fix: inject resolved rule contents into the run-stage directive (#495) - #654
fix: inject resolved rule contents into the run-stage directive (#495)#654wowzoo wants to merge 1 commit into
Conversation
…abs#495) `buildRunStageDirective` shipped `rules_in_context` as paths only, and nothing in the engine forced the conductor to read them. Per-stage steering (the org / team / project / phase memory layers the compile resolved for that stage) could therefore be skipped silently: the paths were listed, the files existed with substantive content, and the artifacts were written without them. Observed on Kiro IDE across two builds and reported independently on 2.5.6. The engine already had the pattern this needs, twelve lines below the defect: `conductor_persona` is delivered by reading `aidlc-common/conductor.md` at directive-build time and baking its contents into the directive, so no skill references it by path. This applies the same treatment to the rule layers. - `aidlc-directive.ts`: new `RuleContent` type and an optional `rules_content?: RuleContent[]` field on `run-stage`, registered in RUN_STAGE_FIELDS and validated by `checkOptionalRulesContent` (shape discipline mirrors `checkOptionalConsumesAbsent`). - `aidlc-orchestrate.ts`: `readRuleContent` reads each resolved rule path best-effort (absent / unreadable / placeholder-only yields no entry), gated on `codekbCtx` so the ctx-less emit path stays byte-identical, mirroring `splitConsumesByPresence`'s own no-ctx skip. - `ruleTextIsSubstantive` drops files whose every line is blank, a heading, a `>` blockquote, or an HTML-comment fence — the exact shape of the shipped `team.md` / `project.md` seeds, so an untouched install gains no directive noise. They appear as soon as they carry a real rule. `rules_in_context` is unchanged and remains the authoritative roster; `rules_content` is an additive subset over the files that carried content. No skill or protocol prose changes: a live Kiro IDE 1.0.212 run confirmed the conductor does not re-read a path whose content the directive already carried — in one `read_files` call it read `team.md` and `project.md` (the two dropped as placeholders) and omitted `org.md` and `phases/ideation.md` (the two injected), so the read boundary matched `rules_content` membership exactly. New t248 covers it at the CLI boundary (7 tests): content present and verbatim, subset-of-roster, placeholder exclusion, a substantive team.md appearing, an emptied org.md dropping out, the phase rule tracking the stage's phase, and the no-memory-tree case leaving the field absent with routing unaffected. t248 is registered in the coverage registry's none->cli spawner list. Verified: bun run check green (parity 5 harnesses + typecheck + lint); bun tests/run-tests.ts --smoke --unit 178 files / 0 failed / 4395 assertions; fixture emit shows org.md (5439 chars) + phases/ideation.md (1158) injected with the two placeholder seeds dropped. The integration tier's t66 and t89 fail on base 257b43a as well (2.5.11 added the claim-sources sensor without refreshing the designer-export golden fixture or the sensor-import fixture dir); both are outside the PR gate's tiers and unrelated to this change.
|
Thanks, Keehyun. The diagnosis and initial implementation here were correct: required rule content must be delivered by deterministic tooling rather than left to discretionary path reads. I ran the focused test from this exact head; all 7 submitted cases passed. Since this opened, #658 merged that same core direction into current The separate persona and supplemental-knowledge retrieval gap you called out is not lost; it is tracked in #694. Issue #495 itself is now resolved by the merged 2.5.33 implementation, although it remains open because changes merged to Please do not spend time rebasing or reworking this branch. I recommend closing #654 as incorporated by #658 and manually closing #495 as resolved. Thank you for the live evidence and initial patch that established the direction. |
Fixes #495.
buildRunStageDirectiveshipsrules_in_contextas paths only, and nothing inthe engine forces the conductor to read them. Per-stage steering could therefore
be skipped silently — the paths listed, the files present with substantive
content, the artifacts written without them.
The engine already has the pattern this needs, twelve lines below the defect:
conductor_personais delivered by readingaidlc-common/conductor.mdatdirective-build time and baking in its contents, so no skill references it by
path. This applies the same treatment to the rule layers.
The change
aidlc-directive.ts— newRuleContenttype; optionalrules_content?: RuleContent[]onrun-stage, registered inRUN_STAGE_FIELDS, validated bycheckOptionalRulesContent(shape disciplinemirrors
checkOptionalConsumesAbsent).aidlc-orchestrate.ts—readRuleContentreads each resolved pathbest-effort; absent, unreadable, or placeholder-only yields no entry. Gated on
codekbCtxso the ctx-less emit path stays byte-identical, mirroringsplitConsumesByPresence's own no-ctx skip.ruleTextIsSubstantivedrops files whose every line is blank, a heading, a>blockquote, or an HTML-comment fence — the exact shape of the shippedteam.md/project.mdseeds, so an untouched install gains no directivenoise. They appear as soon as they carry a real rule.
rules_in_contextis unchanged and remains the authoritative roster;rules_contentis an additive subset over the files that carried content.Limitation — this reads the
defaultspace only.readRuleContentresolveseach entry of
node.rules_in_context, and those paths are baked bymemoryDisplayPath()whenever the graph is compiled — pinned toMEMORY_SPACE = "default", as is therulesDir()the roster is walked from. Nothingon this path consults the active-space cursor, so with a cursor on
teamBtheinjected content is still
aidlc/spaces/default/memory/. That pin is the shippedCOMPILE/DISPLAY-family behaviour documented in
aidlc-graph.tsand this PR doesnot change it — but it is worth stating plainly, because injection makes the
default content deterministic rather than merely listed, and the conductor
demonstrably stops reading paths it has already been given (see the run below).
Making rule resolution cursor-aware is a separate, harness-wide question.
No skill or protocol prose changes — and a live run explains why
I first rewrote the conductor's "read every file in
rules_in_context" clause inall five harness
SKILL.mdfiles, then measured whether it was needed. It isnot, so I reverted it and this PR touches no prose.
A live Kiro IDE 1.0.212 run, instrumented with a temporary
PreToolUseprobe onread_file|read_files(the framework's own hooks cannot see reads — theirmatchers are write/shell only), produced this in a single
read_filescall atintent-capture:rules_contentorg.md(5439 chars)phases/ideation.md(1158)team.md(placeholder)project.md(placeholder)The read boundary matched
rules_contentmembership exactly: the conductorskipped precisely the two files whose contents it had already received, and read
the two it had not — in the same call as the stage file and stage protocol, so
this was not laziness. Injection adds no duplicate read, and the existing prose
needs no change.
Tests
New
t248-directive-rules-content.test.ts, CLI-boundary (the builder has noexports), 7 tests: content present and verbatim, subset-of-roster, placeholder
exclusion, a substantive
team.mdappearing, an emptiedorg.mddropping out,the phase rule tracking the stage's phase, and the no-memory-tree case leaving
the field absent with routing unaffected. Registered in the coverage registry's
none->clispawner list (the honesty ratchet requires the explicit edit).Verification
bun run check— green (parity 5 harnesses, typecheck, lint 540 files)bun tests/run-tests.ts --smoke --unit— 178 files, 0 failed, 4395 assertionsbun scripts/ci-changelog-guard.ts <base>— OK, 137 preserved, 1 neworg.md+phases/ideation.mdinjected, both placeholder seedsdropped; adding a rule to
team.mdmakes it appear immediatelyThe integration tier's
t66andt89also fail on base257b43a3(2.5.11 addedthe
claim-sourcessensor without refreshing the designer-export golden fixtureor the sensor-import fixture dir). Both are outside the PR gate's tiers and
unrelated to this change; happy to file that separately.
Version bumped to 2.5.14 (2.5.12 is #615, 2.5.13 is #653) with a matching
CHANGELOG entry and README badge.
docs/roadmap.mddrops #495 from "Known gaps".Scope note: the two edited files are shared core — byte-identical in all five
dist/trees — so the behaviour lands on every harness, not just one. Worthnaming the asymmetry: the harnesses with a native include (Kiro CLI's agent
resourcesglob, Claude's@-import stub, Codex'sAIDLC_RULES_DIR,opencode's
instructions) already carry the memory tree in ambient context, sofor them this is a per-stage determinism guarantee over a roster the glob does
not distinguish rather than newly available content. On Kiro IDE, which has no
such native include, the injected content is the deterministic path. In both
cases the determinism is over the
defaultspace, per the limitation above.