feat(support-surface): cut over adapters and guidance#205
feat(support-surface): cut over adapters and guidance#205
Conversation
Align status-card handoff guidance with workflow state, treat skipped gates as surfaced results, tighten approval lookup scope, and harden the shared operator-surface regressions.
…upport-surface-cutover # Conflicts: # adapters/shared/specwright-operator-surface.mjs
Eval Smoke Results❌ Smoke evals regressed — 2 regressions, 4 improvements
Posted by Specwright eval-smoke workflow. This comment is updated on each push. |
Code Review — PR #205: feat(support-surface): cut over adapters and guidanceSummaryThis PR does what it sets out to do well: all three adapter session-start hooks now share a single Four issues worth addressing before merge, roughly ordered by impact: [Bug] Codex adapter outer
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -50,23 +47,12 @@ try { | |||
| } | |||
| } | |||
There was a problem hiding this comment.
The Claude Code hook parses ## Correction Summary out of the continuation snapshot and appends it as a separate --- Quality Corrections --- block so the model gets targeted guidance on what to avoid. This hook just includes the raw snapshot. If the omission is intentional (Codex doesn't need this guidance), a comment here would help. If it's an oversight, the extraction logic from the CC hook should be replicated.
| return lines; | ||
| } | ||
|
|
||
| export function renderOperationalWarningLines(options = {}) { |
There was a problem hiding this comment.
renderOperationalWarningLines is exported but has no external consumer — it's only called internally by renderWorkInProgressSummary on line 166. The gate-wiring advisory in the PR body already flags this. Either drop the export keyword or add a comment marking it as an intentional extension point, otherwise the public API surface is misleadingly wide.
| return warnings | ||
| .filter((warning) => { | ||
| const code = normalizeString(warning?.code) ?? ''; | ||
| if (code.startsWith('approval-')) { | ||
| return false; | ||
| } | ||
|
|
||
| return !['missing-closeout', 'branch-mismatch'].includes(code); | ||
| }) |
There was a problem hiding this comment.
The filter suppresses approval-*, missing-closeout, and branch-mismatch codes because those have dedicated rendering sections (formatApprovalStatusLine, formatCloseoutLines, formatBranchStatusLine). This is the right call, but without a comment here it reads as arbitrary exclusion. A future maintainer adding a new code to specwright-status-card.mjs won't know these codes are silently dropped.
Worth adding a one-liner:
| return warnings | |
| .filter((warning) => { | |
| const code = normalizeString(warning?.code) ?? ''; | |
| if (code.startsWith('approval-')) { | |
| return false; | |
| } | |
| return !['missing-closeout', 'branch-mismatch'].includes(code); | |
| }) | |
| .filter((warning) => { | |
| // Approval, closeout, and branch warnings have dedicated rendering sections above; | |
| // suppress them here to avoid duplication. | |
| const code = normalizeString(warning?.code) ?? ''; | |
| if (code.startsWith('approval-')) { | |
| return false; | |
| } | |
| return !['missing-closeout', 'branch-mismatch'].includes(code); | |
| }) |
| }) | ||
| .map((warning) => normalizeString(warning?.summary)) | ||
| .filter(Boolean) | ||
| .slice(0, 2) |
There was a problem hiding this comment.
.slice(0, 2) silently drops any third-or-later warning with no indication to the caller or the user. There's no test for this boundary and no comment explaining why 2 was chosen. If the intent is to keep the session-start surface terse, the cap is reasonable — but it should be a named constant and tested (e.g. assert that a 3-warning fixture only emits 2 WARNING: lines).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2cd092f90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const continuationPath = stateInfo.continuationPath; | ||
| const work = normalizeActiveWork(stateInfo); | ||
| const ownerConflict = findSelectedWorkOwnerConflict(stateInfo); | ||
| const operatorSummary = loadOperatorSurfaceSummary(stateInfo, work); |
There was a problem hiding this comment.
Move this summary construction below the shipped/abandoned early-return check. As written, loadOperatorSurfaceSummary runs even when the hook should exit with no output, which still triggers status-card reads (approvals.md, closeout files). If those files are malformed, summarizeApproval logs an error to stderr, so Codex surfaces startup warnings for work that is already inactive.
Useful? React with 👍 / 👎.
1 participant
Summary
Approval Lineage
STALE (artifact-set-changed), originally approved via/sw-plan.04-adapter-and-support-surface-cutover:STALE (artifact-set-changed), originally approved via/sw-build.freshagainstorigin/mainat ship time.What Changed
renderWorkInProgressSummary/loadOperatorSurfaceSummarypath.sw-status,sw-doctor,sw-init, andsw-guardnow use the same operator vocabulary as the adapters for runtime roots, adoption, and next-step guidance.AGENTS.md/CLAUDE.mdnow teach the same.specwright-local/,git-admin,/sw-adopt, and/sw-statusstory as the runtime surfaces.Why The Agent Implemented It This Way
tests/test-support-surface-cutover-docs.shwas added to the proof set because it is the canonical shell regression that the Claude build harness already executes for support-surface coverage.Acceptance Criteria
Evidence:
adapters/shared/specwright-operator-surface.mjs,adapters/claude-code/hooks/session-start.mjs,adapters/codex/hooks/session-start.mjs, andadapters/opencode/plugin.tsnow share the operator-surface path; covered byevals/tests/test_operator_surface_visibility.py.Evidence:
core/skills/sw-status/SKILL.md,core/skills/sw-doctor/SKILL.md,core/skills/sw-init/SKILL.md, andcore/skills/sw-guard/SKILL.mdnow carry the same runtime-root and adoption vocabulary; covered byevals/tests/test_support_surface_status_card.py.Evidence:
AGENTS.md,CLAUDE.md,adapters/claude-code/CLAUDE.md, and the Codex/Opencode command docs now align on.specwright-local/,git-admin,/sw-adopt, and/sw-status; covered byevals/tests/test_support_surface_status_card.py.Evidence: the shared operator surface and adapter hooks degrade gracefully when closeout or migration state is missing; covered by
evals/tests/test_operator_surface_visibility.py.Evidence:
tests/test-support-surface-cutover-docs.sh,tests/test-claude-code-build.sh, andevals/tests/test_support_surface_status_card.pyfail closed on source/build drift and coverage-marker regression.Spec Conformance
gate-specrun.Gate Summary
bash build/build.shpassed; branch freshness wasfreshat verify and ship timepython -m pytest evals/tests/ -v && bash tests/test-claude-code-build.shpassedrenderOperationalWarningLinesis exported but has no external consumerRemaining Attention
04-adapter-and-support-surface-cutoverif you want the audit trail to match the current unit artifact set.gate-wiringfound one advisory issue:renderOperationalWarningLinesis exported fromadapters/shared/specwright-operator-surface.mjsbut has no external consumer. If it does not need to be part of the public helper surface, make it local to the module.BLOCKfinding.Evidence Links
review-packet.mdand gate evidence for this unit are clone-local, so this PR body inlines the reviewer-usable summary instead of linking to local-only.git/specwright/...artifacts.adapters/shared/specwright-operator-surface.mjsadapters/claude-code/hooks/session-start.mjsadapters/codex/hooks/session-start.mjsadapters/opencode/plugin.tscore/skills/sw-status/SKILL.mdcore/skills/sw-doctor/SKILL.mdcore/skills/sw-init/SKILL.mdcore/skills/sw-guard/SKILL.mdAGENTS.mdCLAUDE.mdadapters/claude-code/CLAUDE.mdevals/tests/test_operator_surface_visibility.pyevals/tests/test_support_surface_status_card.pytests/test-support-surface-cutover-docs.shtests/test-claude-code-build.sh