Skip to content

sync: merge Sunstone fork updates (TRD dependency graph, PRD BDD generation, PRD/TRD labels) - #80

Open
ldangelo wants to merge 25 commits into
mainfrom
sync/sunstone-fork-updates
Open

sync: merge Sunstone fork updates (TRD dependency graph, PRD BDD generation, PRD/TRD labels)#80
ldangelo wants to merge 25 commits into
mainfrom
sync/sunstone-fork-updates

Conversation

@ldangelo

@ldangelo ldangelo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Syncs 25 commits from the Sunstone-Partners fork (sunstone/main) into Fortium's main. These commits are already merged on the fork side via two feature-branch PRs (#4 and #5) plus their supporting regeneration/fixup commits, and a merge commit that folded the fork's main into this repo's local main.

No divergence: origin/main is the exact merge-base with the tip of this branch, so this is a straight fast-forward — no conflicts to resolve.

1. TRD dependency graph + duplicate-work detection (fork PR #4: brumschlag/feat/trd-dependency-graph)

  • Adds foundational TRDs and capability reuse support to /ensemble:create-trd
  • Introduces a TRD dependency graph with duplicate-work detection to avoid redundant TRD authoring
  • Includes review-feedback fixes and regenerated Pi/Codex artifacts for the feature

2. PRD → Reqnroll (xUnit) BDD test generation (fork PR #5: brumschlag/feat/prd-bdd-test-generation)

  • Adds PRD → Reqnroll(xUnit) BDD bindings with a red-gated test-first loop
  • Generates Gherkin feature tests directly from PRDs, with drift detection
  • Rebinds acceptance criteria to the domain decider rather than infrastructure
  • Bundles ensemble-reqnroll into the ensemble-full meta-package and package-lock.json
  • Includes review-feedback fixes and regenerated Pi/Codex artifacts for the feature

3. Human-readable PRD/TRD labels

  • Adds a human-readable label to PRDs/TRDs alongside the existing micro-uuid id, improving readability of generated documents

Supporting commits

  • CI trigger/re-trigger commits (enabling Actions on the fork)
  • YAML quoting fix for [depends:] constraint so it stays a string
  • The merge commit that brought sunstone/main into local main

Test plan

  • CI passes on this PR (validate, test matrix)
  • npm run validate passes
  • npm test passes across affected packages (development, product, reqnroll, full)
  • Spot-check generated Pi/Codex/opencode artifacts are consistent with npm run generate

Summary by CodeRabbit

  • New Features

    • Added Reqnroll/xUnit support for generating acceptance-test projects, implementing bindings, running tests, and detecting binding drift.
    • Added PRD-to-Gherkin feature generation with traceability manifests and feature-drift checks.
    • Added foundational capability workflows with reuse analysis and dependency-graph visualization.
    • Added commands for automated Reqnroll TDD, feature generation, drift checks, and TRD dependency analysis.
    • Added persistent human-readable labels for PRDs and TRDs.
  • Documentation

    • Added guides, references, and specialized automation support for Reqnroll binding development.

Brian Rumschlag and others added 25 commits July 1, 2026 15:02
Phase 1 of avoiding duplicate TRDs and visualizing inter-TRD dependencies.
Builds a deterministic TRD-level dependency graph from the existing cross-TRD
[depends: <trd-slug>#TRD-NNN] annotations and flags likely-duplicate work.

- trd-parser: additively expose documentId, label, and kind from frontmatter
- trd-graph.js (pure): buildRegistry / buildGraph / detectCycles / findOverlaps
  + emitJson/emitMermaid/emitDot. Nodes key on the unique filename slug (what
  cross-TRD refs and beads already resolve against); document_id is a
  correlation attribute and duplicate document_ids are surfaced as warnings
  rather than silently merged. Overlap report flags TRD pairs touching the same
  target files -> extract into one foundational TRD and reference it.
- trd-graph-cli.js: `graph` (--format json|mermaid|dot, exit 2 on cycle) and
  `overlap` subcommands over a directory or explicit paths
- /ensemble:trd-dependency-graph command wraps the CLI (read-only)

Verified on docs/TRD/: 23 nodes; correctly detected the TRD-2026-023
document_id collision (multi-trd-beads-workstream vs trd-staleness-gate) and
real overlapping target files. All 658 development+product tests pass.
Phase 2 of avoiding duplicate TRDs: extract shared technical work once and
reference it, instead of re-specifying it in every PRD's TRD. Builds on the
Phase 1 registry.

- trd-parser: additively expose `capabilities` (frontmatter YAML list or comma
  string -> string[]); `kind` already exposed in Phase 1
- trd-graph: index capabilities per node; add findCapabilityProviders() sorting
  foundational providers first
- trd-graph-cli: new `capabilities` subcommand (--json) — the deterministic
  registry create-trd consults
- create-trd / create-trd-foreman:
  - new "Capability Reuse Check" step — query the capability registry, and when
    a foundational TRD already provides a needed capability, reference it via
    [depends: <foundational-slug>#TRD-NNN] instead of duplicating; record under a
    "## Reused Capabilities" section. Match explicitly by capability token, else
    implicitly by label/title/target-file overlap. Reference by slug/id, never
    label.
  - new --foundational mode: kind: foundational, PRD reference optional
    (capability brief accepted), emits a capabilities: frontmatter list

Foundational deps ride the existing cross-TRD rails resolved by
implement-trd-beads. 662 development+product tests pass.
… string

The trd-dependency-graph command had an unquoted constraint containing
"[depends: <trd-slug>#TRD-NNN]"; YAML parsed the `[ ... : ... ]` as a flow
mapping, turning the constraint into an object. Schema validation missed it
(constraints is additionalProperties), but the pi generator's constraint.trim()
threw `TypeError: constraint.trim is not a function` in CI. Quoting the string
fixes it; a repo-wide scan confirms it was the only non-string constraint/action.
feat(development): TRD dependency graph + foundational TRDs & capability reuse
…tection

Add a deterministic PRD -> Gherkin pipeline plus continuous drift detection,
filling the gap between requirements and tests in the ensemble pipeline.

- prd-parser.js: parse REQ-NNN / AC-NNN-M, splitting each AC sentence into
  Given/When/Then; supports both AC marker forms, H3/H4 REQ headings, inline
  and Priority-line MoSCoW/complexity, and flags free-form / NEEDS CLARIFICATION
- feature-gen.js: emit one .feature per requirement with traceability tags and a
  per-scenario content hash; atomic, idempotent writes + .prd-trace.json manifest
- drift.js: ADDED / REMOVED / CHANGED / IN_SYNC diff keyed by AC id and hash
- prd-cli.js: parse / generate-features / check-drift (exit 2 on drift for CI)
- commands: /ensemble:generate-feature-tests, /ensemble:check-feature-drift
…test-first loop

Make the generated .feature files executable, test-first. The step-matching
layer is deterministic codegen; only step bodies are agentic.

- reqnroll-gen.js: scaffold a runnable Reqnroll+xUnit project from a PRD —
  [Binding] step classes with literal Given/When/Then attributes + Pending()
  bodies, .csproj/reqnroll.json, .reqnroll-trace.json manifest. Step .cs files
  are write-once (never clobber a filled body); features/project/manifest
  regenerate byte-identically
- reqnroll-run.js: pure TRX/console parser + strict isGreen gate (a zero-test
  run is not green) — basis for the red/green gate
- reqnroll-cli.js: generate-bindings / check-binding-drift / run
- check-binding-drift: UNBOUND (PRD step, no binding) / ORPHANED (binding, no
  PRD step), the .feature<->bindings tier of traceability
- packages/reqnroll: tier-4 plugin with the reqnroll-binding-specialist agent
  (fills bodies test-first, never edits attributes) + Reqnroll skill
- commands: /ensemble:generate-reqnroll-bindings, /ensemble:check-binding-drift,
  /ensemble:reqnroll-tdd (scaffold -> RED gate -> implement -> GREEN gate)
- register ensemble-reqnroll in marketplace.json
Wire the new reqnroll plugin into the full bundle the same way dotnet/xunit are:
- symlink agents/reqnroll and skills/reqnroll into packages/full
- register the reqnroll-binding-specialist agent in full plugin.json

The new product commands (generate-feature-tests, check-feature-drift,
generate-reqnroll-bindings, check-binding-drift, reqnroll-tdd) are already
exposed via the existing commands/product symlink.
…ot infrastructure

Acceptance criteria describe decisions, and Given/When/Then maps 1:1 onto a decider:
Given folds prior events into state, When calls decide(command, state), Then asserts
on the emitted events. Make that the default binding target — pure, fast, in-memory,
no HTTP/DB.

- SKILL.md: lead with the decider mapping + example; demote WebApplicationFactory/HTTP
  to an explicit "infrastructure exception" for criteria about the wired pipeline
- reqnroll-binding-specialist: mission + responsibilities now bind domain-first, with
  infrastructure reserved for pipeline-specific criteria

The deterministic generator is unchanged — .feature files and step attributes are
identical regardless of binding target; only the guidance for filling bodies changed.
…cro-uuid id

Micro-uuid document ids are collision-safe but unreadable. Add a short,
human-readable `label` (`<type>-<stem>`, e.g. prd-login-mfa / trd-login-mfa)
as frontmatter metadata — display-only, never a reference key.

- create-prd: set `label: prd-<stem>` (derived from title, author-overridable)
- create-trd / create-trd-foreman: inherit the PRD's stem, swap prefix -> trd-<stem>,
  mirroring how the TRD already reuses the PRD's 8-hex correlation id
- refine-prd / refine-trd: preserve Document ID and Label unchanged on refine
- prd-parser: surface frontmatter `label`; add deriveLabel(id, title) display
  fallback for unlabeled/legacy docs (strips a leading "PRD-YYYY-NNN:" from the
  title so it doesn't double-prefix)
- feature-gen: show `# Label:` in each .feature and record `label` in the
  manifest's prd block — manifests still key on document_id, never the label

Cross-document references remain on the immutable micro-uuid; the label is a
human aid only.
CI (npm ci --legacy-peer-deps) failed with 'Missing: @fortium/ensemble-reqnroll
from lock file' because the new plugin workspace was never added to the lockfile.
Regenerated with --package-lock-only.
feat(product): PRD → Gherkin/Reqnroll BDD test generation with drift detection
# Conflicts:
#	packages/development/tests/trd-parser.test.js
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This change adds deterministic PRD-to-Gherkin and PRD-to-Reqnroll tooling, Reqnroll TDD workflows, foundational TRD support, dependency graph analysis, and integrations for plugin, Codex, and Pi distributions.

Changes

Workflow tooling

Layer / File(s) Summary
PRD parsing, feature generation, and drift checks
packages/product/lib/*, packages/product/commands/*, packages/pi/prompts/*, packages/codex/.codex/skills/commands/*
Adds PRD parsing, Gherkin generation, trace manifests, feature drift checks, labels, and metadata preservation rules.
Reqnroll generation, execution, and TDD
packages/product/lib/reqnroll-*, packages/product/tests/*, packages/reqnroll/*
Adds Reqnroll/xUnit project generation, binding drift checks, test execution, green-gate evaluation, binding guidance, and TDD orchestration.
Foundational TRDs and dependency graphs
packages/development/commands/*, packages/development/lib/trd-*, packages/development/tests/trd-*
Adds foundational capability workflows, capability reuse checks, normalized TRD metadata, dependency graphs, cycle detection, overlap analysis, and graph emitters.
Distribution integration
marketplace.json, packages/full/*, packages/codex/*, packages/pi/*, scripts/generate-codex/*
Registers the Reqnroll package and exposes its agents, skills, commands, prompts, and generated Codex support.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PRD as PRD
  participant Parser as parsePRD
  participant FeatureGen as feature-gen
  participant ReqnrollGen as reqnroll-gen
  participant TestRunner as reqnroll-run
  participant Drift as drift checks

  PRD->>Parser: Parse requirements and acceptance criteria
  Parser->>FeatureGen: Build Gherkin files and trace manifest
  Parser->>ReqnrollGen: Build Reqnroll/xUnit project
  ReqnrollGen->>TestRunner: Run generated acceptance tests
  TestRunner-->>Drift: Report test and binding status
  Drift-->>PRD: Report feature or binding drift
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the fork merge and its main additions: TRD dependency graphs, PRD BDD generation, and PRD/TRD labels.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/sunstone-fork-updates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review: sync/sunstone-fork-updates (PR #80)

Reviewed the fork-sync diff (marketplace + reqnroll package + trd-graph / PRD-to-Gherkin/Reqnroll codegen libs). Overall this is well-structured, pure-function-first code with solid unit tests on the core logic. A few things worth a look before merge.

Code quality

  • Duplicated deriveLabel: the exact same label-derivation function is copy-pasted in packages/development/lib/trd-graph.js and packages/product/lib/prd-parser.js (strip id prefix from title, kebab-case, then <type>-<stem>). Since both already read document_id/label from frontmatter the same way, this would be a good candidate to hoist into a tiny shared util (or one importing the other) to avoid the two implementations drifting apart later.
  • Everything else in the new libs (trd-graph.js, prd-parser.js, feature-gen.js, drift.js, reqnroll-gen.js, reqnroll-run.js) is pure, well-commented on the why, and follows the existing trd-parser.js/trd-cli.js conventions closely (argv parsing, exit-code contract, atomic writes). Good consistency with established codebase patterns.

Potential bugs

  • methodNameFor collision risk (packages/product/lib/reqnroll-gen.js): step dedup in collectSteps keys on normalizeStepText (lowercased, whitespace-collapsed, punctuation preserved), but methodNameFor strips all non-alphanumeric characters before Pascal-casing. Two distinct step texts that differ only in punctuation (e.g. "the user's cart" vs. "the users cart") would produce the same C# method name inside the same generated [Binding] class, causing a duplicate-method compile error in the emitted project. Probably rare in practice, but worth a dedup-on-method-name guard (or an incrementing suffix), since this is emitted, uncompiled C# that no one sees until dotnet build fails.
  • Unescaped labels in emitMermaid/emitDot (packages/development/lib/trd-graph.js): n.label is interpolated directly into Mermaid node text and DOT quoted strings with no escaping. Labels are normally auto-derived (slugified, so safe), but the commit message notes labels are "author-overridable" -- a hand-authored label containing a double quote or bracket/paren would produce malformed Mermaid/DOT output. Low severity (build-time doc generation) but a one-line quote-escape would close it off.

Test coverage

  • Strong coverage on the pure libraries (trd-graph.test.js, prd-parser.test.js, feature-gen.test.js, drift.test.js, reqnroll-gen.test.js, reqnroll-run.test.js, binding-drift.test.js).
  • Gap: the three thin CLI wrappers (trd-graph-cli.js, prd-cli.js, reqnroll-cli.js) have no dedicated tests -- parseArgs, collectEntries's directory-scan/error-handling, and the exit-code contracts (0/1/2, which CI is meant to gate on per the docstrings) aren't directly exercised. Since these exit codes are the explicit CI-gating mechanism (cycle detection, drift detection), a thin test per CLI covering the exit-code branches would be worth adding.

Security

  • No injection/traversal concerns found: all generated filenames derive from regex-constrained ids (REQ-NNN, AC-NNN-M, path.basename), and reqnroll-run.js shells out via execFileSync with an argument array (no shell interpolation).

Other

  • This PR's own test-plan checklist (CI passes / npm run validate / npm test / artifact spot-check) is still unchecked in the description -- worth confirming CI is green before merging, especially given the size (5.9k additions, touching full/codex/pi generated artifacts as well as the source libs).

Nothing here blocks the merge in my view -- the duplication and CLI test gap are the most actionable items; the two potential-bug items are edge cases worth a follow-up rather than a required fix.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/development/commands/create-trd.yaml (1)

265-292: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

expectedOutput.structure drift from newly conditional Acceptance Criteria Traceability logic. Both command YAML files add --foundational-conditional traceability generation/validation but leave their static expectedOutput.structure "Acceptance Criteria Traceability" description unchanged. The already-generated create-trd.md shows the correct target wording ("when a full PRD is supplied"), so regenerating the Markdown from either YAML risks losing this caveat.

  • packages/development/commands/create-trd.yaml#L265-L292: update the "Acceptance Criteria Traceability" expectedOutput.structure description to add "when a full PRD is supplied", matching create-trd.md line 275.
  • packages/development/commands/create-trd-foreman.yaml#L214-L227: update the corresponding "Acceptance Criteria Traceability" expectedOutput.structure description to note it is skipped for --foundational capability briefs.
🤖 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 `@packages/development/commands/create-trd.yaml` around lines 265 - 292, Update
the Acceptance Criteria Traceability descriptions in expectedOutput.structure to
match the conditional generation rules: in
packages/development/commands/create-trd.yaml lines 265-292, state that the
matrix is generated when a full PRD is supplied; in
packages/development/commands/create-trd-foreman.yaml lines 214-227, state that
it is skipped for --foundational capability briefs. Keep the descriptions
consistent with the generated create-trd.md wording.
🟡 Minor comments (13)
packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md-61-63 (1)

61-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add language identifiers to both fenced blocks.

Both skills fail MD040 because the opening fence has no language.

  • packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md#L61-L63: change the opening fence to ```text.
  • packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md#L285-L285: change the opening fence to ```text.
🤖 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 `@packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md`
around lines 61 - 63, Update the opening fenced code blocks to include the text
language identifier in both
packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md
lines 61-63 and
packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md line 285;
change each bare fence to a text-labeled fence while preserving the block
contents.

Source: Linters/SAST tools

packages/codex/AGENTS.md-3-3 (1)

3-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add reqnroll to the documented test skills.

Line 3 reports 11 framework/testing skills. The Key Skills list on Line 31 lists only 10 skills and omits reqnroll. Add reqnroll or correct the count so Codex users can discover the new skill.

Proposed documentation fix
-- Test frameworks: `jest`, `pytest`, `rspec`, `xunit`, `exunit`
+- Test frameworks: `jest`, `pytest`, `rspec`, `xunit`, `exunit`, `reqnroll`
🤖 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 `@packages/codex/AGENTS.md` at line 3, Update the framework/testing skills
documentation in AGENTS.md to include reqnroll in the Key Skills list, and
ensure the reported skill count matches the resulting list.
packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md-233-235 (1)

233-235: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use the label: frontmatter contract consistently.

TRD parsing reads label, so PRD writers also need label: prd-<stem> in generated frontmatter. Add guidance for foundational TRDs to derive label: trd-<stem> from the capability brief when no PRD exists, and keep the same parser behavior in any PRD frontmatter 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 `@packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md` around
lines 233 - 235, Update the TRD frontmatter guidance in
packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md at lines
233-235 to consistently use the label: contract, including guidance for
foundational TRDs to derive label: trd-<stem> from the capability brief when no
PRD exists. Update the PRD frontmatter guidance in
packages/pi/prompts/ensemble-create-prd.md at line 227 and
packages/codex/.codex/skills/commands/ensemble-create-prd/SKILL.md at line 206
to require generated PRDs to include label: prd-<stem> and preserve the existing
parser behavior.
packages/pi/prompts/ensemble-trd-dependency-graph.md-7-9 (1)

7-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the blank line inside the blockquote.

Markdownlint reports MD028 for the blank line between the Mission and Constraints blockquote paragraphs. Remove the blank line or prefix it with >.

Suggested fix
 > **Mission:** ...
-
 > **Constraints:**
🤖 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 `@packages/pi/prompts/ensemble-trd-dependency-graph.md` around lines 7 - 9,
Remove the blank line between the Mission and Constraints paragraphs in the
blockquote in the TRD dependency graph prompt, or prefix that line with “>” so
the entire blockquote remains contiguous and satisfies Markdownlint MD028.

Source: Linters/SAST tools

packages/pi/skills/reqnroll/SKILL.md-111-114 (1)

111-114: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the xUnit category filter for Reqnroll tags.

dotnet test --filter "@AC-001-1" does not match the xUnit Category trait generated from the @AC-001-1 Gherkin tag. Use dotnet test --filter "Category=AC-001-1" instead.

🤖 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 `@packages/pi/skills/reqnroll/SKILL.md` around lines 111 - 114, Update the
Reqnroll test command examples to filter tags through the xUnit Category trait:
replace the tag-based filter with Category=AC-001-1 while preserving the
existing single-scenario behavior.
packages/product/lib/prd-parser.js-41-47 (1)

41-47: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Declare js-yaml as a dependency of @fortium/ensemble-product.

packages/product/lib/prd-parser.js imports js-yaml for frontmatter parsing, but packages/product/package.json does not declare it. Add a direct dependency so PRD frontmatter behavior does not depend on hoisted/root-only resolution.

🤖 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 `@packages/product/lib/prd-parser.js` around lines 41 - 47, Add js-yaml as a
direct runtime dependency in packages/product/package.json, matching the
package’s existing dependency conventions and the version required by the
require('js-yaml') usage in prd-parser.js. Leave the optional loading behavior
in prd-parser.js unchanged.
packages/product/lib/reqnroll-run.js-29-54 (1)

29-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

failed double-counts Pending/undefined-step tests.

failed (Line 33) comes from the TRX Counters attribute, which counts a Pending or undefined-step test as outcome="Failed". The same test is then counted again in pending (Lines 46-51). For example, the TRX_PENDING fixture in reqnroll-run.test.js yields failed: 2 and pending: 2 for the same two tests, so passed + failed + pending does not add up to total. This does not break isGreen(), but it produces a misleading reportRun summary (passed X, failed Y, pending Z, total N) that a developer relies on to distinguish real assertion failures from Pending steps during the RED-gate loop.

Subtract pending from failed so the two categories are mutually exclusive.

🐛 Proposed fix to make failed/pending mutually exclusive
   const total = num(counters, 'total');
   const passed = num(counters, 'passed');
-  const failed = num(counters, 'failed') + num(counters, 'error');
+  const failedRaw = num(counters, 'failed') + num(counters, 'error');
   const skipped = num(counters, 'notExecuted') + num(counters, 'inconclusive');

   // Walk individual results to classify pending / undefined steps.
   let pending = 0;
   const undefinedSteps = [];
   const resultRe = /<UnitTestResult\b([^>]*)>([\s\S]*?)<\/UnitTestResult>|<UnitTestResult\b([^>]*)\/>/gi;
   let m;
   while ((m = resultRe.exec(text)) !== null) {
     ...
   }
+  const failed = Math.max(0, failedRaw - pending);
   return { passed, failed, skipped, pending, total, undefinedSteps };
🤖 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 `@packages/product/lib/reqnroll-run.js` around lines 29 - 54, Update parseTrx
so the returned failed count subtracts the pending count identified while
scanning UnitTestResult entries, keeping pending tests mutually exclusive from
failed tests and ensuring the report totals remain consistent.
packages/pi/prompts/ensemble-generate-reqnroll-bindings.md-7-9 (1)

7-9: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the blank line inside the block quote.

Line 8 triggers markdownlint rule MD028. Keep the line quoted with > or remove the blank 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 `@packages/pi/prompts/ensemble-generate-reqnroll-bindings.md` around lines 7 -
9, Remove the blank line between the quoted Mission paragraph and the quoted
Constraints heading, keeping the block quote contiguous so the markdown
satisfies MD028.

Source: Linters/SAST tools

packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md-62-64 (1)

62-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Label all fenced text blocks.

These fences violate markdownlint rule MD040.

  • packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md#L62-L64: add a fence language in packages/development/commands/trd-dependency-graph.yaml, then regenerate.
  • packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md#L60-L62: add a fence language in packages/product/commands/check-binding-drift.yaml, then regenerate.
  • packages/reqnroll/README.md#L17-L26: change the opening fence to ```text.
🤖 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 `@packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md`
around lines 62 - 64, Label every fenced text block to satisfy markdownlint
MD040. In packages/development/commands/trd-dependency-graph.yaml and
packages/product/commands/check-binding-drift.yaml, add an appropriate fence
language to the command examples and regenerate the corresponding SKILL.md
files; in packages/reqnroll/README.md, change the opening fence to use the text
language.

Source: Linters/SAST tools

packages/development/commands/ensemble/create-trd.md-279-283 (1)

279-283: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced usage code block.

markdownlint-cli2 flags this fenced code block for missing a language (MD040).

📝 Proposed fix
-```
+```text
 /ensemble:create-trd [prd-path] [--team] [--foundational]
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @packages/development/commands/ensemble/create-trd.md around lines 279 - 283,
Update the fenced usage code block under the “Usage” heading in create-trd.md to
include the text language identifier, while preserving the command content
unchanged.


</details>

<!-- cr-comment:v1:329dd2ccda9706af5a283535 -->

_Source: Linters/SAST tools_

</blockquote></details>
<details>
<summary>packages/development/commands/trd-dependency-graph.yaml-10-10 (1)</summary><blockquote>

`10-10`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_

**`--overlap` flag is documented but unused.**

`argument_hint` (line 10) lists `[--overlap]`. The "Analyze" phase workflow (steps 1-3) always runs the overlap subcommand unconditionally, and `expectedOutput.structure` (lines 60-61) lists the overlap report as standard output every run.

Remove `--overlap` from `argument_hint` if it has no effect, or add a step that conditions the overlap subcommand invocation on this flag. Regenerate `commands/ensemble/trd-dependency-graph.md` after the fix.





Also applies to: 47-53, 55-63

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @packages/development/commands/trd-dependency-graph.yaml at line 10, Remove
the unused --overlap option from argument_hint and update the Analyze workflow
and expectedOutput.structure to treat overlap output as unconditional, then
regenerate the corresponding command documentation.


</details>

<!-- cr-comment:v1:b765b41c1e3003bf8b491479 -->

</blockquote></details>
<details>
<summary>packages/development/commands/ensemble/trd-dependency-graph.md-7-7 (1)</summary><blockquote>

`7-7`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_

**Clarify or implement the `--overlap` flag.**

The argument-hint documents `[--overlap]`. The workflow never checks this flag. Step 3 ("Report duplicate work") always runs the overlap report, and "Expected Output" lists it as standard output every run.

Either remove `--overlap` from the argument-hint, since it has no effect, or add a step that skips the overlap report when the flag is absent.





Also applies to: 40-46, 47-54

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @packages/development/commands/ensemble/trd-dependency-graph.md at line 7,
Align the documented --overlap behavior with the workflow: either remove
--overlap from the argument-hint and make duplicate-work reporting
unconditional, or implement flag parsing so Step 3 and its Expected Output
appear only when --overlap is supplied. Apply the same choice consistently
across the argument hint, Step 3, and Expected Output.


</details>

<!-- cr-comment:v1:6e5787b7b5d404e77d0c5252 -->

</blockquote></details>
<details>
<summary>packages/development/lib/trd-graph.js-294-325 (1)</summary><blockquote>

`294-325`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_

**Escape node labels before embedding them in Mermaid/DOT output.**

`emitMermaid` and `emitDot` embed `n.label` directly inside a quoted string (`["…"]` for Mermaid, `"…"` for DOT). `n.label` can be `parsed.label`, a raw, unsanitized string taken straight from TRD frontmatter (see trd-parser.js line 1000). If a label contains a double quote or a newline, the generated Mermaid or DOT syntax breaks.

Escape the label before embedding it in either emitter.



<details>
<summary>🐛 Proposed fix for label escaping</summary>

```diff
+function escapeLabel(label) {
+  return String(label)
+    .replace(/\\/g, '\\\\')
+    .replace(/"/g, '\\"')
+    .replace(/\r?\n/g, ' ');
+}
+
 function emitMermaid(graph) {
   const lines = ['graph TD'];
   for (const n of graph.nodes) {
     const shape = n.kind === 'foundational' ? [`(["`, `"])`] : [`["`, `"]`];
-    lines.push(`  ${mermaidId(n.id)}${shape[0]}${n.label}${shape[1]}`);
+    lines.push(`  ${mermaidId(n.id)}${shape[0]}${escapeLabel(n.label)}${shape[1]}`);
   }
   for (const e of graph.edges) {
     lines.push(`  ${mermaidId(e.from)} --> ${mermaidId(e.to)}`);
   }
   return lines.join('\n') + '\n';
 }

 function emitDot(graph) {
   const lines = ['digraph TRD {', '  rankdir=LR;', '  node [shape=box];'];
   for (const n of graph.nodes) {
     const shape = n.kind === 'foundational' ? ' shape=ellipse' : '';
-    lines.push(`  "${n.id}" [label="${n.label}"${shape}];`);
+    lines.push(`  "${n.id}" [label="${escapeLabel(n.label)}"${shape}];`);
   }
   for (const e of graph.edges) {
     lines.push(`  "${e.from}" -> "${e.to}";`);
   }
   lines.push('}');
   return lines.join('\n') + '\n';
 }
🤖 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 `@packages/development/lib/trd-graph.js` around lines 294 - 325, Escape each
node label before embedding it in the quoted output generated by emitMermaid and
emitDot. Handle at least double quotes and newline characters, while preserving
the existing shape and graph structure; apply the escaping consistently to
n.label in both emitters.
🧹 Nitpick comments (5)
packages/product/tests/reqnroll-gen.test.js (1)

49-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Strengthen the dedupe test with an actual duplicate scenario.

This test only checks that the existing fixture happens to contain no duplicate (keyword, text) pairs. It does not construct a PRD with two ACs sharing identical Given/When/Then text, so it cannot detect a regression in the dedup logic inside collectSteps (see packages/product/lib/reqnroll-gen.js:80-108, which builds a seen set keyed by `${keyword}::${normalizeStepText(text)}`). Add a small inline PRD fixture with two ACs that share a clause and assert collectSteps returns only one entry for it.

🤖 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 `@packages/product/tests/reqnroll-gen.test.js` around lines 49 - 58, The dedupe
test in reqnroll-gen.test.js only validates the current fixture and does not
exercise duplicate handling. Add a small inline PRD fixture containing two
acceptance criteria with identical Given, When, or Then text, invoke
collectSteps on it, and assert the shared keyword/text pair appears only once,
preserving the existing normalized-key behavior.
packages/product/tests/reqnroll-run.test.js (1)

93-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Mock child_process instead of shelling out to a real dotnet binary.

This test calls runTests(dir) directly, which invokes the real execFileSync('dotnet', ...) in reqnroll-run.js. The test's outcome and speed then depend on whether dotnet is installed in the CI environment and on its exact failure behavior in an empty directory, rather than on a controlled, hermetic condition.

Use jest.mock('child_process') to simulate both an ENOENT case and a non-zero exit case, so the test is deterministic and fast regardless of the SDKs installed on the runner.

As per path instructions, packages/*/tests/**/*.{js,jsx,ts,tsx} should Use Jest for most package tests and Vitest for tests in multiplexer-adapters. Use vi.mock() for ESM modules and jest.mock() for CommonJS modules.

🤖 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 `@packages/product/tests/reqnroll-run.test.js` around lines 93 - 111, Update
the runTests test to mock the CommonJS child_process dependency with jest.mock()
instead of invoking the real dotnet binary. Configure the mock to cover both the
ENOENT condition and a non-zero exit from execFileSync, while preserving
assertions that stale reqnroll.trx is removed and the result is not green.

Source: Path instructions

packages/pi/prompts/ensemble-reqnroll-tdd.md (1)

7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the blank line inside the blockquote. markdownlint (MD028) flags a blank line between the **Mission:** and **Constraints:** blockquotes in both pi prompt files; the shared root cause is the pi-prompt generator template inserting a blank line between adjacent blockquote sections.

  • packages/pi/prompts/ensemble-reqnroll-tdd.md#L7-L9: prefix the blank line with > (or remove it) so the two blockquotes merge into one continuous blockquote.
  • packages/pi/prompts/ensemble-check-binding-drift.md#L7-L9: apply the same fix.
🤖 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 `@packages/pi/prompts/ensemble-reqnroll-tdd.md` around lines 7 - 9, Remove or
prefix the blank line separating the Mission and Constraints blockquotes in the
pi-prompt generator output. Apply this change to
packages/pi/prompts/ensemble-reqnroll-tdd.md lines 7-9 and
packages/pi/prompts/ensemble-check-binding-drift.md lines 7-9 so both sections
form one continuous blockquote and satisfy markdownlint MD028.

Source: Linters/SAST tools

packages/product/lib/reqnroll-cli.js (1)

58-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid rebuilding the artifacts twice in runGenerate.

Line 58 calls writeBindingArtifacts, which internally calls buildBindingArtifacts with the full opts (including prdPath). Line 64 calls buildBindingArtifacts again, with a different, smaller opts object, only to read built.manifest.steps.length. This duplicates the PRD-to-artifact build on every invocation and creates a risk: if buildBindingArtifacts starts reading another opts field in the future, the second call's report can silently diverge from what was actually written to disk.

Have writeBindingArtifacts return the manifest (or the full artifacts) so runGenerate does not need a second build call with a different opts shape.

🤖 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 `@packages/product/lib/reqnroll-cli.js` around lines 58 - 71, Avoid the
duplicate artifact build in runGenerate by updating writeBindingArtifacts to
return the manifest or full built artifacts produced with the complete options,
including prdPath. Replace the separate buildBindingArtifacts call in
runGenerate with that returned data when calculating the steps count, while
preserving the existing write behavior and result fields.
packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md (1)

86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a language tag to the "Usage" fenced code blocks. markdownlint (MD040) flags the same missing-language fenced block in all four generated command docs; the shared root cause is a generator template that emits the Usage block without a language hint.

  • packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md#L86-L88: add a language (e.g. bash or text) to the fence around /ensemble:reqnroll-tdd ....
  • packages/product/commands/ensemble/reqnroll-tdd.md#L82-L84: add the same language tag to its Usage fence.
  • packages/product/commands/ensemble/generate-reqnroll-bindings.md#L61-L63: add the same language tag to its Usage fence.
  • packages/codex/.codex/skills/commands/ensemble-generate-reqnroll-bindings/SKILL.md#L65-L67: add the same language tag to its Usage fence.
🤖 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 `@packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md` around
lines 86 - 88, The generated Usage fenced blocks lack language tags, triggering
markdownlint MD040. Update the shared generator template that emits these Usage
blocks to include a consistent language tag such as bash or text, and regenerate
or apply the same change to
packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md lines
86-88, packages/product/commands/ensemble/reqnroll-tdd.md lines 82-84,
packages/product/commands/ensemble/generate-reqnroll-bindings.md lines 61-63,
and
packages/codex/.codex/skills/commands/ensemble-generate-reqnroll-bindings/SKILL.md
lines 65-67.

Source: Linters/SAST tools

🤖 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 `@packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md`:
- Around line 32-33: Update Phase 2 Domain Analysis to branch on foundational
input: when the capability registry is present, consume its named capabilities,
scope, and target files to identify domain boundaries and architecture coverage
instead of scanning for REQ-NNN requirements; retain the existing
requirements-mapping flow for full PRDs.
- Around line 80-82: Update the cross-TRD dependency guidance in the
ensemble-create-trd skill to define one canonical, parser-supported syntax based
on stable document IDs rather than slugs or mutable labels. Align every graph,
refinement, validation, and command parser that creates dependency edges with
this syntax, and update the surrounding references and examples to use it
consistently.

In `@packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md`:
- Line 196: Update the ensemble-refine-trd workflow to branch on the document’s
kind: for foundational TRDs, preserve all frontmatter fields—including
capabilities, PRD references, Document ID, and Label—without regeneration,
accept both micro-UUID and TRD-YYYY-NNN identifiers, and skip the PRD-specific
[satisfies REQ-NNN] coverage requirement. Retain the existing traceability
validation for non-foundational TRDs.

In `@packages/development/commands/create-trd-foreman.yaml`:
- Around line 84-93: The Capability Reuse Check instructions currently record
foundational reuse only in prose, so graph tooling cannot see those
relationships. Update the actions under “Capability Reuse Check” to encode each
reused foundational TRD reference as a supported `[depends: ...]` annotation or
`Dependencies:` entry, while retaining the `## Reused Capabilities` details and
local-only Foreman Dependencies requirement.

In `@packages/pi/prompts/ensemble-create-trd-foreman.md`:
- Around line 24-25: Update Phase 2 domain analysis in
packages/pi/prompts/ensemble-create-trd-foreman.md at lines 24-25 and
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md at
lines 31-32 to branch on the input type: foundational runs must analyze named
capabilities, scope, and target files from the capability registry, while full
PRD runs continue analyzing REQ-NNN requirements. Preserve the existing
registries and ensure both workflows apply the same behavior.
- Around line 185-186: Define a single parser-visible contract for foundational
TRDs’ PRD Reference in the workflow instructions: specify whether the field is
omitted, emitted empty, or populated with a capability-brief reference, while
preserving the required behavior for non-foundational TRDs. Apply the identical
contract at packages/pi/prompts/ensemble-create-trd-foreman.md:185-186 and
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md:189-190.
- Around line 183-185: Complete the foundational no-PRD capability-brief branch
in both packages/pi/prompts/ensemble-create-trd-foreman.md (lines 183-185) and
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md
(lines 187-189): derive TRD_LABEL as trd-<stem> from the brief’s label or title,
use it in frontmatter, and print a foundational-specific next step rather than
PRD-dependent follow-up commands. Keep the existing full-PRD behavior unchanged.
- Around line 79-82: Guard the capability and overlap CLI calls in the workflow
so both are skipped when docs/TR does not exist, while treating the registry as
empty and continuing. Apply this change at
packages/pi/prompts/ensemble-create-trd-foreman.md:79-82 and
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md:76-79,
preserving the existing TRD_GRAPH_CLI resolution and foundational-capability
reuse behavior.
- Around line 81-83: Require explicit capability-token or provider matches
before suppressing task rows in the Foreman workflow. In
packages/pi/prompts/ensemble-create-trd-foreman.md lines 81-83, treat
foundational TRD label/title and target-file overlap, including TRD graph
overlap, only as review warnings and retain the task unless explicit evidence
exists; apply the same change in
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md lines
78-80, while preserving explicit reuse reporting and extraction recommendations.

In `@packages/product/lib/prd-cli.js`:
- Around line 97-121: Implement the documented --force behavior across
runGenerate, writeArtifacts, and writeFileAtomic: pass flags.force into artifact
generation and prevent existing files from being overwritten unless force is
enabled. Ensure skipped versus written artifacts are reflected in the existing
result fields, while preserving dry-run behavior.

In `@packages/product/lib/prd-parser.js`:
- Around line 184-243: Add a seenReq set alongside seenAc and, when processing a
REQ_HEADING_RE match in the main lines loop, normalize the heading id and warn
plus skip creating a requirement if it has already been seen. Preserve the
existing first requirement and continue parsing subsequent unique requirements
normally, using the same duplicate-warning pattern as the AC handling.

In `@packages/product/lib/reqnroll-gen.js`:
- Around line 285-291: Update the artifact-writing loop in
packages/product/lib/reqnroll-gen.js:285-291 so existing step files are merged
with newly generated missing methods while preserving all existing method
bodies, rather than skipped unless --force; retain overwrite behavior for
non-step artifacts and use the existing step-file detection/generation helpers.
Update packages/pi/prompts/ensemble-generate-reqnroll-bindings.md:13-14 and
packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md:47-48
to promise or recommend regeneration only once this additive, body-preserving
merge is available.
- Around line 94-102: The generated binding identity must ignore keywords and
distinguish punctuation variants. Update the deduplication in the generation
flow around key, seen, and out to reject duplicate normalizeStepText(text)
values across all keywords; update methodNameFor usage to append the
corresponding step hash, preserving unique generated C# signatures. Add
regression tests covering same text with different keywords and punctuation
variants such as account-id versus account id.
- Around line 140-144: Update renderStepClass so generated step attributes use
fixed, regex-escaped anchored literals rather than inserting s.text directly,
while preserving quote escaping for the generated source. Update
scanBindingAttributes to decode and compare the literal text, and adjust drift
tests to validate the decoded literal behavior.

In `@packages/product/lib/reqnroll-run.js`:
- Around line 101-119: Update the execFileSync call in runTests to set an
explicit timeout and a larger maxBuffer suitable for verbose dotnet test output.
In its catch block, detect the timeout condition explicitly, assign the
appropriate nonzero exitCode, and preserve captured stdout/stderr handling for
both timeout and other execution failures.

In `@scripts/generate-codex/index.js`:
- Around line 244-266: Extract the shared frontmatter construction and skill
output logic from the existing skill-processing loop and the special reqnroll
block into a helper such as copySkillFile. Have the helper accept the skill
file, package name, and dryRun/verbose options, then call it from both paths
while preserving the current defaults, normalization, and write behavior.

---

Outside diff comments:
In `@packages/development/commands/create-trd.yaml`:
- Around line 265-292: Update the Acceptance Criteria Traceability descriptions
in expectedOutput.structure to match the conditional generation rules: in
packages/development/commands/create-trd.yaml lines 265-292, state that the
matrix is generated when a full PRD is supplied; in
packages/development/commands/create-trd-foreman.yaml lines 214-227, state that
it is skipped for --foundational capability briefs. Keep the descriptions
consistent with the generated create-trd.md wording.

---

Minor comments:
In `@packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md`:
- Around line 61-63: Update the opening fenced code blocks to include the text
language identifier in both
packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md
lines 61-63 and
packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md line 285;
change each bare fence to a text-labeled fence while preserving the block
contents.

In `@packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md`:
- Around line 233-235: Update the TRD frontmatter guidance in
packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md at lines
233-235 to consistently use the label: contract, including guidance for
foundational TRDs to derive label: trd-<stem> from the capability brief when no
PRD exists. Update the PRD frontmatter guidance in
packages/pi/prompts/ensemble-create-prd.md at line 227 and
packages/codex/.codex/skills/commands/ensemble-create-prd/SKILL.md at line 206
to require generated PRDs to include label: prd-<stem> and preserve the existing
parser behavior.

In
`@packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md`:
- Around line 62-64: Label every fenced text block to satisfy markdownlint
MD040. In packages/development/commands/trd-dependency-graph.yaml and
packages/product/commands/check-binding-drift.yaml, add an appropriate fence
language to the command examples and regenerate the corresponding SKILL.md
files; in packages/reqnroll/README.md, change the opening fence to use the text
language.

In `@packages/codex/AGENTS.md`:
- Line 3: Update the framework/testing skills documentation in AGENTS.md to
include reqnroll in the Key Skills list, and ensure the reported skill count
matches the resulting list.

In `@packages/development/commands/ensemble/create-trd.md`:
- Around line 279-283: Update the fenced usage code block under the “Usage”
heading in create-trd.md to include the text language identifier, while
preserving the command content unchanged.

In `@packages/development/commands/ensemble/trd-dependency-graph.md`:
- Line 7: Align the documented --overlap behavior with the workflow: either
remove --overlap from the argument-hint and make duplicate-work reporting
unconditional, or implement flag parsing so Step 3 and its Expected Output
appear only when --overlap is supplied. Apply the same choice consistently
across the argument hint, Step 3, and Expected Output.

In `@packages/development/commands/trd-dependency-graph.yaml`:
- Line 10: Remove the unused --overlap option from argument_hint and update the
Analyze workflow and expectedOutput.structure to treat overlap output as
unconditional, then regenerate the corresponding command documentation.

In `@packages/development/lib/trd-graph.js`:
- Around line 294-325: Escape each node label before embedding it in the quoted
output generated by emitMermaid and emitDot. Handle at least double quotes and
newline characters, while preserving the existing shape and graph structure;
apply the escaping consistently to n.label in both emitters.

In `@packages/pi/prompts/ensemble-generate-reqnroll-bindings.md`:
- Around line 7-9: Remove the blank line between the quoted Mission paragraph
and the quoted Constraints heading, keeping the block quote contiguous so the
markdown satisfies MD028.

In `@packages/pi/prompts/ensemble-trd-dependency-graph.md`:
- Around line 7-9: Remove the blank line between the Mission and Constraints
paragraphs in the blockquote in the TRD dependency graph prompt, or prefix that
line with “>” so the entire blockquote remains contiguous and satisfies
Markdownlint MD028.

In `@packages/pi/skills/reqnroll/SKILL.md`:
- Around line 111-114: Update the Reqnroll test command examples to filter tags
through the xUnit Category trait: replace the tag-based filter with
Category=AC-001-1 while preserving the existing single-scenario behavior.

In `@packages/product/lib/prd-parser.js`:
- Around line 41-47: Add js-yaml as a direct runtime dependency in
packages/product/package.json, matching the package’s existing dependency
conventions and the version required by the require('js-yaml') usage in
prd-parser.js. Leave the optional loading behavior in prd-parser.js unchanged.

In `@packages/product/lib/reqnroll-run.js`:
- Around line 29-54: Update parseTrx so the returned failed count subtracts the
pending count identified while scanning UnitTestResult entries, keeping pending
tests mutually exclusive from failed tests and ensuring the report totals remain
consistent.

---

Nitpick comments:
In `@packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md`:
- Around line 86-88: The generated Usage fenced blocks lack language tags,
triggering markdownlint MD040. Update the shared generator template that emits
these Usage blocks to include a consistent language tag such as bash or text,
and regenerate or apply the same change to
packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md lines
86-88, packages/product/commands/ensemble/reqnroll-tdd.md lines 82-84,
packages/product/commands/ensemble/generate-reqnroll-bindings.md lines 61-63,
and
packages/codex/.codex/skills/commands/ensemble-generate-reqnroll-bindings/SKILL.md
lines 65-67.

In `@packages/pi/prompts/ensemble-reqnroll-tdd.md`:
- Around line 7-9: Remove or prefix the blank line separating the Mission and
Constraints blockquotes in the pi-prompt generator output. Apply this change to
packages/pi/prompts/ensemble-reqnroll-tdd.md lines 7-9 and
packages/pi/prompts/ensemble-check-binding-drift.md lines 7-9 so both sections
form one continuous blockquote and satisfy markdownlint MD028.

In `@packages/product/lib/reqnroll-cli.js`:
- Around line 58-71: Avoid the duplicate artifact build in runGenerate by
updating writeBindingArtifacts to return the manifest or full built artifacts
produced with the complete options, including prdPath. Replace the separate
buildBindingArtifacts call in runGenerate with that returned data when
calculating the steps count, while preserving the existing write behavior and
result fields.

In `@packages/product/tests/reqnroll-gen.test.js`:
- Around line 49-58: The dedupe test in reqnroll-gen.test.js only validates the
current fixture and does not exercise duplicate handling. Add a small inline PRD
fixture containing two acceptance criteria with identical Given, When, or Then
text, invoke collectSteps on it, and assert the shared keyword/text pair appears
only once, preserving the existing normalized-key behavior.

In `@packages/product/tests/reqnroll-run.test.js`:
- Around line 93-111: Update the runTests test to mock the CommonJS
child_process dependency with jest.mock() instead of invoking the real dotnet
binary. Configure the mock to cover both the ENOENT condition and a non-zero
exit from execFileSync, while preserving assertions that stale reqnroll.trx is
removed and the result is not green.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 833c6a38-d1b9-466c-8eae-f683309be015

📥 Commits

Reviewing files that changed from the base of the PR and between 5edaaa2 and 4605729.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (85)
  • marketplace.json
  • packages/codex/.codex/agents/reqnroll-binding-specialist.toml
  • packages/codex/.codex/config.toml
  • packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-check-feature-drift/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-create-prd/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-generate-feature-tests/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-generate-reqnroll-bindings/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-refine-prd/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-reqnroll-tdd/SKILL.md
  • packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md
  • packages/codex/.codex/skills/reqnroll/REFERENCE.md
  • packages/codex/.codex/skills/reqnroll/SKILL.md
  • packages/codex/AGENTS.md
  • packages/development/commands/create-trd-foreman.yaml
  • packages/development/commands/create-trd.yaml
  • packages/development/commands/ensemble/create-trd-foreman.md
  • packages/development/commands/ensemble/create-trd.md
  • packages/development/commands/ensemble/refine-trd.md
  • packages/development/commands/ensemble/trd-dependency-graph.md
  • packages/development/commands/refine-trd.yaml
  • packages/development/commands/trd-dependency-graph.yaml
  • packages/development/lib/trd-graph-cli.js
  • packages/development/lib/trd-graph.js
  • packages/development/lib/trd-parser.js
  • packages/development/tests/trd-graph.test.js
  • packages/development/tests/trd-parser.test.js
  • packages/full/.claude-plugin/plugin.json
  • packages/full/agents/reqnroll
  • packages/full/skills/reqnroll
  • packages/pi/agents/reqnroll-binding-specialist.md
  • packages/pi/prompts/ensemble-check-binding-drift.md
  • packages/pi/prompts/ensemble-check-feature-drift.md
  • packages/pi/prompts/ensemble-create-prd.md
  • packages/pi/prompts/ensemble-create-trd-foreman.md
  • packages/pi/prompts/ensemble-create-trd.md
  • packages/pi/prompts/ensemble-generate-feature-tests.md
  • packages/pi/prompts/ensemble-generate-reqnroll-bindings.md
  • packages/pi/prompts/ensemble-refine-prd.md
  • packages/pi/prompts/ensemble-refine-trd.md
  • packages/pi/prompts/ensemble-reqnroll-tdd.md
  • packages/pi/prompts/ensemble-trd-dependency-graph.md
  • packages/pi/skills/reqnroll/REFERENCE.md
  • packages/pi/skills/reqnroll/SKILL.md
  • packages/product/commands/check-binding-drift.yaml
  • packages/product/commands/check-feature-drift.yaml
  • packages/product/commands/create-prd.yaml
  • packages/product/commands/ensemble/check-binding-drift.md
  • packages/product/commands/ensemble/check-feature-drift.md
  • packages/product/commands/ensemble/create-prd.md
  • packages/product/commands/ensemble/generate-feature-tests.md
  • packages/product/commands/ensemble/generate-reqnroll-bindings.md
  • packages/product/commands/ensemble/refine-prd.md
  • packages/product/commands/ensemble/reqnroll-tdd.md
  • packages/product/commands/generate-feature-tests.yaml
  • packages/product/commands/generate-reqnroll-bindings.yaml
  • packages/product/commands/refine-prd.yaml
  • packages/product/commands/reqnroll-tdd.yaml
  • packages/product/lib/drift.js
  • packages/product/lib/feature-gen.js
  • packages/product/lib/prd-cli.js
  • packages/product/lib/prd-parser.js
  • packages/product/lib/reqnroll-cli.js
  • packages/product/lib/reqnroll-gen.js
  • packages/product/lib/reqnroll-run.js
  • packages/product/tests/binding-drift.test.js
  • packages/product/tests/drift.test.js
  • packages/product/tests/feature-gen.test.js
  • packages/product/tests/fixtures/PRD-sample.md
  • packages/product/tests/prd-parser.test.js
  • packages/product/tests/reqnroll-gen.test.js
  • packages/product/tests/reqnroll-run.test.js
  • packages/reqnroll/.claude-plugin/plugin.json
  • packages/reqnroll/README.md
  • packages/reqnroll/agents/reqnroll-binding-specialist.md
  • packages/reqnroll/agents/reqnroll-binding-specialist.yaml
  • packages/reqnroll/commands/.gitkeep
  • packages/reqnroll/lib/index.js
  • packages/reqnroll/package.json
  • packages/reqnroll/skills/reqnroll/REFERENCE.md
  • packages/reqnroll/skills/reqnroll/SKILL.md
  • scripts/generate-codex/index.js

Comment on lines +32 to +33
- If --foundational and no full PRD exists: accept a short capability brief instead (the shared work to build), skip PRD-structure validation for this run, and build a capability registry from the brief's named capabilities / scope / target files instead of REQ-NNN IDs
- If a full PRD is provided: validate document structure (required sections present), extract key requirements with REQ-NNN IDs, and build requirements registry for traceability tracking

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Feed foundational capabilities into Domain Analysis.

Line 32 builds a capability registry from brief capabilities, scope, and target files. Phase 2 still instructs the agent to scan REQ-NNN requirements and map requirements to domains. A short foundational brief has no REQ-NNN IDs, so capability reuse and architecture coverage can be skipped. Add a foundational branch that consumes the capability registry.

🧰 Tools
🪛 SkillSpector (2.4.4)

[warning] 145: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))


[warning] 156: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))


[warning] 167: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))

