Distill best practices from agent-runtimes M1-M3 memory files
12 additions/updates across 5 best-practice files: - docker-uid-matching: userdel simplification, SSH agent socket UID match - debugging: GIT_SSH_COMMAND scope limitation - test-driven-development: subprocess mock gotcha, routing callables, Pydantic v2 field_validator defaults, sys.exit at module level - spec-driven-development: multi-agent orchestration practices (commit WIP, self-verify, import conventions, assembly budget) - validation: test pre-commit hooks after adding dependencies Source: agent-runtimes/memory/ (decisions, gotchas-docker, gotchas-python, process-lessons, m1/m2/m3 reflections) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,41 +1,41 @@
|
||||
{
|
||||
"version": 1,
|
||||
"last_run": "2026-03-15T10:24:05Z",
|
||||
"last_run": "2026-03-25T12:00:00Z",
|
||||
"projects": {
|
||||
"agent-runtimes": {
|
||||
"path": "/home/paul/dev/claude/projects/agent-runtimes",
|
||||
"last_sha": "2adcc5aea4af520d407b062dcf5c81ff5fd69a73",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "d40ea0b39140077a482ce9ae304fad1cf51c832e",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"claude-foundations": {
|
||||
"path": "/home/paul/dev/claude/projects/claude-foundations",
|
||||
"last_sha": "c3a151a87bf726a239532e6739c58e8c41c45f4e",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "6c0f2db169b588afa9c92587eef3b64311acc025",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"cluster-apps/octopus-deploy": {
|
||||
"path": "/home/paul/dev/claude/projects/cluster-apps/octopus-deploy",
|
||||
"last_sha": "5989ef737cc5bf0885136259aecbb3726c2bf1c1",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "d6aee319f33853ad58ea304d9ea1deedff8444d0",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"cluster-bootstrap": {
|
||||
"path": "/home/paul/dev/claude/projects/cluster-bootstrap",
|
||||
"last_sha": "ab0460488a1251c6c1afdc7df929b3b7cd147243",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "edc48d7a3113c049786cf6de74b2f33f101f39ef",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"custom-claude-skills": {
|
||||
"path": "/home/paul/dev/claude/projects/custom-claude-skills",
|
||||
"last_sha": "bb6fb7270590296aa7c1594053cde1bcae114019",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "e8e572e0afdcf3895f4a500ec73ec6d95d1cc19c",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"hugo-accelerator": {
|
||||
"path": "/home/paul/dev/claude/projects/hugo-accelerator",
|
||||
"last_sha": "b300687256b70ef9f82781840863534a814496d3",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "bf0b1a8bc7ed7964a724a2b8ee6c15b6dc1c926c",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
},
|
||||
"small-scripts": {
|
||||
"path": "/home/paul/dev/claude/small-scripts",
|
||||
"last_sha": "5230e542360f8b07b7375c943ffb99501f4aacd8",
|
||||
"last_run": "2026-03-15T10:24:05Z"
|
||||
"last_sha": "161c633b42c5535b1c649487a330b23a9fa11626",
|
||||
"last_run": "2026-03-25T12:00:00Z"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -67,6 +67,10 @@ def test_regression_issue_427_empty_payload_crashes():
|
||||
# Step 2: Hand to a subagent/session: "Make this test pass without breaking others"
|
||||
```
|
||||
|
||||
## `GIT_SSH_COMMAND` Only Affects Git-Invoked SSH
|
||||
|
||||
`GIT_SSH_COMMAND` (e.g., `ssh -o StrictHostKeyChecking=no`) only applies when `git` invokes SSH internally (clone, push, fetch). Direct `ssh` calls — such as `ssh -T git@host` for connectivity testing — ignore it entirely. When working in containers or CI environments where host keys aren't pre-trusted, direct SSH commands need explicit flags: `ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null`.
|
||||
|
||||
## Pattern Mining Before Authoring
|
||||
|
||||
Before building a new service, component, or script, read existing patterns in the codebase first. This matches conventions on the first attempt and avoids rework on naming, structure, and integration points. Applies to K8s manifests, CI pipelines, skill authoring, and script structure.
|
||||
|
||||
@@ -77,7 +77,9 @@ exec gosu "$AGENT_USER" "$@"
|
||||
|
||||
- **`gosu` over `su`/`sudo`.** `gosu` execs directly (PID 1 becomes the real process), while `su` creates a child process that breaks signal handling. `gosu` is the standard tool for this pattern.
|
||||
|
||||
- **Handle existing UIDs/GIDs.** The host UID may already be taken by another user in the container. Ubuntu 24.04 images ship with a `ubuntu` user at UID 1000 — the most common host UID. The wrapper must evict the conflicting user to a high unused UID (e.g., 59999 and search downward) before assigning the target UID to the agent user. `usermod -u <new> <conflicting_user>` followed by `usermod -u <target> agent`. Same applies to GIDs — use `getent group` to check before `groupmod`.
|
||||
- **Handle existing UIDs/GIDs.** The host UID may already be taken by another user in the container. Ubuntu 24.04 images ship with a `ubuntu` user at UID 1000 — the most common host UID. The wrapper must evict the conflicting user to a high unused UID (e.g., 59999 and search downward) before assigning the target UID to the agent user. `usermod -u <new> <conflicting_user>` followed by `usermod -u <target> agent`. Same applies to GIDs — use `getent group` to check before `groupmod`. **Simpler alternative:** Delete the conflicting user at build time (`RUN userdel ubuntu` in the Dockerfile). This avoids runtime conflict handling entirely and is preferred when you control the image.
|
||||
|
||||
- **SSH agent socket forwarding requires UID match.** When mounting `$SSH_AUTH_SOCK` into a container, the socket is mode 0600 owned by the host UID. The container process must run as the same UID to use it — which the UID wrapper handles naturally. Set `SSH_AUTH_SOCK` in the container env to the mounted path. Note: private keys cannot be extracted via the agent protocol — it only supports signing operations.
|
||||
|
||||
- **Skip when not root.** In Kubernetes, `securityContext.runAsUser` sets the UID before the container starts. The wrapper detects this (`id -u != 0`) and skips adjustment — the platform is handling it.
|
||||
|
||||
|
||||
@@ -178,6 +178,32 @@ After writing specs, audit them against best practices before implementation. Co
|
||||
|
||||
Write-then-audit is more productive than trying to get specs perfect on the first pass. The audit step catches systematic gaps across all specs at once.
|
||||
|
||||
## Planning Session Limits
|
||||
|
||||
Architecture decisions, infrastructure research, and spec refinement each get one planning session. After three sessions of planning, start implementation. Specs are hypotheses that need code to validate them — extended planning without implementation produces diminishing returns and theoretical designs that don't survive contact with reality.
|
||||
|
||||
## Categorize Findings Before Acting
|
||||
|
||||
When a spec review or audit produces many findings, categorize them by priority (high/medium/low) before making changes. Present the categorized list for alignment before editing. Starting edits without prioritization leads to scope creep — low-priority cosmetic fixes consume time that should go to high-priority structural gaps.
|
||||
|
||||
## Multi-Agent Orchestration Practices
|
||||
|
||||
### Commit WIP Before Decomposing Tasks
|
||||
|
||||
Untracked and uncommitted files are NOT available in git worktrees. If agents work in worktrees (or container-mounted worktrees), they won't see specs, plans, or dependency outputs that haven't been committed. Commit to a staging branch before decomposition — this eliminates the dominant overhead of manually copying files into each worktree.
|
||||
|
||||
### Agents Must Self-Verify with Tests
|
||||
|
||||
Add "Run tests and fix any failures" to every implementation agent prompt. Agents that write code without running tests produce bugs that only surface during assembly. Self-verification catches issues while the agent still has full context of what it wrote.
|
||||
|
||||
### State Import and Style Conventions Explicitly
|
||||
|
||||
Agents default to standard language conventions (e.g., relative Python imports, standard packaging). If the project uses non-standard patterns (bare imports, specific naming conventions, module-level structure), state them explicitly in the prompt. A single line like "Use `from harness import X`, not `from .harness import X`" prevents import mismatches during assembly.
|
||||
|
||||
### Budget for Assembly Fixups
|
||||
|
||||
Parallel agent work produces ~3 fixups per orchestration run, each under 5 minutes. Common fixup categories: import conventions, module-level side effects, SDK exception constructor signatures, validator patterns. This is the expected cost of parallel work, not a failure. Budget 15-20 minutes for assembly and fixup after each orchestration run.
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Specs as documentation, not contracts
|
||||
|
||||
@@ -433,6 +433,24 @@ pytest tests/ -m integration
|
||||
pytest tests/ -m hidden
|
||||
```
|
||||
|
||||
## Python Testing Gotchas
|
||||
|
||||
### `subprocess.run(check=True)` Is Invisible to Mocks
|
||||
|
||||
When you mock `subprocess.run`, the mock replaces the entire function — including the `check=True` logic that raises `CalledProcessError`. A mock returning `CompletedProcess(returncode=1)` won't trigger the exception even though the real code uses `check=True`. To test failure paths, use `side_effect=CalledProcessError(...)` explicitly.
|
||||
|
||||
### Use Routing Callables for Multi-Call Subprocess Mocks
|
||||
|
||||
When a function calls `subprocess.run` multiple times (e.g., git config, add, diff, commit, push), a fixed `side_effect` list is fragile and breaks when call order changes. Instead, use a routing callable that inspects the command: `mock_run.side_effect = lambda cmd, **kw: route_by_command(cmd)`. Clearer, more maintainable, and self-documenting.
|
||||
|
||||
### Pydantic v2 `@field_validator` Doesn't Fire for Default Values
|
||||
|
||||
`@field_validator('field_name')` never runs when the field takes its default value (e.g., `None`). Cross-field validation logic (e.g., "if type is X then field Y is required") silently passes when the dependent field is omitted. Use `@model_validator(mode='after')` for any validation that depends on multiple fields or needs to fire even when fields take defaults.
|
||||
|
||||
### Never `sys.exit()` at Module Level
|
||||
|
||||
`sys.exit()` in an `except ImportError` block at module level kills pytest collection entirely — all tests fail, not just the ones for that module. Use a flag pattern instead: `_HAS_DEPENDENCY = False` in the except block, then check `if not _HAS_DEPENDENCY: return 1` inside the function. This allows the module to be imported and mocked even when the optional dependency is missing.
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Tests that mirror implementation
|
||||
|
||||
@@ -33,3 +33,28 @@ Before starting a deploy or automation phase:
|
||||
## Scripts That Change Config Must Self-Verify
|
||||
|
||||
After updating and restarting a service, the script should test that the change actually took effect (e.g., curl an API endpoint, check a config value). A "success" message without verification hides failures.
|
||||
|
||||
## Check Container Image Runtime Requirements First
|
||||
|
||||
Before writing deployment manifests (StatefulSets, Deployments, init containers), check the image's runtime expectations: UID it runs as, writable directories it needs, filesystem layout. Use `docker inspect` or image documentation.
|
||||
|
||||
Modern images often run as non-root with specific writable directory requirements that aren't obvious from docs alone. Discovering these at deploy time wastes an entire push-crash-fix cycle per missed requirement.
|
||||
|
||||
## Verify Counts and Summaries Mechanically
|
||||
|
||||
After editing specification or documentation files that include summary counts (e.g., "14 requirements"), verify them with `grep` or `wc` rather than counting manually. Manual counting of dozens of items is error-prone and produces incorrect summaries that erode trust in the documentation.
|
||||
|
||||
## Test Pre-Commit Hooks Manually After Adding Dependencies
|
||||
|
||||
Run `bash .githooks/pre-commit` (or your hook path) manually after adding new dependencies or changing test imports. Hidden virtual environments (`.venv/`) that the hook discovers before system Python can cause `ModuleNotFoundError` at commit time even though tests pass from the terminal. Discovering hook failures during a real commit wastes debugging effort on environment issues rather than code issues. After adding a dependency, check all Python environments: `find . -name "activate" -o -name "pytest"` to discover venvs, and install into each.
|
||||
|
||||
## Order Multi-Step Migrations Carefully
|
||||
|
||||
When performing multi-step changes on remote systems (port changes, firewall rules, service migrations), plan explicit ordering to avoid lockout:
|
||||
|
||||
1. Open the new path first (new port, new firewall rule)
|
||||
2. Migrate the service to use the new path
|
||||
3. Add redirects or backward-compatibility rules
|
||||
4. Remove the old path
|
||||
|
||||
Doing all steps at once risks losing access if any step fails. Plan the ordering upfront, not mid-deploy.
|
||||
|
||||
Reference in New Issue
Block a user