Skip to content

Ready Label On Confirm - #275

Merged
FrankRay78 merged 1 commit into
mainfrom
feature/274-ready-label-on-confirm
Sep 7, 2026
Merged

Ready Label On Confirm#275
FrankRay78 merged 1 commit into
mainfrom
feature/274-ready-label-on-confirm

Conversation

@FrankRay78

Copy link
Copy Markdown
Owner

Why

/speckit.confirmissue finished by patching the issue body and left no mark on the issue's own state, so "this issue is fully defined" was invisible from the issue list — you had to open each one. ready already exists and already means exactly that. Applying it here also completes the label state machine #271 starts at the other end: review = review pending → removed = reviewed → ready = defined.

What changes

  • /speckit.confirmissue now applies the ready label after it patches the issue body. Additive only — no other label is added, removed or altered, needs triage included.
  • The command's report says whether the label landed. A label failure is never fatal: the decisions are already saved by then, and the report says so.
  • The pipeline description in docs/agentic-workflow.md says the confirm step marks the issue ready.

Non-obvious things a reviewer should know

  • The organic evidence run the issue asks for does not exist yet, and I could not produce one. The issue calls for "one real run linked in the PR: an issue without ready that carries it after /speckit.confirmissue". No open issue is in a state where the command can complete: only Add /study — record what surprised a build, so the harness can be improved from evidence #260 carries a <!-- speckit:review --> comment and all ten of its > _Answer:_ slots are empty, so the command hard-stops at step 2. Apply the ready label when /speckit.confirmissue folds in decisions #274 itself has no review comment and already carries ready, applied by hand. Manufacturing a candidate would mean inventing answers on a real issue or opening a throwaway one — scaffolding of the kind this issue explicitly rejects for the failure path. The next real confirm run on an unlabelled issue is the evidence; this PR ships without it rather than staging it.
  • This is a configuration/tooling change, not production code, so per Constitution I's carve-out the RED-GREEN evidence is the real state failing and passing, not an xUnit test. RED: grep -rn -- "--add-label|--remove-label" .claude/ returned nothing — no command in the repo wrote any label; and Apply the ready label when /speckit.confirmissue folds in decisions #274 carries a ## Confirmed decisions section yet its ready label came from FrankRay78 in the issue timeline, not from the command. GREEN: the same grep now finds the label write, and line 45 of the workflow doc names it. Full suite run anyway and unaffected: 648 passed, 0 failed, 0 skipped, 0 warnings.
  • AC 5 (label failure keeps the decisions saved) is design intent, verified by reading the command text, as the issue directed. Staging it would mean inventing a bogus label name purely to watch an error path.
  • No new gate guards AC 4. An issue with an unanswered or hedging review can't come out ready because the label step sits downstream of step 2's existing hard-stops — the ordering is the guarantee, and it is written into the step so a future editor doesn't move it.
  • Idempotency (AC 3) rests on GitHub treating --add-label of an existing label as a no-op, which preserves the command's stated "re-running is harmless".
  • I also added the label to the command's ## Purpose list, beyond the issue's stated file touchpoints, so that summary doesn't describe a command that now does more than it says.

How to verify

  • Read the new step 6 in .claude/commands/speckit.confirmissue.md — it runs after step 5's body patch, and before step 7's "do not touch the review comment".
  • Confirm nothing in the diff can remove a label: grep -rn -- "--remove-label" .claude/ finds only the prose forbidding it.
  • Check the ordering claim holds against step 2's hard-stops — an unanswered or hedging answer returns before any body patch, so the label step is unreachable.
  • Next time an issue's review comes back fully answered, run /speckit.confirmissue #N on it and confirm the label lands — that is the outstanding evidence noted above.

Related

Closes #274

🤖 Generated with Claude Code

https://claude.ai/code/session_01YaRuiUQHdPMeGugFuvGfeK

/speckit.confirmissue finished by patching the issue body and left no
mark on the issue's own state, so "this issue is fully defined" was
invisible from the issue list. Every `ready` label to date was applied
by hand; nothing in .claude/ wrote a label at all.

Add a label step after the body patch: additive only, so no existing
label is disturbed; idempotent, since adding a label an issue already
carries is a no-op; and non-fatal, so a label failure cannot leave the
decisions unsaved. It sits downstream of step 2's hard-stops, so an
unanswered or hedging review can never come out labelled ready.

RED-GREEN evidence (Constitution I, configuration/tooling carve-out —
this is a prompt-and-docs change, not production code):

  RED   grep -rn -- '--add-label|--remove-label' .claude/  -> no matches
        #274 carries a `## Confirmed decisions` section, yet its `ready`
        label came from FrankRay78, not the command (issue timeline).
        docs/agentic-workflow.md:45 described the step as folding the
        review into a section, nothing more.

  GREEN the same grep now finds the label write in
        .claude/commands/speckit.confirmissue.md, the report section
        states whether the label landed, and line 45 says the step
        labels the issue ready.

Suite unaffected and green: 648 passed, 0 failed, 0 skipped, 0 warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YaRuiUQHdPMeGugFuvGfeK
@FrankRay78

