refactor(agents): route all KAS derives through the shared wrapper

[dwu96]

Fixes #7513

Symptom

PR #7238 introduced derived_agent_permissions (in src/kiro_crew/agent_sdk/drivers/acp.py) as the shared spelling of the KAS derive-plus-fallback, but migrated only one of the three call sites. The other two still spelled the same derive inline, so the {"rules": []} fallback existed in three byte-identical copies — a future divergence between them would have been invisible to any behavioural test. Separately, the two pipeline-conductor probe scripts re-implement readers for three on-disk formats this package itself writes, and their tests used hand-authored fixtures, which cannot detect the owning writer changing its format. The conductor's reclaim decision runs off that classification, so the live failure mode is a running session read as GONE and its work item dispatched twice.

Note: every line number in the issue body had drifted; sites below are located by symbol at main 1d705a03f. The issue calls site A _write_agent_permissions — the enclosing function on main is _seed_kas_permissions (its corrected location matches the issue's described site).

Root cause

• Site A: _seed_kas_permissions (src/kiro_crew/agent.py, import was at :3285, fallback at :3292) — inline allowed_tools_to_permissions + {"rules": []} fallback.
• Site B: _install_conductor_agent, the goal-conductor installer (import was at :5153, fallback at :5160) — same inline spelling.
• Both inline fallbacks were byte-identical to what the wrapper does, which is precisely why the duplication was invisible behaviourally.
• The probe scripts (fleet_probe.py, credit_spend.py) read the session-transcript JSONL shape, the dashboard_ session-filename prefix, and the usage-shard token-row schema with re-implemented readers; only hand-authored fixtures covered them.

Fix

Item 1 (mechanical migration). Replaced both inline derive-plus-fallback sites with a call to the existing derived_agent_permissions wrapper, following the in-tree precedent already at _install_pipeline_conductor_agent (agent.py:5370, the site #7238 migrated). Pure substitution at both sites — argument values and fallback are identical.

One real difference surfaced by the migration (reported, not papered over): the wrapper's allowed_tools: list annotation was narrower than the derive it wraps, whose parameter is Any with fail-closed validation (a non-list — including the absent-key None site A reads via config.get("allowedTools") — yields no policy, and the wrapper then supplies {"rules": []}). mypy rejected the pure substitution at site A on that annotation alone. Widened the wrapper's annotation to object to match the domain of the function it wraps; runtime behaviour is untouched (the derive owns validation, per its TestUnclassifiableEntriesFailClosed / non-list tests).

Boundary baseline. The migration removes agent.py's last two direct kiro_crew.acp imports — the boundary checker counted exactly 2 edges for agent.py, and both were these imports. The count drops to a provable floor of 0, and the shrink-only gate itself mandates the prune for a touched file whose count shrank (--update-baseline, which only lowers/deletes). The agent.py line is therefore removed from .github/agent-sdk-boundary-baseline.txt. No open PR modifies that entry (PR #7325 adds a different file's line; disjoint hunks).

Item 2 (round-trip contract tests). New test/test_pipeline_conductor_probe_roundtrip.py drives the REAL writer for each format and lets the script (loaded via load_skill_script) classify the output:

1. transcript JSONL entry shape — written by ConversationLog.append into the same <data home>/sessions directory the probe derives;
2. dashboard_ filename prefix — the slot key through the real derivation chain (chat_utils._history_key_for → history._safe_key via ConversationLog), probe given only the RAW slot key must not read GONE;
3. usage-shard token-row schema — rows written by persist_token_record → _build_token_record → _write_token_record with a real TurnUsage, summed by credit_spend; includes the rows-are-turns contract and the negative (unmetered) half.

All three real writers were drivable in-process, so no format needed a hand-rolled substitute. The scripts themselves are untouched — their subprocess-free design survives.

Item-1 guard. New test/test_derived_agent_permissions_consumers.py: a textual guard that the inline fallback spelling does not re-grow outside the boundary (kiro_crew/acp/ and the agent_sdk driver). The IMPORT half of the invariant is held by the live CI boundary gate: after this PR's baseline prune, scripts/check_agent_sdk_boundary.py reds any re-grown kiro_crew.acp.kas_permissions import in application code, including dynamic-import spellings a second scanner would miss.

Verification

• Red before green (item 1): the guard was born red against unmodified main, naming src/kiro_crew/agent.py as the only offender (after exempting acp/kas_agents.py, package-internal use inside the boundary). Green after the migration.
• Mutant (a), re-verified after the review-driven guard subtraction: restoring one inline fallback in agent.py reds BOTH the surviving textual guard AND scripts/check_agent_sdk_boundary.py (agent.py:3291 flagged; the baseline no longer lists the file). Restore → both green.
• Red against drifted writers (item 2): each round-trip test was mutation-verified against a deliberately drifted writer, and in every case the sibling hand-fixture tests STAYED GREEN — demonstrating the exact gap being closed:
  • transcript content field renamed in ConversationLog.append → round-trip red, TestFleetProbe fixtures green;
  • _history_key_for prefix changed dashboard: → dash: → round-trip red (false GONE), fixtures green;
  • token row _type renamed in _build_token_record → round-trip red, TestCreditSpend fixtures green.
• Mutants killed (both directions recorded):
  • (a) restore one inline fallback at site A → guard tests red; restore fix → green.
  • (b) wrapper fallback {"rules": []} → None → behavioural tests red at BOTH migrated sites (test_kas_permissions.py::TestTheDiskWriter for site A, test_conductor_agent.py::TestConductorInstaller::test_a_fully_governed_host_still_emits_the_permissions_key for site B) — the divergence the issue said was invisible is now visible at every consumer at once; restore → green.
• Focused suites: test_derived_agent_permissions_consumers.py, test_pipeline_conductor_probe_roundtrip.py, test_kas_permissions.py, test_conductor_agent.py, test_pipeline_conductor_agent.py — 145/145 pass.
• mypy src/kiro_crew: no issues in 1237 files. black/isort/flake8 gates clean.
• scripts/local-gate.py --base origin/main: backend (full) -n auto --dist loadgroup — 225 failed, 78615 passed, 376 skipped, 5 xfailed, 2 errors. The failure set is this host's known environmental baseline (sandbox userns EPERM, AF_UNIX path-length, host-budget/xdist assumptions), proven below.
• Zero-regression proof: full backend suite on this branch vs a git worktree at origin/main (1d705a03f), sorted failing-test id sets diffed both directions: branch 227 failing ids vs origin/main worktree 227 failing ids; comm -23 = 0 lines, comm -13 = 0 lines — the sets are byte-identical in both directions (main run: 225 failed, 78607 passed, 2 errors in 510s).

No frontend surface is touched; there is no visual delta to screenshot. Evidence is the test/gate output above.

Review-driven changes

• First Principles (advisory CONCERNS) — subtraction ADOPTED: deleted test_no_application_code_imports_the_raw_derive. The lane is right that after the baseline prune the CI boundary gate (ci.yml, scripts/check_agent_sdk_boundary.py) permanently holds the import half of the invariant — AST-scanning the same tree, catching dynamic-import spellings the deleted test could not, and redding agent.py on any re-grown import. Two scanners for one symbol drift, and the weaker one loses. The textual fallback-spelling guard (the half nothing else covers) stays and still kills mutant (a); the gate's coverage of the import half was verified empirically, not assumed (see mutant (a) above). This also resolves the lane's Watch item.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 313c001c5d8b2dbfb1274f5beb4bc457febf140c — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Mechanical dedup onto an existing, gate-enforced seam plus round-trip tests that pin real writer output — right shape, no new surface, fully reversible.

[DESIGN-REVIEWED] 313c001

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 313c001c5d8b2dbfb1274f5beb4bc457febf140c — this comment is updated in place on each push.

Review details

The refactor is behavior-preserving: derived_agent_permissions computes Path(agent_filename).stem internally and applies the identical derived if derived is not None else {"rules": []} fallback, matching both removed inline sites exactly. The object typing is safe since allowed_tools_to_permissions fails closed on non-list. agent.py now has zero real kiro_crew.acp imports (the three remaining mentions are comments; the scanner is AST-based), so pruning the shrink-only baseline line to 0 is correct. Both call sites preserve their .get() vs [] access exactly.

No findings.

[OPUS-REVIEWED] 313c001

Verdict parsed from the review's SHA-scoped output markers for commit 313c001c5d8b2dbfb1274f5beb4bc457febf140c.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 313c001c5d8b2dbfb1274f5beb4bc457febf140c: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 313c001c5d8b2dbfb1274f5beb4bc457febf140c and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 313c001

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 313c001c5d8b2dbfb1274f5beb4bc457febf140c: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 313c001c5d8b2dbfb1274f5beb4bc457febf140c — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence checked. Composing the review now.

First-Principles-Verdict: PASS

Completes the migration #7238 left two-thirds undone and closes the fixture-blindness gap behind it; every item traces to a named defect and counted consumers.

What this change ships

Intent: finish #7513 — make the KAS derive-plus-fallback exist in exactly one place, and make the conductor probes' readers red when the real writers drift. This is a FIX.

1. Both remaining inline KAS derives now call the shared wrapper — justified, cause-level
2. Wrapper's allowed_tools annotation widened list → object — declared, forced by the substitution
3. agent.py pruned from the boundary baseline — mandated by the shrink-only gate itself
4. New textual guard pinning the fallback spelling to the wrapper — declared, guards the non-import half
5. New round-trip tests: probes classify what the real writers wrote (3 formats + negative) — justified, closes the false-GONE/double-dispatch blind spot

Counts I ran: derived_agent_permissions has 3 real consumers (agent.py:3291, agent.py:5158, agent.py:5371) — the generalization already earns its shape. Grep for the inline spelling if derived is not None else {"rules": []} in src/ finds only the wrapper (agent_sdk/drivers/acp.py:71): zero unfixed siblings, migration complete. kiro_crew.acp imports remaining in agent.py: 0, so the baseline prune is the gate's own rule, not a choice. The round-trip tests are not a second spelling of test_pipeline_conductor_agent.py's fixture tests — those pin script behavior over frozen inputs; these pin the writer contract (ConversationLog.append, _history_key_for/_safe_key, persist_token_record), a gap the fixtures structurally cannot cover. The prior review round already subtracted the guard's redundant import half; the surviving textual half covers the one slice the boundary gate exempts (boundary-internal trees), so it is not a duplicate.

[FIRST-PRINCIPLES-REVIEWED] 313c001