66: [P0] Add user-approved spec and blueprint file artifacts#87
66: [P0] Add user-approved spec and blueprint file artifacts#87
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds artifact lifecycle actions for ChangesArtifact Lifecycle Actions for Spec and Blueprint
Sequence Diagram(s)sequenceDiagram
participant Client
participant SpecSkill
participant WorkflowConfig
participant Repo
participant Kickoff
Client->>SpecSkill: finalize approved spec
SpecSkill->>WorkflowConfig: load spec_approved actions
SpecSkill->>SpecSkill: validate action shapes
SpecSkill->>Client: prompt (if approval: prompt)
alt approved or auto
SpecSkill->>Repo: write artifact
SpecSkill-->>Kickoff: return artifact status/path
else skipped
SpecSkill-->>Kickoff: return skipped/not_configured
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/configuration.md (1)
168-170: ⚡ Quick winConsider documenting explicit artifact result statuses.
The text defines behavior but not a concrete status vocabulary for existing-file/decline outcomes. Adding canonical statuses would reduce integration ambiguity for kickoff/ticket-update consumers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/configuration.md` around lines 168 - 170, Add a short canonical status vocabulary to the configuration docs describing outcome values for artifact writes and approvals (e.g., statuses such as "created", "skipped_exists", "declined", "approved", "error") and show exactly when each is emitted by the flows mentioned (spec -> spec_approved, blueprint -> blueprint_approved, approval: prompt, approval: auto). Update the paragraph describing default paths and parent-directory creation to state which status is returned for: newly created files (created), when approval: auto skips because file exists (skipped_exists), when a user rejects a prompt (declined), when an approval completes (approved), and when an unexpected failure occurs (error); keep terminology consistent with `{feature}`, `{kind}`, and `{ticket_id}` placeholders and note that statuses are included in same-session handoff context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.beislid/workflow-md-format.md:
- Line 86: The documentation for lifecycle_actions leaves the CLI action
`approval` behavior undefined; update the spec for `lifecycle_actions` (and the
similar paragraph later around the second occurrence) to state a deterministic
default for CLI actions when `approval` is omitted — e.g., "CLI actions default
to `approval: prompt` (or choose `required` if preferred)"; explicitly mention
this applies to `events.kickoff_start.actions[]` and any other CLI-supporting
entries and show the chosen default (`prompt`/`required`/`auto`) and that it can
be overridden by setting `approval` on the action.
In `@docs/configuration.md`:
- Line 168: Change the phrase "creates a missing file without another prompt" to
use the compound modifier "auto-write" (e.g., `approval: auto` creates a missing
file via auto-write) so the doc uses consistent hyphenation; update the sentence
referencing `approval: auto` in the paragraph that also mentions `approval:
prompt`, `spec`/`spec_approved`, and `blueprint`/`blueprint_approved` to read
with "auto-write" (or a similar hyphenated phrasing) while preserving the
meaning that existing files are never overwritten.
In `@skills/break-spec/SKILL.md`:
- Line 12: The precedence logic in break-spec that picks a spec from an artifact
glob needs a tie-breaker: when resolving artifacts (after checking an explicit
handoff spec path), if the glob for plans/*-spec.md or plans/*-prd.md returns
multiple matches, do not auto-select—add a branch to surface an explicit user
prompt asking them to choose the correct file path (e.g., "Which spec file
should I use? [list matches]") and proceed with the selected path; ensure the
same multi-match prompt behavior applies to both plans/*-spec.md and
plans/*-prd.md checks and is used before falling back to tracker docs or session
spec content.
In `@skills/implement/SKILL.md`:
- Line 10: The selection logic for the primary design artifact is ambiguous when
multiple files exist under plans/; change the behavior in SKILL.md so that if
blueprint supplies an explicit design artifact path it is used, otherwise search
plans/ for files matching a deterministic ticket/feature slug (e.g., match
plans/<slug>-design.md to the task’s ticket/feature identifier) and if more than
one file matches, do not auto-pick — prompt the user to confirm the correct
artifact. Update the text that currently reads "read it as your primary input"
to specify deterministic slug matching and add a required confirmation step when
multiple candidates remain; reference the terms "blueprint", "design artifact
path", "plans/<feature>-design.md", and "ticket/feature slug" so the
implementation can locate and enforce this behavior.
---
Nitpick comments:
In `@docs/configuration.md`:
- Around line 168-170: Add a short canonical status vocabulary to the
configuration docs describing outcome values for artifact writes and approvals
(e.g., statuses such as "created", "skipped_exists", "declined", "approved",
"error") and show exactly when each is emitted by the flows mentioned (spec ->
spec_approved, blueprint -> blueprint_approved, approval: prompt, approval:
auto). Update the paragraph describing default paths and parent-directory
creation to state which status is returned for: newly created files (created),
when approval: auto skips because file exists (skipped_exists), when a user
rejects a prompt (declined), when an approval completes (approved), and when an
unexpected failure occurs (error); keep terminology consistent with `{feature}`,
`{kind}`, and `{ticket_id}` placeholders and note that statuses are included in
same-session handoff context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e5699a0-957a-4eab-b25d-2cfec9ad5bc8
📒 Files selected for processing (15)
.beislid/probe-semantics.md.beislid/workflow-md-format.mddocs/configuration.mddocs/skills.mddocs/workflows.mdskills/blueprint/SKILL.mdskills/break-spec/SKILL.mdskills/doctor/SKILL.mdskills/implement/SKILL.mdskills/kickoff/step-4-readiness.mdskills/kickoff/step-5-scope.mdskills/kickoff/step-6-blueprint.mdskills/kickoff/step-8-ticket-update.mdskills/setup/SKILL.mdskills/spec/SKILL.md
Summary
spec_approvedandblueprint_approvedartifact lifecycle action semantics.Verification
python3 scripts/validate_skills.pypython3 scripts/check_skill_size_budgets.pygit diff --checkpython3 tests/agent-smoke/run.py gate ship-it --hosts claude,codex --timeout 900 --changed-onlyReview
3b0b568.Closes #66.
Summary by CodeRabbit
New Features
Documentation