ward#401 Phase 3: drain per-agent bodies out of core into the agent folders, delete the closure bridges #425

New issue

Closed

opened 2026-07-01 15:56:43 +00:00 by coilysiren · 3 comments

coilysiren commented

2026-07-01 15:56:43 +00:00

Owner

Why

Phase 2 (ward#412) built the internal/agentspi seam, the internal/agents/<name>/ folders, the registry, and a large contract-test suite - the structure ward#401 designed. But it moved no per-agent logic out of core. The function bodies are still *Runner methods in cmd/ward/container_bootstrap.go, and each folder reaches back into core through an injected closure wired in cmd/ward/agents_wire.go. Net effect: rg -i claude went 483 -> 565 and container_bootstrap.go barely moved (51 -> 49 claude hits, core non-test cmd/ward 124 -> 126). The seam was added on top of the un-drained scatter, so the count could only rise. Phase 3 is the drain that makes it fall.

Measured inventory (in /substrate/ward):

internal/agentspi: claude 9, codex 8 (new)
internal/agents/<name>: claude 28, codex 29 (new)
cmd/ward core non-test .go: claude 126, codex 90 (the scatter - unchanged)
cmd/ward tests: claude 226 (new)
container_bootstrap.go: 49 claude hits (still the densest cluster)

Part A - rename `agentspi` -> `agentsapi` (do this first, one mechanical sweep)

Kai's call: agentspi reads as a garble. Rename the package to agentsapi (Go-idiomatic, no underscore, matches the existing style and keeps staticcheck ST1003 quiet - NOT agents_api). This is folded into this issue on purpose: the drain in Part B rewrites every agentspi import anyway, so doing the rename as the first commit of the same run avoids the import-line collision that would otherwise churn between two concurrent runs. (The prior engineer on this issue was stopped precisely so this runs as one clean pass.)

Scope of the rename: internal/agentspi/ dir -> internal/agentsapi/, package agentspi -> package agentsapi, every import path .../internal/agentspi -> .../internal/agentsapi, every agentspi. qualifier -> agentsapi., the test files (agentspi_ctx_test.go -> agentsapi_ctx_test.go), and docs/agentspi.md -> docs/agentsapi.md with its in-text references. Land it as its own commit so it is trivially reviewable, then build Part B on top.

Part B - the drain

Relocate each per-agent body from *Runner/container_bootstrap.go into its folder as an agentsapi.Agent method (or package func) operating on agentsapi.RunCtx/HostCtx, then delete the closure field and its agents_wire.go entry and the now-dead core method. The bridge exists so the closure disappears when the body lands home - a folder field pointing back at core is the temporary state Phase 3 ends.

Bodies to move (from agents_wire.go wiring + folder *Fn fields):

claude (internal/agents/claude/): resolveClaudeCreds (+credEnvLines share), writeClaudeCreds, seedClaudeOnboarding, smokeTestClaudeAuth.
codex (internal/agents/codex/): resolveCodexCreds (+credEnvLines), writeCodexCreds, composeCodexConfig.
opencode (internal/agents/opencode/): composeOpencodeConfig, installOpencode.
goose (internal/agents/goose/): composeGooseConfig.

Shared helpers these call (credEnvLines, env/file-writing shims, envFromRunCtx) either move to internal/agentsapi as shared surface or stay in core if genuinely generic - decide per helper, and prefer widening RunCtx/HostCtx over re-reaching into *Runner. When a body needs Runner state not on RunCtx today, thread it through RunCtx (that abstraction exists for exactly this), do not re-inject a closure.

Exit gate (the capstone, ward-local - NOT an aos catalog hook)

Once the drain lands, add a ward-local contract test (sibling of the existing agent contract tests, e.g. a go test that greps cmd/ward non-test .go) that fails if a per-agent literal (claude/codex/goose/opencode/qwen/ollama) appears in core outside a tiny allowlist (the mode->binary registry table and the roster defaults #401 marks as legitimately shared). This is the ratchet that keeps the count from creeping back. It is ward-internal architecture, so it lives as a ward contract test, not a cross-repo catalog validator in agentic-os. Author it last, green.

Sequencing (from #401)

Rename first (Part A, one commit). Then the drain: the mode switch and registry must stay contract-green through the transition - the switch/closures die last, per agent, after that agent's body is home and its contract test passes. Migrate one folder at a time (claude, then codex, then opencode, then goose), each a self-contained commit, so a regression is bisectable. The exit-gate test flips on only after all four are drained.

Done condition

agentspi is gone repo-wide - the package, imports, test files, and docs/agentspi.md are all agentsapi.
Zero *Fn closure fields left in the internal/agents/* folders and zero closure wiring in agents_wire.go.
container_bootstrap.go per-agent creds/config/install methods gone.
Core non-test cmd/ward claude/codex counts down materially (target: container_bootstrap.go near-zero per-agent hits).
The exit-gate contract test green and enforcing.
docs/agentsapi.md + docs/FEATURES.md updated to describe the completed drain (bodies own their folders, core dispatches through the SPI only).

Phase 3 of ward#401, following Phase 2 (ward#412). Inventory + diagnosis from the aos director surface after Kai flagged the growing hit count on ward#401. Rename folded in per Kai (prior engineer stopped to run this as one pass).

## Why Phase 2 (ward#412) built the `internal/agentspi` seam, the `internal/agents/<name>/` folders, the registry, and a large contract-test suite - the structure ward#401 designed. But it moved **no per-agent logic out of core**. The function bodies are still `*Runner` methods in `cmd/ward/container_bootstrap.go`, and each folder reaches back into core through an injected closure wired in `cmd/ward/agents_wire.go`. Net effect: `rg -i claude` went 483 -> 565 and `container_bootstrap.go` barely moved (51 -> 49 claude hits, core non-test cmd/ward 124 -> 126). The seam was added **on top of** the un-drained scatter, so the count could only rise. Phase 3 is the drain that makes it fall. Measured inventory (in /substrate/ward): - `internal/agentspi`: claude 9, codex 8 (new) - `internal/agents/<name>`: claude 28, codex 29 (new) - `cmd/ward` core non-test .go: claude 126, codex 90 (the scatter - unchanged) - `cmd/ward` tests: claude 226 (new) - `container_bootstrap.go`: 49 claude hits (still the densest cluster) ## Part A - rename `agentspi` -> `agentsapi` (do this first, one mechanical sweep) Kai's call: `agentspi` reads as a garble. Rename the package to **`agentsapi`** (Go-idiomatic, no underscore, matches the existing style and keeps staticcheck ST1003 quiet - NOT `agents_api`). This is folded into this issue on purpose: the drain in Part B rewrites every `agentspi` import anyway, so doing the rename as the **first commit** of the same run avoids the import-line collision that would otherwise churn between two concurrent runs. (The prior engineer on this issue was stopped precisely so this runs as one clean pass.) Scope of the rename: `internal/agentspi/` dir -> `internal/agentsapi/`, `package agentspi` -> `package agentsapi`, every import path `.../internal/agentspi` -> `.../internal/agentsapi`, every `agentspi.` qualifier -> `agentsapi.`, the test files (`agentspi_ctx_test.go` -> `agentsapi_ctx_test.go`), and `docs/agentspi.md` -> `docs/agentsapi.md` with its in-text references. Land it as its own commit so it is trivially reviewable, then build Part B on top. ## Part B - the drain Relocate each per-agent body from `*Runner`/`container_bootstrap.go` into its folder as an `agentsapi.Agent` method (or package func) operating on `agentsapi.RunCtx`/`HostCtx`, then **delete the closure field and its `agents_wire.go` entry and the now-dead core method**. The bridge exists so the closure disappears when the body lands home - a folder field pointing back at core is the temporary state Phase 3 ends. Bodies to move (from `agents_wire.go` wiring + folder `*Fn` fields): - **claude** (`internal/agents/claude/`): `resolveClaudeCreds` (+`credEnvLines` share), `writeClaudeCreds`, `seedClaudeOnboarding`, `smokeTestClaudeAuth`. - **codex** (`internal/agents/codex/`): `resolveCodexCreds` (+`credEnvLines`), `writeCodexCreds`, `composeCodexConfig`. - **opencode** (`internal/agents/opencode/`): `composeOpencodeConfig`, `installOpencode`. - **goose** (`internal/agents/goose/`): `composeGooseConfig`. Shared helpers these call (`credEnvLines`, env/file-writing shims, `envFromRunCtx`) either move to `internal/agentsapi` as shared surface or stay in core if genuinely generic - decide per helper, and prefer widening `RunCtx`/`HostCtx` over re-reaching into `*Runner`. When a body needs Runner state not on RunCtx today, thread it through RunCtx (that abstraction exists for exactly this), do not re-inject a closure. ## Exit gate (the capstone, ward-local - NOT an aos catalog hook) Once the drain lands, add a **ward-local contract test** (sibling of the existing agent contract tests, e.g. a `go test` that greps `cmd/ward` non-test `.go`) that **fails if a per-agent literal** (`claude`/`codex`/`goose`/`opencode`/`qwen`/`ollama`) appears in core outside a tiny allowlist (the `mode->binary` registry table and the roster defaults #401 marks as legitimately shared). This is the ratchet that keeps the count from creeping back. It is ward-internal architecture, so it lives as a ward contract test, not a cross-repo catalog validator in agentic-os. Author it last, green. ## Sequencing (from #401) Rename first (Part A, one commit). Then the drain: the mode switch and registry must stay contract-green through the transition - the switch/closures die **last**, per agent, after that agent's body is home and its contract test passes. Migrate one folder at a time (claude, then codex, then opencode, then goose), each a self-contained commit, so a regression is bisectable. The exit-gate test flips on only after all four are drained. ## Done condition - `agentspi` is gone repo-wide - the package, imports, test files, and `docs/agentspi.md` are all `agentsapi`. - Zero `*Fn` closure fields left in the `internal/agents/*` folders and zero closure wiring in `agents_wire.go`. - `container_bootstrap.go` per-agent creds/config/install methods gone. - Core non-test `cmd/ward` claude/codex counts down materially (target: container_bootstrap.go near-zero per-agent hits). - The exit-gate contract test green and enforcing. - `docs/agentsapi.md` + `docs/FEATURES.md` updated to describe the completed drain (bodies own their folders, core dispatches through the SPI only). Phase 3 of ward#401, following Phase 2 (ward#412). Inventory + diagnosis from the aos director surface after Kai flagged the growing hit count on ward#401. Rename folded in per Kai (prior engineer stopped to run this as one pass).

coilyco-ops commented

2026-07-01 15:56:53 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-425 on host kais-macbook-pro-2.local is carrying this issue (reserved 2026-07-01T15:56:51Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-425` on host `kais-macbook-pro-2.local` is carrying this issue (reserved 2026-07-01T15:56:51Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides.  — Claude (she/her), via `ward agent`

coilyco-ops commented

2026-07-01 16:01:37 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-425 on host kais-macbook-pro-2.local is carrying this issue (reserved 2026-07-01T16:01:35Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-425` on host `kais-macbook-pro-2.local` is carrying this issue (reserved 2026-07-01T16:01:35Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides.  — Claude (she/her), via `ward agent`

coilysiren referenced this issue from a commit

2026-07-01 16:55:32 +00:00

test(agents): add the drain exit-gate ratchet + docs (ward#425)

coilysiren closed this issue

2026-07-01 16:55:32 +00:00

coilysiren commented

2026-07-01 16:56:13 +00:00

Author

Owner

WARD-OUTCOME: done - drained every per-agent body into internal/agents/, renamed agentspi->agentsapi, deleted the closure bridges, added a drain ratchet; landed on main.

This one felt like careful demolition more than construction. The seam Phase 2 built was genuinely good scaffolding, so the drain was mostly moving real bodies home and then deleting the temporary bridge - satisfying to watch agents_wire.go shrink from a closure switch down to two tiny helpers.

What fought back: the host-side credential path was more tangled than the issue's bodies-to-move list implied. resolveAgentCreds/credEnvLines/agentCreds all had to flip to the []EnvLine shape at once, and goose isn't a CredentialProvider (it binds ollama via config), so it needed a small special-case rather than clean SPI routing. I grouped claude+codex into one commit for that reason instead of strict per-folder commits - the shared host path didn't bisect cleanly otherwise.

The exit gate took the most thought. A naive grep drowns in comments and Phase-4 switch tables, so I built it on go/parser (string-literal AST nodes only) and shrank the allowlist to exactly two files by moving the launch-argv switches into the mode-table file and swapping incidental roster defaults for the mode constants. It genuinely fails on a leaked literal - I checked.

Confidence is high: every folder has its own tests, the registry contract test still pins argv/signer entry-for-entry, and the full suite is green. Rough edges / follow-ups worth filing: (1) the goose ollama-host resolve staying host-side is a small asymmetry a future pass could fold into the SPI; (2) buildAgentArgv/logAgentArgv/agentSigner are still live switches (Phase 4's job) - the gate allowlists them deliberately; (3) FEATURES.md is right at the 4000-char cap, so the next addition will force a split. Two pre-existing lint findings (a blog Fprintln errcheck, preserveExtraRepo's unused param) were left untouched as out of scope.

WARD-OUTCOME: done - drained every per-agent body into internal/agents/<name>, renamed agentspi->agentsapi, deleted the closure bridges, added a drain ratchet; landed on main. This one felt like careful demolition more than construction. The seam Phase 2 built was genuinely good scaffolding, so the drain was mostly moving real bodies home and then deleting the temporary bridge - satisfying to watch `agents_wire.go` shrink from a closure switch down to two tiny helpers. What fought back: the host-side credential path was more tangled than the issue's bodies-to-move list implied. `resolveAgentCreds`/`credEnvLines`/`agentCreds` all had to flip to the `[]EnvLine` shape at once, and goose isn't a CredentialProvider (it binds ollama via config), so it needed a small special-case rather than clean SPI routing. I grouped claude+codex into one commit for that reason instead of strict per-folder commits - the shared host path didn't bisect cleanly otherwise. The exit gate took the most thought. A naive grep drowns in comments and Phase-4 switch tables, so I built it on go/parser (string-literal AST nodes only) and shrank the allowlist to exactly two files by moving the launch-argv switches into the mode-table file and swapping incidental roster defaults for the mode constants. It genuinely fails on a leaked literal - I checked. Confidence is high: every folder has its own tests, the registry contract test still pins argv/signer entry-for-entry, and the full suite is green. Rough edges / follow-ups worth filing: (1) the goose ollama-host resolve staying host-side is a small asymmetry a future pass could fold into the SPI; (2) buildAgentArgv/logAgentArgv/agentSigner are still live switches (Phase 4's job) - the gate allowlists them deliberately; (3) FEATURES.md is right at the 4000-char cap, so the next addition will force a split. Two pre-existing lint findings (a blog Fprintln errcheck, preserveExtraRepo's unused param) were left untouched as out of scope.