Single-home non-blocking credential reads (I2)

Task 8805 · 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 I2 (architecture §3 rank 2, child 2) Add ONE helper in `apps/jaahd/agents/base.py`: `credential_file_present(path: str, *, max_bytes: int, validate: Callable[[bytes], bool] | None = None) -> bool`. It opens with `O_RDONLY|O_NOFOLLOW|O_NONBLOCK`, accepts a regular file only (fstat), reads at most `max_bytes`, and returns False on any refusal. Build it on the existing `open_regular` / `read_bounded` (`base.py:299-325`), not beside them. Every credential-shape site calls it: `claude_login.py:78-97` (hand-rolled `O_NOFOLLOW`+fstat); `gemini.py:137-158`; `antigravity.py:93-101`; `codex_login.py:125-145`; `codex.py:1074-1080`; `agent_config_dirs.creds_state` (`:100-123`, today a plain `read_text` — a planted FIFO hangs the caller, `apps/jaahd/README.md:500-503`). Each site keeps its own shape check as the `validate` callable (vendor fact stays at the adapter). **Fence**, recognised by function name, not by path: - The AST test lists the credential-reading functions by qualified name (`ClaudeLoginAdapter.credential_present`, `GeminiLogin.credential_present`, `AntigravityLogin.credential_present`, `CodexLogin.credential_present`, `CodexAdapter.credential_state`, `agent_config_dirs.creds_state` — use whatever each class is actually named; pin the exact names in the test). - Each listed function's body must call `credential_file_present` and must contain no `open(`, `os.open(`, `read_text(`, `read_bytes(` or `read_bounded(` call. - The adapter loop (`test_agent_adapters.py:204` style) asserts every registered login adapter's `credential_present` is one of the listed functions or inherits base. - A test plants a FIFO at each adapter's credential path and asserts `credential_present` returns False within 1 s. **PR body:** cite copies 5 → 1 and the LOC delta (about −90 LOC expected). **Docs:** name `credential_file_present` in `apps/jaahd/README.md`'s adapter-seam section (about `:482`) and update the FIFO note at `:500-503` to the present. `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 credential shapes are accepted (each site's validation result must be identical for every currently-accepted and currently-rejected file). - 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. - One `credential_file_present` definition; every listed function calls it — grep + AST fence test cited. - FIFO test: each adapter's `credential_present` returns False within 1 s on a planted FIFO (output cited); before the change `agent_config_dirs.creds_state` would block — show the red→green. - Login adapter tests for Claude, Codex, Gemini, Antigravity green with unchanged accept/reject results.

Outcome

Built credential_file_present in agents/base.py; all 6 credential readers call it (5→1 copies), FIFO no longer hangs creds_state, AST+FIFO fence added. Quick gate green, review lane pass; PR #6566 queued for the lane's full gate. Follow-up draft 8826. Memory: none applied.

Pull request

Artifacts

Activity

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