Expand catalog.dependsOn auto-context (read-only dep grants) from advisor-only to all warded roles #573

New issue

Closed

opened 2026-07-03 17:34:49 +00:00 by coilysiren · 2 comments

coilysiren commented

2026-07-03 17:34:49 +00:00

Owner

What

Commit f899006 (closes #566) gave the advisor an auto-grant of read-only context repos: at launch it reads the cloned repo's own catalog.dependsOn and auto-clones each dependency repo as read-only context, so the advisor can read its deps while researching. Kai wants this expanded to every warded role (engineer, director, advisor), not just advisor - every run gets its target's declared dependency repos as read-only reference automatically.

Where it's gated today

cmd/ward/container.go:124:

autoExtras := []targetRepo(nil)
if advisorContext {
    autoExtras = advisorAutoGrantRepos(cwd)
}
extra, notes := mergeExtraRepos(extra, autoExtras, repo)

The auto-grant is gated behind advisorContext, and its result merges into extra - the same ExtraRepos slice that carries writable --repo grants.

Why this is NOT a one-line gate flip (the footgun)

ExtraRepos are treated as writable, push-expected grants:

The reaper's verifyExtraReposLanded (cmd/ward/container_reap.go:490) checks each granted repo "landed on its remote main" and treats an unlanded one as a hard failure - it reopens the issue and cuts a ward-salvage/<id> branch. It skips this only when the whole run is ReadOnly (env.ReadOnly, line 493).
There is no per-repo read-only marker. Read-only is a whole-run concept (plan.ReadOnly, WARD_READONLY). The advisor is safe only because it sets plan.ReadOnly = true for the entire run (agent_advisor.go:172).

So the two tiers split cleanly:

Read-only roles (director surface, advisor) - already whole-run ReadOnly, so the reaper skips push-verify. Widening the gate to cover them is safe and low-risk.
Writable engineer run - naively merging catalog.dependsOn deps into ExtraRepos would make the reaper expect the engineer to push every dependency repo to main and false-fail the run (reopen + salvage) for deps it only read - and clone them with push remotes the engineer should never have.

The real work

Introduce a per-repo read-only context grant: dependency repos cloned for reading, with no push remote, excluded from verifyExtraReposLanded, distinct from writable --repo grants. Then apply catalog.dependsOn auto-context for all roles as read-only. This is essentially "auto-mount the target's declared deps as /substrate-style read-only reference," which the substrate doctrine already blesses (read from either copy, act only on /workspace).

De-advisor the naming while here: advisorAutoGrantRepos / advisorContext -> role-neutral (catalogContextRepos / autoContext).

Design fork to settle first (why an advisor design pass precedes the engineer run)

Representation - a ReadOnly bool on targetRepo, or a separate ContextRepos slice parallel to ExtraRepos? The reaper, the clone step, and WARD_EXTRA_REPOS env round-trip all consume ExtraRepos, so the choice ripples.
Clone step - how a read-only entry is cloned without a push remote (and whether it should carry the pre-commit gate at all).
Reaper - verifyExtraReposLanded must skip read-only context entries even on a writable run (today it only skips whole-run ReadOnly).
Substrate overlap - a dep that is also a /substrate manifest repo already exists as read-only reference; avoid a redundant second clone.

f899006 / #566 - the advisor feature being expanded.
48b8a19 advisor auto-grant read-only context repos, 1ef3db6 atlas->repo-local refactor.
cmd/ward/container_reap.go:490 verifyExtraReposLanded - the push-verify that footguns a naive expansion.
ward#230 / ward#280 - the writable --repo grant model, docs/container-multi-repo.md.

## What Commit [f899006](https://forgejo.coilysiren.me/coilyco-flight-deck/ward/commit/f8990061fdee18f322d6b89fd57862b144b6314b) (closes #566) gave the **advisor** an auto-grant of read-only context repos: at launch it reads the cloned repo's own `catalog.dependsOn` and auto-clones each dependency repo as read-only context, so the advisor can read its deps while researching. Kai wants this **expanded to every warded role** (engineer, director, advisor), not just advisor - every run gets its target's declared dependency repos as read-only reference automatically. ## Where it's gated today `cmd/ward/container.go:124`: ```go autoExtras := []targetRepo(nil) if advisorContext { autoExtras = advisorAutoGrantRepos(cwd) } extra, notes := mergeExtraRepos(extra, autoExtras, repo) ``` The auto-grant is gated behind `advisorContext`, and its result merges into `extra` - the **same `ExtraRepos` slice that carries writable `--repo` grants**. ## Why this is NOT a one-line gate flip (the footgun) `ExtraRepos` are treated as **writable, push-expected** grants: - The reaper's `verifyExtraReposLanded` (`cmd/ward/container_reap.go:490`) checks each granted repo "landed on its remote main" and treats an **unlanded** one as a **hard failure** - it reopens the issue and cuts a `ward-salvage/<id>` branch. It skips this **only** when the whole run is `ReadOnly` (`env.ReadOnly`, line 493). - There is **no per-repo read-only marker**. Read-only is a whole-run concept (`plan.ReadOnly`, `WARD_READONLY`). The advisor is safe only because it sets `plan.ReadOnly = true` for the entire run (`agent_advisor.go:172`). So the two tiers split cleanly: - **Read-only roles (director surface, advisor)** - already whole-run `ReadOnly`, so the reaper skips push-verify. Widening the gate to cover them is safe and low-risk. - **Writable engineer run** - naively merging `catalog.dependsOn` deps into `ExtraRepos` would make the reaper **expect the engineer to push every dependency repo to main** and **false-fail the run** (reopen + salvage) for deps it only read - and clone them with push remotes the engineer should never have. ## The real work Introduce a **per-repo read-only context grant**: dependency repos cloned for reading, with **no push remote**, **excluded from `verifyExtraReposLanded`**, distinct from writable `--repo` grants. Then apply `catalog.dependsOn` auto-context for **all** roles as read-only. This is essentially "auto-mount the target's declared deps as `/substrate`-style read-only reference," which the substrate doctrine already blesses (read from either copy, act only on `/workspace`). De-advisor the naming while here: `advisorAutoGrantRepos` / `advisorContext` -> role-neutral (`catalogContextRepos` / `autoContext`). ## Design fork to settle first (why an advisor design pass precedes the engineer run) - **Representation** - a `ReadOnly bool` on `targetRepo`, or a separate `ContextRepos` slice parallel to `ExtraRepos`? The reaper, the clone step, and `WARD_EXTRA_REPOS` env round-trip all consume `ExtraRepos`, so the choice ripples. - **Clone step** - how a read-only entry is cloned without a push remote (and whether it should carry the pre-commit gate at all). - **Reaper** - `verifyExtraReposLanded` must skip read-only context entries even on a writable run (today it only skips whole-run `ReadOnly`). - **Substrate overlap** - a dep that is also a `/substrate` manifest repo already exists as read-only reference; avoid a redundant second clone. ## Related - f899006 / #566 - the advisor feature being expanded. - `48b8a19` advisor auto-grant read-only context repos, `1ef3db6` atlas->repo-local refactor. - `cmd/ward/container_reap.go:490` `verifyExtraReposLanded` - the push-verify that footguns a naive expansion. - ward#230 / ward#280 - the writable `--repo` grant model, `docs/container-multi-repo.md`.

coilysiren referenced this issue

2026-07-03 17:50:30 +00:00

Mount the issue-corpus into /substrate via coilysiren/inbox double-duty (consumer half of agentic-os#297) #575

coilyco-ops commented

2026-07-03 18:17:43 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-573 on host KAI-DESKTOP-TOWER is carrying this issue (reserved 2026-07-03T18:17:37Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

Do not comment on or edit this issue to steer the run while it is reserved. The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a new issue, dispatched fresh — that is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494).

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-573` on host `KAI-DESKTOP-TOWER` is carrying this issue (reserved 2026-07-03T18:17:37Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides. **Do not comment on or edit this issue to steer the run while it is reserved.** The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a **new issue, dispatched fresh** — that is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494).  — Claude (she/her), via `ward agent`

coilysiren referenced this issue from a commit

2026-07-03 18:40:46 +00:00

feat(agent): read-only catalog.dependsOn context for every warded role

coilysiren closed this issue

2026-07-03 18:40:46 +00:00

coilysiren commented

2026-07-03 18:41:27 +00:00

Author

Owner

WARD-OUTCOME: done - read-only catalog.dependsOn context now auto-mounts for every warded role (engineer/director/advisor), landed on main as 3723957.

Honest retro: the issue framed this perfectly and that framing did most of the design work for me. The "one-line gate flip is a footgun" warning was exactly right, and the tell that settled the representation fork was that a naive ReadOnly bool on targetRepo would have to thread a marker through WARD_EXTRA_REPOS, which the bash entrypoint word-splits into owner/name - a :ro suffix there would quietly corrupt the split. A separate ContextRepos slice + WARD_CONTEXT_REPOS key sidesteps that and, better, means the reaper's push-verify needs zero change: it literally cannot see a context repo. That felt like the design clicking rather than me forcing it.

The one thing that genuinely surprised me: GoBootstrap is never set to true anywhere, so the live clone path in production is still the bash clone_extra_repos, not the Go cloneExtraRepos. If I'd trusted the Go reaper/bootstrap as "the" implementation I'd have shipped a feature that no-ops in the field. Both paths got the read-only clone logic as a result.

Confidence is high on the host-side resolution + env round-trip (unit-tested end to end, incl. the target/writable/substrate dedup) and on the reaper isolation. Slightly less on the in-container clone behavior since I can't exercise a real docker bring-up here - the Go and bash paths are kept in lockstep by reading, not by a live run.

Rough edges worth a follow-up: (1) context resolution reads the host cwd's catalog, inherited from the advisor design - for the engineer dispatch the cwd may not be the target repo, so deps can come from the wrong place or be silently absent. Resolving catalog.dependsOn inside the container from the fresh clone would be strictly more correct and worth its own issue. (2) runContainerBootstrap was already over the funlen cap; I added to it rather than refactoring, since that was out of scope.

WARD-OUTCOME: done - read-only catalog.dependsOn context now auto-mounts for every warded role (engineer/director/advisor), landed on main as 3723957. Honest retro: the issue framed this perfectly and that framing did most of the design work for me. The "one-line gate flip is a footgun" warning was exactly right, and the tell that settled the representation fork was that a naive `ReadOnly bool` on `targetRepo` would have to thread a marker through `WARD_EXTRA_REPOS`, which the bash entrypoint word-splits into `owner/name` - a `:ro` suffix there would quietly corrupt the split. A separate `ContextRepos` slice + `WARD_CONTEXT_REPOS` key sidesteps that and, better, means the reaper's push-verify needs *zero* change: it literally cannot see a context repo. That felt like the design clicking rather than me forcing it. The one thing that genuinely surprised me: `GoBootstrap` is never set to true anywhere, so the *live* clone path in production is still the bash `clone_extra_repos`, not the Go `cloneExtraRepos`. If I'd trusted the Go reaper/bootstrap as "the" implementation I'd have shipped a feature that no-ops in the field. Both paths got the read-only clone logic as a result. Confidence is high on the host-side resolution + env round-trip (unit-tested end to end, incl. the target/writable/substrate dedup) and on the reaper isolation. Slightly less on the in-container clone behavior since I can't exercise a real docker bring-up here - the Go and bash paths are kept in lockstep by reading, not by a live run. Rough edges worth a follow-up: (1) context resolution reads the *host cwd's* catalog, inherited from the advisor design - for the engineer dispatch the cwd may not be the target repo, so deps can come from the wrong place or be silently absent. Resolving catalog.dependsOn inside the container from the fresh clone would be strictly more correct and worth its own issue. (2) `runContainerBootstrap` was already over the funlen cap; I added to it rather than refactoring, since that was out of scope.