Skip to content

feat(dashboard): say what a Codex install still cannot do - #8684

Merged
chenmingwei23 merged 1 commit into
mainfrom
feat/codex-backend-probe
Sep 5, 2026
Merged

feat(dashboard): say what a Codex install still cannot do#8684
chenmingwei23 merged 1 commit into
mainfrom
feat/codex-backend-probe

Conversation

@iamwhatever

@iamwhatever iamwhatever commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Developer > Agent Backend tells an operator whether a harness is installed, and for Codex that verdict answers less than it looks like it does. codex-acp ships its own Codex binary, so installed really is the whole install fact — and a session can still die on its first turn for want of a credential, with nothing on the page having said which. The remedy has two branches (Codex's own sign-in, or a model_provider in ~/.codex/config.toml when the credentials come from somewhere else entirely), so even a failure message would not have told the operator which one they wanted.

Second, unrelated to Codex but on the same row: KAS sorted under k by policy_id, which put it behind every adapter whose id happens to start earlier — claude, codex.

Why it matters

Codex ships selectable (it is in BASELINE_SELECTABLE_BACKENDS), so this is a live path, not a dormant seam: an operator picks Codex, the chip says installed, the session dies, and the panel never named what was missing. That is the same failure the install probe was added to close, one layer up.

The order matters because KAS is not an adapter — it is kiro-cli's own ACP relay, resolved from the same binary and sharing kiro's install verdict (_probe_kas delegates to _probe_kiro). Splitting the two harnesses that are really one install, with third-party adapters between them, reads to the operator as a rank rather than an alphabet.

What changed (motivation → approach → change)

The credential line is a standing caveat, deliberately NOT a probe line. Reading ~/.codex/auth.json or config.toml would make it a measurement, and a measurement in this panel gates the control: missing disables the chip. The checkable paths are not the only ones that authenticate a Codex — an ambient key, a relocated CODEX_HOME, a CODEX_ACP_BIN adapter carrying its own configuration — and each of those is an operator whose switch would have been disabled while they were already signed in. agent_sdk/backend_install.py names that direction as the more expensive mistake, so the line claims no measurement and says so out loud (Neither is checked here — the adapter reads them, not KiroCrew) rather than letting silence read as a green light. It renders through the existing caveat seam, beside Claude's, so nothing about the probe payload or the wire contract changes.

CODEX is named for the caveat alone and stays out of NAMED: this frontend has no translated label for it, so the chip keeps the server's policy_id rather than rendering an empty chip.

The order is a comparator pin, not a new list. KIRO first (default and floor), then KAS, then the existing byte order over policy_id for the tail — the byte order is kept for the reason it was chosen, that it reproduces the order the probe endpoint already returns and cannot reshuffle per reader locale.

The English string lives in en.manual.json, not en.json: website/docs/i18n-catalog.md makes en.json the codemod's wholesale output, and hand-authored English with no source literal belongs in the manual catalog the codemod never touches. All 11 target catalogs carry the key and en-XA.json is regenerated.

Tests

website/src/test/AgentBackendTab.test.tsx, four cases:

  • both remedy branches reach the reader — the caveat text and ~/.codex/config.toml render on the Codex row.
  • the disclaimer is presentNeither is checked here, which is what stops silence reading as a verified green light.
  • the caveat does not leak to the other harnesses — Kiro CLI and KAS authenticate through Crew's own identity store, so telling their reader to finish a separate sign-in would be false.
  • the chip orderKiro CLI, KAS (kiro-agent), Claude Code, codex. Revert-verified: deleting the two-line KAS pin fails this case and only this case.

Manual verification

website/scripts/capture-agent-backend-probe.mjs gains the two Codex scenes and renders the real built SPA behind fixture API payloads; each shot waits for the literal caveat text (and for codex-acp in the missing scene) before firing, so a frame cannot be taken pre-hydration. Its four existing frames are re-shot because the order change moves KAS in every one of them.

Local gate sweep on this head: all 47 profile gates run, 45 green. Two reds are inherited and reproduce on a pristine base — the backend suite (this diff contains zero Python files; the failures are this host's /local/home symlink spelling, AF_UNIX path too long on a long worktree path, and ownership 403s) and scrub-lint (/home/tést in test/test_atomic_write_named_duplicates.py, present verbatim at origin/main).

Screenshots / video

Codex installed — the install verdict is positive and the caveat is what the row adds:

Agent Backend, Codex installed

Adapter missing — the two lines together, and only the first one disables the chip:

Agent Backend, codex-acp missing

The four pre-existing scenes, re-shot for the new order

local

policy-denied

check failed

restart required

Related Issues

no linked issue: this closes a gap the Codex onboarding table already named as open, rather than a filed report. docs/system-specs/modules/harness-onboarding.md's Codex table records why stage 5 stops at the binary, so the next reader of that table finds the decision rather than re-deriving it.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Two changes to the Agent Backend switch, both about what the row tells an
operator who is choosing between harnesses.

## Being installed is not being signed in

`_probe_codex` covers the binary question completely — the adapter ships its
own Codex binary, so `installed` really is the whole install fact — and a
session can still die on its first turn for want of a credential, with nothing
on the page having said which. The remedy also has two branches, so a failure
message alone would not have told the operator which one they wanted: finish
Codex's own sign-in, or name a `model_provider` in `~/.codex/config.toml` when
the credentials come from somewhere else entirely.

Stated as a standing caveat, next to Claude's, and deliberately NOT probed.
Reading those files would make it a measurement, and a measurement here gates
the control: `missing` disables the chip, and the checkable paths are not the
only ones that authenticate a Codex — an ambient key, a relocated
`CODEX_HOME`, a `CODEX_ACP_BIN` adapter carrying its own configuration. Each
of those is an operator whose switch we would have disabled while they were
already signed in, which `backend_install` itself names as the more expensive
mistake. A sentence that claims no measurement cannot be wrong in that
direction, and it says so out loud rather than letting silence read as a green
light.

`CODEX` is named for the caveat alone and stays out of `NAMED`: this frontend
has no translated label for it, so the chip keeps the server's `policy_id`
rather than rendering empty.

## KAS sits beside Kiro CLI, not behind the adapters

KAS is not an adapter: it is kiro-cli's own ACP relay, resolved from the same
binary and sharing kiro's install verdict (`_probe_kas` delegates to
`_probe_kiro`). Sorted by `policy_id` alone it fell on 'k', which put the two
harnesses that are really one install either side of every adapter whose id
happens to start earlier — `claude`, `codex` — and reads to the operator as a
rank rather than an alphabet. The comparator now pins it second, ahead of the
byte order that still owns the tail.

## Tests

Four cases pin the disclosure and the order: both remedy branches, the
disclaimer, its absence on the harnesses that authenticate through Crew's own
identity store, and the chip sequence (revert-verified — dropping the KAS pin
fails it). The onboarding spec's Codex table records why stage 5 stops at the
binary, and the panel's capture harness gains the two Codex scenes, which also
re-shoots its four existing frames because the order moved KAS in every one.
@iamwhatever
iamwhatever requested a review from a team September 5, 2026 06:49
@iamwhatever
iamwhatever requested a review from a team as a code owner September 5, 2026 06:49
@iamwhatever
iamwhatever requested a review from pepmach September 5, 2026 06:49
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 0b2ae35911bb4979c022de742110866e81cee3c1 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0b2ae35

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

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

A real first-turn failure gets named in the panel; the deliberate non-probe (caveat over measurement) is the right trade and is documented where the next reader looks.

[DESIGN-REVIEWED] 0b2ae35

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

The panel discloses Codex honestly, but refuses to name it: a lowercase wire id codex sits beside "Claude Code", under a header that says the option doesn't exist.

Watch

  • The page description denies the fourth chip. The subtitle "Which agent new sessions start — Kiro CLI, Claude Code, or KAS" (en.manual.json) still enumerates three harnesses while both new screenshots render four chips; a first-time user sees an option the page's own summary says isn't there. Every render of the panel, comprehension friction, persists until edited. Fix: drop the enumeration or add Codex to it.
  • Raw policy_id as the display name. The diff keeps CODEX out of NAMED ("no translated label for Codex"), so the chip and detail row read lowercase codex next to proper names — it reads as a dev artifact, not a product option. The PR itself adds a translated string to all 12 locales, so a "Codex" name entry costs the same one line. Fix: add the NAMED entry with a translated label (icon optional).

Suggestions

  • In codex_signs_in_separately, demote the parenthetical "(CODEX_HOME moves that folder)" — it's nice-to-know context inside the one sentence that carries the remedy; move it after the em-dash clause or cut it.

[UX-REVIEWED] 0b2ae35

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 0b2ae35911bb4979c022de742110866e81cee3c1 — 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: codex is genuinely in BASELINE_SELECTABLE_BACKENDS (src/kiro_crew/acp_backends.py:117), _probe_kas really delegates to the kiro probe (backend_install.py:138), the caveat seam pre-exists for Claude, and the manual-catalog placement matches website/docs/i18n-catalog.md. The one open thread: Claude Code's probe also checks only binary presence (backend_install.py:168), and Claude also authenticates outside Crew — a sibling of the same root cause left without a credential caveat.

First-Principles-Verdict: CONCERNS

The Codex caveat is derived and earns its place; the KAS reorder is a declared-unrelated move justified only by grouping logic, with no named operator who misread the old order.

What this change ships

Intent: tell an operator who installed Codex that a session can still die unsigned-in, and which of two remedies applies. ADDITION.

  1. Codex row gains a standing credential caveat naming both remedies — justified
  2. Caveat says "Neither is checked here" instead of probing — justified (fail-open direction named in backend_install.py)
  3. New i18n key in en.manual.json plus 11 catalogs and en-XA — mandated by i18n invariant
  4. KAS chip moves from fourth to second — declared unrelated, rides along, unjustified move
  5. Frontend CODEX constant, kept out of NAMED — one consumer, justified
  6. Capture script gains two Codex scenes (6 frames) — justified, declared
  7. Four existing screenshots re-shot — consequence of item 4 alone
  8. harness-onboarding.md stage-5 row records the deliberate non-probe — spec-same-commit invariant

Watch

  • The KAS move's only support is "reads to the operator as a rank rather than an alphabet" — no report, misclick, or complaint is named, and the shared-install fact (_probe_kas delegates, backend_install.py:138) explains why adjacency is defensible, not who was failing without it. Its zero option costs nobody anything observable; it also forced re-shooting all four existing frames. Kept at CONCERNS because the shared-install rationale is a real fact, not pure symmetry.
  • Sibling left unfixed, count 1: _probe_claude also verifies only presence of the claude CLI (backend_install.py:168–174), and Claude Code likewise authenticates outside Crew (docs/system-specs/features/claude-code-provider.md:311, "it has to carry credentials across (or CC cannot authenticate at all)"), yet its row's caveat covers permissions only. The Codex mechanism is stronger (the adapter bundles the binary, so the panel's own install command manufactures the trap), so this is accepted-and-deferred, not a demand.

