feat: implement #34 - Light weight option for bugs and issues

[github-actions]

Auto-Implemented via Ensemble Pipeline

This PR was automatically generated using the full Ensemble workflow from issue #34.

Pipeline Phases

1. PRD — /ensemble:create-prd → docs/PRD/issue-34.md
2. TRD — /ensemble:create-trd → docs/TRD/issue-34.md
3. Implementation — /ensemble:implement-trd

Source Issue

Closes #34

⚠️ Review Required

This is AI-generated code. Please review the PRD, TRD, and implementation carefully before merging.

---

Generated by auto-implement workflow

[ldangelo]

Code Review: PR #35

Recommendation: Do not merge until critical issues are resolved.

Critical

1. Schema violation — fix-issue.yaml uses category: development which is not in the allowed enum (analysis, workflow, infrastructure, quality, documentation, git, testing, planning, implementation, deployment). This will fail npm run validate. Should be implementation or workflow.

2. Missing lastUpdated field — All other commands in the package include this field; fix-issue.yaml omits it.

High

3. git add . security risk — The command instructions tell agents to run git add ., which can stage secrets, .env files, or credentials. This directly contradicts the project's own CLAUDE.md guidance. Should specify staging files by name.

4. constraints nesting inconsistency — Constraints are nested under mission.constraints instead of being a top-level key like in sibling commands (create-trd.yaml, etc.).

Medium

5. Missing expectedInput/expectedOutput sections that sibling commands define
6. Emoji usage in generated markdown (inconsistent with project style)
7. PRD/TRD documents dated 2026-02-17 but were generated on 2026-02-16

Low

8. No negative tests or schema validation in the test file — a schema validation test would have caught issue #1
9. Commit format instructions say "Fix: {description}" instead of conventional fix: {description}

---

Minimum fixes before merge:

1. Change category to implementation or workflow
2. Add lastUpdated field
3. Replace git add . with safe file staging
4. Run npm run validate to confirm

[claude]

Code Review — PR #35: Lightweight fix-issue workflow

Overall this is a well-structured addition that meaningfully improves the automation pipeline. The routing logic (bug/task → lightweight fix-issue path vs. full PRD/TRD pipeline) is a sound architectural decision. The review below covers both the good parts and the issues I found.

---

.github/workflows/auto-implement.yml

Issue (medium): git add -A in the Commit step (line 271)

The commit step still uses git add -A for all paths. The CLAUDE.md commit conventions explicitly warn against this ("avoid accidentally including sensitive files"). The fix-issue.yaml command itself correctly instructs agents to use git add {specific-files}, but the CI step does the opposite. In a CI context where the Claude action writes a temp context file to .github/issue-context/, an unexpected artefact that doesn't get cleaned up would silently land in the commit.

The Clean up temporary files step at line 241–242 does run rm -rf .github/issue-context before the commit, which mitigates the immediate risk, but git add -A is still overly broad if any other unexpected file appears in the workspace.

Issue (medium): Heredoc $ISSUE_NUMBER_PLACEHOLDER substitution in PR body (lines 282–325)

The substitution relies on a non-standard shell string replacement (${var//pattern/replace}) applied to the output of a heredoc assigned to a variable. This works in bash but the pattern is fragile — if the heredoc content or ISSUE_NUMBER ever contains characters that confuse shell expansion (e.g., /, &), the result is undefined. Issue titles in ISSUE_TITLE are already used in commit messages and branch names without sanitisation (see below), so this is a related risk surface.

The cleaner approach would be a single printf or injecting the variable directly into the heredoc rather than post-processing.

Issue (medium): ISSUE_TITLE used unsanitised in branch name and commit message (lines 98, 260–268)

ISSUE_TITLE comes from github.event.issue.title and is used verbatim:

• In the branch name via BRANCH_PREFIX/issue-{ISSUE_NUMBER} (safe; title not used here, but commit message uses it)
• In commit message: ${COMMIT_PREFIX}: fix #${ISSUE_NUMBER} - ${ISSUE_TITLE}

If an issue title contains newlines, backticks, or shell metacharacters this could cause the git commit -m to behave unexpectedly. A ISSUE_TITLE=$(echo "$ISSUE_TITLE" | tr -d '\n\r' | head -c 100) guard would prevent the worst cases.

Concern (low): Rate limiting is commented out

The comment on lines 47–62 says "TODO: Re-enable rate limiting after testing is complete." If this merges and the TODO is forgotten, the workflow has no protection against a flood of auto-implement triggers. Recommend either re-enabling it in this PR or creating a follow-up issue to track it.

Good: workflow routing logic is clean

The Detect issue type step (lines 64–84) using toJSON(github.event.issue.labels.*.name) + grep -qi is a pragmatic approach. The toJSON expression produces a JSON array string (e.g., ["bug","auto-implement"]) so the grep '"bug"' match against quoted labels is correct and avoids false positives from labels like "debug-tool". Well done.

Good: if-no-files-found: ignore on artifact upload

Avoids spurious failures when the fix-issue path doesn't produce PRD/TRD docs.

---

packages/development/commands/fix-issue.yaml

Issue (low): Collaborative Planning spawns 4 orchestrators sequentially for every fix

The planning step (Phase 1, Step 2) delegates to product-management-orchestrator, tech-lead-orchestrator, infrastructure-orchestrator, and qa-orchestrator in sequence. For a lightweight bug-fix workflow this is heavier than the name implies — 4 full orchestrator calls adds significant latency and token cost before any code is changed. The original issue (#34) is specifically about a "lightweight option". Consider making 2 of the orchestrators optional (e.g., only escalate to infrastructure/product orchestrators if the analysis step flags architectural impact) or parallelising them via concurrent Task calls.

Concern (low): description < 20 words ambiguity trigger

The user interview trigger fires when the description is fewer than 20 words. A clean, precise 15-word bug description shouldn't trigger an interview. A word count heuristic is fragile — consider checking for vague terms as the primary signal and word count only as a secondary indicator.

Good: explicit git add {specific-files} in PR creation instructions

Phase 3 Step 2 explicitly says git add {specific-files} (stage only files modified during this workflow; never use git add . or git add -A). This is aligned with CLAUDE.md conventions and the security-conscious pattern from the existing project.

Good: retry: 2 on Test Validation

Clear ceiling on auto-fix retries with a hard HALT prevents infinite loops.

---

packages/development/tests/fix-issue-command.test.js

Good: structural test coverage

The tests validate YAML schema structure, parameter types, phase ordering, agent delegation, model selection, error handling, and TodoWrite integration. For a YAML-driven command definition this is the right approach.

Issue (low): tests are purely structural, no behaviour coverage

All 415 lines test the YAML structure rather than any runtime behaviour. This is partially justified because the command is declarative, but there are no tests covering edge cases like:

• What happens when neither --issue nor description is provided?
• Does the skip-tests flag actually suppress the retry block in the instructions?
• Is the branch name slug generation logic tested?

These can't easily be unit-tested from YAML alone — but a comment or follow-up issue tracking integration test gaps would be valuable.

Issue (low): js-yaml dependency assumed to be available

The test file requires js-yaml directly, but packages/development/package.json needs to declare it as a devDependency. If it is already present as a transitive dependency this works, but it's fragile. Confirm js-yaml is in devDependencies.

---

docs/PRD/issue-34.md and docs/TRD/issue-34.md

These are comprehensive AI-generated documents that provide good rationale for the implementation. No issues.

---

Summary

Severity	Count	Key items
Medium	3	git add -A in CI; heredoc substitution fragility; unsanitised ISSUE_TITLE
Low	4	Rate limiting TODO; 4 orchestrators for "lightweight" path; ambiguity trigger heuristic; no js-yaml devDependency declared

The medium issues should be addressed before merge. The low issues can be follow-up work.

---

Reviewed by Claude Sonnet 4.5 via ensemble code review workflow