feat(operator-surface): cut over shared operator surfaces#192
Conversation
Eval Smoke Results✅ Smoke evals clean — 0 regressions, 5 improvements
Posted by Specwright eval-smoke workflow. This comment is updated on each push. |
Code Review — PR #192: feat(operator-surface): cut over shared operator surfacesOverall this is solid work. The centralization goal is achieved cleanly, and the regression harness (Python + shell) gives good surface coverage. Several issues worth addressing before merge, ranging from a correctness risk to minor hygiene. 🔴 Issues to fix1. Dead branch in The first two branches are byte-for-byte identical — both return // Lines 13-19 — first guard is dead
if (stateInfo?.layout === 'shared' && stateInfo?.repoStateRoot) {
return join(stateInfo.repoStateRoot, 'work', work.workId);
}
if (stateInfo?.repoStateRoot) {
return join(stateInfo.repoStateRoot, 'work', work.workId); // same
}Merge into 2. Silent error swallowing in A bare 🟡 Issues to address3. Capitalization inconsistency in Every other bullet point in this section starts with a capital letter ( 4. No test coverage for the The new 5. Both test harnesses hardcode 🔵 Nits6.
7.
✅ Positive callouts
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109db1c863
ℹ️ 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".
Summary
This unit cuts the primary operator surfaces over to a shared closeout and approval summary model, so Claude recovery hooks, the Opencode plugin, and status/setup/health guidance all surface the same human-useful state instead of requiring hidden context.
Approval Lineage
design: APPROVED via/sw-planand still current against the design artifact set.unit-spec(03-operator-surface-cutover): STALE (artifact-set-changed) becauseplan.mdgained## As-Built Notesafter/sw-buildcaptured approval.What Changed
adapters/shared/specwright-operator-surface.mjsso Claude and Opencode render the same closeout and approval summary.adapters/claude-code/hooks/session-start.mjsandadapters/opencode/plugin.tsover to the shared state/operator-surface helpers without dropping lock, ownership, shipping, or continuation warnings.build/build.shandadapters/opencode/package.jsonso the shared helper ships with the Opencode adapter.core/skills/sw-status/SKILL.md,core/skills/sw-init/SKILL.md,core/skills/sw-guard/SKILL.md, andcore/skills/sw-doctor/SKILL.mdaround closeout visibility, approval reason visibility, and explicitgit.runtime.*guidance.evals/tests/test_operator_surface_visibility.py,tests/test-hooks.sh,tests/test-opencode-plugin.sh, andtests/test-config-validation-visibility-docs.sh.Why The Agent Implemented It This Way
review-packet presencewording insw-statusbecause the full configured test command proved older publication-mode regressions still depend on that phrase.pytestpath, not only through shell-only proofs.Acceptance Criteria
AC-1PASSImplementation:
adapters/claude-code/hooks/session-start.mjs:24,adapters/opencode/plugin.ts:186Evidence:
TestSessionStartSurface.test_session_start_replays_shared_digest_and_approval_summaryinevals/tests/test_operator_surface_visibility.py:234, shell hook coverage intests/test-hooks.sh:565, Opencode wiring assertions intests/test-opencode-plugin.sh:128AC-2PASSImplementation:
adapters/shared/specwright-operator-surface.mjs:10,adapters/shared/specwright-operator-surface.mjs:95,adapters/claude-code/hooks/session-start.mjs:80Evidence:
TestSessionStartSurface.test_session_start_names_missing_closeout_and_approvalinevals/tests/test_operator_surface_visibility.py:223, shell hook coverage intests/test-hooks.sh:545AC-3PASSImplementation:
core/skills/sw-status/SKILL.md:46Evidence:
TestOperatorSurfaceContracts.test_status_skill_keeps_approval_reason_and_closeout_visibility_togetherinevals/tests/test_operator_surface_visibility.py:261, doc regression checks intests/test-config-validation-visibility-docs.sh:79AC-4PASSImplementation:
core/skills/sw-init/SKILL.md:124,core/skills/sw-guard/SKILL.md:68,core/skills/sw-guard/SKILL.md:97Evidence:
TestOperatorSurfaceContracts.test_runtime_guidance_uses_runtime_keys_without_collapsing_into_work_artifactsinevals/tests/test_operator_surface_visibility.py:266, doc regression checks intests/test-config-validation-visibility-docs.sh:57AC-5PASSImplementation:
core/skills/sw-doctor/SKILL.md:65,core/skills/sw-doctor/SKILL.md:83,core/skills/sw-doctor/SKILL.md:94Evidence:
TestOperatorSurfaceContracts.test_runtime_guidance_uses_runtime_keys_without_collapsing_into_work_artifactsinevals/tests/test_operator_surface_visibility.py:266, doc regression checks intests/test-config-validation-visibility-docs.sh:72AC-6PASSImplementation:
evals/tests/test_operator_surface_visibility.py:223,tests/test-hooks.sh:545,tests/test-opencode-plugin.sh:114,tests/test-config-validation-visibility-docs.sh:57Evidence:
TestSessionStartSurface.test_session_start_names_missing_closeout_and_approvalinevals/tests/test_operator_surface_visibility.py:223,TestSessionStartSurface.test_session_start_replays_shared_digest_and_approval_summaryinevals/tests/test_operator_surface_visibility.py:234, plus the recordedcommands.testPASS for the full validation pathSpec Conformance
AC-1throughAC-6all passed in the canonical compliance matrix for this unit.workflow-legibility-devex, so behavioralIC-B*criteria were intentionally not evaluated in this unit-level ship step.Gate Summary
Remaining Attention
unit-specapproval lineage is stale and should be refreshed or consciously accepted as stale review context..specwright/AUDIT.mdis currently modified locally but is outside this unit's file-change map and is not part of this PR.Evidence
This repository uses clone-local work-artifact mode, so the reviewer-usable approval, rationale, conformance, and remaining-attention summaries are inlined above rather than depending on local-only artifact links.