Subtractions

  • Drop the KAS comparator pin (the two if (a === KAS) lines in AgentBackendTab.tsx:293–294, its test, and the four re-shot frames) and let it return with a named observation of an operator misled by the byte order; the Codex caveat stands alone without it.

[FIRST-PRINCIPLES-REVIEWED] 0b2ae35

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 0b2ae35911bb4979c022de742110866e81cee3c1 — this comment is updated in place on each push.

Review details

Everything verifies: the en.manual key resolves to pages.developer.agentBackendTab.codex_signs_in_separately (matching the code), all 13 required catalogs carry the key (en.json is the generated base merged with en.manual), the KAS-pinning comparator is a consistent total order, and the caveat function gates correctly on CODEX. No security boundary, credential path, or command construction is touched. The candidate pass found nothing, and no groundable Step 2 finding surfaces.

No findings.

[OPUS-REVIEWED] 0b2ae35

Verdict parsed from the review's SHA-scoped output markers for commit 0b2ae35911bb4979c022de742110866e81cee3c1.

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

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 5, 2026
@chenmingwei23
chenmingwei23 merged commit 6d1b517 into main Sep 5, 2026
92 of 99 checks passed
@chenmingwei23
chenmingwei23 deleted the feat/codex-backend-probe branch September 5, 2026 07:49
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 5, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Dispositions for the UX Review CONCERNS on 0b2ae35911bb. The PR merged as 6d1b517 before these were posted, so two of the three are follow-up work rather than in-PR changes.

  • The page description denies the fourth chipaccepted, follow-up. Legitimate and pre-existing: developerPage.tabs.agentBackend.description has enumerated three harnesses since Codex shipped selectable in feat(acp): wire Codex in behind an enforced tool-permission route #7963, so the subtitle contradicted the control under it before this PR and the new screenshots only made it visible. The fix is written and sitting on a branch — the enumeration is DROPPED rather than extended to four, because a list in prose rots on every harness that lands and the chips below it are already the list ("Which agent new sessions start with", plus the same clause removed from the other 11 catalogs and en-XA regenerated). It will land as its own PR.

  • Raw policy_id as the display nameneeds a decision. The rule cuts both ways and the repo states both halves: NAME's own comment says a core agent that ships selectable earns a translated entry (Codex now does), while AgentBackendTab.test.tsx's "offers an agent this frontend has no name for, under its policy id" case uses codex as its example of exactly that. Adding the label therefore also means moving that test onto a fictional registered id — which is arguably more faithful to what it is testing, but it rewrites a pinned contract to change a product name, so it is the maintainer's call rather than a reviewer-driven edit. Put to the maintainer; not filed as an issue.

  • Demote the (CODEX_HOME moves that folder) parentheticalrebutted (disproportional). It reads as nice-to-know but it is load-bearing: without it the sentence names ~/.codex/config.toml as the path, which is simply the wrong file for anyone whose CODEX_HOME points elsewhere — and that operator is precisely the one the caveat exists for, since their credential is the one the panel cannot see. Moving it after the em-dash clause would attach it to "the adapter reads them", which is a different claim. Kept inside the remedy sentence, where the qualification applies.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Dispositions for the First Principles CONCERNS on 0b2ae35911bb.

  • Drop the KAS comparator pin — no named operator misread the old orderrebutted. The maintainer requested the move directly, in the session that produced this PR ("同时把KAS挪到第二位"), after seeing the four-chip panel. That is the named observation the finding says is missing; it arrived as a maintainer instruction rather than a bug report, which is why the diff cites the shared-install fact (_probe_kas delegates to _probe_kiro) as the reason the request is right rather than as the reason to invent it. The re-shot frames are not a cost of the move so much as a consequence of keeping committed evidence true: a frame that shows the old order would misdescribe the code it documents.

  • Sibling left unfixed: _probe_claude also verifies only presenceaccepted-and-deferred#8698 (deferred-finding, assigned, Due: 2026-10-05). The finding is correct and its own verdict is the right one: Claude Code authenticates outside Crew too, so the same gap exists on that row. It is deferred rather than fixed here because the Codex mechanism is strictly stronger — codex-acp bundles its own Codex binary, so the panel's own install command manufactures the trap, while an operator who installed the claude CLI has already been through its sign-in. Closing it needs a claim about Claude's auth paths that nothing in this diff established, plus a second sentence in 12 catalogs. The issue records the rule any fix must follow: state the remedy, claim no measurement, and do not probe the credential file.

@bolichen97 bolichen97 mentioned this pull request Sep 8, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants