feat: chore(prompts): formalize cross-prompt contract registry for develop + research machines - #324
Merged
Merged
Conversation
canesin
force-pushed
the
feat/chore-prompts-formalize-cross_GH_322
branch
from
April 11, 2026 15:31
520a05e to
d4d2c76
Compare
* chore(wip): checkpoint [skip ci] * fix: use login shell in runShellSync fallback for full user PATH Change bash -c to bash -lc in the runShellSync fallback path so that user profile additions (go, cargo, etc.) are available when the MCP server runs from a non-login context (systemd, VS Code, Claude Code). Closes #306 * fix: pre-review cleanup — correct cmd field to match -lc flag
Adds src/machines/prompt-contracts.js as the single source of truth for artifact section names shared between producer and consumer prompts across develop and research machines. Rebased onto origin/dev, integrating Pike's rules prose from PR #321: - Adds "Data Structure Review" to PLANREVIEW.md contract sections. - Extends the contract with optional sectionDescriptions so producer prompts preserve their rich per-section prose while consumers still pull just the section-name list via renderCritiqueSectionList. - Adds findingFieldExamples + renderFindingExample() for the REVIEW_FINDINGS.md example block in quality-review. Producers (plan-review, planning, issue-draft, quality-review, and the research machines) now source section names from the contract so any future drift fails the test suite instead of silently breaking downstream verdict parsers.
- REVIEW_FINDINGS.md contract: add renderReviewFindingsTemplate() that sources the full block (title, finding heading, verdict line) from the registry so the quality-review producer template no longer hardcodes headings that could drift from findingFields. - issue-draft, planning: replace fragile secs[N] positional indexing with renderSectionsWithDescriptions(), a name-keyed helper that throws on missing or unknown section keys so reordering fails loudly instead of silently misaligning prose to the wrong heading. - renderSpecArchitectExample: throw on unknown mode instead of silently dropping required top-level sections. - Add tests covering the new helpers and error paths.
- renderIssueBacklogExample: iterate named sections instead of
sections[0..2] positional indexing; throw if the 'issues' slot is
removed so silent key reordering cannot move issue objects into the
wrong top-level field.
- renderCritiqueSectionList: join the final item with 'and' instead of
'or' — consumers must address every critique section, not any one
of them. The previous 'or' wording weakened the implementation
prompt's reach over the critique.
- Tests:
- new producer-import check asserts each registered producer file
imports from prompt-contracts.js, catching regressions to
hardcoded section prose
- renderIssueBacklogExample test now asserts the issue array lives
at json.issues (by name) rather than json[sections[0]]
- renderCritiqueSectionList test now asserts ' and ' conjunction
and the absence of ' or '
Dismissed: (2) renderSpecArchitectExample hardcoded 'issueSpecs' key
is already protected by the existing "all build/ingest sections are
present in JSON" test; (4) renderIdeaIssueMarkdown flat-marker test
scope is out of this PR's drift-prevention goal.
- Tighten renderSpecArchitectExample tests to exact top-level key equality for both build and ingest modes. Previously the tests only asserted required keys were present; a stale key left in modeExamples after a section was removed from modes.*.sections would slip through silently. Now any drift between the two sources of truth fails the test. - Add a prompt-consumer import check: every consumer listed in a contract's consumers[] must import from prompt-contracts.js, with an inline allowlist for the two research runtime consumers (issue-publish, spec-render) that only destructure JS fields from the artifact and never embed section names in their own prompts. The rationale is documented in the test body so the allowlist cannot silently expand. Dismissed (repeat): renderIdeaIssueMarkdown nested-field assertions remain out of this PR's section-name-drift scope — that function predates the contract work and its test strategy is a separate concern.
- implementation.machine.js: rewrite Step 1 prose so the Question
section name isn't duplicated outside the render helper. The
renamed "open question" / "open item" language no longer hardcodes
any section label, so PLANREVIEW.md's Questions section can be
renamed without updating this consumer.
- prompt-contracts.js: buildIssueFieldsExample() now throws when any
declared issueField lacks an entry in issueFieldExamples instead
of silently falling back to "string" — schema drift for array and
object-typed fields will fail at load time.
- research/issue-backlog and research/spec-architect contracts: add
explicit examples for every declared issueField. This uncovered
and fixed real gaps (title, objective, problem, verification,
estimated_effort, notes, domain were missing).
- Tests:
- new test asserts every issueField has an explicit example
- producer import test now also requires the literal contract key
(or a dedicated helper for the artifact) to appear in source,
catching producers that import the registry but then re-hardcode
their section names
- consumer import test relabels RUNTIME_CONSUMERS as
PASSIVE_CONSUMERS and documents the rationale more clearly
Dismissed (repeat): renderSectionsWithDescriptions requires a
description map by design — producers own dynamic per-section prose
while the registry owns canonical section names, and the helper's
loud-failure behavior is the intended drift guard.
- renderRequiredSections, renderCritiqueSectionList, getSections now guard against contract entries without a flat 'sections' array (REVIEW_FINDINGS.md, research/spec-architect.json) and throw descriptive errors instead of a cryptic .map/undefined TypeError. - parseReviewVerdict tests now assert that every declared finding field survives round-tripping through the parser, not just that result.findings is truthy. assert.ok(findings) passed for any non-empty string, which is too weak a guarantee. - Consumer test restructure. The previous "all consumers must import the registry" test could pass when quality-review imported for its producer role while leaving PLAN.md/ISSUE.md references untouched. Replaced it with an ACTIVE_CONSUMER_PAIRS allowlist that documents the one (file, contract) pair that actually embeds contract section names in prompts — implementation.machine.js consuming PLANREVIEW.md — and requires the literal key on that single pair. All other consumer-contract pairs are passive file readers and don't need the stricter check. Dismissed (4th repeat): renderIdeaIssueMarkdown nested-field checks remain out of this PR's section-name-drift scope.
- prompt-contracts.test.js: resolve contract producer/consumer file paths against the repo root (derived from import.meta.url) instead of process.cwd(). Tests now pass when invoked from any working directory, not just the repo root. Dismissed: the systemd-run.js `bash -c` → `bash -lc` change and its accompanying test are from PR #313 (already merged to main), not part of this PR's cross-prompt-contract scope. Any regression concerns about login-shell side effects belong in a separate PR against that change, not this one.
canesin
force-pushed
the
feat/chore-prompts-formalize-cross_GH_322
branch
from
April 12, 2026 03:13
d4d2c76 to
78f8f13
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Metadata
2. Problem
The develop and research workflow machines rely on hand-edited string literals for markdown artifact sections (e.g., PLANREVIEW.md, PLAN.md, REVIEW_FINDINGS.md). Because the producer's section list and the consumer's section list are maintained independently across different files, they can drift out of sync. For example, PR #321 found that 'Data Structure Review' was added to src/machines/develop/plan-review.machine.js (producer) but missing from src/machines/develop/implementation.machine.js (consumer).
Furthermore, hard parsers like parsePlanVerdict (in src/machines/develop/plan-review.machine.js) and parseReviewVerdict (in src/machines/develop/quality-review.machine.js) key off specific headings, and silently break or downgrade to fallback logic if the section names in the prompts drift from what the regex expects, without any failing tests to warn developers.
Closes #322