One bounded session walk everywhere (I4)

Task 8807 · Done · Coder · created · updated

## Original requirement (epic 8783, verbatim) > Review implementation of model provider interfaces: > > - Claude, Codex, Antigravity, Pi, ACP, > > Identify opportunity to unify, move more code from specific to common, make it more industry standard, more reliable. Do web research to potentially adopt a better architecture. > > Implement top ideas. Settled inputs: PRD (BA task 8784) and architecture (task 8785) — read them with `task_read` 8784 / 8785 and their `prd` / `architecture` artifacts. Decisions there are settled; realize them, do not reopen them. **Quality bar (PRD, verbatim).** A change that passes AC5 by moving code into a grab-bag module with vendor `if` branches inside it FAILS. Common code must be vendor-neutral, and vendor facts stay as data on the adapter or catalogue row. A review artifact that lists ideas without evidence or ranking FAILS AC1/AC2. ## Scope — idea I4 (architecture §3 rank 4, child 4) Port `gemini_runner._chat_files` (`apps/jaahd/agents/gemini_runner.py:1115-1231`) and `codex._rollout_candidates` (`apps/jaahd/agents/codex.py:1462-1534`; its "one home" docstring `:1468` is false) onto `base.SessionTreeWalk` (`apps/jaahd/agents/base.py:237`). Extend the walk with any option both need (a predicate, a depth) as constructor parameters, never a vendor branch. Keep each caller's partial-walk signal (`ChatScanFailed`). One scan budget, lstat/nlink and symlink rule everywhere. **Fence**, scoped to session/transcript discovery: an AST test over each adapter's `discover_session_id`, `transcript`, `session_members` and their private helpers (list them by name, including `gemini_runner._chat_files` and `codex._rollout_candidates`) bans `os.scandir`, `os.walk`, `os.listdir`, `Path.iterdir`, `Path.glob` and `Path.rglob`; these functions must walk through `SessionTreeWalk`. Non-walk directory reads stay legal and out of scope: `gemini_runner.py:1015` (has-run probe), `subagents.py:223`, and the `antigravity_transcript.py` index reads. **PR body:** cite copies 3 → 1 and the LOC delta (about −150 LOC expected). **Docs:** name the extended `SessionTreeWalk` options in `apps/jaahd/README.md`'s adapter-seam section. `python3 tools/check_context.py` passes. Credentials & reach: AC1–AC3, AC5, AC7, AC9, AC10 (repo tests, `./dc check full`, `check_context.py`) — Nothing beyond the worktree — verified (worktree present). Out of scope: From PRD §6 (verbatim) — do NOT touch: - **Do NOT** move Claude subscription (oauth) runs onto ACP, or change `Capabilities.runtime` for `claude` oauth (ADR-0167). - **Do NOT** delete the legacy terminal launch path (`fleet_launch`/`fleet_dispatch` exec lines, `answer_delivery` tmux paste, `ComposerTui`). It still serves Claude subscriptions and non-Linux nodes. - **Do NOT** replace or wrap ACP with A2A, an LLM gateway, LiteLLM-style proxies or direct vendor SDKs (D3). - **Do NOT** rename agent verbs, MCP tool names or ACP extension methods (ADR-0166 table), lifecycle states, `terminal_type` values or DB columns. - **Do NOT** drive `agy`'s TUI (S1, ADR-0165 anti-pattern), and do not fork an upstream ACP adapter unless no option closes the gap (ADR-0167 rule). - **Do NOT** add a new provider, or bump any vendor CLI pin other than `agy` (and only if S1 requires it). - **Do NOT** touch the cloud compute "provider" work of epic 8760 (AWS/GCP/Azure node providers, `NodeLink` storage adapters). That is a different "provider". - **Do NOT** redesign console pages (Settings › Providers, Brains roster, task page, Audit › Sessions). The only console delta is Antigravity runs gaining ACP chat/live-control views and their catalogue gap text. - **Do NOT** make ACP available on Windows or macOS nodes (`acp_runtime.py:75` Linux gate stays). - **Do NOT** build ideas ranked below the top 3–5. File them (AC6). - **Do NOT** change the API key handling invariant: no model key in an engine's environment or `launch.json` (ADR-0165 self-check). - Do NOT change which session/rollout files are discovered or their order of preference (behaviour-preserving, PRD S2); any extra filtering a caller had moves into a constructor parameter. - Your mechanism's diff must not touch the other unification children's mechanisms (I1–I5 are separate PRs; file overlap is fine, scope overlap is not). Done means: **AC5 — Top 3–5 ideas built, each single-homed and fenced (R5)** (PRD, verbatim). Pass per built idea: - (i) the mechanism has exactly one implementation; - (ii) every former copy calls it; - (iii) a test (census or fence) fails when a second copy or a vendor literal is re-added at the guarded sites; - (iv) the merged PR cites before/after copy count and LOC. A built idea that leaves a residual copy names it in its PR body as a follow-up. → verify, per idea: 1. Grep the repo for the old mechanism's signature (e.g. the per-adapter staged-write pattern): one definition remains. 2. Add a scratch copy at a guarded site locally and run the fence test: it fails. Revert the scratch copy. 3. Read the PR: before/after figures are present. **AC7 — Behaviour preserved, by test (R7, S2)** (PRD, verbatim). Pass, all green on the merged develop: - `apps/jaahd/tests/test_agent_adapters.py` (Claude launch.sh + settings byte-identical to the goldens); - the ACP replay fixtures under `apps/jaahd/tests/acp_replay/`; - `tools/tests/test_provider_capabilities.py`; - every provider's adapter tests; - `./dc check full`. → verify: the merge lane's `./dc check full` GATE_RESULT `status=pass` on each PR. Any golden that had to be regenerated is called out in that PR with the reason. A golden change for any lane other than Antigravity is a failure unless it is byte-neutral in behaviour. **AC9 — No new vendor branches (R5)** (PRD, verbatim). Pass: - No new `provider ==` / vendor-literal branch is introduced in launch sites, `apps/jaahd/acp/` or `apps/jaahd/provider_health.py` dispatch. - The existing census still passes. - Vendor literals in `acp/` (C9) are either removed or kept with a recorded reason. → verify: `git diff develop...<merge> -- apps/jaahd/acp apps/jaahd/fleet_launch.py apps/jaahd/fleet_dispatch.py` shows no added vendor names. `./dc test apps/jaahd/tests/test_agent_adapters.py` is green. **AC10 — Docs current (R8)**, this child's part (the README `:551/:552` "No ACP host exists yet" line and the Antigravity section are owned by the Antigravity ACP child — do not edit them here): - `apps/jaahd/README.md` describes the present; the new common helper is named in the adapter-seam section. - `python3 tools/check_context.py` passes. → verify: run `python3 tools/check_context.py`, which prints no failure; grep the adapter-seam section for the helper name. - `_chat_files` and `_rollout_candidates` walk via `SessionTreeWalk`; AST fence green and fails on a scratch `os.scandir` in a guarded function (output cited). - Gemini and Codex session-discovery/transcript tests green, including a partial-walk (budget exhausted) case that still raises/reports `ChatScanFailed` the same way.

Outcome

I4: Gemini _chat_files and Codex _rollout_candidates now use base.SessionTreeWalk (new admit/newest_first options); walks 3→1, net −30 production LOC (~−150 expected); AST fence added. Quick gate and changed-area tests green; PR #6563 queued. Memory: none applied.

Pull request

Artifacts

Activity

  • Draft
  • Ready
  • Runner started
  • Runner ended (done)