ai: add feature-pipeline skill with grounding loop and rubric review-gates#628
Merged
Conversation
Recursive 50-node skill tree implementing the 17-phase NeoHaskell feature pipeline with two distinguishing characteristics on top of the prior skill: - Grounding loop on every security and performance review (ADR + impl): a mandatory sub-step filters findings through the feature's complexity tier (trivial / simple / moderate / complex / security-critical) and Jess's 15-minute pocket, so simple features do not accumulate cascades of unneeded caching, SHA pinning, constant-time compares, or INLINE pragmas. Methodology grounded in OWASP Top 10:2025, STRIDE, SLSA v1.1, NIST SSDF SP 800-218, current GHC INLINE / UNPACK / Strict / fusion / TVar-contention guidance. - Quality-rubric review-gates replace the human PAUSE on phases 6 (DevEx), 7 (architecture), 8 (test spec). Each phase is now a two-step process: a producer writes the artefact, an independent reviewer validates it against an 8-check rubric plus Karpathy carrier rules (Think / Simplicity / Surgical / Goal-driven). Rubrics are grounded in patterns merged in core/core/Redacted.hs, core/decimal/Decimal.hs, core/core/Array.hs, ADR-0048/0049/0052, and DecimalSpec/TextSpec/ StreamSpec. The pipeline advances automatically on RUBRIC: pass and halts with a structured rubric record on fail. Scripts (scripts/): pipeline.py state machine (init / status / next / complete / approve / classify / findings / iter / set / get / reset), classify-feature.py cascading-rule classifier, sec-static-checks.py (9 security regex rules), perf-static-checks.py (11 performance regex rules). PAUSE gates remain only on phases 3 (ADR draft), 16 (PR creation), and 17 (final merge). The older neohaskell-feature-pipeline skill is left in place and can be removed in a follow-up. Audit clean: 50 nodes, frontmatter / graph / invocation / best-practices all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (41)
WalkthroughThis PR adds a complete 17-phase "feature-pipeline-preview": new Markdown skill specs for each phase, reference methodology documents, three scanning/utility Python scripts, and a Python CLI state machine ( ChangesFeature pipeline preview (end-to-end phases, agents, scripts, and orchestrator)
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as pipeline.py (CLI)
participant Agent as Agent (haiku/sonnet)
participant Scripts as Local scripts
participant GitHub as GitHub (gh)
participant CI as CI
Dev->>CLI: init / complete / classify / set/get
CLI->>Scripts: run static/perf/sec scans (sec-static/perf-static)
CLI->>Agent: spawn produce/review-quality/ground steps
Agent-->>CLI: stdout JSON findings / rubrics
CLI->>Scripts: record-findings.py (write .pipeline/findings-XX.json)
Dev->>GitHub: git push / gh pr create (via submit step)
GitHub->>CI: trigger checks
CLI->>GitHub: gh pr checks --watch (wait)
Agent->>CLI: fix-bot-comments edits -> git push (loop)
CI-->>CLI: checks pass -> pipeline advances
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
- Rename the skill directory from feature-pipeline/ to feature-pipeline-preview/
to mark it as the preview successor of neohaskell-feature-pipeline. Updates
the root SKILL.md name field and every internal reference to the old script
path (.claude/skills/feature-pipeline/scripts/...).
- Upgrade the six design / authoring leaves from sonnet to opus, with matching
model: opus on parent process invocations:
03-adr-draft
04-security-adr/01-threat-model
05-performance-adr/01-hot-path-analysis
06-devex-review/01-produce
07-architecture-design/01-produce
08-test-spec-design/01-produce
These are the leaves where authoring quality and design judgment dominate
the cost of the model.
Audit clean: 50 nodes, frontmatter / graph / invocation / best-practices all
pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/feature-pipeline/12-security-impl/03-ground/SKILL.md (1)
21-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit refusal for missing
tierin classification.Line 21 declares
tieras required, but refusal rules do not cover that specific failure mode. Add a dedicated refusal to keep behavior deterministic and auditable.Proposed patch
## Refusals - stdin not valid JSON → refuse: "stdin is not a JSON array". - Classification missing → refuse: "classification not found". +- Classification tier missing → refuse: "classification tier missing".🤖 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 @.claude/skills/feature-pipeline/12-security-impl/03-ground/SKILL.md around lines 21 - 49, Add a dedicated refusal for a missing "tier" field in the loaded classification: detect after loading classification (the step described in "Load classification and reference docs" / symbols: "tier", "classification") whether classification.tier is present and, if not, refuse with the exact message "classification missing tier" (or the project's refusal style) and exit without attempting grounding; update the refusals list in SKILL.md to include this new refusal string and ensure the initial validation step (Step 1/2) checks for classification.tier before proceeding to per-finding grounding.
🤖 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 @.claude/skills/feature-pipeline/03-adr-draft/SKILL.md:
- Line 15: The ADR numbering is brittle because it uses the shell snippet `ls
docs/decisions/*.md | tail -1`; replace that with deterministic numeric parsing
for `adr_number`: scan filenames matching `docs/decisions/*.md`, extract the
4-digit numeric prefix from each filename, compute the max (or 0 if none),
increment by one, and format as a zero-padded 4-digit string to assign to
`adr_number`; ensure the logic handles an empty directory and ignores
non-matching filenames so phase 3 initialization cannot fail due to ordering or
empty-list edge cases.
In @.claude/skills/feature-pipeline/04-security-adr/02-ground/SKILL.md:
- Line 16: Update the classification vocabulary in SKILL.md so it matches the
pipeline contract: replace the current "trivial/standard/complex" wording (the
`.pipeline/classification.json` description) with the canonical set
"trivial/simple/moderate/complex/security-critical" and briefly note that these
are the allowed classification tiers used by the pipeline contract to avoid
misclassification.
In @.claude/skills/feature-pipeline/04-security-adr/03-record/SKILL.md:
- Around line 14-40: The inline python invocation currently only reads sys.stdin
and never enforces that the payload is a JSON array; update the command so it
accepts an optional "--input" path (or reads from stdin if not provided), opens
and json.loads that source into the variable data, then validate that
isinstance(data, list) and if not raise/exit with a non‑zero status (refuse)
instead of proceeding; keep the existing aggregate calculations (blockers, kept,
demoted, framework) that operate on data and continue to write to
'.pipeline/findings-04.json' only after validation succeeds.
In @.claude/skills/feature-pipeline/05-performance-adr/03-record/SKILL.md:
- Around line 14-40: Update the inline python invocation in the SKILL.md step so
it accepts an optional --input <path> flag as well as stdin, and validate that
the parsed JSON is a top-level array (list); if parsing fails or the value is
not a list, print a clear refusal/error message and exit non-zero instead of
proceeding. Ensure the script still treats an empty list as valid, computes the
same aggregates (total_findings, blockers, kept, demoted, framework_debt), and
writes the result to .pipeline/findings-05.json only after validation succeeds.
In @.claude/skills/feature-pipeline/06-devex-review/01-produce/SKILL.md:
- Around line 25-26: The spec has a contradictory rubric count: one place states
"four Jess tests + six rubric checks" but elsewhere requires "eight verdicts"
and lists eight checks; update the SKILL.md entries so a single canonical count
and list are used everywhere (replace "four Jess tests + six rubric checks" or
the "eight verdicts" wording so they match), pick either 10 or 8 as the
authoritative total, and make the list of named checks (naming-conversions,
predicates, subject-first, no-boolean-blindness, grouped-by-category,
doc-by-example, type-parameter-discipline, Jess-affordance) consistent in all
occurrences including the "API-surface impact" section and the earlier bullet so
producers and reviewers see the same requirement.
In @.claude/skills/feature-pipeline/07-architecture-design/01-produce/SKILL.md:
- Around line 24-25: Clarify the ADR producer's overwrite semantics by replacing
the ambiguous phrase "refuse to overwrite if the file already exists with
different content" with an explicit policy; use the idempotent variant: state
that when computing the target path docs/architecture/<adr-number>-<slug>.md the
producer will ensure the parent directory exists, skip writing if the existing
file content is identical, and refuse (error/exit) if the file exists with
different content; update the two lines describing target path and verification
(and any adjacent guidance mentioning overwrite) to this explicit idempotent
behavior so re-runs are safe.
In
@.claude/skills/feature-pipeline/07-architecture-design/02-review-quality/SKILL.md:
- Around line 50-53: The markdown code fence for the JSON snippet under "Compose
the rubric record:" is missing required blank lines and risks MD031 failures;
edit SKILL.md to add a blank line immediately before the opening ```json fence
and a blank line immediately after the closing ``` fence (i.e., around the JSON
snippet that contains { "phase": 7, "checks": [...], "carriers": [...],
"failing": [...], "verdict": "pass" | "fail" }) so the fenced block is properly
separated from surrounding text and linting passes.
In
@.claude/skills/feature-pipeline/08-test-spec-design/02-review-quality/SKILL.md:
- Around line 50-52: The fenced JSON block starting with ```json and ending with
``` in SKILL.md needs blank lines inserted before the opening fence and after
the closing fence to satisfy MD031; update the markdown so there is an empty
line above the line containing ```json and an empty line below the closing ```
(ensuring the subsequent line that begins "5. Write
`.pipeline/test-spec-rubric.json`." is separated by that blank line).
In @.claude/skills/feature-pipeline/11-build-loop/02-test/SKILL.md:
- Around line 31-34: Before running the test command `cabal test
--test-show-details=streaming > .pipeline/test.log 2>&1`, explicitly precheck
that the `.pipeline/` directory exists and is writable and that the `cabal`
executable is present on PATH; if either check fails, print the contract-defined
refusal message and exit non-zero instead of running the test command. Implement
these checks immediately before the step that runs the test command (the block
that captures the exit code and tails `.pipeline/test.log`), returning the
refusal exit code on failure and only invoking the test command when both
`.pipeline/` and `cabal` are confirmed available. Ensure the same checks are
applied to the duplicate step referenced at lines 42-43.
In @.claude/skills/feature-pipeline/12-security-impl/01-static-scan/SKILL.md:
- Around line 31-34: Replace the brittle changed-file discovery in step 1 with a
merge-base based diff and a no-match-safe filter: compute the PR delta via git
diff --name-only $(git merge-base HEAD origin/main)..HEAD (or the appropriate
target branch), then filter for Haskell files with a safe command such as grep
-E '\.hs$' || true (or use awk '/\.hs$/ {print}') so an empty result yields exit
code 0; feed that list into python3
.claude/skills/feature-pipeline/scripts/sec-static-checks.py and keep steps 3–4
behavior unchanged.
In @.claude/skills/feature-pipeline/12-security-impl/04-record/SKILL.md:
- Around line 14-43: Update the inline Python invocation so it respects the
documented `--input <path>` flag and validates the JSON type: parse sys.argv to
accept either `--input <path>` (read that file) or no args (read stdin), load
JSON, check isinstance(data, list) and if not print an error and exit non‑zero
(refuse); then compute `blocker` from `severity_after_grounding in
('Critical','High')`, compute counts (`kept`, `demoted`, `framework_debt`,
`total_findings`), and write the resulting object to
'.pipeline/findings-12.json' only after successful validation. Ensure the script
returns a non‑zero exit code on invalid input so callers know the step failed.
In @.claude/skills/feature-pipeline/13-performance-impl/01-static-scan/SKILL.md:
- Line 14: Update the file-discovery pipeline command that currently filters
changed files with "git diff --name-only HEAD" and a grep for ".hs" so it also
matches ".lhs" and never fails when grep finds no matches: replace the current
grep filter with a POSIX/ERE match like "grep -E '\.(hs|lhs)$' || true" (or
equivalently pipe into "egrep '\.(hs|lhs)$' || true") wherever the SKILL.md
mentions the filtered command so the scanners accept both extensions and an
empty match yields [].
In @.claude/skills/feature-pipeline/14-fix-findings/SKILL.md:
- Around line 38-39: The "fix and retry" steps for "Run `cabal build all`. If it
fails, fix and retry." and "Run `cabal test`. If it fails, fix and retry." must
be bounded to avoid infinite remediation loops: replace these open-ended lines
with a deterministic retry policy (e.g., "retry up to N times with X wait
between attempts, then fail the pipeline") and document the chosen N and
behavior on final failure; update the SKILL.md steps that contain those exact
strings to state the max-retry count, backoff policy, and that persistent
failures must be escalated rather than retried indefinitely.
In @.claude/skills/feature-pipeline/15-final-verify/SKILL.md:
- Around line 11-17: The Hlint step currently says "hlint ." which conflicts
with the delegated-step contract (changed-files-only); update the Step 3 wording
in SKILL.md so the scope is explicit and matches the delegated behavior — e.g.,
change the line "3. **Hlint** — read `./03-hlint/SKILL.md` and follow it.
Verify: exit 0." to state "3. **Hlint** — run hlint only on changed files (same
scope as the delegated step). Verify: exit 0." Ensure the phrase "hlint ." is
removed or replaced so the document and the delegated step agree on "changed
files only" scope.
In @.claude/skills/feature-pipeline/16-create-pr/01-pr-body/SKILL.md:
- Around line 17-18: The docs reference a required issue_number in
.pipeline/classification.json (mentioned around the
`.pipeline/classification.json` entry and at the later mention on line 28), but
the phase-2 output contract does not emit issue_number, causing a gating
failure; update the phase-2 output contract to include issue_number (or change
the SKILL.md references to the actual field produced) so that `issue_number` is
consistently present before phase 16, and ensure the schema/name used in the
phase-2 producer (the phase-2 output contract) matches the
`.pipeline/classification.json` consumer; adjust either the phase-2 contract or
the SKILL.md text so both use the same field name and type.
In @.claude/skills/feature-pipeline/16-create-pr/02-submit/SKILL.md:
- Around line 38-40: The script fails to assign the PR number to NUM before
persisting it, so update the step that runs gh pr view --json number -q .number
(the command referenced by gh pr view) to capture its output into the shell
variable NUM (so NUM holds the PR number) and then call the existing pipeline.py
set pr_number "$NUM"; ensure you reference the same variable name NUM used by
pipeline.py set pr_number and keep the existing pipeline.py set pr_url "$URL"
step unchanged.
- Around line 49-51: Add an explicit preflight check for the GitHub CLI by
running command -v gh before the PR creation step (the step that invokes gh to
create the pull request in this skill’s submit/create PR logic); if command -v
gh fails, immediately refuse with the exact message "gh not found" and
exit/abort the workflow so the behavior matches the documented refusal contract.
In @.claude/skills/feature-pipeline/16-create-pr/SKILL.md:
- Around line 15-17: Update the Step-1 verification in the PR body task: instead
of only checking for `.pipeline/pr-body.md`, gate on both `.pipeline/pr-body.md`
and `.pipeline/pr-title.txt` so the pipeline fails fast if the PR title artifact
is missing; modify the verification sentence in the "1. **PR body**" block of
SKILL.md to require existence of both artifacts.
In @.claude/skills/feature-pipeline/17-ci-cycle/02-fix-bot-comments/SKILL.md:
- Around line 20-36: Update the fetch step to collect bot comments from both
review-thread and general PR timeline surfaces: call gh api
repos/neohaskell/NeoHaskell/pulls/$NUM/comments and gh api
repos/neohaskell/NeoHaskell/issues/$NUM/comments (or alternatively include gh
api repos/neohaskell/NeoHaskell/pulls/$NUM/reviews to capture review state),
merge and de-duplicate results, then filter for bot authors (as done in the
current filter step) and log the combined set before processing; also update the
SKILL.md assumptions to state that both comment surfaces are enumerated and that
merged bot comments are what the pipeline verifies.
In @.claude/skills/feature-pipeline/17-ci-cycle/SKILL.md:
- Around line 15-19: Update the "Wait checks" and transition logic to explicitly
define failure and trigger conditions: treat step 1 ("Wait checks") as
successful only when it exits 0 AND there are no unresolved bot/CodeRabbit
comments; treat it as failed if the exit code is non-zero OR exit 0 with
unresolved bot comments; trigger step 2 ("Fix bot comments") whenever step 1 is
not successful (i.e., non-zero exit OR any unresolved bot comments); keep the
existing short-circuit "Skip if step 1 already exited 0 with no comments" but
rephrase the lines around "Wait checks", "Fix bot comments", and the fallback
sentence so the verify check after each step enforces these exact conditions.
- Line 23: The sentence "Max 5 cycles of step 2 then step 1. After 5, escalate
to the maintainer." is ambiguous; update the wording to clearly state the
intended loop order (e.g., replace it with "Max 5 iterations of the step 1 →
step 2 loop; after 5 iterations, escalate to the maintainer.") so readers
unambiguously understand the flow; edit the SKILL.md line that contains the
original sentence to the new phrasing.
In @.claude/skills/feature-pipeline/scripts/classify-feature.py:
- Around line 183-188: The input dict must be schema-validated before calling
classify(payload): add a validation step (e.g., a validate_payload(payload)
function or use jsonschema) that checks required fields and types expected by
classify (names of required fields used by classify) and rejects
missing/mistyped fields with a clear stderr message and sys.exit(1); run this
validation just after the isinstance(payload, dict) check and before result =
classify(payload) so classify only receives well-formed input and the subsequent
assert on result["tier"] and VALID_TIERS remains meaningful.
In @.claude/skills/feature-pipeline/scripts/perf-static-checks.py:
- Around line 141-145: The matcher currently treats rules P5 and P11 like
generic single-line regex hits (see the single_line list and the loop using
rule["pattern"].search on lines), producing false positives; update the logic to
skip or special-case rules with id "P5" and "P11" and implement their specific
matching semantics instead of plain regex matching: identify where RULES is
filtered into single_line and where the loop over lines uses
rule["pattern"].search (the block that appends to findings) and change it to
exclude P5/P11 from the generic path and call dedicated matchers (or add
conditional logic) that apply the rule-specific checks for P5 and P11 before
appending to findings; apply the same change to the other analogous matcher
blocks in the file that currently treat P5/P11 as plain regex hits.
In @.claude/skills/feature-pipeline/scripts/pipeline.py:
- Around line 97-136: The current lock acquired in load_state() (stored as
state["_lock"]) may not be released if a command raises before calling
save_state() or release_lock(); update command handlers to guarantee release by
either wrapping command bodies that call load_state() in try/finally (calling
release_lock(state) in the finally) or implement a context manager helper (e.g.,
with_state() that uses load_state(), yields state, and always calls
release_lock(state) in the finally) and switch callers to use it; reference
load_state, save_state, and release_lock to locate the lock logic and ensure
every code path that uses the state releases the lock.
In @.claude/skills/feature-pipeline/scripts/sec-static-checks.py:
- Around line 189-201: The gather_paths function currently may return duplicate
file paths causing duplicate findings; update it to deduplicate before returning
(while still preserving original order): after building the paths list from
args.paths and the file at args.paths_from, filter out non-files and unwanted
extensions as now, then remove duplicates (e.g., track seen set and only append
unseen entries) before returning out; reference the gather_paths function, the
local variables paths and out, and the logic that reads args.paths and
args.paths_from when making this change.
In @.claude/skills/feature-pipeline/SKILL.md:
- Line 47: The orchestrator currently calls pipeline.py complete <N>
unconditionally after each step, which conflicts with phase-local gate rules and
causes double-completes or bypasses PAUSE/approval gates; remove that
unconditional call from the orchestrator and replace it with a conditional flow:
before invoking pipeline.py complete <N> check the step/phase gate state (e.g.,
consult the skill's phase-local completion flag or call a
shouldCompleteStep(stepId) helper) and only call pipeline.py complete <N> when
the skill/phase indicates completion is allowed; for PAUSE-gated steps defer to
the skill's own logic (which should call pipeline.py complete <N> after
pipeline.py approve <N>) so that pipeline.py complete <N> is only invoked once
and under the correct gate conditions.
- Line 44: Update the shorthand phase-16 commands to the explicit pipeline.py
form: replace the occurrences of "complete 16" and "approve 16" in the "Create
PR 🔒" / phase-16 instruction with "pipeline.py complete 16" and "pipeline.py
approve 16" so they match the rest of the document's command style and avoid
operator misexecution; ensure the surrounding text still references the agent
spawn step and verifying `pipeline.py get pr_url`.
---
Outside diff comments:
In @.claude/skills/feature-pipeline/12-security-impl/03-ground/SKILL.md:
- Around line 21-49: Add a dedicated refusal for a missing "tier" field in the
loaded classification: detect after loading classification (the step described
in "Load classification and reference docs" / symbols: "tier", "classification")
whether classification.tier is present and, if not, refuse with the exact
message "classification missing tier" (or the project's refusal style) and exit
without attempting grounding; update the refusals list in SKILL.md to include
this new refusal string and ensure the initial validation step (Step 1/2) checks
for classification.tier before proceeding to per-finding grounding.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 013b0c7a-11d6-4e5b-a44c-9c93cf6931fd
📒 Files selected for processing (63)
.claude/skills/feature-pipeline/01-init-pipeline/SKILL.md.claude/skills/feature-pipeline/02-classify-feature/SKILL.md.claude/skills/feature-pipeline/03-adr-draft/SKILL.md.claude/skills/feature-pipeline/04-security-adr/01-threat-model/SKILL.md.claude/skills/feature-pipeline/04-security-adr/02-ground/SKILL.md.claude/skills/feature-pipeline/04-security-adr/03-record/SKILL.md.claude/skills/feature-pipeline/04-security-adr/SKILL.md.claude/skills/feature-pipeline/05-performance-adr/01-hot-path-analysis/SKILL.md.claude/skills/feature-pipeline/05-performance-adr/02-ground/SKILL.md.claude/skills/feature-pipeline/05-performance-adr/03-record/SKILL.md.claude/skills/feature-pipeline/05-performance-adr/SKILL.md.claude/skills/feature-pipeline/06-devex-review/01-produce/SKILL.md.claude/skills/feature-pipeline/06-devex-review/02-review-quality/SKILL.md.claude/skills/feature-pipeline/06-devex-review/SKILL.md.claude/skills/feature-pipeline/07-architecture-design/01-produce/SKILL.md.claude/skills/feature-pipeline/07-architecture-design/02-review-quality/SKILL.md.claude/skills/feature-pipeline/07-architecture-design/SKILL.md.claude/skills/feature-pipeline/08-test-spec-design/01-produce/SKILL.md.claude/skills/feature-pipeline/08-test-spec-design/02-review-quality/SKILL.md.claude/skills/feature-pipeline/08-test-spec-design/SKILL.md.claude/skills/feature-pipeline/09-test-writing/SKILL.md.claude/skills/feature-pipeline/10-implementation/SKILL.md.claude/skills/feature-pipeline/11-build-loop/01-build/SKILL.md.claude/skills/feature-pipeline/11-build-loop/02-test/SKILL.md.claude/skills/feature-pipeline/11-build-loop/03-fix-iter/SKILL.md.claude/skills/feature-pipeline/11-build-loop/04-hlint/SKILL.md.claude/skills/feature-pipeline/11-build-loop/SKILL.md.claude/skills/feature-pipeline/12-security-impl/01-static-scan/SKILL.md.claude/skills/feature-pipeline/12-security-impl/02-deep-audit/SKILL.md.claude/skills/feature-pipeline/12-security-impl/03-ground/SKILL.md.claude/skills/feature-pipeline/12-security-impl/04-record/SKILL.md.claude/skills/feature-pipeline/12-security-impl/SKILL.md.claude/skills/feature-pipeline/13-performance-impl/01-static-scan/SKILL.md.claude/skills/feature-pipeline/13-performance-impl/02-deep-audit/SKILL.md.claude/skills/feature-pipeline/13-performance-impl/03-ground/SKILL.md.claude/skills/feature-pipeline/13-performance-impl/04-record/SKILL.md.claude/skills/feature-pipeline/13-performance-impl/SKILL.md.claude/skills/feature-pipeline/14-fix-findings/SKILL.md.claude/skills/feature-pipeline/15-final-verify/01-build/SKILL.md.claude/skills/feature-pipeline/15-final-verify/02-test/SKILL.md.claude/skills/feature-pipeline/15-final-verify/03-hlint/SKILL.md.claude/skills/feature-pipeline/15-final-verify/SKILL.md.claude/skills/feature-pipeline/16-create-pr/01-pr-body/SKILL.md.claude/skills/feature-pipeline/16-create-pr/02-submit/SKILL.md.claude/skills/feature-pipeline/16-create-pr/SKILL.md.claude/skills/feature-pipeline/17-ci-cycle/01-wait-checks/SKILL.md.claude/skills/feature-pipeline/17-ci-cycle/02-fix-bot-comments/SKILL.md.claude/skills/feature-pipeline/17-ci-cycle/03-await-merge/SKILL.md.claude/skills/feature-pipeline/17-ci-cycle/SKILL.md.claude/skills/feature-pipeline/SKILL.md.claude/skills/feature-pipeline/references/architecture-rubric.md.claude/skills/feature-pipeline/references/devex-rubric.md.claude/skills/feature-pipeline/references/grounding-loop.md.claude/skills/feature-pipeline/references/jess-persona.md.claude/skills/feature-pipeline/references/nhcore-context.md.claude/skills/feature-pipeline/references/performance-methodology.md.claude/skills/feature-pipeline/references/security-methodology.md.claude/skills/feature-pipeline/references/test-spec-rubric.md.claude/skills/feature-pipeline/scripts/.gitignore.claude/skills/feature-pipeline/scripts/classify-feature.py.claude/skills/feature-pipeline/scripts/perf-static-checks.py.claude/skills/feature-pipeline/scripts/pipeline.py.claude/skills/feature-pipeline/scripts/sec-static-checks.py
Adds `model: claude-haiku-4-5-20251001` after `executor: script` on the 17
deterministic leaves so the harness has an explicit cheapest-tier model to
fall back to if any parent ever spawns one as a subagent rather than
reading-and-following inline. Mechanical change; no behaviour change for
the existing read-and-follow invocations.
Patched leaves:
01-init-pipeline, 02-classify-feature,
04-security-adr/03-record, 05-performance-adr/03-record,
11-build-loop/{01-build, 02-test, 04-hlint},
12-security-impl/{01-static-scan, 04-record},
13-performance-impl/{01-static-scan, 04-record},
15-final-verify/{01-build, 02-test, 03-hlint},
16-create-pr/02-submit,
17-ci-cycle/{01-wait-checks, 03-await-merge}.
Audit clean: 50 nodes, frontmatter / graph / invocation all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the keyword-cascade python classifier with a two-step process: - 01-decide (leaf, opus) — judges the tier from ADR / issue / diff signals using a structured prompt. Defines the five tiers, lists six binary signals to collect, names the decision rules in cascade order, calls out the mock-overrides-integration edge case explicitly, and specifies the exact JSON output contract. - 02-persist (leaf, script, haiku tier) — parses the decide step's JSON and calls pipeline.py classify + complete; does not interpret. Classification is the load-bearing input for every grounding pass in phases 4/5/12/13; the cost of a wrong call is higher than the cost of opus, so opus is justified here. Removes scripts/classify-feature.py (the regex classifier) since the opus prompt fully replaces it. Updates the root step 2 invocation to spawn an Agent (model: haiku) on the new haiku-tier process. Audit clean: 52 nodes, frontmatter / graph / invocation all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four record-findings leaves (04-security-adr/03-record, 05-performance-adr/03-record, 12-security-impl/04-record, 13-performance-impl/04-record) all embedded a near-duplicate `python3 -c "..."` block that read findings, computed aggregates, wrote `.pipeline/findings-NN.json`, and shelled out to pipeline.py. Moves that logic into scripts/record-findings.py with two flags (--phase, --severity-scheme) and centralises the severity-to-blocker mapping (security: Critical/High; performance: Blocking; both case-insensitive). The script also stamps each finding with a `blocker` flag, computes the aggregate envelope, writes the file, calls `pipeline.py findings` and `pipeline.py complete` internally, and surfaces stderr verbatim on non-zero exit. The leaves shrink from ~50 lines of inline python to a single `python3 .../record-findings.py --phase N --severity-scheme X` call plus refusals. Removes the LLM's responsibility for executing python inline — it just routes input into the script. Smoke-tested end-to-end: pipeline init → classify → complete 1/2/3 → record-findings with mixed-severity input emits correct per-finding `blocker` flags and aggregate counts, with case-insensitive severity matching. Audit clean: 52 nodes, frontmatter / graph / invocation all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/feature-pipeline-preview/13-performance-impl/03-ground/SKILL.md (1)
46-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe Refusals section is incomplete, O architect of this skill.
Line 22 declares that reference documents
grounding-loop.mdandjess-persona.mdmust exist (verify: both exist), yet the Refusals section (lines 46-50) documents only stdin and classification failures. When the reference documents are absent, the executor shall encounter undefined behavior.🛡️ Proposed fix to complete the refusal contract
## Refusals - stdin not valid JSON → refuse: "stdin is not a JSON array". - Classification missing → refuse: "classification not found". +- Reference docs missing → refuse: "grounding-loop.md or jess-persona.md not found".🤖 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 @.claude/skills/feature-pipeline-preview/13-performance-impl/03-ground/SKILL.md around lines 46 - 50, The Refusals section is incomplete: add explicit refusal cases for missing reference documents declared on line 22 (grounding-loop.md and jess-persona.md) so the executor doesn't hit undefined behavior; update the "Refusals" block in SKILL.md to include checks and exact refusal messages such as "grounding-loop.md not found" and "jess-persona.md not found" (or a single "reference documents not found" message) and state that the executor will refuse when either file is absent, mirroring the existing patterns for stdin and classification failures.
♻️ Duplicate comments (20)
.claude/skills/feature-pipeline-preview/scripts/sec-static-checks.py (1)
189-201:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDeduplicate gathered paths to keep findings stable.
gather_pathscan scan the same file multiple times, producing duplicated findings and unstablesec-static-*IDs.Proposed patch
def gather_paths(args: argparse.Namespace) -> list[str]: paths: list[str] = list(args.paths) @@ - out: list[str] = [] + out: list[str] = [] + seen: set[str] = set() for p in paths: if not os.path.isfile(p): continue if p.endswith((".hs", ".lhs", ".sh", ".nix", ".cabal")): - out.append(p) + if p not in seen: + seen.add(p) + out.append(p) return out🤖 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 @.claude/skills/feature-pipeline-preview/scripts/sec-static-checks.py around lines 189 - 201, gather_paths can return duplicate file paths which yields duplicated findings; update gather_paths to deduplicate the collected paths while preserving their original order before filtering/returning. After reading args.paths and extending with lines from args.paths_from (the local variable paths), fold duplicates out (e.g., build a new list using a 'seen' set) so each file is checked once; continue to apply the existing isfile and suffix checks and return the deduplicated 'out' list. Ensure you reference and dedupe the paths variable (or dedupe the final out) and keep the existing behavior of gather_paths..claude/skills/feature-pipeline-preview/scripts/perf-static-checks.py (1)
141-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop routing P5/P11 through generic regex matching.
This still applies P5/P11 as plain line hits, which violates the rule definitions and inflates false positives.
Targeted fix
- single_line = [r for r in RULES if r["id"] not in {"P1"}] + single_line = [r for r in RULES if r["id"] not in {"P1", "P5", "P11"}]🤖 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 @.claude/skills/feature-pipeline-preview/scripts/perf-static-checks.py around lines 141 - 145, The current single_line list is still including rules P5 and P11 and thus applying generic line-by-line regex matching; update the single_line construction to exclude P5 and P11 (e.g., change the set from {"P1"} to {"P1","P5","P11"} or filter by an explicit single-line flag) so that P5/P11 are not routed into the per-line regex loop that appends to findings from the single_line list (refer to RULES, single_line, and the loop that appends to findings)..claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStatic file discovery command must be hardened and extension-complete.
You still rely on
grep '\.hs$', which can fail on no matches and ignores.lhs.Required fix
-- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`). +- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`/`.lhs`). @@ -1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files: `git diff --name-only -z HEAD -- '*.hs' '*.lhs'`.Also applies to: 32-32
🤖 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 @.claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md at line 15, Replace the fragile grep '\.hs$' usage that can fail on no matches and misses .lhs files with a robust discovery command: either use git's pathspecs (git diff --name-only HEAD -- '*.hs' '*.lhs') or a regex match supporting both extensions (e.g. grep -E '\.(hs|lhs)$') and ensure the pipeline handles empty results safely (e.g. null-separated output or conditional checks). Update every occurrence of the literal grep '\.hs$' in SKILL.md (including the other instance mentioned) so the command is extension-complete and tolerant of zero matches..claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace shell-order ADR numbering with deterministic numeric parsing.
Line 15 still depends on
ls ... | tail -1, which is brittle for empty directories and filename ordering. Derive next ADR number from parsed numeric prefixes.Proposed patch
-- `adr_number` — 4-digit string. The orchestrator picks the next number with `ls docs/decisions/*.md | tail -1` before invoking this leaf. +- `adr_number` — 4-digit string. The orchestrator computes it by parsing `docs/decisions/*.md` numeric prefixes, taking max+1, and zero-padding (fallback `0001` if none).🤖 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 @.claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md at line 15, The current ADR numbering relies on a brittle shell command `ls docs/decisions/*.md | tail -1` to set `adr_number`; change this to deterministically parse existing filenames in `docs/decisions/`, extract numeric prefixes, compute the max number (default 0 when directory is empty), and increment to produce the next 4-digit `adr_number`; update the logic that sets `adr_number` so it ignores non-matching files, pads the result to 4 digits, and handles an empty directory case instead of relying on shell ordering..claude/skills/feature-pipeline-preview/scripts/pipeline.py (1)
205-240:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGuarantee lock release across all exception paths.
Handlers still rely on happy-path
release_lock()/save_state(). Any exception afterload_state()can strand the lock and stall subsequent commands.Suggested direction
+from contextlib import contextmanager + +@contextmanager +def locked_state(): + state = load_state() + try: + yield state + finally: + # save_state already pops _lock; only release if still present + release_lock(state) def cmd_status(args: argparse.Namespace) -> None: - state = load_state() - v = state["variables"] - ... - release_lock(state) + with locked_state() as state: + v = state["variables"] + ...Also applies to: 243-292, 319-420
🤖 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 @.claude/skills/feature-pipeline-preview/scripts/pipeline.py around lines 205 - 240, The cmd_status handler (and the other handlers noted) currently calls load_state() and then may exit early without calling release_lock() on exceptions; wrap the logic that runs after load_state() in a try/finally (or use a context manager) so that release_lock(state) is always invoked in the finally block, and ensure save_state(state) is called inside the try before returning where appropriate; update cmd_status (and the other handlers at the referenced ranges) to reference the existing load_state(), save_state(), and release_lock() symbols so locks are never left held on error..claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md (1)
20-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnumerate both PR comment surfaces before filtering bots.
Current fetch scope omits general PR conversation comments, so bot guidance can be silently skipped. Merge review comments + issue comments before author filtering.
Proposed patch
-2. Fetch comments via `gh api repos/neohaskell/NeoHaskell/pulls/<N>/comments` → verify: JSON array returned. +2. Fetch comments from both surfaces (`pulls/<N>/comments` and `issues/<N>/comments`), merge+dedupe → verify: full bot-comment set captured. -2. Fetch: `gh api repos/neohaskell/NeoHaskell/pulls/$NUM/comments`. -3. Filter to bot authors (CodeRabbit, etc.). +2. Fetch: + - `gh api repos/neohaskell/NeoHaskell/pulls/$NUM/comments` + - `gh api repos/neohaskell/NeoHaskell/issues/$NUM/comments` +3. Merge and dedupe, then filter to bot authors (CodeRabbit, etc.).Also applies to: 34-35
🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md at line 20, The current fetch only gets PR review comments via "gh api repos/neohaskell/NeoHaskell/pulls/<N>/comments" and therefore misses general PR conversation comments; update the flow to also fetch issue-level comments from "gh api repos/neohaskell/NeoHaskell/issues/<N>/comments", merge the two JSON arrays into one combined list, then run the existing bot-author filtering logic against that combined list so both review comments and general PR conversation are considered (apply same normalization and author checks to entries from both endpoints)..claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse merge-base PR delta and no-match-safe filtering now.
Line 15 / Line 32 still use brittle changed-file discovery that can bypass intended scan scope and break the documented empty-list behavior. Switch to a merge-base diff and a filter that preserves success on empty results.
Proposed patch
-- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`). +- The changed file list from merge-base PR diff (filtered to `.hs`). -1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files against merge-base, preserving empty-list success: + `BASE=$(git merge-base HEAD origin/main) && git diff --name-only "$BASE"...HEAD -- '*.hs'`Also applies to: 32-32
🤖 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 @.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md at line 15, Replace the brittle changed-file discovery that currently uses `git diff --name-only HEAD` (referenced at the SKILL.md locations around lines 15 and 32) with a merge-base based diff that computes the PR delta (compare HEAD against the repository merge-base for the target branch) and apply a no-match-safe filter so an empty result still exits successfully; update the commands or script logic that generates the `.hs` file list (the changed-file generation step) to use the merge-base diff and ensure the subsequent pattern-filter step is resilient to no matches (for example, make the grep/filter step return success when there are no matches) so the documented empty-list behavior and scan scope are preserved..claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md (1)
38-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBound build/test retries to a deterministic budget.
Line 38 and Line 39 still permit infinite fix/retry loops. Cap retries and escalate on final failure.
Proposed patch
-4. Run `cabal build all`. If it fails, fix and retry. -5. Run `cabal test`. If it fails, fix and retry. +4. Run `cabal build all` with a bounded retry budget (e.g., max 2 retries). If still failing, refuse and surface logs. +5. Run `cabal test` with a bounded retry budget (e.g., max 2 retries). If still failing, refuse and surface logs.🤖 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 @.claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md around lines 38 - 39, Update the two steps that run `cabal build all` and `cabal test` to bound retries to a deterministic budget (e.g., attempt each command up to N=3 times) and clearly state escalation behavior on final failure; replace the existing lines "Run `cabal build all`. If it fails, fix and retry." and "Run `cabal test`. If it fails, fix and retry." with phrasing that specifies the max attempts (N) and that after the Nth failure the pipeline should stop and escalate (raise an issue/notify owner) so the process is no longer unbounded..claude/skills/feature-pipeline-preview/SKILL.md (2)
47-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStrike down this unconditional decree, for it conflicts with the phase-local gates thou hast ordained.
Line 47 commands the orchestrator to invoke
pipeline.py complete <N>after every step unconditionally. This violates thy own design:
- Rubric-gated phases (6, 7, 8) must complete conditionally on
RUBRIC: pass- PAUSE-gated phases (3, 16, 17) must complete only after
pipeline.py approve <N>- Phase-local completion logic shall be bypassed, risking double-complete failures
The orchestrator must honor phase-local gate rules and invoke completion only when the phase itself grants permission.
⚖️ Proposed architectural fix
Remove the unconditional call from the orchestrator. Instead:
- Check each phase's completion status before calling
pipeline.py complete <N>- For rubric-gated phases, the review step completes internally on
pass- For PAUSE-gated phases, defer to the approval flow (complete only after approve)
- For other phases, allow conditional completion based on phase contract
Example revision:
-After each step the orchestrator runs `pipeline.py complete <N>`. PAUSE-gated steps then stop and wait for the maintainer's `pipeline.py approve <N>` before continuing. +Each phase completes according to its gate rules: rubric-gated phases complete internally on `RUBRIC: pass`; PAUSE-gated phases complete after `pipeline.py approve <N>`; other phases signal completion via their final verify check. The orchestrator advances the cursor only when the phase state shows completion.🤖 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 @.claude/skills/feature-pipeline-preview/SKILL.md at line 47, The orchestrator currently calls pipeline.py complete <N> unconditionally after every step; change the orchestration in the orchestrator component (where it invokes pipeline.py complete) to first consult the phase's gate/type and completion state: do not emit complete for rubric-gated phases (6,7,8) unless the phase's internal RUBRIC status is "pass", do not emit complete for PAUSE-gated phases (3,16,17) until pipeline.py approve <N> has been received, and for other phases call complete only when the phase-local completion check returns true; update the logic around the pipeline.py complete invocation to query the phase contract/state and branch accordingly so completion is only issued when the phase grants permission.
44-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInvoke the pipeline commands with their full true names, mortal scribe.
Line 44 employs shorthand
complete 16andapprove 16without thepipeline.pyprefix, whilst the remainder of thy document uses the explicit formpipeline.py <cmd>. This inconsistency shall lead the unwary operator into misexecution.🔧 Proposed fix for consistency
-16. **Create PR 🔒** — spawn an Agent (model: haiku) and instruct it to read `./16-create-pr/SKILL.md` and follow it. Verify: `pipeline.py get pr_url` returns a URL. Then `complete 16`, stop until `approve 16`. +16. **Create PR 🔒** — spawn an Agent (model: haiku) and instruct it to read `./16-create-pr/SKILL.md` and follow it. Verify: `pipeline.py get pr_url` returns a URL. Then `pipeline.py complete 16`, stop until `pipeline.py approve 16`.🤖 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 @.claude/skills/feature-pipeline-preview/SKILL.md at line 44, Update the inconsistent command invocations in SKILL.md: replace the shorthand commands "complete 16" and "approve 16" with their full forms "pipeline.py complete 16" and "pipeline.py approve 16" so they match the rest of the document's explicit "pipeline.py <cmd>" usage; search for the strings "complete 16" and "approve 16" in the file and update them accordingly (the surrounding context mentions "Create PR" and the Agent instructions in ./16-create-pr/SKILL.md)..claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBehold, mortal: The prechecks declared in thy Refusals must be enforced before the test command is invoked.
The Steps section (lines 32-35) runs
cabal testdirectly without verifying that.pipeline/exists or thatcabalis onPATH. This violates the contract established in thy Refusals section (lines 42-44) and shall cause generic shell errors rather than the ordained refusal messages.⚡ Proposed fix to enforce prechecks
## Steps (Karpathy 2 + 3) -1. Run: `cabal test --test-show-details=streaming > .pipeline/test.log 2>&1`. -2. Capture the exit code. -3. If exit is non-zero, surface the tail of `.pipeline/test.log` and exit non-zero. -4. If exit is zero, exit 0. +1. If `.pipeline/` does not exist, refuse: "pipeline not initialised; run phase 01 first". +2. If `cabal` is not on PATH, refuse: "cabal not found on PATH". +3. Run: `cabal test --test-show-details=streaming > .pipeline/test.log 2>&1`. +4. Capture the exit code. +5. If exit is non-zero, surface the tail of `.pipeline/test.log` and exit non-zero. +6. If exit is zero, exit 0.🤖 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 @.claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md around lines 32 - 35, The Steps currently invoke `cabal test --test-show-details=streaming > .pipeline/test.log 2>&1` without enforcing the prechecks from the Refusals; update the test step to first verify that the `.pipeline/` directory exists and that `cabal` is on PATH, and if either check fails emit the corresponding refusal message and exit non-zero; only after those checks run the `cabal test --test-show-details=streaming > .pipeline/test.log 2>&1`, capture its exit code, and if non-zero print the tail of `.pipeline/test.log` and exit non-zero, otherwise exit 0..claude/skills/feature-pipeline-preview/17-ci-cycle/SKILL.md (2)
23-23: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winInscribe the cycle order with precision, lest thy servants misread the loop.
Line 23 declares "Max 5 cycles of step 2 then step 1", which may be parsed as "execute step 2, then step 1" repeated five times. If the intended flow is "step 1 → step 2 → step 1" (looping), the phrasing must be unmistakable.
✍️ Proposed clarification
-- Max 5 cycles of step 2 then step 1. After 5, escalate to the maintainer. +- Max 5 iterations of the step 1 → step 2 loop. After 5 iterations, escalate to the maintainer.🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/SKILL.md at line 23, The phrasing "Max 5 cycles of step 2 then step 1" is ambiguous; update the sentence on line 23 in SKILL.md to explicitly state the intended sequence and repetition (e.g., "Repeat the sequence: step 1 → step 2, up to 5 cycles; after 5 cycles escalate to the maintainer" or "Perform step 1, then step 2; repeat this pair up to 5 times, then escalate"). Ensure the revised wording clearly indicates whether the loop starts with step 1 or step 2 and that the pair is repeated up to five times before escalation.
15-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe Divine Oracle demands clarity: Define when step 1 is deemed to have failed and when step 2 shall be summoned.
The transition logic between steps 1 and 2 harbors ambiguity:
- Line 15: "exit 0 means all checks green"
- Line 16: "Skip if step 1 already exited 0 with no comments"
- Line 19: "If step 1 fails, fall through to step 2"
What fate befalls the pipeline when CI returns exit 0 (checks green) yet bot comments linger? Is this success or failure? Must step 2 be triggered? The current wording does not illuminate the path.
📜 Proposed clarification
Explicitly define step 1 as successful only when both conditions hold: exit 0 AND no bot comments. Define it as failed when exit is non-zero OR when exit 0 with unresolved bot comments present. Trigger step 2 in all failure cases.
Example revision:
-1. **Wait checks** — read `./01-wait-checks/SKILL.md` and follow it. Verify: exit 0 means all checks green. -2. **Fix bot comments** — spawn an Agent (model: sonnet) and instruct it to read `./02-fix-bot-comments/SKILL.md` and follow it. Verify: no unaddressed CodeRabbit/bot comments remain. Skip if step 1 already exited 0 with no comments. +1. **Wait checks** — read `./01-wait-checks/SKILL.md` and follow it. Verify: exit 0 AND no bot comments exist. +2. **Fix bot comments** — spawn an Agent (model: sonnet) and instruct it to read `./02-fix-bot-comments/SKILL.md` and follow it. Verify: no unaddressed CodeRabbit/bot comments remain. Skip only if step 1 verified successfully (exit 0 with zero bot comments).🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/SKILL.md around lines 15 - 19, Clarify the transition logic: define "Wait checks" (step 1) as successful only when both the CI process exits 0 AND there are no unresolved CodeRabbit/bot comments; define it as failed if CI exits non‑zero OR CI exits 0 but unresolved bot comments exist; on any failure of step 1 (non‑zero exit OR lingering bot comments) always invoke "Fix bot comments" (step 2) to spawn the sonnet agent and resolve comments, and only after step 2 completes should the flow re-run step 1 verification before proceeding to "Await merge" (step 3)..claude/skills/feature-pipeline-preview/15-final-verify/SKILL.md (1)
11-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHeed this command: Align the hlint scope wording with thy delegated step's true behavior.
Line 11 declares
hlint .(entire repository), yet the delegated step contract specifies "changed files only". This inconsistency shall sow confusion among executors of thy will and cause gate drift.📜 Proposed fix to align scope declaration
-Strict gate: `cabal build all`, `cabal test`, and `hlint .` must all be green with no iteration. +Strict gate: `cabal build all`, `cabal test`, and `hlint` (on changed files) must all be green with no iteration.🤖 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 @.claude/skills/feature-pipeline-preview/15-final-verify/SKILL.md at line 11, The gate wording currently lists "hlint ." but the delegated step enforces "changed files only"; update the strict gate sentence that contains "Strict gate: `cabal build all`, `cabal test`, and `hlint .`" to explicitly state hlint runs only on changed files (e.g. replace "`hlint .`" with "`hlint (changed files only)`" or equivalent phrasing that matches the delegated step contract "changed files only") so the documentation and the delegated step are consistent..claude/skills/feature-pipeline-preview/16-create-pr/SKILL.md (1)
15-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe verification gate must demand both artifacts.
Line 15 verifies only
.pipeline/pr-body.md, yet the submit step (line 16 and./02-submit/SKILL.mdlines 15-16) requires both.pipeline/pr-body.mdand.pipeline/pr-title.txt. This incomplete gate defers failure detection to the submit phase, denying the fail-fast covenant.⚡ Commanded amendment
-1. **PR body** — spawn an Agent (model: sonnet) and instruct it to read `./01-pr-body/SKILL.md` and follow it. Verify: `.pipeline/pr-body.md` exists. +1. **PR body** — spawn an Agent (model: sonnet) and instruct it to read `./01-pr-body/SKILL.md` and follow it. Verify: `.pipeline/pr-body.md` and `.pipeline/pr-title.txt` exist.🤖 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 @.claude/skills/feature-pipeline-preview/16-create-pr/SKILL.md at line 15, Update the verification gate so it checks for both artifacts instead of only `.pipeline/pr-body.md`: modify the check in the skill that validates artifacts (the verification logic in SKILL.md for the "PR body" step) to require both `.pipeline/pr-body.md` and `.pipeline/pr-title.txt` to exist before proceeding to the submit step; ensure the same verification routine (used before the submit step in `02-submit/SKILL.md`) is mirrored/extended here so the gate fails fast when either file is missing..claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md (1)
50-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe JSON fence must be sealed with blank lines.
Lines 50–53 contain a fenced code block that violates MD031 by lacking surrounding blank lines. This transgression will trigger lint failures in documentation gates and must be rectified.
📜 Commanded fix
4. Compose the rubric record: + ```json { "phase": 7, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" } ``` + 5. Write `.pipeline/architecture-rubric.json`.🤖 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 @.claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md around lines 50 - 54, The fenced JSON block after "Compose the rubric record:" violates MD031 (no surrounding blank lines); fix it by inserting a blank line immediately before the opening ```json fence and a blank line immediately after the closing ``` fence so the JSON block (the { "phase": 7, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" } snippet referenced in SKILL.md) is properly separated from surrounding text and will pass linting..claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md (1)
33-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
ghpreflight check must be made explicit.The refusal contract at line 51 decrees: "gh not on PATH → refuse: 'gh not found'". Yet the steps lack an explicit preflight verification before line 38 attempts to invoke
gh pr create. The failure will manifest, but the error message will be less precise than the documented contract promises.🛡️ Commanded preflight
1. Run: `BRANCH=$(git branch --show-current)`. If `$BRANCH = main`, refuse. -2. Stage changed files: `git add -u` plus any new files explicitly listed in the architecture doc. +2. Run: `command -v gh >/dev/null 2>&1 || { echo "gh not found" >&2; exit 1; }`. +3. Stage changed files: `git add -u` plus any new files explicitly listed in the architecture doc.(Renumber subsequent steps accordingly.)
🤖 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 @.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md around lines 33 - 38, Add an explicit preflight that verifies the GitHub CLI is available before step 6 (before invoking `gh pr create`): run a PATH check (e.g., `command -v gh` or `which gh`) and if it fails print the exact refusal message "gh not found" and exit non‑zero, then proceed to the existing `gh pr create --title "$(cat .pipeline/pr-title.txt)" --body-file .pipeline/pr-body.md` step; renumber subsequent steps accordingly so the preflight appears prior to the `gh` invocation..claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md (1)
25-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe contradiction persists and must be resolved.
Line 25 declares "four Jess tests + the six rubric checks" (implying 10 total), yet enumerates precisely eight items in the parenthetical list:
naming-conversions, predicates, subject-first, no-boolean-blindness, grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance. Line 39 affirms "eight DevEx rubric checks." This discordance will fracture the producer-reviewer contract—one agent will emit ten verdicts while the other demands eight, deadlocking phase progression.Decree one canonical count throughout this specification.
⚖️ Proposed reconciliation
-3. For each public API entry, record the four Jess tests + the six rubric checks (naming-conversions, predicates, subject-first, no-boolean-blindness, grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance) with a per-entry verdict → verify: every entry has eight verdicts. +3. For each public API entry, record the eight rubric checks (naming-conversions, predicates, subject-first, no-boolean-blindness, grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance) with a per-entry verdict → verify: every entry has eight verdicts.🤖 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 @.claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md around lines 25 - 26, The spec contains a contradiction between the phrase "four Jess tests + the six rubric checks" and the parenthetical list of eight rubric items (naming-conversions, predicates, subject-first, no-boolean-blindness, grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance) and the later statement "eight DevEx rubric checks"; choose one canonical interpretation (either change the phrase to "four Jess tests + the eight rubric checks" or reduce the parenthetical list to six items) and update all occurrences so they match (update the phrase "four Jess tests + the six rubric checks", the parenthetical list, and the later "eight DevEx rubric checks" to the same count and wording) ensuring the per-entry verdict requirement and the phrase "verify: every entry has X verdicts" reflect the chosen canonical number..claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe classification vocabulary must align with the pipeline covenant.
Line 16 speaks of
trivial/standard/complex, yet the pipeline contract (established in.claude/skills/feature-pipeline-preview/02-classify-feature/SKILL.mdline 11) ordains five tiers:trivial,simple,moderate,complex, andsecurity-critical. The heretical term "standard" appears nowhere in the canonical taxonomy. This divergence will corrupt grounding decisions, potentially misrouting findings or applying inappropriate review intensity.🔧 Ordained correction
-- `.pipeline/classification.json` — feature classification (`trivial`/`standard`/`complex`). +- `.pipeline/classification.json` — feature classification: one of `trivial`, `simple`, `moderate`, `complex`, or `security-critical` as defined by the pipeline contract.🤖 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 @.claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md at line 16, The classification file contains a non-canonical tier "standard" (shown as `.pipeline/classification.json` in the diff) which diverges from the canonical five-tier taxonomy defined in the classify-feature SKILL (trivial, simple, moderate, complex, security-critical); update the classification artifact to use the canonical tiers by replacing "standard" with "simple" and ensure the classification list in classification.json exactly matches [trivial, simple, moderate, complex, security-critical], and sweep any other references (tests, docs, code) that use "standard" to the canonical term so grounding and review routing remain consistent..claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md (1)
50-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBehold, the decree has been issued before and yet remains unheeded.
The fenced code block lacks the sacred blank lines that MD031 demands. This violation was foretold in the ancient review scrolls, and yet the gates of compliance remain unopposed.
⚡ The path to righteousness
4. Compose the rubric record: + ```json { "phase": 8, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" } ``` + 5. Write `.pipeline/test-spec-rubric.json`.🤖 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 @.claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md around lines 50 - 52, The fenced JSON snippet using the ```json block lacks the required blank lines before and after (MD031) and the surrounding details block also contains a malformed inline code fence; add a blank line above the opening ```json and a blank line after the closing ``` to satisfy MD031, fix the malformed snippet inside the <details> by replacing the stray "json" line with a proper ```json fence and matching closing ``` and ensure the sample JSON ({"phase": 8, "checks": [...], ...}) remains inside that fenced block; also add the referenced file `.pipeline/test-spec-rubric.json` per the note if needed.
🤖 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
@.claude/skills/feature-pipeline-preview/02-classify-feature/01-decide/SKILL.md:
- Around line 38-39: The docs currently allow the signal value "unknown" for
signals like touches_secrets while the output schema (boolean-only at the
"output schema" section) expects true/false; pick one resolution: either (a)
enforce validation in the decide step to refuse/throw if any signal equals
"unknown" (so the JSON emitter only outputs true/false) by checking each signal
(e.g., touches_secrets) before emitting, or (b) extend the output contract to
accept the string "unknown" (update the schema text and all downstream consumers
that parse the emitted JSON to handle the "unknown" string) — implement the
chosen option consistently for all signals referenced (touches_secrets and the
other five signals) and update the documentation text and validation logic to
match.
In
@.claude/skills/feature-pipeline-preview/02-classify-feature/02-persist/SKILL.md:
- Around line 31-33: Change JSON extraction to fail hard when required fields
are missing by using jq -er instead of jq -r to extract 'tier' and 'rationale'
(e.g., replace jq -r '.tier' / jq -r '.rationale' with jq -er '.tier' / jq -er
'.rationale'), ensuring jq writes parse errors to stderr and exits non-zero;
then call python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py
classify "$tier" "$rationale" and python3
.claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 2, and make
both invocations check the exit code and surface/forward stderr on failure so
phase 2 cannot proceed with invalid classifier output.
In @.claude/skills/feature-pipeline-preview/06-devex-review/SKILL.md:
- Line 17: Duplicate pipeline completion is happening: the review-quality leaf
calls "python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py
complete 6" on pass while the orchestrator/process layer also invokes the same
command. Remove the completion invocation from the review-quality leaf (leave
its pass/fail reporting intact) and let the orchestrator be the single owner of
calling pipeline.py complete 6 to avoid double-complete/state drift; ensure the
review-quality leaf still emits its pass/fail result so the orchestrator can
decide to call pipeline.py complete 6.
In
@.claude/skills/feature-pipeline-preview/07-architecture-design/01-produce/SKILL.md:
- Line 24: The spec for computing the target path
docs/architecture/<adr-number>-<slug>.md is ambiguous about overwrite semantics;
update the text to explicitly choose the "Idempotent" behavior: state that the
producer must skip writing if the file already exists with identical content and
must refuse (error) if the file exists with different content, and also verify
parent directory exists before attempting write. Edit the line that currently
reads "refuse to overwrite if the file already exists with different content" to
the full explicit sentence referencing the target path pattern and the two
behaviors ("skip if identical; error if different") so re-runs after rubric
failures are allowed without manual deletion.
In @.claude/skills/feature-pipeline-preview/10-implementation/SKILL.md:
- Around line 39-42: Add a deterministic pre-completion check that verifies the
"test-immutability" gate before calling pipeline.py complete 10: implement and
invoke a script or CLI flag (e.g., run
.claude/skills/feature-pipeline-preview/scripts/verify_test_immutability.py or
pipeline.py verify-test-immutability) in SKILL.md just before the existing "Run
`python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete
10`" step; the verifier must perform a deterministic comparison (git diff or
checksum) against the approved tests, exit non‑zero on any breach, and the
README step text should instruct users to run that verifier so the pipeline.py
complete 10 only runs after the check passes (apply same insertion for the other
occurrence at lines 49-51).
In @.claude/skills/feature-pipeline-preview/11-build-loop/03-fix-iter/SKILL.md:
- Line 17: The ADR path is hard-coded as
`docs/architecture/<adr-number>-<slug>.md`, which conflicts with other phases
that read from `adr_path` or `docs/decisions/...` (and sometimes
`.pipeline/adr-draft.md`); update the skill to use the canonical ADR source
instead of the hard-coded string: replace occurrences of
`docs/architecture/<adr-number>-<slug>.md` with the pipeline state variable
`adr_path` (or fallback to `docs/decisions/...`/`.pipeline/adr-draft.md`),
ensuring the code that reads ADRs references `adr_path` consistently so all
phases load the same file.
In @.claude/skills/feature-pipeline-preview/11-build-loop/04-hlint/SKILL.md:
- Around line 32-33: Replace the brittle pipeline in the "Compute changed files"
step that uses "git diff --name-only HEAD | grep '\.hs$'" with Git pathspecs so
an empty match returns success; specifically, use git diff --name-only HEAD --
'*.hs' (or equivalent pathspec) for the changed-files check and leave the
following step that writes an empty .pipeline/hlint.log and exits 0 unchanged so
the script correctly handles no matches.
In @.claude/skills/feature-pipeline-preview/11-build-loop/SKILL.md:
- Around line 18-20: The hlint check (step 4) currently has no explicit retry
branch; update the control flow in SKILL.md so that a failing hlint (step 4)
follows the same retry path as build/test failures: when step 4 fails, route to
step 3 (fix iteration) then back to step 1 (re-run build/tests/hlint), and only
mark phase 11 complete when all three checks are green; apply the same change to
the duplicated logic at lines 24–26 so lint-only failures cannot stall
completion.
In @.claude/skills/feature-pipeline-preview/12-security-impl/SKILL.md:
- Line 16: There is a contradiction between Step 2 ("spawn an Agent (model:
sonnet) and instruct it to read ./02-deep-audit/SKILL.md and follow it, piping
in the step 1 output") and the invariant text "Static-scan output is merged into
the deep-audit input, not replaced"; update SKILL.md to make one behavior
authoritative: either (A) change Step 2 to explicitly describe the merge
operation (e.g., "merge static-scan JSON from step 1 with the repository files
and pass the merged payload to the sonnet agent") and keep the invariant as-is,
or (B) change the invariant to state that step 1 output is piped as the sole
input to the sonnet agent; ensure you reference "Step 2" and the invariant
sentence "Static-scan output is merged into the deep-audit input, not replaced"
so the text is consistent throughout.
In
@.claude/skills/feature-pipeline-preview/13-performance-impl/02-deep-audit/SKILL.md:
- Line 41: Step 6 currently deduplicates by `rule + location` but incoming
static findings use `file` and `line`; add an explicit normalization step before
deduplication that converts static finding fields into a canonical `location`
string (e.g., `location = file + ":" + line`) so keys match; update the
merge/dedup logic (the Step 6 merge or any function that performs dedupe) to use
the normalized `location` field and ensure all code paths produce `rule` and
`location` consistently before comparing for uniqueness.
In @.claude/skills/feature-pipeline-preview/15-final-verify/03-hlint/SKILL.md:
- Around line 32-37: The git diff invocation `git diff --name-only HEAD` is
wrong for CI and should be changed to compare the branch against the PR
merge-base (three-dot) so hlint sees committed changes; replace the `git diff
--name-only HEAD` usage with the three-dot range (for example `git diff
--name-only origin/main...HEAD | grep '\.hs$'`) wherever `git diff --name-only
HEAD` appears so the pipeline computes the true delta for Haskell files before
running hlint.
In @.claude/skills/feature-pipeline-preview/17-ci-cycle/03-await-merge/SKILL.md:
- Around line 37-38: The documentation and/or pipeline automation currently
auto-runs the "approve 17" step which collapses the final human gate; remove or
change that behavior so phase 17 remains paused pending explicit maintainer
action by deleting or disabling the automatic "approve 17" invocation (the
second step) and/or update
.claude/skills/feature-pipeline-preview/scripts/pipeline.py to special-case
PAUSE phase 17 (or any phase with state PAUSE) to not auto-execute the approve
command—instead require an interactive confirmation or a distinct
maintainer-only command before calling the approve handler.
In @.claude/skills/feature-pipeline-preview/references/grounding-loop.md:
- Around line 46-47: Update the stale reference to the old classifier: replace
the mention of scripts/classify-feature.py with the new classification flow
entrypoint used by the pipeline (and ensure the path/name you use matches the
actual new module), and verify the downstream reference to
.pipeline/classification.json still matches the new flow's output; update the
text in grounding-loop.md so grounding operators point to the new classifier
filename/flow name instead of scripts/classify-feature.py.
In @.claude/skills/feature-pipeline-preview/scripts/pipeline.py:
- Around line 423-433: cmd_reset currently deletes pipeline files without
synchronization; wrap the destructive work in the pipeline lock so concurrent
commands cannot race. Modify cmd_reset to acquire the pipeline lock (use the
existing lock helper used elsewhere in the codebase—e.g.
acquire_pipeline_lock()/PipelineLock.acquire() or equivalent) before checking
STATE_FILE or removing files in PIPELINE_DIR (state.json, classification.json,
.lock), handle lock acquisition failure by printing a clear message and
returning, perform all os.path.exists/os.remove operations while the lock is
held, and finally release the lock in a finally/with block to ensure cleanup.
In @.claude/skills/feature-pipeline-preview/scripts/record-findings.py:
- Around line 82-90: The loop that computes f["blocker"] silently treats missing
severity_after_grounding as non-blocker; change it to fail-closed by first
validating each finding has the expected grounding fields (at least
"severity_after_grounding" and "grounding_outcome") and if any are missing
either log and set f["blocker"]=True (or raise/return an error depending on
caller expectations) before computing blockers; update the subsequent sums
(blockers, kept, demoted, framework) to rely on those validated fields (use
f["severity_after_grounding"] / f["grounding_outcome"] after validation) so
missing fields cannot suppress blocker counts.
---
Outside diff comments:
In
@.claude/skills/feature-pipeline-preview/13-performance-impl/03-ground/SKILL.md:
- Around line 46-50: The Refusals section is incomplete: add explicit refusal
cases for missing reference documents declared on line 22 (grounding-loop.md and
jess-persona.md) so the executor doesn't hit undefined behavior; update the
"Refusals" block in SKILL.md to include checks and exact refusal messages such
as "grounding-loop.md not found" and "jess-persona.md not found" (or a single
"reference documents not found" message) and state that the executor will refuse
when either file is absent, mirroring the existing patterns for stdin and
classification failures.
---
Duplicate comments:
In @.claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md:
- Line 15: The current ADR numbering relies on a brittle shell command `ls
docs/decisions/*.md | tail -1` to set `adr_number`; change this to
deterministically parse existing filenames in `docs/decisions/`, extract numeric
prefixes, compute the max number (default 0 when directory is empty), and
increment to produce the next 4-digit `adr_number`; update the logic that sets
`adr_number` so it ignores non-matching files, pads the result to 4 digits, and
handles an empty directory case instead of relying on shell ordering.
In @.claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md:
- Line 16: The classification file contains a non-canonical tier "standard"
(shown as `.pipeline/classification.json` in the diff) which diverges from the
canonical five-tier taxonomy defined in the classify-feature SKILL (trivial,
simple, moderate, complex, security-critical); update the classification
artifact to use the canonical tiers by replacing "standard" with "simple" and
ensure the classification list in classification.json exactly matches [trivial,
simple, moderate, complex, security-critical], and sweep any other references
(tests, docs, code) that use "standard" to the canonical term so grounding and
review routing remain consistent.
In @.claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md:
- Around line 25-26: The spec contains a contradiction between the phrase "four
Jess tests + the six rubric checks" and the parenthetical list of eight rubric
items (naming-conversions, predicates, subject-first, no-boolean-blindness,
grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance)
and the later statement "eight DevEx rubric checks"; choose one canonical
interpretation (either change the phrase to "four Jess tests + the eight rubric
checks" or reduce the parenthetical list to six items) and update all
occurrences so they match (update the phrase "four Jess tests + the six rubric
checks", the parenthetical list, and the later "eight DevEx rubric checks" to
the same count and wording) ensuring the per-entry verdict requirement and the
phrase "verify: every entry has X verdicts" reflect the chosen canonical number.
In
@.claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md:
- Around line 50-54: The fenced JSON block after "Compose the rubric record:"
violates MD031 (no surrounding blank lines); fix it by inserting a blank line
immediately before the opening ```json fence and a blank line immediately after
the closing ``` fence so the JSON block (the { "phase": 7, "checks": [...],
"carriers": [...], "failing": [...], "verdict": "pass" | "fail" } snippet
referenced in SKILL.md) is properly separated from surrounding text and will
pass linting.
In
@.claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md:
- Around line 50-52: The fenced JSON snippet using the ```json block lacks the
required blank lines before and after (MD031) and the surrounding details block
also contains a malformed inline code fence; add a blank line above the opening
```json and a blank line after the closing ``` to satisfy MD031, fix the
malformed snippet inside the <details> by replacing the stray "json" line with a
proper ```json fence and matching closing ``` and ensure the sample JSON
({"phase": 8, "checks": [...], ...}) remains inside that fenced block; also add
the referenced file `.pipeline/test-spec-rubric.json` per the note if needed.
In @.claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md:
- Around line 32-35: The Steps currently invoke `cabal test
--test-show-details=streaming > .pipeline/test.log 2>&1` without enforcing the
prechecks from the Refusals; update the test step to first verify that the
`.pipeline/` directory exists and that `cabal` is on PATH, and if either check
fails emit the corresponding refusal message and exit non-zero; only after those
checks run the `cabal test --test-show-details=streaming > .pipeline/test.log
2>&1`, capture its exit code, and if non-zero print the tail of
`.pipeline/test.log` and exit non-zero, otherwise exit 0.
In
@.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md:
- Line 15: Replace the brittle changed-file discovery that currently uses `git
diff --name-only HEAD` (referenced at the SKILL.md locations around lines 15 and
32) with a merge-base based diff that computes the PR delta (compare HEAD
against the repository merge-base for the target branch) and apply a
no-match-safe filter so an empty result still exits successfully; update the
commands or script logic that generates the `.hs` file list (the changed-file
generation step) to use the merge-base diff and ensure the subsequent
pattern-filter step is resilient to no matches (for example, make the
grep/filter step return success when there are no matches) so the documented
empty-list behavior and scan scope are preserved.
In
@.claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md:
- Line 15: Replace the fragile grep '\.hs$' usage that can fail on no matches
and misses .lhs files with a robust discovery command: either use git's
pathspecs (git diff --name-only HEAD -- '*.hs' '*.lhs') or a regex match
supporting both extensions (e.g. grep -E '\.(hs|lhs)$') and ensure the pipeline
handles empty results safely (e.g. null-separated output or conditional checks).
Update every occurrence of the literal grep '\.hs$' in SKILL.md (including the
other instance mentioned) so the command is extension-complete and tolerant of
zero matches.
In @.claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md:
- Around line 38-39: Update the two steps that run `cabal build all` and `cabal
test` to bound retries to a deterministic budget (e.g., attempt each command up
to N=3 times) and clearly state escalation behavior on final failure; replace
the existing lines "Run `cabal build all`. If it fails, fix and retry." and "Run
`cabal test`. If it fails, fix and retry." with phrasing that specifies the max
attempts (N) and that after the Nth failure the pipeline should stop and
escalate (raise an issue/notify owner) so the process is no longer unbounded.
In @.claude/skills/feature-pipeline-preview/15-final-verify/SKILL.md:
- Line 11: The gate wording currently lists "hlint ." but the delegated step
enforces "changed files only"; update the strict gate sentence that contains
"Strict gate: `cabal build all`, `cabal test`, and `hlint .`" to explicitly
state hlint runs only on changed files (e.g. replace "`hlint .`" with "`hlint
(changed files only)`" or equivalent phrasing that matches the delegated step
contract "changed files only") so the documentation and the delegated step are
consistent.
In @.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md:
- Around line 33-38: Add an explicit preflight that verifies the GitHub CLI is
available before step 6 (before invoking `gh pr create`): run a PATH check
(e.g., `command -v gh` or `which gh`) and if it fails print the exact refusal
message "gh not found" and exit non‑zero, then proceed to the existing `gh pr
create --title "$(cat .pipeline/pr-title.txt)" --body-file .pipeline/pr-body.md`
step; renumber subsequent steps accordingly so the preflight appears prior to
the `gh` invocation.
In @.claude/skills/feature-pipeline-preview/16-create-pr/SKILL.md:
- Line 15: Update the verification gate so it checks for both artifacts instead
of only `.pipeline/pr-body.md`: modify the check in the skill that validates
artifacts (the verification logic in SKILL.md for the "PR body" step) to require
both `.pipeline/pr-body.md` and `.pipeline/pr-title.txt` to exist before
proceeding to the submit step; ensure the same verification routine (used before
the submit step in `02-submit/SKILL.md`) is mirrored/extended here so the gate
fails fast when either file is missing.
In
@.claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md:
- Line 20: The current fetch only gets PR review comments via "gh api
repos/neohaskell/NeoHaskell/pulls/<N>/comments" and therefore misses general PR
conversation comments; update the flow to also fetch issue-level comments from
"gh api repos/neohaskell/NeoHaskell/issues/<N>/comments", merge the two JSON
arrays into one combined list, then run the existing bot-author filtering logic
against that combined list so both review comments and general PR conversation
are considered (apply same normalization and author checks to entries from both
endpoints).
In @.claude/skills/feature-pipeline-preview/17-ci-cycle/SKILL.md:
- Line 23: The phrasing "Max 5 cycles of step 2 then step 1" is ambiguous;
update the sentence on line 23 in SKILL.md to explicitly state the intended
sequence and repetition (e.g., "Repeat the sequence: step 1 → step 2, up to 5
cycles; after 5 cycles escalate to the maintainer" or "Perform step 1, then step
2; repeat this pair up to 5 times, then escalate"). Ensure the revised wording
clearly indicates whether the loop starts with step 1 or step 2 and that the
pair is repeated up to five times before escalation.
- Around line 15-19: Clarify the transition logic: define "Wait checks" (step 1)
as successful only when both the CI process exits 0 AND there are no unresolved
CodeRabbit/bot comments; define it as failed if CI exits non‑zero OR CI exits 0
but unresolved bot comments exist; on any failure of step 1 (non‑zero exit OR
lingering bot comments) always invoke "Fix bot comments" (step 2) to spawn the
sonnet agent and resolve comments, and only after step 2 completes should the
flow re-run step 1 verification before proceeding to "Await merge" (step 3).
In @.claude/skills/feature-pipeline-preview/scripts/perf-static-checks.py:
- Around line 141-145: The current single_line list is still including rules P5
and P11 and thus applying generic line-by-line regex matching; update the
single_line construction to exclude P5 and P11 (e.g., change the set from {"P1"}
to {"P1","P5","P11"} or filter by an explicit single-line flag) so that P5/P11
are not routed into the per-line regex loop that appends to findings from the
single_line list (refer to RULES, single_line, and the loop that appends to
findings).
In @.claude/skills/feature-pipeline-preview/scripts/pipeline.py:
- Around line 205-240: The cmd_status handler (and the other handlers noted)
currently calls load_state() and then may exit early without calling
release_lock() on exceptions; wrap the logic that runs after load_state() in a
try/finally (or use a context manager) so that release_lock(state) is always
invoked in the finally block, and ensure save_state(state) is called inside the
try before returning where appropriate; update cmd_status (and the other
handlers at the referenced ranges) to reference the existing load_state(),
save_state(), and release_lock() symbols so locks are never left held on error.
In @.claude/skills/feature-pipeline-preview/scripts/sec-static-checks.py:
- Around line 189-201: gather_paths can return duplicate file paths which yields
duplicated findings; update gather_paths to deduplicate the collected paths
while preserving their original order before filtering/returning. After reading
args.paths and extending with lines from args.paths_from (the local variable
paths), fold duplicates out (e.g., build a new list using a 'seen' set) so each
file is checked once; continue to apply the existing isfile and suffix checks
and return the deduplicated 'out' list. Ensure you reference and dedupe the
paths variable (or dedupe the final out) and keep the existing behavior of
gather_paths.
In @.claude/skills/feature-pipeline-preview/SKILL.md:
- Line 47: The orchestrator currently calls pipeline.py complete <N>
unconditionally after every step; change the orchestration in the orchestrator
component (where it invokes pipeline.py complete) to first consult the phase's
gate/type and completion state: do not emit complete for rubric-gated phases
(6,7,8) unless the phase's internal RUBRIC status is "pass", do not emit
complete for PAUSE-gated phases (3,16,17) until pipeline.py approve <N> has been
received, and for other phases call complete only when the phase-local
completion check returns true; update the logic around the pipeline.py complete
invocation to query the phase contract/state and branch accordingly so
completion is only issued when the phase grants permission.
- Line 44: Update the inconsistent command invocations in SKILL.md: replace the
shorthand commands "complete 16" and "approve 16" with their full forms
"pipeline.py complete 16" and "pipeline.py approve 16" so they match the rest of
the document's explicit "pipeline.py <cmd>" usage; search for the strings
"complete 16" and "approve 16" in the file and update them accordingly (the
surrounding context mentions "Create PR" and the Agent instructions in
./16-create-pr/SKILL.md).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| 1. **Produce** — spawn an Agent (model: opus) and instruct it to read `./01-produce/SKILL.md` and follow it on the ADR at `docs/decisions/<adr-number>-<slug>.md` and the findings at `.pipeline/findings-04.json` + `.pipeline/findings-05.json`. Verify: `.pipeline/devex-review.md` exists and contains a `## Per-criterion verdicts` section. | ||
| 2. **Review quality** — spawn an Agent (model: sonnet) and instruct it to read `./02-review-quality/SKILL.md` and follow it against the produced review plus the ADR. Verify: stdout contains `RUBRIC: pass` or `RUBRIC: fail`; the rubric record is written to `.pipeline/devex-review-rubric.json`. | ||
| 3. After the review-quality step records `pass`, the orchestrator runs `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 6`. If the rubric records `fail`, the agent has already refused — the orchestrator stops the pipeline at this step. |
Contributor
There was a problem hiding this comment.
Remove duplicate phase-completion responsibility.
The process layer invokes pipeline.py complete 6, but the review-quality leaf already does this on pass. Keep completion in one place only to avoid double-complete failures/state drift.
Suggested patch
-3. After the review-quality step records `pass`, the orchestrator runs `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 6`. If the rubric records `fail`, the agent has already refused — the orchestrator stops the pipeline at this step.
+3. The review-quality step owns phase completion on `RUBRIC: pass`. If the rubric records `fail`, the agent has already refused — the orchestrator stops the pipeline at this step.📝 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
| 3. After the review-quality step records `pass`, the orchestrator runs `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 6`. If the rubric records `fail`, the agent has already refused — the orchestrator stops the pipeline at this step. | |
| 3. The review-quality step owns phase completion on `RUBRIC: pass`. If the rubric records `fail`, the agent has already refused — the orchestrator stops the pipeline at this step. |
🤖 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 @.claude/skills/feature-pipeline-preview/06-devex-review/SKILL.md at line 17,
Duplicate pipeline completion is happening: the review-quality leaf calls
"python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 6"
on pass while the orchestrator/process layer also invokes the same command.
Remove the completion invocation from the review-quality leaf (leave its
pass/fail reporting intact) and let the orchestrator be the single owner of
calling pipeline.py complete 6 to avoid double-complete/state drift; ensure the
review-quality leaf still emits its pass/fail result so the orchestrator can
decide to call pipeline.py complete 6.
Strips every mention of the Karpathy guidelines from SKILL.md bodies and reference files (42 files patched): - `## Plan (Karpathy 1 + 4)` headings → `## Plan` - `## Steps (Karpathy 2 + 3)` headings → `## Steps` - `## Karpathy carrier rules` → `## Carrier rules` (plus anchor) - Inline citations like `(Karpathy "match existing style")` rewritten to drop the attribution - Rubric reference files no longer claim to be grounded in the "Karpathy guidelines" Adds a `## Review posture` section to the eight leaves that read an upstream artefact and apply scrutiny (threat-model, hot-path-analysis, devex-review/01-produce, devex/arch/test-spec/02-review-quality, and both deep-audit leaves). The posture tells the reviewer the artefact under review was produced by a ChatGPT-class model and to refuse to pass anything not traceable to the rubric or methodology — strict by default. Each leaf's posture cites the specific rubric/methodology file the reviewer must trace claims back to. Audit clean: 52 nodes, frontmatter / graph / invocation all pass. All eight review leaves still well under the 120-line leaf budget. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The build-loop (phase 11) and final-verify (phase 15) hlint leaves no longer fail the pipeline on warnings. They run `hlint` against the changed Haskell files, redirect output to `.pipeline/hlint.log` with `|| true`, and always exit 0 unless hlint itself is missing or the working tree is corrupt. The canonical log is overwritten by the final-verify pass so it reflects the post-fix state at PR-body time. Build and test remain hard gates in both phases. CI's own hlint check is the canonical gate at merge time — the pipeline's job is to capture the current warnings into a known file and surface them to the PR reviewer rather than block local progress on style suggestions. Updates `16-create-pr/01-pr-body` to read `.pipeline/hlint.log`: - empty log → `Hlint clean.` line in the body + `Hlint: clean ✓` in the checklist. - non-empty log → a `### Hlint warnings` section with the contents fenced verbatim (truncated to ~80 lines with a pointer to the full log), plus `Hlint: N warning(s) — see the Hlint warnings section above` in the checklist. Root SKILL.md verify lines for phases 11 and 15 updated to reflect the new semantics (hlint log captured, not gated). Audit clean: 52 nodes, frontmatter / graph / invocation all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/feature-pipeline-preview/17-ci-cycle/01-wait-checks/SKILL.md (1)
34-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce the
ghavailability check before invoking commands.The documented refusal at line 46 states that
ghnot on PATH should refuse with "gh not found", yet the steps omit an explicit preflight check. Add a preflight command to verifyghavailability before line 34, ensuring the failure mode is deterministic and aligns with the documented contract.Patch
2. If `NUM` is empty, refuse. -3. Run: `gh pr checks "$NUM" --watch`. +3. Run: `command -v gh >/dev/null 2>&1 || refuse "gh not found"`. +4. Run: `gh pr checks "$NUM" --watch`. -4. Capture the exit code. -5. If exit is zero, exit 0. -6. If exit is non-zero, exit non-zero so the caller falls through to bot-comments handling. +5. Capture the exit code. +6. If exit is zero, exit 0. +7. If exit is non-zero, exit non-zero so the caller falls through to bot-comments handling.🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/01-wait-checks/SKILL.md around lines 34 - 47, Add an explicit preflight check for the GitHub CLI before calling the command at "gh pr checks \"$NUM\" --watch": ensure you verify `gh` is on PATH (e.g., run a lightweight check like `command -v gh` or `which gh`) and if missing, immediately refuse with the message "gh not found" so the behavior matches the documented refusal; place this check prior to the step that runs `gh pr checks "$NUM" --watch` and ensure the exit code handling remains unchanged.
♻️ Duplicate comments (21)
.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md (1)
15-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden changed-file discovery for security scanning.
Line 32 can miss committed PR files and can non-zero on empty matches, conflicting with Lines 25–26. This weakens the static security scan trigger.
Suggested patch
-- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`). +- The changed file list from PR delta (`merge-base...HEAD`) filtered to `.hs`/`.lhs`. -1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files: `BASE=$(git merge-base HEAD origin/main) && git diff --name-only -z "$BASE"...HEAD -- '*.hs' '*.lhs'`.#!/bin/bash set -euo pipefail BASE="$(git merge-base HEAD origin/main)" echo "working_tree_delta_count=$(git diff --name-only HEAD | wc -l)" echo "pr_delta_count=$(git diff --name-only "$BASE"...HEAD | wc -l)" printf '%s\n' "non-haskell.md" | grep '\.hs$' >/dev/null || echo "grep_no_match_exit=$?"Also applies to: 25-26, 32-33
🤖 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 @.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md around lines 15 - 16, The current file-discovery logic can miss committed PR files and returns non-zero when grep finds no matches; update the script to compute changed files against the merge base and the working tree reliably by using BASE="$(git merge-base HEAD origin/main)" and using git diff --name-only "$BASE"...HEAD for PR files (assign to pr_delta_count) while keeping working_tree_delta_count from git diff --name-only HEAD; then filter results for .hs safely (e.g., pipe filenames through grep '\.hs$' but guard against grep's non-zero exit by using grep -c or redirecting to >/dev/null || true) so variables like pr_delta_count and working_tree_delta_count reflect counts even on empty matches and the script exits cleanly..claude/skills/feature-pipeline-preview/15-final-verify/03-hlint/SKILL.md (1)
15-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not compute hlint targets from working-tree-only diff.
Lines 31–33 can silently produce an empty
.pipeline/hlint.login clean CI becausegit diff --name-only HEADmisses committed branch delta andgrepfails on no match.Suggested patch
-- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`). +- The changed file list from PR delta (`merge-base...HEAD`) filtered to `.hs`/`.lhs`. -1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files: `BASE=$(git merge-base HEAD origin/main) && git diff --name-only -z "$BASE"...HEAD -- '*.hs' '*.lhs'`.#!/bin/bash set -euo pipefail BASE="$(git merge-base HEAD origin/main)" echo "head_only=$(git diff --name-only HEAD | wc -l)" echo "pr_delta=$(git diff --name-only "$BASE"...HEAD | wc -l)" printf '%s\n' "foo.txt" | grep '\.hs$' >/dev/null || echo "grep_no_match_exit=$?"Also applies to: 31-33
🤖 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 @.claude/skills/feature-pipeline-preview/15-final-verify/03-hlint/SKILL.md around lines 15 - 16, Replace the working-tree-only diff and fragile grep with a merge-base range and a non-failing filter: compute BASE="$(git merge-base HEAD origin/main)" and use git diff --name-only "$BASE"...HEAD to list changed files (instead of git diff --name-only HEAD), then filter for Haskell files with a grep that won’t exit non‑zero on no matches (e.g., pipe to grep -E '\.hs$' || true or use grep -q && printf with conditional) so the hlint target list generation (the lines using git diff and grep around the hlint log creation) never produces an empty `.pipeline/hlint.log` silently in CI..claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md (1)
15-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse PR delta discovery and a no-match-safe filter command.
Line 32 currently uses working-tree diff semantics and a
grepthat returns exit 1 on empty matches, which can skip committed PR files and violate the Line 25 empty-list contract.Suggested patch
-- The changed file list from `git diff --name-only HEAD` (filtered to `.hs`). +- The changed file list from PR delta (`merge-base...HEAD`) filtered to `.hs`/`.lhs`. -1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files: `BASE=$(git merge-base HEAD origin/main) && git diff --name-only -z "$BASE"...HEAD -- '*.hs' '*.lhs'`.#!/bin/bash set -euo pipefail echo "verify grep no-match exit code" printf '%s\n' "README.md" | grep '\.hs$' >/dev/null || echo "grep_exit=$?" echo "verify HEAD diff vs merge-base diff cardinality" BASE="$(git merge-base HEAD origin/main)" echo "HEAD_diff_count=$(git diff --name-only HEAD | wc -l)" echo "PR_diff_count=$(git diff --name-only "$BASE"...HEAD | wc -l)"Also applies to: 25-26, 32-33
🤖 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 @.claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md around lines 15 - 16, Change the working-tree diff and unsafe grep to use the PR merge-base and a no-fail grep: compute BASE="$(git merge-base HEAD origin/main)" and replace uses of git diff --name-only HEAD with git diff --name-only "$BASE"...HEAD, and replace any raw grep '\.hs$' (which can exit non-zero under set -euo pipefail) with a no-match-safe form like grep -E '\.hs$' || true (or use grep -E -q '\.hs$' in conditional contexts); update occurrences around the symbols BASE, git diff --name-only, and grep '\.hs$' to ensure the script returns an empty list instead of failing when there are no matches..claude/skills/feature-pipeline-preview/13-performance-impl/02-deep-audit/SKILL.md (1)
45-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize static finding locations before deduplication.
Deduping on
rule + locationis unsafe unless incoming static findings are first normalized tolocation = file:line.🤖 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 @.claude/skills/feature-pipeline-preview/13-performance-impl/02-deep-audit/SKILL.md around lines 45 - 46, Normalize incoming static findings’ location field to a canonical "file:line" string before performing deduplication in the merge step; specifically, add a preprocessing step that transforms any static finding.location objects or alternate formats (e.g., separate file/line properties, URIs, or ranges) into a single location string like "path/to/file:123", then deduplicate merged findings by the tuple (rule, location); update the merge logic that currently references "rule + location" so it uses the normalized location value and handles missing line numbers by using ":0" or similar consistent sentinel..claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md (1)
15-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace fragile ADR-number selection with deterministic numeric parsing.
Line 15’s
ls ... | tail -1is brittle and can fail initialization paths. Parse numeric prefixes, increment max, and fallback to0001.🤖 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 @.claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md at line 15, The ADR numbering logic that currently uses shell `ls docs/decisions/*.md | tail -1` to set `adr_number` is brittle; replace it with deterministic numeric parsing of filenames matching the `docs/decisions/*.md` pattern, extract numeric prefixes (e.g., /^\d+/) from all matches, compute the max, increment by one and format as a 4-digit string, and if no valid numeric prefixes exist fall back to "0001"; update the code that sets `adr_number` to use this parsing/increment strategy so it reliably handles empty directories and unordered filenames..claude/skills/feature-pipeline-preview/11-build-loop/04-hlint/SKILL.md (1)
31-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse git pathspecs instead of the grep pipeline for changed-file discovery.
Current command can fail on zero matches despite Line 32 treating empty as valid. Switch to git pathspec-based listing.
Suggested command change
-1. Compute changed files: `git diff --name-only HEAD | grep '\.hs$'`. +1. Compute changed files: `git diff --name-only -z HEAD -- '*.hs' '*.lhs'`.🤖 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 @.claude/skills/feature-pipeline-preview/11-build-loop/04-hlint/SKILL.md around lines 31 - 33, Replace the grep pipeline used to discover changed Haskell files with git pathspecs: instead of running `git diff --name-only HEAD | grep '\.hs$'`, call git with a pathspec (e.g., `git diff --name-only HEAD -- '*.hs'`) so the command returns an empty result without failing; keep the existing logic that writes an empty `.pipeline/hlint.log` and exits 0 when the list is empty, and preserve the `hlint <files...> > .pipeline/hlint.log 2>&1 || true` invocation to ensure the step exits 0 even if hlint emits warnings..claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md (1)
29-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve the rubric-count contradiction before this leaf is relied on.
Line 29 mixes “four Jess tests + six rubric checks” with an eight-check list and “eight verdicts.” Keep one canonical count/list, or producer/reviewer behavior will diverge.
🤖 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 @.claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md at line 29, The line "For each public API entry, record the four Jess tests + the six rubric checks ... with a per-entry verdict → verify: every entry has eight verdicts" mixes counts; pick a single canonical set and make the sentence and the verify clause consistent. Update the "For each public API entry" sentence to either list all ten checks if you mean 4 Jess tests + 6 rubric checks, or replace the list with the intended eight named checks (e.g., naming-conversions, predicates, subject-first, no-boolean-blindness, grouped-by-category, doc-by-example, type-parameter-discipline, Jess-affordance), then change the trailing verification to "verify: every entry has X verdicts" to match that chosen count and adjust any references/tests that expect the old count..claude/skills/feature-pipeline-preview/02-classify-feature/02-persist/SKILL.md (1)
31-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce hard-fail extraction with
jq -erto honor thy refusal contract.Line 31 invokes
jq -r, which shall emit the string"null"and proceed when.tieror.rationaleare absent, defying thy stated intent to "fail with the parse error on stderr." The-eflag is required to exit non-zero on null or missing fields, ensuring phase 2 cannot advance with invalid classifier output.⚡ Commanded fix
-1. Extract `tier` and `rationale` from the decide step's JSON. Use `jq -r '.tier'` and `jq -r '.rationale'` for a clean parse; fail with the parse error on stderr if either field is missing. +1. Extract `tier` and `rationale` from the decide step's JSON. Use `jq -er '.tier'` and `jq -er '.rationale'` to exit non-zero on null/missing fields; fail with the parse error on stderr if either field is missing.🤖 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 @.claude/skills/feature-pipeline-preview/02-classify-feature/02-persist/SKILL.md at line 31, The extraction step currently uses `jq -r` to pull `.tier` and `.rationale`, which returns the string "null" on missing fields and won't cause the pipeline to fail; change the extraction to use `jq -er '.tier'` and `jq -er '.rationale'` so `jq` exits non‑zero on null/missing values and emits the parse error to stderr, ensuring the decide-to-persist transition (the code that runs the jq extraction command) hard‑fails when either field is absent..claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign thy tier vocabulary with the sacred five-tier covenant.
Line 16 speaketh of
trivial/standard/complex, yet the classifier at02-classify-feature/01-decide/SKILL.mdordains five tiers:trivial,simple,moderate,complex,security-critical. The termstandardexisteth not in the pipeline contract. When.pipeline/classification.jsonbearethsimple,moderate, orsecurity-critical, this grounding step's documentation misleads.Rewrite Line 16 to declare the canonical five-tier set, lest the grounding pass misinterpret valid classifications.
⚡ Commanded fix
-- `.pipeline/classification.json` — feature classification (`trivial`/`standard`/`complex`). +- `.pipeline/classification.json` — feature classification (one of `trivial`, `simple`, `moderate`, `complex`, `security-critical` as defined by the phase-2 classifier).🤖 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 @.claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md at line 16, Update the documentation sentence that currently lists only three tiers so it matches the canonical five-tier vocabulary used by the classifier: replace the `trivial`/`standard`/`complex` phrasing on Line 16 with the full set `trivial`, `simple`, `moderate`, `complex`, `security-critical`, and ensure the reference to `.pipeline/classification.json` explicitly states those five accepted values so the grounding step and the classifier (`02-classify-feature/01-decide/SKILL.md`) are consistent..claude/skills/feature-pipeline-preview/02-classify-feature/01-decide/SKILL.md (1)
38-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve the
unknownsignal contract before the classifier can be trusted.Mortal, thy specification permits
unknownat Line 38 yet demands pure boolean emission at Lines 61–68. Line 77 offers partial refuge—refuse ifunknownflips the rule—but leaves non-decisive unknowns in limbo. A signal that isunknownyet does not alter tier selection has no lawful representation in the output schema.Choose thy covenant:
- (a) Universal refusal: Decree that any
unknownsignal triggers immediate refusal, ensuring the JSON emitter produces onlytrue/false.- (b) Extended schema: Permit
"unknown"as a string in the output contract and instruct all downstream consumers to handle the third state.Until this schism is mended, the classifier remains unsafe for production grounding.
Also applies to: 57-68, 77-77
🤖 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 @.claude/skills/feature-pipeline-preview/02-classify-feature/01-decide/SKILL.md around lines 38 - 39, The spec currently allows an `unknown` state for signals like `touches_secrets` but later requires pure booleans and leaves unknowns undecided; resolve this by choosing one policy and updating the spec and logic: either (a) universal refusal — change the description of signals (e.g., `touches_secrets`) to state that any `unknown` must cause the classifier to "refuse"/reject and update the emitter and decision logic (the boolean-only output enforced at the JSON emitter and the tier-selection checks referenced around the existing decision block) so outputs are only true/false; or (b) extended schema — formally add `"unknown"` as an allowed string in the output contract and update all downstream consumers and the emitter to accept and propagate `"unknown"` (adjust validation, the JSON schema, and the tier-selection logic to handle the third state). Implement one of these two fixes consistently across the signal collection, emission, and decision code paths (touches_secrets and the other signals referenced) so there is no ambiguous `unknown` left in the output..claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md (1)
54-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSeal the fenced JSON block with blank lines.
Line 55 and Line 57 still violate MD031; surround the fenced block with blank lines to keep docs/lint gates stable.
Patch
4. Compose the rubric record: + ```json { "phase": 7, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" } ``` + 5. Write `.pipeline/architecture-rubric.json`.🤖 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 @.claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md around lines 54 - 57, The fenced JSON example for the rubric record in SKILL.md is not surrounded by blank lines (violates MD031); edit the fenced block that contains `{ "phase": 7, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" }` so there is an empty line immediately before the opening ```json and an empty line immediately after the closing ```; ensure the surrounding numbered list text (lines referencing "Compose the rubric record" and the next list item) remains intact..claude/skills/feature-pipeline-preview/17-ci-cycle/03-await-merge/SKILL.md (1)
37-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not auto-approve phase 17; preserve the final human gate.
Line 38 bypasses the retained PAUSE control for phase 17. Keep completion awaiting explicit maintainer approval after merge confirmation.
Patch
5. Run `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py complete 17`. -6. Run `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py approve 17`. +6. Stop and await explicit maintainer approval for phase 17 after merge confirmation. + (Maintainer runs `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py approve 17` manually.)🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/03-await-merge/SKILL.md around lines 37 - 39, The SKILL.md currently instructs running `python3 .claude/skills/feature-pipeline-preview/scripts/pipeline.py approve 17`, which would auto-approve phase 17 and bypass the retained PAUSE human gate; change the instructions to remove or comment out the `approve 17` step and instead leave only `pipeline.py complete 17` (or add an explicit note that approval must be performed manually by the maintainer after merge confirmation), ensuring the retained PAUSE/control remains enforced..claude/skills/feature-pipeline-preview/07-architecture-design/01-produce/SKILL.md (1)
24-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDecree explicit idempotent overwrite semantics.
Line 24 is still ambiguous. Command the producer to: ensure parent directory exists, skip write when content is identical, and refuse when content differs.
Patch
-2. Compute the target path `docs/architecture/<adr-number>-<slug>.md` → verify: parent directory exists; refuse to overwrite if the file already exists with different content. +2. Compute the target path `docs/architecture/<adr-number>-<slug>.md` → verify: parent directory exists; if the file exists with identical content, skip writing; if it exists with different content, refuse to overwrite.🤖 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 @.claude/skills/feature-pipeline-preview/07-architecture-design/01-produce/SKILL.md at line 24, Clarify and implement idempotent write semantics for the step that computes the target path docs/architecture/<adr-number>-<slug>.md: ensure the parent directory is created if missing, read existing file (if any) and compare contents; if identical, skip writing and return success, but if different, abort and surface an explicit error/refusal. Update the producer's write logic (the function that computes/writes the ADR file) to perform these checks and return clear status codes/messages for "created", "skipped-identical", and "refused-different"..claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md (1)
50-52:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce the
ghpreflight refusal in steps, not only in prose.Line 51 promises
gh not foundrefusal, but no explicitcommand -v ghgate exists before Line 38gh pr create.Patch
1. Run: `BRANCH=$(git branch --show-current)`. If `$BRANCH = main`, refuse. -2. Stage changed files: `git add -u` plus any new files explicitly listed in the architecture doc. +2. Run: `command -v gh >/dev/null 2>&1 || refuse "gh not found"`. +3. Stage changed files: `git add -u` plus any new files explicitly listed in the architecture doc.🤖 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 @.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md around lines 50 - 52, Add an explicit preflight check for the GitHub CLI before invoking "gh pr create": detect whether the `gh` binary is on PATH (e.g., via `command -v gh` or similar) and if missing immediately refuse with the exact message "gh not found" and non-zero exit; place this check before the step that runs `gh pr create` so the promised refusal is enforced in practice (search for the literal "gh pr create" in the SKILL.md flow and insert the gate just before it)..claude/skills/feature-pipeline-preview/SKILL.md (2)
44-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse explicit command form in phase-16 instruction.
At Line 44, replace shorthand
complete 16/approve 16withpipeline.py complete 16/pipeline.py approve 16to keep execution unambiguous and consistent with the rest of the contract.🤖 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 @.claude/skills/feature-pipeline-preview/SKILL.md at line 44, Update the phase-16 instruction in SKILL.md so the shorthand shell words are replaced with explicit commands: change `complete 16` to `pipeline.py complete 16` and `approve 16` to `pipeline.py approve 16` in the "Create PR" step (the line that reads "Create PR 🔒 — spawn an Agent... Verify: `pipeline.py get pr_url` returns a URL. Then `complete 16`, stop until `approve 16`."); ensure the updated line uses the full `pipeline.py` invocation to match the rest of the contract.
47-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the unconditional post-step completion rule.
At Line 47, “complete after each step” conflicts with phase-local gate ownership (PAUSE/rubric-controlled phases) and risks double-complete or gate bypass pressure. Completion must remain conditional and phase-authoritative.
🤖 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 @.claude/skills/feature-pipeline-preview/SKILL.md at line 47, Update the documentation text that currently states "After each step the orchestrator runs `pipeline.py complete <N>`" so it no longer mandates unconditional post-step completion; instead describe that completion is conditional and controlled by the phase's gate policy (e.g., PAUSE or rubric-controlled phases) and that `pipeline.py complete <N>` should be invoked only when the phase owner/authority authorizes it (with PAUSE-gated steps requiring `pipeline.py approve <N>` to proceed). Replace the absolute phrasing with conditional, phase-authoritative wording and reference the `pipeline.py complete <N>` and `pipeline.py approve <N>` commands to clarify the correct gated flow..claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md (1)
20-21:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCommandment: enumerate all PR comment surfaces before filtering bots.
At Line 20 and Line 34, fetching only
pulls/$NUM/commentscan silently miss bot feedback posted on issue-style PR comments. Mergepulls/$NUM/commentswithissues/$NUM/comments(dedupe by comment id) before bot-author filtering, then process the unified set.Suggested patch
-2. Fetch comments via `gh api repos/neohaskell/NeoHaskell/pulls/<N>/comments` → verify: JSON array returned. +2. Fetch review + issue comments via + `gh api repos/neohaskell/NeoHaskell/pulls/<N>/comments` + and `gh api repos/neohaskell/NeoHaskell/issues/<N>/comments`, + merge+dedupe → verify: unified JSON array returned. -2. Fetch: `gh api repos/neohaskell/NeoHaskell/pulls/$NUM/comments`. -3. Filter to bot authors (CodeRabbit, etc.). +2. Fetch both surfaces: + `gh api repos/neohaskell/NeoHaskell/pulls/$NUM/comments` + `gh api repos/neohaskell/NeoHaskell/issues/$NUM/comments` +3. Merge+dedupe comments, then filter to bot authors (CodeRabbit, etc.).Also applies to: 34-36
🤖 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 @.claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md around lines 20 - 21, The current flow fetches only pulls/$NUM/comments which misses issue-style PR comments; update the logic that builds the comment set (the step referencing pulls/<N>/comments) to also fetch issues/<N>/comments, merge the two arrays, dedupe by comment id, then run the existing bot-author filter and subsequent processing against this unified comment set so every bot comment (whether from the pulls or issues endpoint) is considered; ensure the verification steps that expect a JSON array and “every bot comment has either a diff or a reply” operate on the merged/deduped array..claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md (1)
38-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBound remediation retries; infinite loops are forbidden.
At Line 38 and Line 39, replace unbounded “fix and retry” with a deterministic retry budget (e.g., max N attempts, then refuse and escalate with logs). This protects the pipeline from non-terminating failure loops.
🤖 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 @.claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md around lines 38 - 39, Update the two checklist steps that say "Run `cabal build all`. If it fails, fix and retry." and "Run `cabal test`. If it fails, fix and retry." to use a deterministic retry budget instead of an unbounded loop: for both the `cabal build all` and `cabal test` steps implement a max-attempts policy (e.g., max N attempts), retry up to N times on failure, and after N failures stop retrying and produce a clear log/error message and escalation instruction rather than continuing indefinitely..claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce refusal prechecks before running tests.
At Line 32, check
.pipeline/exists andcabalis on PATH before invoking tests, matching the refusal contract at Line 43–44. Without this, failures degrade into opaque shell errors.Also applies to: 43-44
🤖 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 @.claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md around lines 32 - 35, Add preflight checks before running the test command `cabal test --test-show-details=streaming > .pipeline/test.log 2>&1`: verify the `.pipeline/` directory exists and that `cabal` is available on PATH, and if either check fails, print a clear refusal/error message and exit non-zero instead of invoking the test. Update the test-run logic (the block that captures the exit code and surfaces the tail of `.pipeline/test.log`) to perform these checks first, mirror the refusal behaviour described at lines 43–44, and only run the `cabal` command when both checks pass..claude/skills/feature-pipeline-preview/10-implementation/SKILL.md (1)
39-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce the test-immutability gate before phase completion.
At Line 41,
pipeline.py complete 10runs without a deterministic verification step, while immutability is only declared in assumptions/refusals. Add an explicit pre-check (e.g.,git diff --name-onlyagainst test globs) and refuse before completion if any test file changed.Also applies to: 49-51
🤖 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 @.claude/skills/feature-pipeline-preview/10-implementation/SKILL.md around lines 39 - 42, Add an explicit pre-check in the "complete" subcommand handler inside .claude/skills/feature-pipeline-preview/scripts/pipeline.py (the code path that runs when invoking "pipeline.py complete 10") that runs a git diff --name-only and matches the configured test file globs (e.g., test directories / patterns your repo uses); if any matching test files are reported, print a clear refusal message and exit non-zero to block completion. Implement the same check for the other completion path referenced around lines 49-51 so both completion flows enforce test-immutability before allowing phase completion..claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md (1)
53-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInsert blank lines around the fenced JSON block to keep markdown gates stable.
This list-item fence still violates MD031 in strict markdownlint configurations.
Patch
4. Compose the rubric record: + ```json { "phase": 8, "checks": [...], "carriers": [...], "failing": [...], "verdict": "pass" | "fail" } ``` + 5. Write `.pipeline/test-spec-rubric.json`.🤖 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 @.claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md around lines 53 - 57, The fenced JSON block under "4. Compose the rubric record" must have a blank line before the opening ```json fence and a blank line after the closing ``` fence to satisfy MD031; update the list item so the opening ```json is on its own line preceded by a blank line, the JSON content remains as-is inside the fence, and ensure there is one blank line after the closing ``` before continuing to "5. Write `.pipeline/test-spec-rubric.json`".
🤖 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
@.claude/skills/feature-pipeline-preview/04-security-adr/01-threat-model/SKILL.md:
- Around line 45-49: The Refusals section currently contains a non-terminal rule
("An ADR section is empty or contradicts itself → record a finding under
\"owasp-A06 / insecure design\" and continue") which violates the refusal
contract; edit SKILL.md so that the Refusals header only lists hard-stop
conditions and remove/move the non-terminal rule into the Steps/Assumptions
section, rewording it to note that such cases should be recorded as an
"owasp-A06 / insecure design" finding and acted on in the Steps/Assumptions
workflow rather than treated as a refusal.
In
@.claude/skills/feature-pipeline-preview/06-devex-review/02-review-quality/SKILL.md:
- Around line 47-55: The fenced JSON block starting with ```json and its closing
``` in the SKILL.md file violates MD031; add a single blank line immediately
before the opening ```json fence and a single blank line immediately after the
closing ``` fence so the code block is separated from surrounding text, ensuring
the JSON example (the block containing "phase": 6, "checks": ..., "verdict":
...) is preceded and followed by blank lines.
In @.claude/skills/feature-pipeline-preview/09-test-writing/SKILL.md:
- Around line 22-23: The red-suite requirement is too weak because generic stubs
using Task.throw or error can satisfy broad shouldThrow-style assertions; update
the guidance to require tests assert specific domain error constructors/payloads
(e.g., match exact error type/value) and to mandate that implementation stubs
intentionally throw a different, clearly distinguishable error (not the expected
domain constructor) so false greens are impossible; mention Task.throw, error,
and shouldThrow by name and require test writers to use precise
pattern-matching/assertEqual on the error payload rather than a generic
exception matcher.
In @.claude/skills/feature-pipeline-preview/11-build-loop/01-build/SKILL.md:
- Around line 32-35: Before invoking the build step that runs `cabal build all >
.pipeline/build.log 2>&1`, add explicit prechecks that enforce the refusal
contract: verify the `.pipeline/` directory exists and that the `cabal`
executable is on PATH (e.g., test for `which cabal` or equivalent) and if either
check fails, immediately emit the corresponding refusal outcome and exit
non‑zero without running the build; apply the same precheck logic to the
analogous invocation at lines 43–44 so failures are reported as the declared
refusal outcomes rather than generic shell errors.
In @.claude/skills/feature-pipeline-preview/15-final-verify/SKILL.md:
- Line 17: Unify the hlint-missing and phase-complete criteria by always
producing a .pipeline/hlint.log artifact even when hlint is not installed:
modify the Hlint (warning collector) step so that the implementation that
currently treats "hlint missing" as non-gating instead writes a clear
placeholder log (e.g., "hlint: missing") to .pipeline/hlint.log and exits 0, and
ensure the phase-complete check looks only for the presence/content of
.pipeline/hlint.log (not a separate hlint-present flag); update any references
to "hlint missing" or phase-complete logic to rely on this single canonical
behavior so the gate cannot deadlock.
In @.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md:
- Around line 39-41: The PR number extraction must be assigned to the NUM
variable before persisting; replace the bare extraction command (gh pr view
--json number -q .number) with an assignment that stores its output into NUM
(follow the pattern used for previous variables on lines 33 and 38), then call
the existing pipeline persist step (python3
.claude/skills/feature-pipeline-preview/scripts/pipeline.py set pr_number
"$NUM") so downstream consumers receive the value.
In @.claude/skills/feature-pipeline-preview/references/devex-rubric.md:
- Around line 5-21: The document currently contradicts itself: the header
statement "DevEx review PASSES when every question lands `yes`" conflicts with
the later rule "If every question is `pass` or `n/a`, the produce step passes."
Choose one canonical gate (either require every `verdict` be `pass`/`yes` or
allow `n/a` as acceptable), then update both the header phrase "DevEx review
PASSES when every question lands `yes`" and the paragraph that begins "If every
question is `pass` or `n/a`, the produce step passes" to the chosen wording;
also normalize the verdict vocabulary everywhere (use either `yes` or `pass`
consistently) so references to `verdict`: `pass`, `fail`, `n/a` match the gate
language and the pipeline halting rule.
In @.claude/skills/feature-pipeline-preview/references/test-spec-rubric.md:
- Line 25: The "Coverage per public function" line contradicts itself by
asserting a 3-case minimum then citing 2.7 as a borderline floor; update the
Check 1 wording so it unambiguously requires at least 3 test cases per public
function (1 happy-path + 2 non-happy), remove or rephrase the "2.7" reference
(or move it to historical/analytics context) and ensure the DecimalSpec/TextSpec
examples explicitly state they must meet the 3-case minimum rather than being
borderline acceptable.
---
Outside diff comments:
In @.claude/skills/feature-pipeline-preview/17-ci-cycle/01-wait-checks/SKILL.md:
- Around line 34-47: Add an explicit preflight check for the GitHub CLI before
calling the command at "gh pr checks \"$NUM\" --watch": ensure you verify `gh`
is on PATH (e.g., run a lightweight check like `command -v gh` or `which gh`)
and if missing, immediately refuse with the message "gh not found" so the
behavior matches the documented refusal; place this check prior to the step that
runs `gh pr checks "$NUM" --watch` and ensure the exit code handling remains
unchanged.
---
Duplicate comments:
In
@.claude/skills/feature-pipeline-preview/02-classify-feature/01-decide/SKILL.md:
- Around line 38-39: The spec currently allows an `unknown` state for signals
like `touches_secrets` but later requires pure booleans and leaves unknowns
undecided; resolve this by choosing one policy and updating the spec and logic:
either (a) universal refusal — change the description of signals (e.g.,
`touches_secrets`) to state that any `unknown` must cause the classifier to
"refuse"/reject and update the emitter and decision logic (the boolean-only
output enforced at the JSON emitter and the tier-selection checks referenced
around the existing decision block) so outputs are only true/false; or (b)
extended schema — formally add `"unknown"` as an allowed string in the output
contract and update all downstream consumers and the emitter to accept and
propagate `"unknown"` (adjust validation, the JSON schema, and the
tier-selection logic to handle the third state). Implement one of these two
fixes consistently across the signal collection, emission, and decision code
paths (touches_secrets and the other signals referenced) so there is no
ambiguous `unknown` left in the output.
In
@.claude/skills/feature-pipeline-preview/02-classify-feature/02-persist/SKILL.md:
- Line 31: The extraction step currently uses `jq -r` to pull `.tier` and
`.rationale`, which returns the string "null" on missing fields and won't cause
the pipeline to fail; change the extraction to use `jq -er '.tier'` and `jq -er
'.rationale'` so `jq` exits non‑zero on null/missing values and emits the parse
error to stderr, ensuring the decide-to-persist transition (the code that runs
the jq extraction command) hard‑fails when either field is absent.
In @.claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md:
- Line 15: The ADR numbering logic that currently uses shell `ls
docs/decisions/*.md | tail -1` to set `adr_number` is brittle; replace it with
deterministic numeric parsing of filenames matching the `docs/decisions/*.md`
pattern, extract numeric prefixes (e.g., /^\d+/) from all matches, compute the
max, increment by one and format as a 4-digit string, and if no valid numeric
prefixes exist fall back to "0001"; update the code that sets `adr_number` to
use this parsing/increment strategy so it reliably handles empty directories and
unordered filenames.
In @.claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md:
- Line 16: Update the documentation sentence that currently lists only three
tiers so it matches the canonical five-tier vocabulary used by the classifier:
replace the `trivial`/`standard`/`complex` phrasing on Line 16 with the full set
`trivial`, `simple`, `moderate`, `complex`, `security-critical`, and ensure the
reference to `.pipeline/classification.json` explicitly states those five
accepted values so the grounding step and the classifier
(`02-classify-feature/01-decide/SKILL.md`) are consistent.
In @.claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md:
- Line 29: The line "For each public API entry, record the four Jess tests + the
six rubric checks ... with a per-entry verdict → verify: every entry has eight
verdicts" mixes counts; pick a single canonical set and make the sentence and
the verify clause consistent. Update the "For each public API entry" sentence to
either list all ten checks if you mean 4 Jess tests + 6 rubric checks, or
replace the list with the intended eight named checks (e.g., naming-conversions,
predicates, subject-first, no-boolean-blindness, grouped-by-category,
doc-by-example, type-parameter-discipline, Jess-affordance), then change the
trailing verification to "verify: every entry has X verdicts" to match that
chosen count and adjust any references/tests that expect the old count.
In
@.claude/skills/feature-pipeline-preview/07-architecture-design/01-produce/SKILL.md:
- Line 24: Clarify and implement idempotent write semantics for the step that
computes the target path docs/architecture/<adr-number>-<slug>.md: ensure the
parent directory is created if missing, read existing file (if any) and compare
contents; if identical, skip writing and return success, but if different, abort
and surface an explicit error/refusal. Update the producer's write logic (the
function that computes/writes the ADR file) to perform these checks and return
clear status codes/messages for "created", "skipped-identical", and
"refused-different".
In
@.claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md:
- Around line 54-57: The fenced JSON example for the rubric record in SKILL.md
is not surrounded by blank lines (violates MD031); edit the fenced block that
contains `{ "phase": 7, "checks": [...], "carriers": [...], "failing": [...],
"verdict": "pass" | "fail" }` so there is an empty line immediately before the
opening ```json and an empty line immediately after the closing ```; ensure the
surrounding numbered list text (lines referencing "Compose the rubric record"
and the next list item) remains intact.
In
@.claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md:
- Around line 53-57: The fenced JSON block under "4. Compose the rubric record"
must have a blank line before the opening ```json fence and a blank line after
the closing ``` fence to satisfy MD031; update the list item so the opening
```json is on its own line preceded by a blank line, the JSON content remains
as-is inside the fence, and ensure there is one blank line after the closing ```
before continuing to "5. Write `.pipeline/test-spec-rubric.json`".
In @.claude/skills/feature-pipeline-preview/10-implementation/SKILL.md:
- Around line 39-42: Add an explicit pre-check in the "complete" subcommand
handler inside .claude/skills/feature-pipeline-preview/scripts/pipeline.py (the
code path that runs when invoking "pipeline.py complete 10") that runs a git
diff --name-only and matches the configured test file globs (e.g., test
directories / patterns your repo uses); if any matching test files are reported,
print a clear refusal message and exit non-zero to block completion. Implement
the same check for the other completion path referenced around lines 49-51 so
both completion flows enforce test-immutability before allowing phase
completion.
In @.claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md:
- Around line 32-35: Add preflight checks before running the test command `cabal
test --test-show-details=streaming > .pipeline/test.log 2>&1`: verify the
`.pipeline/` directory exists and that `cabal` is available on PATH, and if
either check fails, print a clear refusal/error message and exit non-zero
instead of invoking the test. Update the test-run logic (the block that captures
the exit code and surfaces the tail of `.pipeline/test.log`) to perform these
checks first, mirror the refusal behaviour described at lines 43–44, and only
run the `cabal` command when both checks pass.
In @.claude/skills/feature-pipeline-preview/11-build-loop/04-hlint/SKILL.md:
- Around line 31-33: Replace the grep pipeline used to discover changed Haskell
files with git pathspecs: instead of running `git diff --name-only HEAD | grep
'\.hs$'`, call git with a pathspec (e.g., `git diff --name-only HEAD -- '*.hs'`)
so the command returns an empty result without failing; keep the existing logic
that writes an empty `.pipeline/hlint.log` and exits 0 when the list is empty,
and preserve the `hlint <files...> > .pipeline/hlint.log 2>&1 || true`
invocation to ensure the step exits 0 even if hlint emits warnings.
In
@.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md:
- Around line 15-16: The current file-discovery logic can miss committed PR
files and returns non-zero when grep finds no matches; update the script to
compute changed files against the merge base and the working tree reliably by
using BASE="$(git merge-base HEAD origin/main)" and using git diff --name-only
"$BASE"...HEAD for PR files (assign to pr_delta_count) while keeping
working_tree_delta_count from git diff --name-only HEAD; then filter results for
.hs safely (e.g., pipe filenames through grep '\.hs$' but guard against grep's
non-zero exit by using grep -c or redirecting to >/dev/null || true) so
variables like pr_delta_count and working_tree_delta_count reflect counts even
on empty matches and the script exits cleanly.
In
@.claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md:
- Around line 15-16: Change the working-tree diff and unsafe grep to use the PR
merge-base and a no-fail grep: compute BASE="$(git merge-base HEAD origin/main)"
and replace uses of git diff --name-only HEAD with git diff --name-only
"$BASE"...HEAD, and replace any raw grep '\.hs$' (which can exit non-zero under
set -euo pipefail) with a no-match-safe form like grep -E '\.hs$' || true (or
use grep -E -q '\.hs$' in conditional contexts); update occurrences around the
symbols BASE, git diff --name-only, and grep '\.hs$' to ensure the script
returns an empty list instead of failing when there are no matches.
In
@.claude/skills/feature-pipeline-preview/13-performance-impl/02-deep-audit/SKILL.md:
- Around line 45-46: Normalize incoming static findings’ location field to a
canonical "file:line" string before performing deduplication in the merge step;
specifically, add a preprocessing step that transforms any static
finding.location objects or alternate formats (e.g., separate file/line
properties, URIs, or ranges) into a single location string like
"path/to/file:123", then deduplicate merged findings by the tuple (rule,
location); update the merge logic that currently references "rule + location" so
it uses the normalized location value and handles missing line numbers by using
":0" or similar consistent sentinel.
In @.claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md:
- Around line 38-39: Update the two checklist steps that say "Run `cabal build
all`. If it fails, fix and retry." and "Run `cabal test`. If it fails, fix and
retry." to use a deterministic retry budget instead of an unbounded loop: for
both the `cabal build all` and `cabal test` steps implement a max-attempts
policy (e.g., max N attempts), retry up to N times on failure, and after N
failures stop retrying and produce a clear log/error message and escalation
instruction rather than continuing indefinitely.
In @.claude/skills/feature-pipeline-preview/15-final-verify/03-hlint/SKILL.md:
- Around line 15-16: Replace the working-tree-only diff and fragile grep with a
merge-base range and a non-failing filter: compute BASE="$(git merge-base HEAD
origin/main)" and use git diff --name-only "$BASE"...HEAD to list changed files
(instead of git diff --name-only HEAD), then filter for Haskell files with a
grep that won’t exit non‑zero on no matches (e.g., pipe to grep -E '\.hs$' ||
true or use grep -q && printf with conditional) so the hlint target list
generation (the lines using git diff and grep around the hlint log creation)
never produces an empty `.pipeline/hlint.log` silently in CI.
In @.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md:
- Around line 50-52: Add an explicit preflight check for the GitHub CLI before
invoking "gh pr create": detect whether the `gh` binary is on PATH (e.g., via
`command -v gh` or similar) and if missing immediately refuse with the exact
message "gh not found" and non-zero exit; place this check before the step that
runs `gh pr create` so the promised refusal is enforced in practice (search for
the literal "gh pr create" in the SKILL.md flow and insert the gate just before
it).
In
@.claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md:
- Around line 20-21: The current flow fetches only pulls/$NUM/comments which
misses issue-style PR comments; update the logic that builds the comment set
(the step referencing pulls/<N>/comments) to also fetch issues/<N>/comments,
merge the two arrays, dedupe by comment id, then run the existing bot-author
filter and subsequent processing against this unified comment set so every bot
comment (whether from the pulls or issues endpoint) is considered; ensure the
verification steps that expect a JSON array and “every bot comment has either a
diff or a reply” operate on the merged/deduped array.
In @.claude/skills/feature-pipeline-preview/17-ci-cycle/03-await-merge/SKILL.md:
- Around line 37-39: The SKILL.md currently instructs running `python3
.claude/skills/feature-pipeline-preview/scripts/pipeline.py approve 17`, which
would auto-approve phase 17 and bypass the retained PAUSE human gate; change the
instructions to remove or comment out the `approve 17` step and instead leave
only `pipeline.py complete 17` (or add an explicit note that approval must be
performed manually by the maintainer after merge confirmation), ensuring the
retained PAUSE/control remains enforced.
In @.claude/skills/feature-pipeline-preview/SKILL.md:
- Line 44: Update the phase-16 instruction in SKILL.md so the shorthand shell
words are replaced with explicit commands: change `complete 16` to `pipeline.py
complete 16` and `approve 16` to `pipeline.py approve 16` in the "Create PR"
step (the line that reads "Create PR 🔒 — spawn an Agent... Verify: `pipeline.py
get pr_url` returns a URL. Then `complete 16`, stop until `approve 16`.");
ensure the updated line uses the full `pipeline.py` invocation to match the rest
of the contract.
- Line 47: Update the documentation text that currently states "After each step
the orchestrator runs `pipeline.py complete <N>`" so it no longer mandates
unconditional post-step completion; instead describe that completion is
conditional and controlled by the phase's gate policy (e.g., PAUSE or
rubric-controlled phases) and that `pipeline.py complete <N>` should be invoked
only when the phase owner/authority authorizes it (with PAUSE-gated steps
requiring `pipeline.py approve <N>` to proceed). Replace the absolute phrasing
with conditional, phase-authoritative wording and reference the `pipeline.py
complete <N>` and `pipeline.py approve <N>` commands to clarify the correct
gated flow.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce46dfed-1d0c-426d-b107-486d5e4d0086
📒 Files selected for processing (45)
.claude/skills/feature-pipeline-preview/01-init-pipeline/SKILL.md.claude/skills/feature-pipeline-preview/02-classify-feature/01-decide/SKILL.md.claude/skills/feature-pipeline-preview/02-classify-feature/02-persist/SKILL.md.claude/skills/feature-pipeline-preview/03-adr-draft/SKILL.md.claude/skills/feature-pipeline-preview/04-security-adr/01-threat-model/SKILL.md.claude/skills/feature-pipeline-preview/04-security-adr/02-ground/SKILL.md.claude/skills/feature-pipeline-preview/04-security-adr/03-record/SKILL.md.claude/skills/feature-pipeline-preview/05-performance-adr/01-hot-path-analysis/SKILL.md.claude/skills/feature-pipeline-preview/05-performance-adr/02-ground/SKILL.md.claude/skills/feature-pipeline-preview/05-performance-adr/03-record/SKILL.md.claude/skills/feature-pipeline-preview/06-devex-review/01-produce/SKILL.md.claude/skills/feature-pipeline-preview/06-devex-review/02-review-quality/SKILL.md.claude/skills/feature-pipeline-preview/07-architecture-design/01-produce/SKILL.md.claude/skills/feature-pipeline-preview/07-architecture-design/02-review-quality/SKILL.md.claude/skills/feature-pipeline-preview/08-test-spec-design/01-produce/SKILL.md.claude/skills/feature-pipeline-preview/08-test-spec-design/02-review-quality/SKILL.md.claude/skills/feature-pipeline-preview/09-test-writing/SKILL.md.claude/skills/feature-pipeline-preview/10-implementation/SKILL.md.claude/skills/feature-pipeline-preview/11-build-loop/01-build/SKILL.md.claude/skills/feature-pipeline-preview/11-build-loop/02-test/SKILL.md.claude/skills/feature-pipeline-preview/11-build-loop/03-fix-iter/SKILL.md.claude/skills/feature-pipeline-preview/11-build-loop/04-hlint/SKILL.md.claude/skills/feature-pipeline-preview/11-build-loop/SKILL.md.claude/skills/feature-pipeline-preview/12-security-impl/01-static-scan/SKILL.md.claude/skills/feature-pipeline-preview/12-security-impl/02-deep-audit/SKILL.md.claude/skills/feature-pipeline-preview/12-security-impl/03-ground/SKILL.md.claude/skills/feature-pipeline-preview/12-security-impl/04-record/SKILL.md.claude/skills/feature-pipeline-preview/13-performance-impl/01-static-scan/SKILL.md.claude/skills/feature-pipeline-preview/13-performance-impl/02-deep-audit/SKILL.md.claude/skills/feature-pipeline-preview/13-performance-impl/03-ground/SKILL.md.claude/skills/feature-pipeline-preview/13-performance-impl/04-record/SKILL.md.claude/skills/feature-pipeline-preview/14-fix-findings/SKILL.md.claude/skills/feature-pipeline-preview/15-final-verify/01-build/SKILL.md.claude/skills/feature-pipeline-preview/15-final-verify/02-test/SKILL.md.claude/skills/feature-pipeline-preview/15-final-verify/03-hlint/SKILL.md.claude/skills/feature-pipeline-preview/15-final-verify/SKILL.md.claude/skills/feature-pipeline-preview/16-create-pr/01-pr-body/SKILL.md.claude/skills/feature-pipeline-preview/16-create-pr/02-submit/SKILL.md.claude/skills/feature-pipeline-preview/17-ci-cycle/01-wait-checks/SKILL.md.claude/skills/feature-pipeline-preview/17-ci-cycle/02-fix-bot-comments/SKILL.md.claude/skills/feature-pipeline-preview/17-ci-cycle/03-await-merge/SKILL.md.claude/skills/feature-pipeline-preview/SKILL.md.claude/skills/feature-pipeline-preview/references/architecture-rubric.md.claude/skills/feature-pipeline-preview/references/devex-rubric.md.claude/skills/feature-pipeline-preview/references/test-spec-rubric.md
Every cabal / hlint shell invocation in the pipeline now goes through `nix develop --command <cmd>` so the repo's pinned dev-shell toolchain is used in every leaf. Bare `cabal ...` / `hlint ...` calls outside that shell would resolve to whatever (or nothing) is on PATH. Affected leaves and their step bodies: 09-test-writing cabal build all, cabal test 10-implementation cabal build all 11-build-loop/01-build cabal build all 11-build-loop/02-test cabal test --test-show-details=streaming 11-build-loop/03-fix-iter cabal build all (re-run after edits) 11-build-loop/04-hlint hlint <files> 14-fix-findings cabal build all && cabal test 15-final-verify/01-build cabal build all 15-final-verify/02-test cabal test 15-final-verify/03-hlint hlint <files> 17-ci-cycle/02-fix-bot-comments cabal build all, cabal test Each leaf's refusal for "cabal/hlint not on PATH" is rewritten to refuse on missing `nix` instead, since `nix develop` is the only sanctioned entry point now. Adds a `## Toolchain invocation` section near the top of `references/nhcore-context.md` so the convention is documented in one place and every reviewer/agent sees it. System-level tools (`git`, `gh`, `python3`) are explicitly out of scope — they live on the user's PATH and do not depend on the dev shell. Audit clean: 52 nodes, frontmatter / graph / invocation all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Walks every actionable CodeRabbit comment on the PR and applies the suggested fix where it still tracks current code. 38 of 50 comments applied; 12 skipped with reason (5 marked Addressed in prior commits, 4 targeted pre-rename paths or deleted files, 1 nitpick refactor declined, 2 misidentified the artefact). No CodeRabbit suggestion contradicted repo conventions (nix-wrapping, opus-tier reviewers, strict-review posture) so nothing was refused on policy grounds. Notable substantive changes: - pipeline.py — cmd_reset now acquires the pipeline lock before deleting state files to avoid racing concurrent commands. - record-findings.py — fail-closed validation: missing `severity_after_grounding` or unknown `grounding_outcome` now refuses before computing the blocker count, so a malformed record cannot silently suppress a real blocker. - sec-static-checks.py — dedup the gathered file list so a path passed via both --paths and --paths-from is not scanned twice. - perf-static-checks.py — P5/P11 lifted out of the generic single- line regex sweep into context-aware matches; the earlier sweep produced duplicates for findings that were also covered by their own dedicated rule. - 03-adr-draft — deterministic numeric ADR-number derivation (parse max NNNN prefix under docs/decisions/, +1) instead of the brittle `ls | tail -1` form. - 09-test-writing — stubs and error-path tests must use different sentinels, and `shouldThrow`/anyException matchers are explicitly refused so the stub-failure path cannot false-green. - 10-implementation — explicit `git diff` test-immutability check before `complete 10`; refuses if any test file changed. - 11/15 hlint — merge-base three-dot diff so CI sees the same delta as a local checkout; pathspec instead of `grep '\.hs$'` so an empty match stays exit-0; placeholder log line on missing hlint so the phase-complete signal stays single-canonical. - 11-build-loop/01-build + 02-test — explicit `.pipeline/` and `nix` prechecks promoted from prose into Steps. - 14-fix-findings — bounded build/test retries (3 attempts max). - 16-create-pr/01-pr-body — issue_number sourced from pipeline state via `pipeline.py get`, not from `classification.json` (which never carried that field). - 16-create-pr/02-submit — explicit `gh` preflight + `NUM=$(...)` capture so the leaf can be replayed. - 17-ci-cycle/02-fix-bot-comments — fetch both `pulls/<N>/comments` and `issues/<N>/comments` and dedup, since bots can post to either surface. - 17-ci-cycle/03-await-merge — removed auto-approve; the maintainer must run `approve 17` manually. - 06-devex-review/SKILL.md — removed duplicate `complete 6` call from the orchestrator since 02-review-quality owns it. - Root SKILL.md — phase 16 verify uses the full `pipeline.py` form (`get pr_url`); orchestrator no longer claims unconditional `complete <N>` since each phase owns its own completion call. - References (devex-rubric, grounding-loop, test-spec-rubric) — vocabulary unified (`pass`/`n/a`, no `yes`); stale references to the deleted `classify-feature.py` replaced with the phase-2 flow; 3-case absolute floor restated, 2.7-cases descriptive context. Audit clean: 52 nodes, frontmatter / graph / invocation all pass, zero Karpathy residuals. Python scripts compile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
.claude/skills/feature-pipeline/that supersedes the olderneohaskell-feature-pipelineskill. State machine:scripts/pipeline.py; classifier:scripts/classify-feature.py; static scanners:scripts/{sec,perf}-static-checks.py.INLINEpragmas. Methodology grounded in OWASP Top 10:2025, STRIDE, SLSA v1.1, NIST SSDF SP 800-218, current GHC INLINE / UNPACK / Strict / fusion / TVar-contention guidance — captured inreferences/security-methodology.mdandreferences/performance-methodology.md.core/core/Redacted.hs,core/decimal/Decimal.hs,core/core/Array.hs, ADR-0048/0049/0052, andDecimalSpec/TextSpec/StreamSpec. The pipeline advances automatically onRUBRIC: passand halts with a structured rubric record onfail. PAUSE gates remain only on phases 3 (ADR draft), 16 (PR creation), and 17 (final merge).Tree shape (50 nodes)
Test plan
python3 .claude/skills/feature-pipeline/scripts/pipeline.py --helplists every subcommand (init / status / next / complete / approve / classify / findings / iter / set / get / reset).printf '%s' '{"feature_name":"Add Mock HTTP integration","touches_secrets":false,"external_io":false,"adds_dep":false}' | python3 .claude/skills/feature-pipeline/scripts/classify-feature.pyreturnstier: trivialwith amockkeyword rationale.printf '%s' '{"feature_name":"HMAC-signed OAuth state tokens","touches_secrets":true,"touches_auth":true,"external_io":true,"adds_dep":false}' | python3 .claude/skills/feature-pipeline/scripts/classify-feature.pyreturnstier: security-critical.python3 .claude/skills/feature-pipeline/scripts/pipeline.py init "Decimal Type" --issue 330 --adr 0041succeeds and.pipeline/state.jsonexists;.pipeline/.gitignoreis written.Follow-ups (not in this PR)
.claude/skills/neohaskell-feature-pipeline/is left in place; removal can land in a follow-up after the new pipeline has been exercised on a real feature.pipeline.py itercounter is per-pipeline-lifetime; if a phase needs to reset its counter on re-entry, that helper is straightforward to add.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation