Excuses (5) every reflect Backlog item carries a handle - #555
Open
EdbertChan wants to merge 11 commits into
Open
Excuses (5) every reflect Backlog item carries a handle#555EdbertChan wants to merge 11 commits into
EdbertChan wants to merge 11 commits into
Conversation
…view claim: A reflect summary fails its check when any Backlog item lacks a task id, issue or PR id, or an explicit 'declined by user'. Review lane: policy Safety invariant: Accepted, Route-to-automate-me, and Rejected items are not checked by this rule and are unaffected. Effectiveness measurement: Script tests: a summary whose Backlog item names a land-stack no-op fix with no handle fails; the same item with an Invoker workflow id passes; a summary with only Accepted items passes; an unparseable input exits with a distinct unchecked code. Slice rationale: One check script and its tests; the reflect step that runs it is a separate commit in the same PR. Architectural effect: A reflect Backlog item cannot be parked without a durable pointer to where it will be done. Goal: Stop reflect findings from being named and then never landing. Motivation: A corpus reflect found a reflect pass that named an Invoker land-stack bug as a fix, asked 'Still waiting on your call: which items to apply', never got an answer, and the fix is still absent from origin/master. 367 reflect outputs held 672 Backlog items with no tracking. Alternative considerations: Auto-dispatching every Backlog item was set aside because process and worker items still need approval. A prose-only reminder was set aside because the pattern persisted with prose. Implementation details: Add engine/skills/reflect/scripts/check_backlog_handles.py. It takes a reflect summary as a file argument or stdin, finds the Backlog section, and exits 0 when every item has a handle, 1 with the offending items listed when any lacks one, and 2 when the input cannot be parsed (unchecked, never pass). Add its unittest module next to the existing reflect script tests. Non-goals: No change to token_audit.py, top_sessions.py, or reflect hooks. Leave the files of open PRs #482, #500 and #528 alone; if a change would overlap them, stop and report instead. Layer: domain Feature state: active Files: - engine/skills/reflect/scripts/check_backlog_handles.py - engine/skills/reflect/scripts/tests/test_check_backlog_handles.py Change types: - engine/skills/reflect/scripts/check_backlog_handles.py: create - engine/skills/reflect/scripts/tests/test_check_backlog_handles.py: create Acceptance criteria: - `python3 -m unittest discover -s engine/skills/reflect/scripts/tests -v` exits 0. - `python3 scripts/check_skill_file_refs.py` exits 0. - `python3 scripts/check_no_new_comments.py` exits 0. Exit code: 0 Invoker-Finalize-Id: 4f5887b4-1f94-4a5e-9be7-2b2f39b8caa2
…claim: In engine/skills/reflect/SKILL.md step 7 (Summarize), add one line requiring `python3 engine/skills/reflect/scripts/check_backlog_handles.py` to exit 0 on the summary before it is presented, and treating exit 2 as unchecked. Review lane: docs Safety invariant: Accepted, Route-to-automate-me, and Rejected items are not checked by this rule and are unaffected. Effectiveness measurement: Script tests: a summary whose Backlog item names a land-stack no-op fix with no handle fails; the same item with an Invoker workflow id passes; a summary with only Accepted items passes; an unparseable input exits with a distinct unchecked code. Slice rationale: The docs for this slice's code, as a separate commit in the same PR. Architectural effect: None beyond the prose describing the code change. Goal: Stop reflect findings from being named and then never landing. Motivation: A corpus reflect found a reflect pass that named an Invoker land-stack bug as a fix, asked 'Still waiting on your call: which items to apply', never got an answer, and the fix is still absent from origin/master. 367 reflect outputs held 672 Backlog items with no tracking. Alternative considerations: Auto-dispatching every Backlog item was set aside because process and worker items still need approval. A prose-only reminder was set aside because the pattern persisted with prose. Implementation details: In engine/skills/reflect/SKILL.md step 7 (Summarize), add one line requiring `python3 engine/skills/reflect/scripts/check_backlog_handles.py` to exit 0 on the summary before it is presented, and treating exit 2 as unchecked. Non-goals: No code, test, or install change. Leave the files of open PRs #482, #500 and #528 alone; if a change would overlap them, stop and report instead. Layer: docs Feature state: active Files: - engine/skills/reflect/SKILL.md Change types: - engine/skills/reflect/SKILL.md: modify Acceptance criteria: - `python3 scripts/check_skill_file_refs.py` exits 0. Exit code: 0 Invoker-Finalize-Id: fcf6766c-53cf-484f-888e-ede36b045a81
…iew claim: `python3 -m unittest discover -s engine/skills/reflect/scripts/tests -v` passes on the finished branch. Review lane: proof Safety invariant: Verification is read-only and alters no repository file. Effectiveness measurement: The command's exit code is the direct measurement. Slice rationale: One check per proof task. Architectural effect: None; verification only. Goal: Prove the slice. Motivation: Running the check is the proof. Alternative considerations: The full suite was not required because the slice touches one component with its own tests. Implementation details: Run `python3 -m unittest discover -s engine/skills/reflect/scripts/tests -v`. Non-goals: No mutations. Layer: app_regression Feature state: active Layer exception: allowed. Verification and the terminal scrub run after the docs commit so they check the final branch the PR will carry. Acceptance criteria: - The command exits 0. Exit code: 0 Invoker-Finalize-Id: 24031acf-11e9-4b8d-808d-5530499b53dd
…iew claim: `python3 scripts/check_skill_file_refs.py` passes on the finished branch. Review lane: proof Safety invariant: Verification is read-only and alters no repository file. Effectiveness measurement: The command's exit code is the direct measurement. Slice rationale: One check per proof task. Architectural effect: None; verification only. Goal: Prove the slice. Motivation: Running the check is the proof. Alternative considerations: The full suite was not required because the slice touches one component with its own tests. Implementation details: Run `python3 scripts/check_skill_file_refs.py`. Non-goals: No mutations. Layer: app_regression Feature state: active Layer exception: allowed. Verification and the terminal scrub run after the docs commit so they check the final branch the PR will carry. Acceptance criteria: - The command exits 0. Exit code: 0 Invoker-Finalize-Id: dba3123d-57af-46ec-b8af-eb6481140bce
…iew claim: `python3 scripts/check_no_new_comments.py` passes on the finished branch. Review lane: proof Safety invariant: Verification is read-only and alters no repository file. Effectiveness measurement: The command's exit code is the direct measurement. Slice rationale: One check per proof task. Architectural effect: None; verification only. Goal: Prove the slice. Motivation: Running the check is the proof. Alternative considerations: The full suite was not required because the slice touches one component with its own tests. Implementation details: Run `python3 scripts/check_no_new_comments.py`. Non-goals: No mutations. Layer: app_regression Feature state: active Layer exception: allowed. Verification and the terminal scrub run after the docs commit so they check the final branch the PR will carry. Acceptance criteria: - The command exits 0. Exit code: 0 Invoker-Finalize-Id: da3fd8eb-1d95-41db-b63c-272e6fd3e660
…ry-handle-2/g0.t0.a-a4d1297d3-b2f9b884
…ry-handle-3/g0.t0.a-a4d44101b-08e4c2c0
…No ephemeral inter-task handoff files remain in the worktree before the merge gate. Review lane: cleanup Safety invariant: The scrub script only checks for known handoff artifact names and never touches source, tests, or other repository files. Effectiveness measurement: The script exits non-zero if any handoff artifact remains. Slice rationale: Required terminal scrub for every implementation workflow. Architectural effect: None; hygiene only. Goal: Leave the branch free of handoff artifacts. Motivation: Handoff files must not reach the PR. Alternative considerations: Manual cleanup was set aside as non-deterministic. Implementation details: Run scripts/scrub-handoff-artifacts.sh. Non-goals: No product edits. Layer: app_regression Feature state: active Layer exception: allowed. Verification and the terminal scrub run after the docs commit so they check the final branch the PR will carry. Acceptance criteria: - The command exits 0. Exit code: 0 Invoker-Finalize-Id: 5e755152-9e1b-47d7-b85b-dfa428399625
…a16c5858c-91d15cf7 — Review claim: No ephemeral inter-task handoff files remain in the worktree before the merge gate. Review lane: cleanup Safety invariant: The scrub script only checks for known handoff artifact names and never touches source, tests, or other repository files. Effectiveness measurement: The script exits non-zero if any handoff artifact remains. Slice rationale: Required terminal scrub for every implementation workflow. Architectural effect: None; hygiene only. Goal: Leave the branch free of handoff artifacts. Motivation: Handoff files must not reach the PR. Alternative considerations: Manual cleanup was set aside as non-deterministic. Implementation details: Run scripts/scrub-handoff-artifacts.sh. Non-goals: No product edits. Layer: app_regression Feature state: active Layer exception: allowed. Verification and the terminal scrub run after the docs commit so they check the final branch the PR will carry. Acceptance criteria: - The command exits 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review summaries now require each pending follow-up to point to tracked work.
This prevents findings from being named and then forgotten without a durable next step.
Accepted, routed, and rejected findings keep their existing treatment.
Review Claim
Approve the check that requires every pending follow-up to carry a durable tracking handle.
Review Lane
behavior
Review Unit
engine-runtime
Safety Invariant
Accepted, Route-to-automate-me, and Rejected items are not checked and remain unaffected.
Slice Rationale
This is one review claim: the check, its focused tests, and the instruction that runs it ship together.
The code and documentation are separate commits because the documentation has no meaning without the check.
Non-goals
Architecture
Before
graph TD A["Review builds a summary"] --> B["Summary is presented"]After
graph TD A["Review builds a summary"] --> B["Backlog handle check"] B -->|"exit 0"| C["Summary is presented"] B -->|"exit 1 or 2"| D["Summary is blocked or unchecked"]Test Plan
Test Plan
python3 -m unittest discover -s engine/skills/reflect/scripts/tests -v— passed.python3 scripts/check_skill_file_refs.py— passed.python3 scripts/check_no_new_comments.py— passed.bash scripts/scrub-handoff-artifacts.sh— passed; no ephemeral handoff files remain.python3 scripts/check_skill_test_coverage.py --base main --head HEAD— passed.python3 scripts/check_skills_three_harnesses.py— passed.python3 scripts/check_ecosystem_boundaries.py— passed.python3 engine/skills/make-pr/scripts/preflight.py --base main --dry-run— passed; one engine-runtime review unit.Revert Plan
Revert Plan
git revert <sha>.