🤖 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 `@packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md` around
lines 32 - 33, Update Phase 2 Domain Analysis to branch on foundational input:
when the capability registry is present, consume its named capabilities, scope,
and target files to identify domain boundaries and architecture coverage instead
of scanning for REQ-NNN requirements; retain the existing requirements-mapping
flow for full PRDs.

Comment on lines +80 to +82
- If a foundational TRD already provides the capability: DO NOT generate duplicate tasks for it. Instead add a cross-TRD dependency [depends: <foundational-slug>#TRD-NNN] (or #PR-N) on the task that needs it, and record it under a '## Reused Capabilities' section (capability -> foundational TRD label + document id)
- If a needed capability is clearly reusable across PRDs but no foundational TRD exists yet, recommend extracting it: suggest running /ensemble:create-trd <prd> --foundational to create a shared TRD, rather than embedding the work here
- Reference foundational work by slug / document id only -- never by label (labels are display-only and may change)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
fd -a 'SKILL.md|.*refinement.*|.*graph.*|.*validation.*|.*depends.*|micro-uuid|TRD|PR' packages/codex 2>/dev/null | head -200

echo
echo "== exact dependency refs in target skill =="
sed -n '70,90p' packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md || true

echo
echo "== depends reference occurrences =="
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' 'depends:|`#PR-`|micro-UUID|micro uuid|canonical|slug|`#TRD`|TRD-NNN|document id|document-id|immutable' packages/codex 2>/dev/null | head -300

Repository: FortiumPartners/ensemble

Length of output: 30741


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# List package top-level skills/commands structure and inspect likely parsers.
echo "== packages/codex tree (focused) =="
git ls-files packages/codex | sed -n '1,220p'

echo
echo "== files likely containing parser/graph/validation =="
git ls-files packages/codex | grep -Ei '(parse|graph|node|task|validation|schema|ref|doc|document|id|uuid)' | sort

Repository: FortiumPartners/ensemble

Length of output: 5995


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant line ranges =="
for f in \
  packages/codex/.codex/skills/commands/ensemble-configure-team/SKILL.md \
  packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md \
  packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md \
  packages/codex/.codex/skills/commands/ensemble-create-prd/SKILL.md
do
  echo "--- $f"
  ast-grep outline "$f" --view compact | head -80 || true
  echo
done

echo "== exact configured snippets =="
sed -n '30,110p' packages/codex/.codex/skills/commands/ensemble-configure-team/SKILL.md
echo "---"
sed -n '25,65p' packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md
sed -n '145,165p' packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md
echo "---"
sed -n '35,75p' packages/codex/.codex/skills/commands/ensemble-trd-dependency-graph/SKILL.md
echo "---"
sed -n '210,224p' packages/codex/.codex/skills/commands/ensemble-create-prd/SKILL.md

echo "== parser CLI references/imports =="
rg -n --hidden --glob '!node_modules/**' 'TRD_CLI|trd-graph-cli|br\s+list|b-|b.*graph|trd-graph|graph|dependsOn|taskIds|nodes|edges|micro-UUID|micro UUID|uuid' packages/codex bin lib . 2>/dev/null | head -300 || true

Repository: FortiumPartners/ensemble

Length of output: 50382


Use a stable, parser-supported cross-TRD dependency format.

[depends: <foundational-slug>#TRD-NNN] and #PR-N mix mutable slug/label-based references with existing TRD-NNN dependency parsing and PRD micro-UUID document IDs. Define one canonical document-ID-based syntax and update graph, refinement, validation, and command parsers that build dependency edges.

🧰 Tools
🪛 SkillSpector (2.4.4)

[warning] 145: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))


[warning] 156: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))


[warning] 167: [EA1] Unrestricted Tool Access: Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Remediation: Restrict tool access to only the tools required for the skill's stated purpose. Use an explicit allowlist rather than granting blanket access.

(Excessive Agency (EA1))

🤖 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 `@packages/codex/.codex/skills/commands/ensemble-create-trd/SKILL.md` around
lines 80 - 82, Update the cross-TRD dependency guidance in the
ensemble-create-trd skill to define one canonical, parser-supported syntax based
on stable document IDs rather than slugs or mutable labels. Align every graph,
refinement, validation, and command parser that creates dependency edges with
this syntax, and update the surrounding references and examples to use it
consistently.

Write the refined TRD with version history and changelog

- Bump version in frontmatter (increment patch: e.g. 1.0.0 -> 1.0.1)
- Preserve the Document ID and Label frontmatter fields unchanged -- never regenerate them on refine (beads and PRD/TRD correlation depend on the stable micro UUID id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files named SKILL.md around ensemble-refine:"
fd -p 'packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md' . || true
echo

file="packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md"
if [ -f "$file" ]; then
  echo "File exists. Line count:"
  wc -l "$file"
  echo
  echo "Relevant sections:"
  rg -n "Document ID|Label|TRD-YYYY-NNN|REQ-NNN|foundation|foundational|capabilities|frontmatter|refine" "$file" -C 3
fi

Repository: FortiumPartners/ensemble

Length of output: 7204


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find create-trd related files:"
fd -i 'create.*trd|trd|TRD' packages/codex/.codex 2>/dev/null | sed -n '1,120p'
echo

echo "Search micro-UUIDs, foundational metadata, and document-id parsing in skills:"
rg -n "micro|uuid|UUID|foundational|capabilities|document.?id|Document ID|TRD-YYYY-NNN|REQ-NNN" packages/codex/.codex/skills -S -C 2 | sed -n '1,240p'

Repository: FortiumPartners/ensemble

Length of output: 32954


🌐 Web query:

sketchy-documents codex create-trd foundational TRD document-id capabilities documentation

💡 Result:

The terms "sketchy-documents" and "create-trd" appear to originate from distinct domains, as there is no single authoritative system or standard linking them directly. 1. Sketchy (R package): The term "sketchy" refers to an R package designed to create custom research compendiums (standard folder structures for data analysis) [1][2][3]. It has no functional relationship to "codex" or "TRD" creation workflows [3]. 2. Codex and TRD Workflows: - TRD (Technical Requirements Document): A TRD is a standard software engineering document defining technical architecture, infrastructure, and requirements [4][5][6]. - create-trd: This is often a command-line tool or agent skill used in AI-assisted development environments (e.g., within Claude or Codex-based ecosystems) to bootstrap or scaffold documentation [7][8]. - Codex: Refers to AI-driven coding environments or specialized skills (like formal-docx-workflow or codex-guides) that help developers manage, document, and polish project artifacts [9][10][11][12]. 3. Foundational TRD Documentation: - Foundational TRDs are intended to define architecture patterns (the "how" and "where") before concrete implementation [8]. - Documentation standards typically require specific metadata (e.g., feature, gate, deployment.model) and unique, traceable Document IDs (e.g., #TRD-NNN) to ensure searchability and alignment across project phases [7][8][13]. - A Traceability Matrix is frequently used in these documents to link specific requirements to design decisions and test cases [13]. If you are encountering these terms in a specific development environment, they likely refer to a custom "skill" or plugin installed in your agent's configuration (such as a local ~/.codex/skills/ directory) designed to enforce a specific documentation standard for your organization [10][14][12].

Citations:


Make TRD refinement preserve foundational frontmatter and IDs.

create-trd generates kind: foundational, optional PRD references, and capabilities for foundational briefs, and both micro-UUID and TRD-YYYY-NNN IDs. The refine skill only reads legacy TRD-YYYY-NNN, still requires [satisfies REQ-NNN] coverage, and preserves only Document ID and Label. Update refinement to branch by kind, accept both ID formats, retain all frontmatter including capabilities, and skip PRD-specific REQ-NNN traceability checks for foundational TRDs.

🤖 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 `@packages/codex/.codex/skills/commands/ensemble-refine-trd/SKILL.md` at line
196, Update the ensemble-refine-trd workflow to branch on the document’s kind:
for foundational TRDs, preserve all frontmatter fields—including capabilities,
PRD references, Document ID, and Label—without regeneration, accept both
micro-UUID and TRD-YYYY-NNN identifiers, and skip the PRD-specific [satisfies
REQ-NNN] coverage requirement. Retain the existing traceability validation for
non-foundational TRDs.

Comment on lines +84 to +93
title: Capability Reuse Check
description: Reuse existing foundational work instead of duplicating it (dedup-by-reference)
actions:
- "Resolve TRD_GRAPH_CLI to first existing path among: ${CLAUDE_PLUGIN_ROOT}/lib/trd-graph-cli.js, packages/development/lib/trd-graph-cli.js. If missing, print error and HALT."
- "Run: node \"$TRD_GRAPH_CLI\" capabilities docs/TRD --json to list capabilities already provided by foundational TRDs; if docs/TRD does not exist yet, treat the registry as empty and continue"
- "For each technical capability this PRD needs, check the registry: EXPLICIT match = a listed capability token; otherwise IMPLICIT match by comparing needed work to foundational TRD labels/titles and target files (also: node \"$TRD_GRAPH_CLI\" overlap docs/TRD)"
- "If a foundational TRD already provides the capability: DO NOT emit duplicate task rows for it. Keep the Foreman Dependencies column local-only; record external reuse under a '## Reused Capabilities' section as capability -> foundational slug/document id/ref (<foundational-slug>#TRD-NNN or #PR-N)"
- "If a needed capability is clearly reusable but no foundational TRD exists, recommend extracting it via /ensemble:create-trd <prd> --foundational instead of embedding it here"
- "Reference foundational work by slug / document id only -- never by label"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether trd-graph.js parses a "## Reused Capabilities" section, or only inline [depends: slug#ID] annotations.
fd trd-graph.js packages/development/lib
rg -n "Reused Capabilities" packages/development/lib/trd-graph.js
rg -n "depends:" packages/development/lib/trd-graph.js

Repository: FortiumPartners/ensemble

Length of output: 282


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
fd -H 'trd-graph|trd-foreman|create-trd' packages/development -t f

echo
echo "== trd-graph outline =="
ast-grep outline packages/development/lib/trd-graph.js --view compact || true

echo
echo "== trd-graph relevant sections =="
wc -l packages/development/lib/trd-graph.js
sed -n '1,260p' packages/development/lib/trd-graph.js

echo
echo "== tests mentioning Graph depends / reused / Reused / deps =="
fd -H '.test.js$' packages/development/lib tests lib -x sh -c 'if rg -q "depend|reuse|Reused|graph|deps" "$1"; then echo "--- $1"; rg -n -C 3 "depend|reuse|Reused|graph|deps" "$1"; fi' sh {}

echo
echo "== command snippets =="
sed -n '1,180p' packages/development/commands/create-trd.yaml
sed -n '50,120p' packages/development/commands/create-trd-foreman.yaml

Repository: FortiumPartners/ensemble

Length of output: 26396


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== trd-graph remaining sections =="
sed -n '260,339p' packages/development/lib/trd-graph.js

echo
echo "== parser and cross-trd deps =="
fd -H 'trd-parser.js|cross-trd-deps.js|trd-parser.test.js|cross-trd-deps.test.js' packages/development -x sh -c 'echo "--- $1"; wc -l "$1"; rg -n -C 4 "parseTRD|dependsOn|deps|targetFiles|parseQualifiedRef|depends:" "$1"' sh {}

echo
echo "== trd-graph tests =="
sed -n '1,360p' packages/development/tests/trd-graph.test.js

echo
echo "== read-only extraction of relevant implementation facts =="
python3 - <<'PY'
from pathlib import Path
import re
for p in [Path('packages/development/lib/trd-graph.js'), Path('packages/development/lib/trd-parser.js'), Path('packages/development/lib/cross-trd-deps.js')]:
    txt = p.read_text()
    print(f'[{p}]')
    for needle in ["dependsOn", "targetFiles", "capabilities", "dependencies", "Reused Capabilities", "[depends:", "parseQualifiedRef", "parseTRD"]:
        print(f'  contains {needles!r}:', needle in txt)
PY

Repository: FortiumPartners/ensemble

Length of output: 26568


Encode Foreman reuse as dependency annotations if graph cycles matter.

packages/development/lib/trd-graph.js builds TRD graph edges from task dependsOn values, and packages/development/lib/trd-parser.js only derives dependsOn from [depends: ...] annotations / Dependencies: lines. A ## Reused Capabilities prose section does not create graph edges, so Foreman TRD references in it will not drive trd-graph-cli.js graph, cycle detection, or PR-boundary overlap results unless an annotation is added.

🤖 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 `@packages/development/commands/create-trd-foreman.yaml` around lines 84 - 93,
The Capability Reuse Check instructions currently record foundational reuse only
in prose, so graph tooling cannot see those relationships. Update the actions
under “Capability Reuse Check” to encode each reused foundational TRD reference
as a supported `[depends: ...]` annotation or `Dependencies:` entry, while
retaining the `## Reused Capabilities` details and local-only Foreman
Dependencies requirement.

Comment on lines +24 to +25
2. If --foundational and no full PRD exists: accept a short capability brief instead (the shared work to build), skip PRD-structure validation for this run, and build a capability registry from the brief's named capabilities / scope / target files instead of REQ-NNN IDs
3. If a full PRD is provided: validate document structure (required sections present), extract key requirements with REQ-NNN IDs, and build requirements registry for traceability tracking

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the capability registry throughout foundational runs.

Both workflows create a capability registry but later analyze only REQ-NNN requirements. Add foundational domain analysis from capabilities, scope, and target files.

  • packages/pi/prompts/ensemble-create-trd-foreman.md#L24-L25: add the foundational input to Phase 2 domain analysis.
  • packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md#L31-L32: apply the same branch in the Codex workflow.
📍 Affects 2 files
  • packages/pi/prompts/ensemble-create-trd-foreman.md#L24-L25 (this comment)
  • packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md#L31-L32
🤖 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 `@packages/pi/prompts/ensemble-create-trd-foreman.md` around lines 24 - 25,
Update Phase 2 domain analysis in
packages/pi/prompts/ensemble-create-trd-foreman.md at lines 24-25 and
packages/codex/.codex/skills/commands/ensemble-create-trd-foreman/SKILL.md at
lines 31-32 to branch on the input type: foundational runs must analyze named
capabilities, scope, and target files from the capability registry, while full
PRD runs continue analyzing REQ-NNN requirements. Preserve the existing
registries and ensure both workflows apply the same behavior.

Comment on lines +94 to +102
const key = `${keyword}::${normalizeStepText(text)}`;
if (seen.has(key)) continue;
seen.add(key);
out.push({
keyword,
text,
reqId: req.id,
method: methodNameFor(keyword, text),
hash: hashStep(keyword, text),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files matching reqnroll-gen.js:\n'
fd -a 'reqnroll-gen\.js$' .

file="$(fd 'reqnroll-gen\.js$' . | head -n1)"
printf '\nFile: %s\n' "$file"
wc -l "$file"
printf '\nRelevant lines 1-180:\n'
sed -n '1,180p' "$file" | cat -n

printf '\nSearch generated method names and normalize/hash usages:\n'
rg -n "methodNameFor|normalizeStepText|hashStep|method:" "$file" -C 3

printf '\nSearch reqnroll references/docs in repo:\n'
rg -n "Reqnroll|reqnroll|step definition|binding|keyword" README* docs packages -S --glob '!node_modules' | head -n 200

Repository: FortiumPartners/ensemble

Length of output: 36905


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd 'reqnroll-gen\.js$' . | head -n1)"

printf 'Behavioral probe for deduplication and method-name collisions:\n'
node - <<'JS' "$file"
const reqnroll = require(process.argv[2]);
const { methodNameFor, hashStep, normalizeStepText, collectSteps } = reqnroll;

const cases = [
  { keyword: 'Given', text: 'there is an account' },
  { keyword: 'Then', text: 'there is an account' },
  { keyword: 'Given', text: 'account-id' },
  { keyword: 'Given', text: 'account id' },
];

console.log(JSON.stringify(cases.map(s => ({
  ...s,
  normalized: normalizeStepText(s.text),
  method: methodNameFor(s.keyword, s.text),
  hash: hashStep(s.keyword, s.text)
})), null, 2));

const prd = {
  reqs: [{
    id: 'T-1',
    acs: [{
      given: 'there is an account',
      when: 'I click submit',
      then: 'there is an account',
      ands: [],
    }],
  }],
};
console.log('collectSteps output:', JSON.stringify(collectSteps(prd), null, 2));

const methods = new Set();
for (const s of cases) methods.add(methodNameFor(s.keyword, s.text));
console.log('distinct methods:', methods.size, 'vs inputs:', cases.length);
console.log('collisions:', methods.size < cases.length
  ? cases.filter(s => [...methods].filter(m => m === methodNameFor(s.keyword, s.text)).length > 1).map(s => ({s, collisions: [...cases].filter(c => methodNameFor(c.keyword, c.text) === methodNameFor(s.keyword, s.text)).map(c => c.text)}))
  : '-');
JS

printf '\nPackage and test file references for reqnroll-gen functions:\n'
rg -n "methodNameFor|hashStep|normalizeStepText|collectSteps|reqnroll-gen" packages test tests __tests__ test-files . --glob '!node_modules' | head -n 200

Repository: FortiumPartners/ensemble

Length of output: 7275


🌐 Web query:

Reqnroll step definitions Gherkin keywords does not use keyword in binding matching

💡 Result:

In Reqnroll, Gherkin keywords (such as Given, When, Then, And, But) are not taken into account when the framework attempts to match a step in your feature file to a step definition [1][2]. When Reqnroll executes a step, it uses only the text following the Gherkin keyword to find a matching step definition [1][2]. Because keywords are ignored during this matching process, you cannot define separate step definitions for a Given, When, Then, And, or But step if they share the exact same text [1][2]. For example, if you have two steps defined as "Given I log in" and "When I log in", Reqnroll treats the matching text as "I log in" for both, and they will resolve to the same step definition method [1]. Step definition attributes (such as [Given], [When], or [Then]) are used to register the binding, but they do not enforce that a step must be preceded by that specific keyword in the feature file [3][4]. Using the [StepDefinition] attribute can also allow a single method to match any of the standard step keywords [3]. If you need to distinguish behavior based on the keyword, you would need to implement that logic inside your step definition method or use a different approach, as the binding engine itself does not differentiate steps based on these keywords [1][2].

Citations:


Use one collision-safe identity for generated bindings.

Reqnroll ignores Gherkin keywords during matching, while the current C# method generator discards punctuation. A Given and Then with the same normalized text, or step texts such as account-id and account id, can produce duplicate method signatures in one generated class.

Reject duplicate normalized step text across all keywords, append s.hash to generated method names, and add regression tests for both cases.

🤖 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 `@packages/product/lib/reqnroll-gen.js` around lines 94 - 102, The generated
binding identity must ignore keywords and distinguish punctuation variants.
Update the deduplication in the generation flow around key, seen, and out to
reject duplicate normalizeStepText(text) values across all keywords; update
methodNameFor usage to append the corresponding step hash, preserving unique
generated C# signatures. Add regression tests covering same text with different
keywords and punctuation variants such as account-id versus account id.

Comment on lines +140 to +144
for (const s of steps) {
const attr = attributeFor(s.keyword);
lines.push('');
lines.push(` [${attr}(@"${s.text.replace(/"/g, '""')}")]`);
lines.push(` public void ${s.method}() => throw new PendingStepException();`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | sed -n '1,120p' | rg 'packages/product/lib/reqnroll-gen.js|test|spec|packages' || true

echo "== target file outline/size =="
wc -l packages/product/lib/reqnroll-gen.js
ast-grep outline packages/product/lib/reqnroll-gen.js || true

echo "== relevant source lines =="
cat -n packages/product/lib/reqnroll-gen.js | sed -n '1,230p'

echo "== scans for attribute/scanBinding tests/usages =="
rg -n "scanBindingAttributes|Reqnroll|Cucumber|expression|regex|attributeFor|Reqnroll|annotations|@" packages -S -C 3 || true

Repository: FortiumPartners/ensemble

Length of output: 50383


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate reqnroll files =="
git ls-files | rg 'reqnroll-gen\.js|reqnroll|Reqnroll|scanBinding|drift' || true

echo "== target file =="
if [ -f packages/product/lib/reqnroll-gen.js ]; then
  wc -l packages/product/lib/reqnroll-gen.js
  cat -n packages/product/lib/reqnroll-gen.js | sed -n '1,260p'
fi

echo "== focused rg =="
rg -n "scanBindingAttributes|attributeFor|Reqnroll|Cucumber|RegEx|regex|drift|drifts|drifts" packages -S || true

Repository: FortiumPartners/ensemble

Length of output: 50382


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== drift source lines =="
cat -n packages/product/lib/drift.js | sed -n '1,220p'

echo "== binding drift tests =="
cat -n packages/product/tests/binding-drift.test.js | sed -n '1,180p'

echo "== reqnroll gen tests relevant sections =="
cat -n packages/product/tests/reqnroll-gen.test.js | sed -n '1,180p'

echo "== deterministic probe for generated attributes and drift decoder =="
node - <<'JS'
const fs = require('fs');
const source = fs.readFileSync('packages/product/lib/reqnroll-gen.js', 'utf8');
const extract = (name) => {
  const start = source.indexOf(`function ${name}(`);
  if (start === -1) throw new Error(`missing ${name}`);
  const end = source.indexOf('\n}\n', start) + 3;
  let depth = 0, bodyEnd = -1;
  for (let i = source.indexOf('{', start); i < source.length; i++) {
    if (source.charAt(i) === '{') depth++;
    else if (source.charAt(i) === '}') {
      depth--;
      if (depth === 0) { bodyEnd = i + 1; break; }
    }
  }
  return `[${name}]=\n${source.substring(start, bodyEnd)}`;
};
for (const name of ['attributeFor','normalizeStepText','hashStep','methodNameFor','collectSteps','renderStepClass','scanBindingAttributes']) console.log(extract(name));

const cases = [
  'add 2 * 3',
  'contains "quoted"',
  '{}({})',
  'contains /regex/',
  'contains ${expr}',
  '^anchored',
  'contains <text>',
];
const prd = {
  documentId: 'TEST',
  version: '1',
  reqs: [{
    id: 'REQ-001',
    title: 'Probe',
    acs: [{ id: 1, given: 'contains 2 * 3', when: 'add {}({})', then: 'contains "quoted"' }],
  }],
};
const build = eval(`${source.replace(/exports\./g, '')}\nmodule.exports.buildBindingArtifacts;`);
const buildLib = { buildBindingArtifacts, renderFeature: () => '' };
Object.assign(global.module || require.main, { exports: buildLib }); // no-op guard
const out = eval(source.replace(/module\.exports\.|\nexports\./g, '\nconst ').replace(/\nexports\./g, '\nconst ').replace(/require\('\.\/feature-gen'\)/, '"{}"'));
console.log(JSON.stringify({ cases, attributeForCase: cases.map(c => (s) => eval(source.match(/function attributeFor[\s\S]*?^}/m)[0])(c) ), renderedSteps: (() => {
  const { steps } = collectSteps(prd);
  return steps.map(x => ({ original: x.text, generatedAttr: `[Given(@"${x.text.replace(/"/g, '""')}")]` }));
})(), driftMatches: (() => {
  const stepTexts = [
    ...new Set(prd.reqs.flatMap((r) => r.acs.flatMap((a) => [a.given, a.when, a.then].filter(Boolean)))),
  ];
  const cs = stepTexts.map((text) => `    [Given(@"${text.replace(/"/g, '""')}")]\n    public void Method() {}`);
  const bindings = scanBindingAttributes(cs.join('\n')).map(entry => entry[0]);
  return stepTexts.map((text) => `${text} <= bound: ${bindings.includes(text)}`);
})() });
JS

echo "== fetch Reqnroll docs snippets for anchor/regex expression =="
web_search "Reqnroll Cucumber expressions regular expressions anchored regex force regular expression"

Repository: FortiumPartners/ensemble

Length of output: 13874


🌐 Web query:

Reqnroll documentation regular expression anchors at start end expressions

💡 Result:

In Reqnroll, regular expression anchors (^ and $) play a critical role in the framework's automatic detection of step definition types [1][2]. Reqnroll supports both Cucumber Expressions and Regular Expressions for step definitions [1][3]. Because both types can be defined within the same project, Reqnroll uses a set of heuristics to determine which type an expression belongs to [1][2]. The presence of regular expression anchors at the start (^) or end ($) of an expression is the primary rule used to force that expression to be treated as a Regular Expression [1][2]. If an expression does not contain these anchors, Reqnroll evaluates other criteria—such as the presence of Cucumber Expression placeholders (e.g., {int}) or common regex patterns—to decide how to handle it [1][2]. If you are migrating legacy code or want to ensure a specific expression is always treated as a regex, the recommended practice is to explicitly include these ^ and $ anchors [4][3]. Alternatively, you can bypass the automatic detection and explicitly define the type by using the ExpressionType parameter in your step definition attribute [1][2]: [When("Your pattern", ExpressionType = ExpressionType.RegularExpression)] public void YourMethod {... } This provides complete control over how Reqnroll interprets your step definition, regardless of whether anchors are present [1][2].

Citations:


Escape generated step expressions as literals.

renderStepClass inserts PRD text directly into attribute strings. Text containing Reqnroll expression, regex, or interpolation syntax can bind incorrectly or match parameters not present on the generated zero-argument method. Generate fixed, regex-escaped anchors and update scanBindingAttributes + drift tests to compare decoded literal text.

🤖 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 `@packages/product/lib/reqnroll-gen.js` around lines 140 - 144, Update
renderStepClass so generated step attributes use fixed, regex-escaped anchored
literals rather than inserting s.text directly, while preserving quote escaping
for the generated source. Update scanBindingAttributes to decode and compare the
literal text, and adjust drift tests to validate the decoded literal behavior.

Comment on lines +285 to +291
for (const f of artifacts.files) {
const dest = path.join(outDir, f.relPath);
fs.mkdirSync(path.dirname(dest), { recursive: true });
// WRITE-ONCE: never clobber a (possibly filled) step binding unless --force.
if (isStepFile(f.relPath) && fs.existsSync(dest) && !opts.force) {
skipped.push(dest);
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Make write-once binding generation additive.

The generator skips an existing requirement step file. The documented regeneration workflow therefore cannot add a new binding without --force, which discards completed bodies.

  • packages/product/lib/reqnroll-gen.js#L285-L291: merge missing generated methods into existing step files while retaining existing bodies.
  • packages/pi/prompts/ensemble-generate-reqnroll-bindings.md#L13-L14: keep this promise only after additive merging is implemented.
  • packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md#L47-L48: direct users to regeneration only after it can add missing stubs safely.
📍 Affects 3 files
  • packages/product/lib/reqnroll-gen.js#L285-L291 (this comment)
  • packages/pi/prompts/ensemble-generate-reqnroll-bindings.md#L13-L14
  • packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md#L47-L48
🤖 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 `@packages/product/lib/reqnroll-gen.js` around lines 285 - 291, Update the
artifact-writing loop in packages/product/lib/reqnroll-gen.js:285-291 so
existing step files are merged with newly generated missing methods while
preserving all existing method bodies, rather than skipped unless --force;
retain overwrite behavior for non-step artifacts and use the existing step-file
detection/generation helpers. Update
packages/pi/prompts/ensemble-generate-reqnroll-bindings.md:13-14 and
packages/codex/.codex/skills/commands/ensemble-check-binding-drift/SKILL.md:47-48
to promise or recommend regeneration only once this additive, body-preserving
merge is available.

Comment on lines +101 to +119
function runTests(projectDir, opts = {}) {
const args = ['test', '--logger', 'trx;LogFileName=reqnroll.trx', '--results-directory', '.'];
if (opts.filter) args.push('--filter', opts.filter);
const trxPath = path.join(projectDir, 'reqnroll.trx');
try {
fs.rmSync(trxPath, { force: true });
} catch {
/* stale TRX cleanup is best-effort */
}


let exitCode = 0;
let raw = '';
try {
raw = execFileSync('dotnet', args, { cwd: projectDir, encoding: 'utf8', stdio: 'pipe' });
} catch (err) {
exitCode = typeof err.status === 'number' ? err.status : 1;
raw = `${err.stdout || ''}\n${err.stderr || ''}`;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add a timeout and larger buffer to the dotnet test call.

execFileSync at Line 115 has no timeout and no maxBuffer. If dotnet test stalls (for example, on a hung test or a blocked NuGet restore), this call blocks indefinitely because execFileSync is synchronous. A verbose run can also exceed the default 1 MB maxBuffer and throw instead of returning output. This function drives the automated red/green gate in the reqnroll-tdd workflow, so a hang here stalls the whole loop.

Add explicit timeout and maxBuffer options, and handle the timeout case explicitly in the catch block.

🛡️ Proposed fix to bound the external call
   try {
-    raw = execFileSync('dotnet', args, { cwd: projectDir, encoding: 'utf8', stdio: 'pipe' });
+    raw = execFileSync('dotnet', args, {
+      cwd: projectDir,
+      encoding: 'utf8',
+      stdio: 'pipe',
+      timeout: opts.timeoutMs || 5 * 60 * 1000,
+      maxBuffer: 20 * 1024 * 1024,
+    });
   } catch (err) {
     exitCode = typeof err.status === 'number' ? err.status : 1;
     raw = `${err.stdout || ''}\n${err.stderr || ''}`;
   }
🤖 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 `@packages/product/lib/reqnroll-run.js` around lines 101 - 119, Update the
execFileSync call in runTests to set an explicit timeout and a larger maxBuffer
suitable for verbose dotnet test output. In its catch block, detect the timeout
condition explicitly, assign the appropriate nonzero exitCode, and preserve
captured stdout/stderr handling for both timeout and other execution failures.

Comment on lines +244 to +266

const reqnrollSkillFile = path.join(PACKAGES_DIR, 'reqnroll', 'skills', 'reqnroll', 'SKILL.md');
if (fs.existsSync(reqnrollSkillFile)) {
const packageName = 'reqnroll';
const dest = path.join(OUTPUT_DIR, 'skills', packageName, 'SKILL.md');
const raw = readFile(reqnrollSkillFile);
const parsed = parseFrontmatter(raw);
const frontmatter = Object.keys(parsed.data).length > 0
? { ...parsed.data, 'user-invocable': parsed.data['user-invocable'] ?? true }
: {
name: packageName,
description: firstMeaningfulLine(parsed.content),
'user-invocable': true,
};

writeFile(dest, `${yamlFrontmatter(frontmatter)}\n${normalizeSkillBody(parsed.content)}\n`, dryRun, verbose);

const referencePath = path.join(PACKAGES_DIR, 'reqnroll', 'skills', 'reqnroll', 'REFERENCE.md');
if (fs.existsSync(referencePath)) {
writeFile(path.join(OUTPUT_DIR, 'skills', packageName, 'REFERENCE.md'), `${normalizeSkillBody(readFile(referencePath))}\n`, dryRun, verbose);
}
count += 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract shared logic for copying a skill file.

Lines 251-259 duplicate the frontmatter-default construction and writeFile call already implemented in the loop at Lines 232-240. A future change to the shared default-frontmatter logic requires updating both places to stay in sync.

Extract a helper, e.g. copySkillFile(skillFile, packageName, { dryRun, verbose }), and call it from both the loop and this special case.

♻️ Proposed refactor sketch
+function copySkillFile(skillFile, packageName, { dryRun, verbose }) {
+  const dest = path.join(OUTPUT_DIR, 'skills', packageName, 'SKILL.md');
+  const raw = readFile(skillFile);
+  const parsed = parseFrontmatter(raw);
+  const frontmatter = Object.keys(parsed.data).length > 0
+    ? { ...parsed.data, 'user-invocable': parsed.data['user-invocable'] ?? true }
+    : {
+        name: packageName,
+        description: firstMeaningfulLine(parsed.content),
+        'user-invocable': true,
+      };
+  writeFile(dest, `${yamlFrontmatter(frontmatter)}\n${normalizeSkillBody(parsed.content)}\n`, dryRun, verbose);
+}
+
 for (const skillFile of skillFiles) {
   const packageName = path.basename(path.dirname(path.dirname(skillFile)));
-  const dest = path.join(OUTPUT_DIR, 'skills', packageName, 'SKILL.md');
-  const raw = readFile(skillFile);
-  const parsed = parseFrontmatter(raw);
-  const frontmatter = Object.keys(parsed.data).length > 0
-    ? { ...parsed.data, 'user-invocable': parsed.data['user-invocable'] ?? true }
-    : {
-        name: packageName,
-        description: firstMeaningfulLine(parsed.content),
-        'user-invocable': true,
-      };
-  writeFile(dest, `${yamlFrontmatter(frontmatter)}\n${normalizeSkillBody(parsed.content)}\n`, dryRun, verbose);
+  copySkillFile(skillFile, packageName, { dryRun, verbose });
   count += 1;
 }

 const reqnrollSkillFile = path.join(PACKAGES_DIR, 'reqnroll', 'skills', 'reqnroll', 'SKILL.md');
 if (fs.existsSync(reqnrollSkillFile)) {
-  const packageName = 'reqnroll';
-  const dest = path.join(OUTPUT_DIR, 'skills', packageName, 'SKILL.md');
-  const raw = readFile(reqnrollSkillFile);
-  const parsed = parseFrontmatter(raw);
-  const frontmatter = Object.keys(parsed.data).length > 0
-    ? { ...parsed.data, 'user-invocable': parsed.data['user-invocable'] ?? true }
-    : {
-        name: packageName,
-        description: firstMeaningfulLine(parsed.content),
-        'user-invocable': true,
-      };
-  writeFile(dest, `${yamlFrontmatter(frontmatter)}\n${normalizeSkillBody(parsed.content)}\n`, dryRun, verbose);
+  copySkillFile(reqnrollSkillFile, 'reqnroll', { dryRun, verbose });
   const referencePath = path.join(PACKAGES_DIR, 'reqnroll', 'skills', 'reqnroll', 'REFERENCE.md');
   if (fs.existsSync(referencePath)) {
     writeFile(path.join(OUTPUT_DIR, 'skills', 'reqnroll', 'REFERENCE.md'), `${normalizeSkillBody(readFile(referencePath))}\n`, dryRun, verbose);
   }
   count += 1;
 }
🤖 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 `@scripts/generate-codex/index.js` around lines 244 - 266, Extract the shared
frontmatter construction and skill output logic from the existing
skill-processing loop and the special reqnroll block into a helper such as
copySkillFile. Have the helper accept the skill file, package name, and
dryRun/verbose options, then call it from both paths while preserving the
current defaults, normalization, and write behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant