feat: centralize workflow files in claude_constructor directory #34
Conversation
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
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughConsolidates workflow artifacts under claude_constructor/{issue_key}/, changes many read/write paths to that layout, adds resume-detection UX and an implementation-summary command, replaces SlashCommand usages with Skill calls, updates state file format/fields, updates docs/.gitignore, and bumps plugin/package versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Orchestrator as Agent/Orchestrator
participant FS as FileSystem
participant Git as Git/PR
participant SkillSvc as Skill
Note over User,Orchestrator: Resume detection + implementation-summary flow
User->>Orchestrator: request /feature or /implementation-summary (issue_key)
Orchestrator->>FS: ensure `claude_constructor/{issue_key}/` exists
FS-->>Orchestrator: dir exists / created
Orchestrator->>FS: read `state_management.md`, `specification.md`, `review.md`, `security_review.md`
FS-->>Orchestrator: return state, spec, reviews, agent statuses
Orchestrator->>SkillSvc: invoke Skill to fetch issue/issue updates or post comments
SkillSvc-->>Orchestrator: issue data / post confirmation
Orchestrator->>Git: query branch, commits, PR metadata
Git-->>Orchestrator: return branch/commits/PR URL
Orchestrator->>FS: write/update `implementation_summary.md` and update `state_management.md`
FS-->>Orchestrator: write confirmation
Orchestrator->>User: return summary excerpt and file path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.gitignore(1 hunks)README.md(3 hunks)plugins/claude-constructor/agents/code-reviewer.md(3 hunks)plugins/claude-constructor/agents/increment-implementer.md(1 hunks)plugins/claude-constructor/agents/requirements-definer.md(1 hunks)plugins/claude-constructor/agents/security-reviewer.md(2 hunks)plugins/claude-constructor/commands/create-state-management-file.md(1 hunks)plugins/claude-constructor/commands/feature.md(3 hunks)plugins/claude-constructor/commands/implement-increment.md(1 hunks)
🔇 Additional comments (9)
.gitignore (1)
4-5: LGTM!Re-adding
node_modules/and addingclaude_constructor/correctly prevents generated artifacts and dependencies from being tracked.plugins/claude-constructor/agents/increment-implementer.md (1)
31-35: LGTM!The path update to
claude_constructor/{issue_key}/review.mdis correct and the guidance for reading existing review feedback is clear. The conditional logic appropriately handles both initial and revision modes.plugins/claude-constructor/agents/requirements-definer.md (1)
42-44: LGTM!The specification file path correctly uses the new centralized location without timestamps. The reliance on git history for versioning is a cleaner approach. Directory creation is handled upstream by
create-state-management-file.mdin the orchestrated workflow.plugins/claude-constructor/agents/code-reviewer.md (1)
122-154: LGTM!The review file path update is correct, and the logic appropriately distinguishes between first review creation (using Write tool) and subsequent iterations (appending). The assumption that
claude_constructor/{issue_key}/directory already exists is valid given thatcreate-state-management-file.mdcreates it in the orchestrated workflow.plugins/claude-constructor/commands/create-state-management-file.md (1)
18-23: LGTM!The directory-first approach is correct. Using
mkdir -pensures idempotency, so repeated execution or concurrent attempts won't fail. The step order (create directory → create file → write content) is logical and establishes the foundation for other agents' file writes.plugins/claude-constructor/agents/security-reviewer.md (1)
25-26: LGTM!The security review path update is correct, and the new iteration tracking guidance is well-structured. Preserving previous review findings across iterations creates a clear audit trail for agents fixing security issues. The timestamped approach and numbering convention are consistent with the code-reviewer pattern.
Also applies to: 33-33, 89-89, 91-99
plugins/claude-constructor/commands/implement-increment.md (1)
29-33: LGTM!The path update and the added note about subagents automatically checking
claude_constructor/{issue-key}/review.mdare correct. This eliminates redundant passing of review content and improves the orchestration clarity.plugins/claude-constructor/commands/feature.md (1)
55-79: LGTM!The artifact path updates to
claude_constructor/{issue-key}/security_review.mdandclaude_constructor/{issue-key}/review.mdare correct and consistent. The workflow steps correctly route to these files for both initial implementation and revision loops (lines 61 and 78).README.md (1)
139-139: Documentation accurately reflects the centralized artifact structure.The updates correctly document the new
claude_constructor/{issue_key}/consolidation across all three changed sections:
- Tips for Success (line 139) guides users to the centralized directory
- State Management section (line 339) explicitly points to the new path
- Generated files example (lines 529-533) shows the four artifact files in their new locations
Documentation is clear and consistent with the PR's restructuring objectives.
Also applies to: 339-339, 529-533
Hurblat
force-pushed
the
hurblat/feat-single-workflow-directory
branch
3 times, most recently
from
01b2a8d to
191857a
Compare
Hurblat
force-pushed
the
hurblat/feat-single-workflow-directory
branch
from
191857a to
3fc8c8b
Compare
Hurblat
force-pushed
the
hurblat/feat-single-workflow-directory
branch
from
b07db42 to
0bd73ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@plugins/claude-constructor/commands/feature.md`:
- Around line 33-37: When scanning the claude_constructor directory to determine
the next prompt key, match existing folders by the numeric prefix (e.g., use a
regex like ^prompt-(\d+)(?:-|$)) instead of exact names like prompt-{number};
update the logic that "List the contents of the `claude_constructor` directory"
and "Determine the next issue key" to extract the numeric part from any folder
starting with prompt-{number}-* (or prompt-{number}) to compute maxNumber and
then emit prompt-{maxNumber+1}-{short-description} so existing
prompt-1-something entries are detected and collisions avoided.
In `@README.md`:
- Around line 116-129: Add the missing SlashCommand permission for
"/implementation-summary" to the example permissions arrays so workflows that
call /implementation-summary don't fail; update the permissions block that
contains entries like "SlashCommand(/feature:*)" and the similar block at the
later occurrence (the one around the "allow" list with
"SlashCommand(/create-pull-request:*)" etc.) to include
"SlashCommand(/implementation-summary:*)" alongside the other SlashCommand
entries.
- Around line 528-533: Update the README entry that currently shows
specification_{timestamp}.md to match the actual generated filename (change to
specification.md if timestamps were removed); locate the list block containing
state_management.md, specification_{timestamp}.md, review.md, security_review.md
and replace specification_{timestamp}.md with the correct filename used by the
tooling so the documentation and actual output remain consistent.
- Around line 59-77: The step numbering in README.md is duplicated: the second
"3." (the "Add the local marketplace" block) should be renumbered to "4." and
the following "4. Install the plugin:" should become "5." so the sequence reads
1..5; update the numeric prefixes in the three affected headings/blocks (the
first "3. Run with issue key:", the second "3. Add the local marketplace:" and
"4. Install the plugin:") to ensure sequential numbering and consistent
ordering.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
179-179: Update state file path to match new directory structure.This line still references the old path
state_management/{issue_key}.md, which is inconsistent with the new centralized structure documented in lines 122-123.📝 Proposed fix
-- **Check state files:** `state_management/{issue_key}.md` shows detailed progress +- **Check state files:** `claude_constructor/{issue_key}/state_management.md` shows detailed progress
Hurblat
force-pushed
the
hurblat/feat-single-workflow-directory
branch
from
554f081 to
3e3ad2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
179-179: Stale path reference in Tips section.Line 179 still references the old path
state_management/{issue_key}.md, but should now point toclaude_constructor/{issue_key}/state_management.mdto match the new structure.📝 Proposed fix
-- **Check state files:** `state_management/{issue_key}.md` shows detailed progress +- **Check state files:** `claude_constructor/{issue_key}/state_management.md` shows detailed progress
🤖 Fix all issues with AI agents
In `@plugins/claude-constructor/commands/implementation-summary.md`:
- Around line 26-29: Update Step 3 in implementation-summary.md to explicitly
state the behavior when review files are missing: when
claude_constructor/$1/review.md or claude_constructor/$1/security_review.md do
not exist, include a line in the summary such as "No review performed" for the
missing file, set review iteration count to 0, and set final verdict to "N/A"
(or an equivalent placeholder); reference the files by name
(claude_constructor/$1/review.md and claude_constructor/$1/security_review.md)
and ensure the wording is added to the bullet list under "Read Review Files" so
consumers know exactly what to include in the summary when files are absent.
- Around line 58-62: The "Acceptance Criteria Status" section currently
hardcodes all items as completed; add a step to compute real status instead of
assuming `[x]`—implement a function like getAcceptanceCriteriaStatus() (or
extractAcceptanceCriteriaStatus()) that reads the spec or state store and
returns a boolean per criterion, then update the template renderer that
generates the "Acceptance Criteria Status" block to emit `[x]` or `[ ]` based on
those booleans; ensure the renderer or workflow step that populates the
Acceptance Criteria Status header consumes getAcceptanceCriteriaStatus() results
so the markdown reflects actual completion.
- Around line 86-90: Remove the hardcoded "Quality Gates: All passed" line in
the "Code Review" block and either replace it with a dynamic placeholder (e.g.,
{quality_gates_status}) that gets populated by your workflow, or delete the line
entirely so only "{count}" and "{APPROVED}" remain; update whatever generator or
template that emits implementation-summary.md to supply quality_gates_status
from CI/QA results if you choose the placeholder route, and ensure the
template/function that produces this section (the "Code Review" block) no longer
asserts a passing state without verification.
- Around line 31-34: Update the "Gather Git Information" step to use the PR's
actual base branch instead of the hardcoded origin/HEAD: extract baseRefName
from the output of the previously-run gh pr view call (the PR info retrieved in
Step 5) and substitute that value into the git commands (replace origin/HEAD in
git log --oneline origin/HEAD..HEAD, git diff --stat origin/HEAD..HEAD, etc.);
also implement a local-dev fallback that uses origin/HEAD when no PR context /
baseRefName is available and document that the summary may be incomplete in that
fallback case.
♻️ Duplicate comments (1)
plugins/claude-constructor/commands/feature.md (1)
33-37: Verify the prompt directory pattern matching logic.The instructions say to check for directories named
prompt-{number}(e.g.,prompt-1,prompt-2), but actual directories would have a suffix likeprompt-1-implement-cli. This could cause the pattern matching to miss existing directories when computing the next number.
- Move all generated files to single directory per issue - Add implementation summary as final workflow step - Update create-pull-request to ignore claude_constructor folder - Update documentation to reflect new structure
Track workflow progress markers (requirementsApproved, specificationApproved, workingBranch, pullRequestUrl) in state management file. Detect existing workflow state on /feature invocation and offer to resume from last checkpoint. Support resuming mid-implementation by skipping completed agents. Also improves implementation-summary.md with dynamic base branch detection, actual acceptance criteria status, and proper handling of missing review files.
Hurblat
force-pushed
the
hurblat/feat-single-workflow-directory
branch
from
3e3ad2f to
57395d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@plugins/claude-constructor/commands/feature.md`:
- Around line 102-103: The archive path template
"claude_constructor/{issue-key}-archived-{timestamp}/" is ambiguous and may
produce filesystem-unsafe names; update the docs to specify a concrete, portable
timestamp format (e.g. UTC ISO-like compact form "YYYYMMDDThhmmssZ" or Unix
epoch milliseconds) and state timezone (UTC) and separator rules so implementers
can reliably generate names without characters like ":"; reference the archive
pattern string "claude_constructor/{issue-key}-archived-{timestamp}/" and
clarify that {timestamp} must follow the chosen format.
In `@plugins/claude-constructor/commands/git-checkout.md`:
- Around line 26-28: Clarify and implement the update step for the state
management file ($2): open and read the file, locate the "Workflow Progress"
section (create it if missing), search within that section for the workingBranch
field (key named workingBranch), set its value to "feat/$1" (update if present,
add the key if absent), and write the updated content back to the same file;
ensure the implementation handles parsing the file format (e.g., YAML/JSON)
consistently and preserves other sections.
In `@plugins/claude-constructor/commands/implement-increment.md`:
- Around line 33-47: The documentation's resume logic refers to statuses like
"in_progress" and "needs_revision" but the example under the "## Implementation
Agents Status" section only shows "pending", causing ambiguity; update the
example under the "## Implementation Agents Status" header to include and
demonstrate all supported status values ("pending", "in_progress",
"needs_revision", "completed") with sample revision counts so implementers know
the exact expected format for parsing and resuming (e.g., show lines for
agent-3: in_progress (revision: 1), agent-4: completed (revision: 2), agent-5:
needs_revision (revision: 1)) and ensure the resume logic references these exact
status tokens when parsing agent states.
🧹 Nitpick comments (3)
plugins/claude-constructor/commands/create-state-management-file.md (1)
18-32: State management file creation structure is well-designed.The two-step approach (create directory → create file) is correct, and using
mkdir -pis safe as it handles existing directories gracefully. The Workflow Progress fields (requirementsApproved, specificationApproved, workingBranch, pullRequestUrl) provide good tracking for the workflow state.However, there's a minor formatting inconsistency in the markdown template.
📝 Suggested formatting improvement
The markdown template shows
Issue Key: {$1}with curly braces, but the bash variable expansion syntax is just$1. Consider clarifying whether this is meant to be a literal placeholder or should be interpolated:If it should show the actual issue key value:
-Issue Key: {$1} +Issue Key: $1If it should remain as a placeholder for later substitution:
# Keep as-is, but document that {$1} is a placeholderplugins/claude-constructor/commands/create-pull-request.md (1)
44-49: Clarify the conditional logic for issue status updates.The condition uses AND/OR logic: skip update if
silentModeistrueORissueTrackingProvideris"prompt". This means:
- Updates when: silentMode=false AND issueTrackingProvider≠"prompt"
- Skips when: silentMode=true OR issueTrackingProvider="prompt"
The log message "Silent mode: Would have updated..." may be misleading when the skip is due to
issueTrackingProvider="prompt"rather than silent mode.💬 Suggested improvement for clarity
Consider separate log messages for different skip reasons:
8. **Check Silent Mode for Issue Status Update**: - If `silentMode` is `false` AND `issueTrackingProvider` is NOT `"prompt"`: - Use the SlashCommand tool to execute `/update-issue $1 "Code Review" $2` - - If `silentMode` is `true` OR `issueTrackingProvider` is `"prompt"`: - - Log: "Silent mode: Would have updated issue $1 status to 'Code Review'" + - Otherwise: + - If `silentMode` is `true`: + - Log: "Silent mode: Would have updated issue $1 status to 'Code Review'" + - Else (issueTrackingProvider is "prompt"): + - Log: "Prompt mode: Skipping issue status update for $1" - Skip the issue updateplugins/claude-constructor/commands/feature.md (1)
48-68: Clarify UX for completed workflows (Check 1) and error handling.The resume detection algorithm is comprehensive, but there are two areas needing clarification:
Check 1 (line 52): When
implementation_summary.mdexists, the resume point is "Complete - offer to start fresh". However, the UX flow section doesn't show what this looks like to the user. Should it skip the AskUserQuestion and go directly to archiving? Or show a different prompt?Error handling: The algorithm assumes files are well-formed (e.g., "latest review has APPROVED"). What happens if a file exists but is corrupted, incomplete, or doesn't contain the expected markers?
Consider adding:
- A specific UX flow example for the "workflow already complete" case
- Guidance on handling malformed/incomplete state files (e.g., treat as corrupted and offer to start fresh)
Transform passive Open Questions sections into interactive resolution during sign-off. Questions tagged as [STRUCTURED] get resolved via multiple-choice UI, while [OPEN-ENDED] questions remain for manual feedback. - Add question classification format to requirements-definer and specification-writer - Add interactive resolution workflow to sign-off commands - Add question format validation to auditors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SlashCommand was the legacy tool name - it has been merged into the Skill tool in current Claude Code versions. Update all references: - .claude/settings.json: permission rules now use Skill() format - Command/agent files: tool invocations use "Skill tool" wording - security-reviewer.md: tools field in frontmatter Also adds missing /implementation-summary to permissions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add explicit documentation of all valid agent statuses (pending, in_progress, needs_revision, completed, failed) with descriptions. Resolves ambiguity when resuming workflows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- git-checkout: add explicit steps for updating workingBranch field - feature: specify timestamp format (YYYYMMDD-HHMMSS) for archives Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@plugins/claude-constructor/commands/create-pull-request.md`:
- Around line 36-41: Guard the PR URL update by first capturing and validating
the output of the `gh pr create` command before writing to the state file: when
`silentMode` is false, ensure the script that runs `gh pr create` captures its
stdout/stderr, check that the captured string contains a valid URL (non-empty
and matches a PR URL pattern) and only then update the `pullRequestUrl` field in
the Workflow Progress section of the state management file ($2); if capture
fails or validation fails, do not write an empty/invalid URL—log or surface the
error and skip the update (or write an explicit null/error marker) so
`pullRequestUrl` is never set to an invalid value.
In `@plugins/claude-constructor/commands/git-checkout.md`:
- Around line 26-31: The step that updates the state management file needs
explicit error handling and a clear field format/insertion rule: when reading
the state management file ($2) and locating the "## Workflow Progress" section,
fail gracefully if the file is missing (throw/return a clear error) or create
the file and a new "## Workflow Progress" section if not present; treat the
`workingBranch:` field as a YAML-style key/value line ("workingBranch: feat/$1")
and if the key exists replace its value, otherwise append a new line
"workingBranch: feat/$1" at the end of the "## Workflow Progress" section;
ensure all file reads/writes catch and surface IO/parsing errors with
descriptive messages referencing the file and section.
In `@plugins/claude-constructor/commands/implement-increment.md`:
- Around line 33-61: Specify exact parsing and semantics for the "##
Implementation Agents Status" block: parse lines matching the regex ^-
(?P<id>[^:]+): (?P<status>pending|in_progress|needs_revision|completed|failed)
\(revision: (?P<rev>\d+)\)$ to extract agent-id, status and numeric revision;
treat any non-matching or malformed line as warning-and-default-to "pending
(revision: 0)" and log the issue. Define revision semantics: revision is
zero-based (revision: 0 = first attempt), max attempts is 3 meaning allowed
revisions are 0..2 (stop and mark "failed" when revision >= 3), and increment
revision by 1 immediately after a failed attempt before scheduling a retry
(preserve existing revision counts when resuming). For invalid status values
found during parsing, coerce to "pending" with revision preserved or set to 0 if
missing, and log a warning that the status was normalized.
- Line 20: The Skill tool commands in implement-increment.md are missing the
required third argument (the state management file path); update the
`/update-issue` invocation to include $2 (i.e. `/update-issue $1 "In Progress"
$2`) and update the `/create-comment` invocation to include $2 and the intended
message placeholder (i.e. `/create-comment $1 "Claude Code implementation
started for [specification-file-name]" $2`) so both commands pass the
state-management-file-path parameter expected by the workflow.
In `@plugins/claude-constructor/commands/requirements-sign-off.md`:
- Around line 23-27: The "Parse Open Questions" spec for extracting STRUCTURED
options lacks a defined option format; update the documentation in
requirements-sign-off.md and the parser logic that reads the "### Open
Questions" section to require and/or accept canonical option formats (e.g., "A.
Option text") and also normalize common variants ("A) Option", "A: Option", "-
A: Description") by extracting the option label (A/B/C/...) and option text;
ensure the parser that looks for `[STRUCTURED]` questions validates presence of
at least one option and fails gracefully with a clear error message when options
cannot be parsed.
- Around line 34-37: Stop editing the specification file (do not move items into
the "### Resolved Questions" section or remove the "[STRUCTURED]" tag there);
instead change the workflow so resolved answers are appended to the state
management file (e.g., a JSON/YAML state store) and keep the spec read-only
after creation, moving the code that currently mutates the spec (the logic that
parses answered questions and writes them back into "### Resolved Questions")
into a new/updated function that writes resolution records to the state file and
marks questions resolved there while preserving original spec content and
metadata.
- Around line 28-32: Clarify and add explicit looping: when resolving
"STRUCTURED" questions in the "Resolve Structured Questions Interactively" step,
iterate over the full list in sequential batches of up to 4 until all are
processed; for each batch call the AskUserQuestion tool with the batch's
questions, await and collect responses, then continue to the next batch
(preserving original order) until no STRUCTURED questions remain. Ensure the
instruction references "STRUCTURED", "batches of up to 4", and "AskUserQuestion
tool" so implementers understand to loop and collect responses across multiple
batches.
In `@plugins/claude-constructor/commands/review-pull-request.md`:
- Line 40: The command using the Skill tool currently sends a literal
placeholder string in `/create-comment $1 "[user feedback summary and changes
made in response]" $2`; update this to pass a dynamically generated summary
instead—have the workflow/agent produce a concise feedback summary from steps
3–6 (e.g., aggregate identified issues and the changes made) and substitute that
generated text into the second argument of `/create-comment` so the posted
comment contains the actual PR feedback and changes rather than the placeholder.
In `@plugins/claude-constructor/commands/specification-sign-off.md`:
- Line 69: Update the `/create-comment` invocation in specification-sign-off.md
to include the missing third argument: the state-management-file-path; locate
the incorrect command that currently reads `/create-comment [issue-key]
"[specification details and assumptions]"` and change it to include the required
`[state-management-file-path]` argument so the Skill tool call supplies all
three parameters (`[issue-key]`, `"[comment-text]"`,
`[state-management-file-path]`).
♻️ Duplicate comments (1)
plugins/claude-constructor/commands/feature.md (1)
33-38: Clarify pattern matching to include directory suffixes.Line 34 states "Check for existing directories named using the pattern
prompt-{number}" but actual directories include suffixes (e.g.,prompt-1-implement-cli). This ambiguity could cause the logic to miss existing directories and regenerate colliding keys.The description should explicitly state that the pattern matches directories with the prefix
prompt-{number}-to prevent key collisions.📝 Proposed clarification
-- Check for existing directories named using the pattern `prompt-{number}` (e.g., `prompt-1`, `prompt-2`) +- Check for existing directories matching the prefix pattern `prompt-{number}-` (e.g., `prompt-1-*`, `prompt-2-*`) +- Extract the numeric portion after `prompt-` and before the next `-` or end of name - Determine the next issue key: - If no `prompt-{number}` directories exist: use `prompt-1-{short-description}` - If at least one exists: find the maximum number and use `prompt-{maxNumber+1}-{short-description}`
Comment on lines
+36
to
+41
| 7. **Update Workflow Progress with PR URL**: | ||
| - If `silentMode` is `false`: | ||
| - Extract the PR URL from the `gh pr create` output (it outputs the URL on success) | ||
| - Read the state management file ($2) | ||
| - Update `pullRequestUrl:` to `pullRequestUrl: [extracted-url]` in the Workflow Progress section | ||
| - If `silentMode` is `true`: |
There was a problem hiding this comment.
Guard PR URL update against missing/failed gh pr create output.
Step 7 depends on extracting the URL, but Step 6 doesn’t instruct capturing output or handling failure. That can write an empty/invalid URL into the state file.
✅ Suggested wording tweak
- - If `silentMode` is `false`:
- - Extract the PR URL from the `gh pr create` output (it outputs the URL on success)
- - Read the state management file ($2)
- - Update `pullRequestUrl:` to `pullRequestUrl: [extracted-url]` in the Workflow Progress section
+ - If `silentMode` is `false`:
+ - Capture `gh pr create` output into a variable
+ - If the URL is present, update `pullRequestUrl:` in the Workflow Progress section
+ - If missing, log a failure and leave `pullRequestUrl:` unchanged🤖 Prompt for AI Agents
In `@plugins/claude-constructor/commands/create-pull-request.md` around lines 36 -
41, Guard the PR URL update by first capturing and validating the output of the
`gh pr create` command before writing to the state file: when `silentMode` is
false, ensure the script that runs `gh pr create` captures its stdout/stderr,
check that the captured string contains a valid URL (non-empty and matches a PR
URL pattern) and only then update the `pullRequestUrl` field in the Workflow
Progress section of the state management file ($2); if capture fails or
validation fails, do not write an empty/invalid URL—log or surface the error and
skip the update (or write an explicit null/error marker) so `pullRequestUrl` is
never set to an invalid value.
Comment on lines
+26
to
+31
| 5. **Update Workflow Progress**: | ||
| - Read the state management file ($2) | ||
| - Locate the `## Workflow Progress` section | ||
| - Find the `workingBranch:` field and replace its value with `feat/$1` | ||
| - If field doesn't exist, add `workingBranch: feat/$1` to the section | ||
| - Write the updated content back to the state management file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Specify error handling and field format.
Step 5 lacks crucial implementation details:
- Missing error handling: No guidance for what to do if the state management file or Workflow Progress section doesn't exist.
- Format ambiguity: The exact format of the
workingBranch:field isn't specified (is it YAML-styleworkingBranch: value, Markdown, etc.?). - Insertion point unclear: Line 30 says to add the field "to the section" if it doesn't exist, but doesn't specify where in the section (beginning, end, specific position).
📝 Proposed enhancement
5. **Update Workflow Progress**:
- Read the state management file ($2)
- Locate the `## Workflow Progress` section
+ - If the section doesn't exist, create it after the Settings section
- Find the `workingBranch:` field and replace its value with `feat/$1`
- - If field doesn't exist, add `workingBranch: feat/$1` to the section
+ - If field doesn't exist, add `workingBranch: feat/$1` on a new line at the end of the section
+ - Format: `workingBranch: feat/$1` (YAML-style key-value pair)
- Write the updated content back to the state management file🤖 Prompt for AI Agents
In `@plugins/claude-constructor/commands/git-checkout.md` around lines 26 - 31,
The step that updates the state management file needs explicit error handling
and a clear field format/insertion rule: when reading the state management file
($2) and locating the "## Workflow Progress" section, fail gracefully if the
file is missing (throw/return a clear error) or create the file and a new "##
Workflow Progress" section if not present; treat the `workingBranch:` field as a
YAML-style key/value line ("workingBranch: feat/$1") and if the key exists
replace its value, otherwise append a new line "workingBranch: feat/$1" at the
end of the "## Workflow Progress" section; ensure all file reads/writes catch
and surface IO/parsing errors with descriptive messages referencing the file and
section.
Comment on lines
+33
to
+61
| - **Check for existing Implementation Agents Status** (resume support): | ||
| - Read the state management file ($2) | ||
| - If `## Implementation Agents Status` section already exists: | ||
| - Parse existing agent statuses and revision counts | ||
| - Skip agents marked as "completed" - their work is preserved | ||
| - Resume agents marked as "in_progress" or "needs_revision" | ||
| - Preserve existing revision counts when resuming | ||
| - Log: "Resuming implementation - skipping N completed agents" | ||
| - If section does not exist, create it fresh | ||
|
|
||
| **Agent Status Format**: | ||
|
|
||
| ```markdown | ||
| ## Implementation Agents Status | ||
|
|
||
| - agent-1: pending (revision: 0) | ||
| - agent-2: in_progress (revision: 0) | ||
| - agent-3: needs_revision (revision: 1) | ||
| - agent-4: completed (revision: 0) | ||
| - agent-5: failed (revision: 2) | ||
| ``` | ||
|
|
||
| Valid statuses: | ||
|
|
||
| - `pending`: Not yet started | ||
| - `in_progress`: Currently being worked on | ||
| - `needs_revision`: Audit failed, awaiting re-implementation | ||
| - `completed`: Finished and audit passed | ||
| - `failed`: Max revisions reached or unrecoverable error |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Clarify parsing logic and revision count semantics.
The resume support is well-structured, but lacks implementation details:
-
Parsing specification: Line 36 says "Parse existing agent statuses and revision counts" but doesn't specify the expected format or regex pattern. Should explicitly state:
- agent-id: status (revision: N). -
Revision count semantics: The example shows
revision: 0for pending/in_progress agents, but line 79 mentions "max revisions reached (3 attempts)". Clarify:- Does revision start at 0 (first attempt) or 1?
- Is "3 attempts" inclusive (0,1,2) or exclusive?
- When does revision increment - before or after each attempt?
-
Malformed data handling: No guidance for what to do if the status format is malformed or contains invalid status values.
📝 Proposed enhancement
- **Check for existing Implementation Agents Status** (resume support):
- Read the state management file ($2)
- If `## Implementation Agents Status` section already exists:
- - Parse existing agent statuses and revision counts
+ - Parse existing agent statuses using format: `- agent-id: status (revision: N)`
+ - If format is malformed, log warning and treat as fresh start
- Skip agents marked as "completed" - their work is preserved
- Resume agents marked as "in_progress" or "needs_revision"
- - Preserve existing revision counts when resuming
+ - Preserve existing revision counts (0-indexed, where 0 = first attempt)
- Log: "Resuming implementation - skipping N completed agents"
- If section does not exist, create it freshAnd clarify revision semantics:
- Increment revision counter for tracking
- - Repeat audit cycle until pass or max revisions reached (3 attempts)
+ - Repeat audit cycle until pass or max revisions reached (revision 2 = 3 total attempts)
- If max revisions reached, mark as "failed" and handle as agent failure📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **Check for existing Implementation Agents Status** (resume support): | |
| - Read the state management file ($2) | |
| - If `## Implementation Agents Status` section already exists: | |
| - Parse existing agent statuses and revision counts | |
| - Skip agents marked as "completed" - their work is preserved | |
| - Resume agents marked as "in_progress" or "needs_revision" | |
| - Preserve existing revision counts when resuming | |
| - Log: "Resuming implementation - skipping N completed agents" | |
| - If section does not exist, create it fresh | |
| **Agent Status Format**: | |
| ```markdown | |
| ## Implementation Agents Status | |
| - agent-1: pending (revision: 0) | |
| - agent-2: in_progress (revision: 0) | |
| - agent-3: needs_revision (revision: 1) | |
| - agent-4: completed (revision: 0) | |
| - agent-5: failed (revision: 2) | |
| ``` | |
| Valid statuses: | |
| - `pending`: Not yet started | |
| - `in_progress`: Currently being worked on | |
| - `needs_revision`: Audit failed, awaiting re-implementation | |
| - `completed`: Finished and audit passed | |
| - `failed`: Max revisions reached or unrecoverable error | |
| - **Check for existing Implementation Agents Status** (resume support): | |
| - Read the state management file ($2) | |
| - If `## Implementation Agents Status` section already exists: | |
| - Parse existing agent statuses using format: `- agent-id: status (revision: N)` | |
| - If format is malformed, log warning and treat as fresh start | |
| - Skip agents marked as "completed" - their work is preserved | |
| - Resume agents marked as "in_progress" or "needs_revision" | |
| - Preserve existing revision counts (0-indexed, where 0 = first attempt) | |
| - Log: "Resuming implementation - skipping N completed agents" | |
| - If section does not exist, create it fresh | |
| **Agent Status Format**: |
🤖 Prompt for AI Agents
In `@plugins/claude-constructor/commands/implement-increment.md` around lines 33 -
61, Specify exact parsing and semantics for the "## Implementation Agents
Status" block: parse lines matching the regex ^- (?P<id>[^:]+):
(?P<status>pending|in_progress|needs_revision|completed|failed) \(revision:
(?P<rev>\d+)\)$ to extract agent-id, status and numeric revision; treat any
non-matching or malformed line as warning-and-default-to "pending (revision: 0)"
and log the issue. Define revision semantics: revision is zero-based (revision:
0 = first attempt), max attempts is 3 meaning allowed revisions are 0..2 (stop
and mark "failed" when revision >= 3), and increment revision by 1 immediately
after a failed attempt before scheduling a retry (preserve existing revision
counts when resuming). For invalid status values found during parsing, coerce to
"pending" with revision preserved or set to 0 if missing, and log a warning that
the status was normalized.
Comment on lines
34
to
37
| c. Update specification file: | ||
| - Move answered questions to `### Resolved Questions` section (create if needed) | ||
| - Format: Question title + "**Answer:** [selected option with description]" | ||
| - Remove the `[STRUCTURED]` tag from resolved questions |
There was a problem hiding this comment.
Reconsider modifying the specification file post-creation.
The workflow modifies the specification file by moving questions to a "Resolved Questions" section. This raises several concerns:
- Versioning conflict: The PR states it relies on "git history for versioning" rather than timestamps, but modifying the specification file after creation complicates this approach.
- Concurrency: If multiple processes or users access the specification simultaneously, modifications could conflict.
- Audit trail: Moving content between sections loses the original context of when questions were open vs. resolved.
Consider storing resolved question answers in the state management file instead, keeping the specification as a read-only artifact after initial creation.
🤖 Prompt for AI Agents
In `@plugins/claude-constructor/commands/requirements-sign-off.md` around lines 34
- 37, Stop editing the specification file (do not move items into the "###
Resolved Questions" section or remove the "[STRUCTURED]" tag there); instead
change the workflow so resolved answers are appended to the state management
file (e.g., a JSON/YAML state store) and keep the spec read-only after creation,
moving the code that currently mutates the spec (the logic that parses answered
questions and writes them back into "### Resolved Questions") into a new/updated
function that writes resolution records to the state file and marks questions
resolved there while preserving original spec content and metadata.
- Standardize placeholder notation: {issue_key} not {issue-key}
- Fix workflow step references across all agents and commands
- Update example timestamp year from 2024 to 2026
- Add /implementation-summary to README internal commands
- Remove redundant approval check in specification-sign-off
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add missing state-management-file-path ($2) argument to Skill commands - Document option parsing rules for STRUCTURED questions (canonical format, accepted variants, validation with graceful fallback) - Add explicit batching loop logic for AskUserQuestion tool - Replace placeholder with dynamic summary generation in review-pull-request Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Consolidates all workflow artifacts into issue-specific subdirectories under
claude_constructor/{issue_key}/instead of scattered top-level directories.Old structure:
New structure:
Changes:
claude_constructor/to .gitignoreBenefits:
Test plan
/featureworkflow with Linear/Jira issue/featureworkflow with prompt mode🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.