Repository navigation
spec: sprint-close Expected Behavior matches the warn-only script - #486
Conversation
The sprint-execution Expected Behavior claimed `sprint-close.sh` "runs only when every Plan item is `[x]` or explicitly struck in Progress". The script never enforced that: it prints `Warning: N todo, M in-flight items remaining` and closes anyway. A shipped sprint file closed at `2/9 tasks completed`, so the spec was describing a rail that does not exist. The spec moves, not the script. Whether an item is "explicitly struck in Progress" is prose the session writes and no deterministic check can read, so a script-side gate could only count checkboxes and would refuse exactly the legitimate close the precedent shows. The irreversible part -- `status: completed` never flips back -- is already a Hard Constraint, and adding a refusal plus a `--force` escape hatch would be a new rail against the charter's subtraction direction. The bullet now states what one successful run does (flip to `status: completed`, append `N/M tasks completed`), that remaining `[ ]` / `[~]` items only warn, and that deciding to finish, strike, or carry them is the session's call before the close. Decision row appended 2026-09-20 (append-only table, no existing row touched). No other file claimed the refusal. Verified: node --test 188 tests / 187 pass / 0 fail / 1 skipped; smoke 167 passed / 0 failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9gZYk7FcYSLAZBBe7JXvZ
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Astra round 1: the spec allowed strike-or-carry closes while the SKILL.md Complete rail still said the sprint stays open until its Plan is done. Both now say the same thing; sprint-close.sh is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9gZYk7FcYSLAZBBe7JXvZ
|
Independent review round 1 (GPT-6 Astra via codex exec): CHANGES REQUESTED — SKILL.md Complete rail still said the sprint stays open until its Plan is done, contradicting the new spec bullet. Fixed: the rail now states the same close condition (every item |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0ab9355e2
ℹ️ 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".
| - No two sprint files with `status: active` declare overlapping scope — overlap fails loud through the one shared `scopesOverlap` predicate (`component:` equality or `scope:` path-prefix collision; surfaced by `sprint-init` refusal, `sprint-state` `OVERLAPPING_TRACKS`, and the doctor's `Active tracks overlap on scope` verdict). Disjoint-scope tracks coexist as a portfolio; a single active track behaves exactly as before; once more than one track is active, any track without a declared axis cannot be proven disjoint and surfaces an informational doctor warning. | ||
| - Every `[~]` line carries a PR or branch pointer in-line — never an unmoored `[~]`. | ||
| - One successful `sprint-close.sh` invocation flips the sprint to `status: completed` and appends final Progress; it runs only when every Plan item is `[x]` or explicitly struck in Progress. | ||
| - One successful `sprint-close.sh` invocation flips the sprint to `status: completed` and appends the final Progress entry (`N/M tasks completed`); remaining `[ ]` / `[~]` items only print a warning and never block the close, so before closing the session decides whether to finish them, strike them with a Progress entry, or carry them to the next sprint. |
There was a problem hiding this comment.
Align the live close contracts with warn-only closure
This amendment permits closing with unfinished Plan items, but the live execution contract still says “the sprint stays open until its Plan is done” in skills/dev-backlog/SKILL.md:122, while spec/system-map.md:38 says to close a sprint only when its whole Plan is done. Agents consulting these authoritative surfaces will therefore apply conflicting completion rules; update them in the same change, or retain the previous capability behavior, so sprint closure has one consistent contract.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
|
Independent review round 2 (GPT-6 Astra): LGTM. Non-blocking: PR body claimed two files / no behaviour change; body updated. |
Summary
spec/capabilities.md(sprint-execution → Expected Behaviors) claimed:skills/dev-backlog/scripts/sprint-close.shhas never enforced that. It counts checkboxes, printsWarning: $CB_TODO todo, $CB_IN_FLIGHT in-flight items remaining, and proceeds to flipstatus: completedand append the Progress entry. A shipped sprint file under.dev-backlog/sprints/records2026-09-17: Sprint closed. 2/9 tasks completed., so the claimed rail was contradicted in this repo's own history.This PR moves the spec text to the truth. Three files:
spec/capabilities.md— the one Expected Behavior bullet rewritten; one decision row appended (2026-09-20). The Decisions table stays append-only; no existing row was edited or deleted.CHANGELOG.md— one line at the top of## [Unreleased]→### Changed.No script behaviour change. The session's close condition in the SKILL.md Complete rail is restated the same way (review round 1).
New bullet
Why the spec moved rather than the script
2/9precedent shows — a sprint whose remaining items were deliberately carried, not forgotten.status: completedback toactive" is an existing Hard Constraint in the same capability. That is the part that cannot be undone; an unchecked box at close is not.--forcewould be a new rail. Adding both runs against the charter's subtraction direction, and--forceflags get typed reflexively — the gate would cost upkeep and buy nothing.The judgement stays with the session, which is where it already lived: finish the item, strike it with a Progress entry, or carry it to the next sprint.
Scope check
spec/,skills/,README.md, andCLAUDE.mdwere grepped for other sentences claiming sprint-close refuses or requires every item done (runs only when,every Plan item,refusenear close). The capabilities bullet was the only one.SKILL.md's per-sprint rail already described the script accurately; the per-task rail said "the sprint stays open until its Plan is done" and now states the same close condition as the spec bullet (every item[x], or the rest struck or carried with a Progress entry)..dev-backlog/sprints/*was not touched — completed sprints are immutable history.Verification
Both run chained with
&&; the chain exited 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01X9gZYk7FcYSLAZBBe7JXvZ
Independent review