Copy link
Copy Markdown
Owner Author

@claude Review this pull request. Analyse the code changes and provide feedback covering:

  • Bugs or correctness issues
  • Security concerns (including any flagged by static analysis)
  • Adherence to the project conventions in CLAUDE.md
  • Test coverage — do the tests adequately cover the new behaviour?
  • Any spec/test-plan mismatches

End the review with a recommendation whether to merge the PR, and if not, what you suggest needs addressing first.

@claude

claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @FrankRay78's task in 2m 15s —— View job


Review complete

  • Gather context (diff, changed files, related docs)
  • Review .claude/commands/speckit.confirmissue.md changes
  • Review docs/agentic-workflow.md change
  • Check adherence to CLAUDE.md / constitution conventions
  • Assess test coverage claims (config/tooling carve-out)
  • Check spec/test-plan mismatches

Summary

Small, well-scoped change: adds a step 6 (gh issue edit <number> --add-label ready) to /speckit.confirmissue, renumbers the old step 6 → 7, and updates the chat-report checklist and the pipeline table in docs/agentic-workflow.md. Both changed files are prose/prompt content — no .cs files touched, so the constitution's config/tooling carve-out (Principle I) applies rather than xUnit coverage.

Correctness

  • The new step is placed correctly: after step 5 (body patch) and before the renamed step 7 ("do not touch the review comment"), so a label failure can never leave the decisions unsaved. Confirmed against step 2's hard-stops (unanswered/hedging answers stop before step 3, well before the label step) — the ordering claim in the PR body holds.
  • --add-label is additive-only; nothing in the diff introduces --remove-label. Verified: grep -rn -- "--remove-label" .claude/ only turns up the prose forbidding it.
  • No stale cross-references: grepped the file for step [0-9] mentions and other docs referencing /speckit.confirmissue — nothing points at the old step-6/7 numbering.
  • Line 45 of docs/agentic-workflow.md does name the label update, as the PR body claims — checked directly.
  • Renumbering read cleanly top-to-bottom: ## Purpose (item 4), Workflow step 6/7, and the ## Output to the user bullet list are all internally consistent.

Security

No concerns. This is a markdown prompt change with no executable code; the gh issue edit invocation follows the same <owner>/<repo> / <number> placeholder pattern already used elsewhere in the file (step 5's gh api call), so no new injection surface.

CLAUDE.md / constitution adherence

  • Correctly classified as a configuration/tooling change, not production code — the RED/GREEN evidence in the commit message (grep for --add-label absent → present) is the right kind of evidence per Principle I's carve-out, and matches [[feedback_guard_outgrows_the_fix]]-style reasoning: there's no meaningful automatable CI gate for "does this prompt correctly instruct an agent," so a hand-rolled test would be exactly the kind of bespoke scaffolding Principle I warns against.
  • No hard-wrapped prose in the new lines — each added paragraph/bullet is a single line, consistent with the project's soft-wrap convention.
  • PR body is honest about a real gap: no organic end-to-end run of the new step exists yet, and the author explains why one couldn't be manufactured without inventing scaffolding. That's the right call per Principle IX's spirit (outcome over manufactured mechanism) rather than staging a throwaway issue.

Test coverage

No .cs production code changed, so no xUnit coverage is expected. Given the file only affects an LLM-driven prompt workflow, "coverage" here is necessarily the RED/GREEN grep evidence plus a future real run — both are documented. The dotnet suite (648/0/0 per the PR body) is unaffected, which is expected since no .cs file is touched.

Spec/test-plan mismatches

None found — there's no specs/274-... folder for this issue (consistent with [[feedback_no_spec_references]] — no spec-kit cycle was run for a change this small), and nothing in the diff references a spec path.

Minor observations (non-blocking)

  • The PR references PR Automated pre-specification review on labelled issues #271 (review label auto-apply/removal) as "the other end of the same label state machine," but I could not find that logic anywhere in the current .claude/ or .github/workflows/ — it appears to live in a not-yet-merged or separate PR. Worth double-checking the cross-reference still resolves to the right PR when this merges, but it's prose context, not a defect in this diff.
  • If the ready label is ever deleted or renamed in the repo's label set, gh issue edit --add-label ready will error — the step already treats any label failure as non-fatal, so this is handled, just noting there's no auto-create-label fallback (correctly out of scope for this PR).

Recommendation

Approve / merge. The change is minimal, correctly ordered relative to the existing hard-stops, additive-only as claimed, and the docs cross-reference checks out. The only open item — an organic end-to-end run — is explicitly called out by the author as outstanding evidence to capture on the next real /speckit.confirmissue run, which is a reasonable thing to ship without rather than block on.
· branch feature/274-ready-label-on-confirm

@FrankRay78
FrankRay78 merged commit 4c53d16 into main Sep 7, 2026
3 checks passed
@FrankRay78
FrankRay78 deleted the feature/274-ready-label-on-confirm branch September 7, 2026 12:44
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.

Apply the ready label when /speckit.confirmissue folds in decisions

1 participant