fix(engram): resolve project before scoped search - #2022
Open
dnlrsls wants to merge 10 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSDD orchestrator assets now resolve the canonical Engram project before scoped memory operations. They handle unique, ambiguous, and missing projects. Routing, initialization, strict-TDD, and apply/verify guards skip project-scoped Engram access when no project is resolved. Tests cover coordinator contracts and exclusions. ChangesEngram project resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant Engram
participant User
Orchestrator->>Engram: Call engram_mem_current_project
Engram-->>Orchestrator: Return canonical project or alternatives
Orchestrator->>User: Request selection when ambiguous
Orchestrator->>Engram: Run scoped memory operation with canonical key
Orchestrator->>Engram: Check SDD state only when project and artifact store permit it
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md`:
- Around line 128-134: Make the no-project branch consistent with each SDD Init
Guard by defining one shared behavior that skips project-scoped Engram
initialization or stops with a clear user-facing explanation; never derive a
basename, use an empty key, or perform broad search. Apply this at
internal/assets/antigravity/sdd-orchestrator.md:128-134,
internal/assets/codex/sdd-orchestrator.md:212-218,
internal/assets/qwen/sdd-orchestrator.md:111-118, and
internal/assets/windsurf/sdd-orchestrator.md:112-119, while preserving the
existing project-scoped flow when a canonical project key is available.
In `@internal/assets/assets_test.go`:
- Around line 53-76: Extend
TestCoordinatorEngramProjectResolutionPrecedesScopedCalls to inspect each
asset’s content before the matched project-resolution contract and fail if that
prefix contains operational mem_context or mem_search invocations. Exclude the
contract text itself from this scan, while preserving the existing exact
contract-count and coordinator coverage assertions.
In `@internal/assets/cursor/sdd-orchestrator.md`:
- Around line 131-137: The SDD project-resolution rules conflict with the
mandatory project-scoped SDD Init Guard when no project is available. Update the
downstream SDD Init Guard in internal/assets/cursor/sdd-orchestrator.md:131-137,
the model-capable coordinator in
internal/assets/generic/sdd-orchestrator.md:113-119, and the Kiro coordinator in
internal/assets/kiro/sdd-orchestrator.md:181-187 to explicitly skip
project-scoped initialization and continue without Engram persistence until a
project is selected; preserve the existing unique and ambiguous project
behavior.
In `@internal/assets/generic/sdd-orchestrator.md`:
- Around line 272-278: Change the “Engram Project Resolution (MANDATORY)”
heading in the SDD orchestrator documentation from level three to level two so
it follows the document’s heading hierarchy and satisfies MD001; leave the
section content unchanged.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: be70dd6b-40d1-4a47-adee-47b16985046d
📒 Files selected for processing (14)
internal/assets/antigravity/sdd-orchestrator.mdinternal/assets/assets_test.gointernal/assets/claude/sdd-orchestrator-workflow.mdinternal/assets/claude/sdd-orchestrator.mdinternal/assets/codex/sdd-orchestrator.mdinternal/assets/cursor/sdd-orchestrator.mdinternal/assets/gemini/sdd-orchestrator.mdinternal/assets/generic/sdd-orchestrator.mdinternal/assets/hermes/sdd-orchestrator.mdinternal/assets/kimi/sdd-orchestrator.mdinternal/assets/kiro/sdd-orchestrator.mdinternal/assets/opencode/sdd-orchestrator.mdinternal/assets/qwen/sdd-orchestrator.mdinternal/assets/windsurf/sdd-orchestrator.md
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/assets/assets_test.go (1)
83-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake apply-asset exclusion independent of heading level.
The test rejects only
### Engram Project Resolution (MANDATORY). It will miss the same coordinator instructions if an apply asset uses##or no heading. Check for the contract text itself, or use a heading-independent marker.🤖 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 `@internal/assets/assets_test.go` around lines 83 - 91, Update the apply-asset validation loop to detect the coordinator project-resolution contract independently of Markdown heading level. In the checks around MustRead(path), match the distinctive instruction text or another heading-independent marker rather than only the exact “### Engram Project Resolution (MANDATORY)” heading.
♻️ Duplicate comments (1)
internal/assets/assets_test.go (1)
65-80: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate every coordinator contract occurrence.
strings.Indexalways selects the first contract. Forgeneric/sdd-orchestrator.md, the test expects two contracts but checks ordering only before the first one. A scoped call could appear before the second small-model contract and the test would still pass. Iterate over every contract occurrence and validate each coordinator section.🤖 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 `@internal/assets/assets_test.go` around lines 65 - 80, Update the ordering validation in the test loop around strings.Count and strings.Index to inspect every project-resolution contract occurrence, not only the first. Iterate through each contract position and check the preceding coordinator section for mem_context or mem_search invocations, while preserving the existing expected-count assertions and failure messages.
🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md`:
- Line 142: Align the Native SDD Dispatcher Guards so `sdd-init/{project}`
lookup occurs only after a canonical project and artifact store are established;
otherwise follow the explicit no-project path and continue without
project-scoped Engram persistence. Apply this behavior in
`internal/assets/antigravity/sdd-orchestrator.md` lines 142-142,
`internal/assets/codex/sdd-orchestrator.md` lines 226-226,
`internal/assets/cursor/sdd-orchestrator.md` lines 143-145,
`internal/assets/generic/sdd-orchestrator.md` lines 127-127,
`internal/assets/kiro/sdd-orchestrator.md` lines 193-195,
`internal/assets/qwen/sdd-orchestrator.md` lines 125-129, and
`internal/assets/windsurf/sdd-orchestrator.md` lines 124-126.
---
Outside diff comments:
In `@internal/assets/assets_test.go`:
- Around line 83-91: Update the apply-asset validation loop to detect the
coordinator project-resolution contract independently of Markdown heading level.
In the checks around MustRead(path), match the distinctive instruction text or
another heading-independent marker rather than only the exact “### Engram
Project Resolution (MANDATORY)” heading.
---
Duplicate comments:
In `@internal/assets/assets_test.go`:
- Around line 65-80: Update the ordering validation in the test loop around
strings.Count and strings.Index to inspect every project-resolution contract
occurrence, not only the first. Iterate through each contract position and check
the preceding coordinator section for mem_context or mem_search invocations,
while preserving the existing expected-count assertions and failure messages.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3417f3f-50f0-42cf-ae4d-4389512ca836
📒 Files selected for processing (8)
internal/assets/antigravity/sdd-orchestrator.mdinternal/assets/assets_test.gointernal/assets/codex/sdd-orchestrator.mdinternal/assets/cursor/sdd-orchestrator.mdinternal/assets/generic/sdd-orchestrator.mdinternal/assets/kiro/sdd-orchestrator.mdinternal/assets/qwen/sdd-orchestrator.mdinternal/assets/windsurf/sdd-orchestrator.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/assets/antigravity/sdd-orchestrator.md (1)
142-142: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle
noneas a separate artifact-store mode.These assets define
noneas inline-only. The dispatcher wording maps every non-OpenSpec choice to Engram, and theOtherwisebranch inSDD Init Guardstill searches Engram. When the documentednonemode is selected, the coordinator can fail or persist data through a backend the user did not select. Use explicit mode branches:engramandhybridmay use Engram only with a canonical project;openspecuses file artifacts;noneskips Engram persistence.
internal/assets/antigravity/sdd-orchestrator.md#L142-L142: make initialization conditional on an Engram-permitting store.internal/assets/cursor/sdd-orchestrator.md#L145-L145: skip Engram initialization fornone.internal/assets/generic/sdd-orchestrator.md#L127-L127: preserve inline-only behavior fornone.internal/assets/kiro/sdd-orchestrator.md#L195-L195: add an explicitnonebranch before Engram search.internal/assets/qwen/sdd-orchestrator.md#L125-L125: preventnonefrom entering the Engram init path.internal/assets/windsurf/sdd-orchestrator.md#L126-L126: preventnonefrom entering the Engram init path.internal/assets/assets_test.go#L60-L60: replace the shared gate string with explicit artifact-store behavior and test thenonepath.🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md` at line 142, Handle artifact-store mode explicitly so none remains inline-only and never initializes or searches Engram. In internal/assets/antigravity/sdd-orchestrator.md:142-142, internal/assets/cursor/sdd-orchestrator.md:145-145, internal/assets/generic/sdd-orchestrator.md:127-127, internal/assets/kiro/sdd-orchestrator.md:195-195, internal/assets/qwen/sdd-orchestrator.md:125-125, and internal/assets/windsurf/sdd-orchestrator.md:126-126, allow Engram only for engram or hybrid with a canonical project, use file artifacts for openspec, and add an explicit none path that skips persistence; update internal/assets/assets_test.go:60-60 to replace the shared gate assertion with explicit artifact-store behavior covering none.
🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md`:
- Line 138: Add a terminal no-project guard before every Engram status, TDD,
apply, verify, and persistence operation: in
internal/assets/antigravity/sdd-orchestrator.md:138,
codex/sdd-orchestrator.md:222, cursor/sdd-orchestrator.md:141,
generic/sdd-orchestrator.md:123, kiro/sdd-orchestrator.md:191,
qwen/sdd-orchestrator.md:121, and windsurf/sdd-orchestrator.md:122, prevent
projectless change-topic lookups; additionally gate the project-scoped
apply/verify searches at codex/sdd-orchestrator.md:330-337 and
kiro/sdd-orchestrator.md:300-309, and the TDD search at
windsurf/sdd-orchestrator.md:400-410. Extend
internal/assets/assets_test.go:82-99 to verify these later project-scoped calls
are also skipped without a canonical project.
In `@internal/assets/assets_test.go`:
- Line 101: Extend the dispatcher condition in the asset coverage test to
include claude/sdd-orchestrator-workflow.md alongside the existing provider
paths. Keep the existing seven asset checks unchanged so the lazy-loaded Claude
workflow is validated for its Native SDD Dispatcher Guard.
---
Outside diff comments:
In `@internal/assets/antigravity/sdd-orchestrator.md`:
- Line 142: Handle artifact-store mode explicitly so none remains inline-only
and never initializes or searches Engram. In
internal/assets/antigravity/sdd-orchestrator.md:142-142,
internal/assets/cursor/sdd-orchestrator.md:145-145,
internal/assets/generic/sdd-orchestrator.md:127-127,
internal/assets/kiro/sdd-orchestrator.md:195-195,
internal/assets/qwen/sdd-orchestrator.md:125-125, and
internal/assets/windsurf/sdd-orchestrator.md:126-126, allow Engram only for
engram or hybrid with a canonical project, use file artifacts for openspec, and
add an explicit none path that skips persistence; update
internal/assets/assets_test.go:60-60 to replace the shared gate assertion with
explicit artifact-store behavior covering none.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8f067688-1c1c-4c5d-9132-ecc649944072
⛔ Files ignored due to path filters (13)
testdata/golden/combined-claude-claudemd.goldenis excluded by!testdata/**testdata/golden/combined-windsurf-global-rules.goldenis excluded by!testdata/**testdata/golden/sdd-antigravity-rulesmd.goldenis excluded by!testdata/**testdata/golden/sdd-claude-claudemd.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-lowcost.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-powerful.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd.goldenis excluded by!testdata/**testdata/golden/sdd-cursor-rules.goldenis excluded by!testdata/**testdata/golden/sdd-gemini-geminimd.goldenis excluded by!testdata/**testdata/golden/sdd-kiro-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-opencode-multi-settings.goldenis excluded by!testdata/**testdata/golden/sdd-vscode-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-windsurf-global-rules.goldenis excluded by!testdata/**
📒 Files selected for processing (8)
internal/assets/antigravity/sdd-orchestrator.mdinternal/assets/assets_test.gointernal/assets/codex/sdd-orchestrator.mdinternal/assets/cursor/sdd-orchestrator.mdinternal/assets/generic/sdd-orchestrator.mdinternal/assets/kiro/sdd-orchestrator.mdinternal/assets/qwen/sdd-orchestrator.mdinternal/assets/windsurf/sdd-orchestrator.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/assets/hermes/sdd-orchestrator.md (1)
141-148: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGuard all later Engram lookups when no canonical project exists.
Line 147 says to continue without an invented project and without broad search. However, the following
Native SDD Dispatcher Guardstill instructs the coordinator to checksdd-init/{project}when the artifact store is not established. TheSDD Init Guardalso searches that key without an explicit no-project branch.Add the no-project skip before both lookups. Only use
sdd-init/{project}afterengram_mem_current_projectreturns a canonical project.🤖 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 `@internal/assets/hermes/sdd-orchestrator.md` around lines 141 - 148, Add an explicit no-project branch to both the Native SDD Dispatcher Guard and SDD Init Guard, skipping their artifact-store and sdd-init/{project} lookups when no canonical project is returned. Ensure sdd-init/{project} is queried only after engram_mem_current_project provides a canonical project, while preserving the existing unique and ambiguous project behavior.internal/assets/codex/sdd-orchestrator.md (1)
146-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the Markdown lint warning on the launch example.
markdownlint-cli2reports MD031 and MD040 at Line 147. Add blank lines around the fenced block and specify a language such astext.🤖 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 `@internal/assets/codex/sdd-orchestrator.md` around lines 146 - 155, Update the sdd-design launch example in the surrounding Markdown by adding blank lines immediately before and after its fenced code block, and specify an explicit fence language such as text to satisfy MD031 and MD040.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.
Outside diff comments:
In `@internal/assets/codex/sdd-orchestrator.md`:
- Around line 146-155: Update the sdd-design launch example in the surrounding
Markdown by adding blank lines immediately before and after its fenced code
block, and specify an explicit fence language such as text to satisfy MD031 and
MD040.
In `@internal/assets/hermes/sdd-orchestrator.md`:
- Around line 141-148: Add an explicit no-project branch to both the Native SDD
Dispatcher Guard and SDD Init Guard, skipping their artifact-store and
sdd-init/{project} lookups when no canonical project is returned. Ensure
sdd-init/{project} is queried only after engram_mem_current_project provides a
canonical project, while preserving the existing unique and ambiguous project
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4dd5e352-b264-4383-9ea3-f91e8e185a65
⛔ Files ignored due to path filters (13)
testdata/golden/combined-claude-claudemd.goldenis excluded by!testdata/**testdata/golden/combined-windsurf-global-rules.goldenis excluded by!testdata/**testdata/golden/sdd-antigravity-rulesmd.goldenis excluded by!testdata/**testdata/golden/sdd-claude-claudemd.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-lowcost.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-powerful.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd.goldenis excluded by!testdata/**testdata/golden/sdd-cursor-rules.goldenis excluded by!testdata/**testdata/golden/sdd-gemini-geminimd.goldenis excluded by!testdata/**testdata/golden/sdd-kiro-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-opencode-multi-settings.goldenis excluded by!testdata/**testdata/golden/sdd-vscode-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-windsurf-global-rules.goldenis excluded by!testdata/**
📒 Files selected for processing (13)
internal/assets/antigravity/sdd-orchestrator.mdinternal/assets/claude/sdd-orchestrator-workflow.mdinternal/assets/claude/sdd-orchestrator.mdinternal/assets/codex/sdd-orchestrator.mdinternal/assets/cursor/sdd-orchestrator.mdinternal/assets/gemini/sdd-orchestrator.mdinternal/assets/generic/sdd-orchestrator.mdinternal/assets/hermes/sdd-orchestrator.mdinternal/assets/kimi/sdd-orchestrator.mdinternal/assets/kiro/sdd-orchestrator.mdinternal/assets/opencode/sdd-orchestrator.mdinternal/assets/qwen/sdd-orchestrator.mdinternal/assets/windsurf/sdd-orchestrator.md
dnlrsls
added
the
size:exception
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/assets/antigravity/sdd-orchestrator.md (2)
151-153: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winApply the artifact-store gate to every Engram operation.
The dispatcher requires both a canonical project and an artifact store that permits Engram. The initialization, strict-TDD, and apply-progress rules check only the project. An OpenSpec-only session can still perform project-scoped Engram searches.
internal/assets/antigravity/sdd-orchestrator.md#L151-L153: gatesdd-initlookup byengramorhybrid.internal/assets/antigravity/sdd-orchestrator.md#L356-L356: gate strict-TDD capability lookup byengramorhybrid.internal/assets/codex/sdd-orchestrator.md#L251-L253: gate initialization lookup by artifact store.internal/assets/codex/sdd-orchestrator.md#L357-L358: gate apply/verify context searches by artifact store.internal/assets/cursor/sdd-orchestrator.md#L154-L156: gate initialization lookup by artifact store.internal/assets/cursor/sdd-orchestrator.md#L369-L370: gate strict-TDD lookup by artifact store.internal/assets/generic/sdd-orchestrator.md#L136-L138: gate initialization lookup by artifact store.internal/assets/generic/sdd-orchestrator.md#L387-L388: gate strict-TDD lookup by artifact store.internal/assets/kiro/sdd-orchestrator.md#L204-L206: gate initialization lookup by artifact store.internal/assets/kiro/sdd-orchestrator.md#L313-L314: gate apply/verify context searches by artifact store.internal/assets/qwen/sdd-orchestrator.md#L130-L136: gate initialization and dispatcher-related Engram access by artifact store.internal/assets/qwen/sdd-orchestrator.md#L332-L333: gate strict-TDD lookup by artifact store.internal/assets/windsurf/sdd-orchestrator.md#L135-L137: gate initialization lookup by artifact store.internal/assets/windsurf/sdd-orchestrator.md#L415-L416: gate strict-TDD lookup by artifact store.internal/assets/claude/sdd-orchestrator-workflow.md#L113-L113: gate initialization lookup by artifact store.internal/assets/claude/sdd-orchestrator-workflow.md#L287-L287: gate strict-TDD lookup by artifact store.internal/assets/claude/sdd-orchestrator-workflow.md#L289-L291: gate apply-progress lookup by artifact store.🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md` around lines 151 - 153, Apply the artifact-store gate to every Engram operation: require an Engram-enabled store (engram or hybrid) in the SDD Init Guard, strict-TDD, and apply/verify lookups. Update internal/assets/antigravity/sdd-orchestrator.md lines 151-153 and 356; internal/assets/codex/sdd-orchestrator.md lines 251-253 and 357-358; internal/assets/cursor/sdd-orchestrator.md lines 154-156 and 369-370; internal/assets/generic/sdd-orchestrator.md lines 136-138 and 387-388; internal/assets/kiro/sdd-orchestrator.md lines 204-206 and 313-314; internal/assets/qwen/sdd-orchestrator.md lines 130-136 and 332-333; internal/assets/windsurf/sdd-orchestrator.md lines 135-137 and 415-416; and internal/assets/claude/sdd-orchestrator-workflow.md lines 113, 287, and 289-291. Ensure OpenSpec-only sessions skip project-scoped Engram initialization, searches, and persistence.
149-149: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the
noneartifact-store contract.These assets define
noneas inline-only, but their dispatcher text treats every state without OpenSpec as Engram-backed. A selectednonestore can therefore trigger Engram initialization or persistence.
internal/assets/antigravity/sdd-orchestrator.md#L149-L149: handlenoneseparately fromengram.internal/assets/cursor/sdd-orchestrator.md#L152-L152: handlenoneseparately fromengram.internal/assets/generic/sdd-orchestrator.md#L134-L134: handlenoneseparately fromengram.internal/assets/kiro/sdd-orchestrator.md#L202-L202: handlenoneseparately fromengram.internal/assets/windsurf/sdd-orchestrator.md#L133-L133: handlenoneseparately fromengram.🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md` at line 149, Update the artifact-store routing around the native dispatcher in internal/assets/antigravity/sdd-orchestrator.md:149-149, internal/assets/cursor/sdd-orchestrator.md:152-152, internal/assets/generic/sdd-orchestrator.md:134-134, internal/assets/kiro/sdd-orchestrator.md:202-202, and internal/assets/windsurf/sdd-orchestrator.md:133-133 to handle `none` separately from `engram`. For `none`, preserve inline-only behavior: do not initialize or persist through Engram and do not perform project-scoped Engram lookups; retain the existing Engram routing only for `engram`.internal/assets/generic/sdd-orchestrator.md (1)
283-290: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep the small-model section behind the full SDD gates.
The
model-smallsection defines SDD execution modes and Engram access rules, but it does not include theNative SDD Dispatcher GuardorSDD Init Guard. If this section can be rendered independently, small model coordinators can bypass artifact-store routing and initialization. Add compact guards here or ensure the renderer always injects them.🤖 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 `@internal/assets/generic/sdd-orchestrator.md` around lines 283 - 290, The model-small section must enforce the same SDD execution gates as the full SDD flow. Add compact Native SDD Dispatcher Guard and SDD Init Guard instructions within the model-small section, or update its rendering path to always inject both guards before execution and Engram access.
🤖 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 `@internal/assets/antigravity/sdd-orchestrator.md`:
- Line 145: Unscoped Engram fallback is not consistently gated by explicit
cross-project recall. In internal/assets/antigravity/sdd-orchestrator.md:145,
internal/assets/cursor/sdd-orchestrator.md:148,
internal/assets/generic/sdd-orchestrator.md:130,
internal/assets/qwen/sdd-orchestrator.md:128,
internal/assets/windsurf/sdd-orchestrator.md:129, and
internal/assets/claude/sdd-orchestrator-workflow.md:36, update the no-project
contract so the referenced later delegation/non-SDD rules only permit unscoped
Engram searches or persistence during explicit cross-project recall; preserve
valid OpenSpec/file behavior and prohibit invented keys or broad searches
otherwise.
In `@internal/assets/assets_test.go`:
- Around line 67-90: The test duplicates coordinator paths in dispatcherPaths
and paths, allowing their coverage to diverge. Update the test around
dispatcherPaths and paths to use a single path slice and derive the dispatcher
lookup set from it, or explicitly assert both collections contain exactly the
same entries while preserving the existing eight-path coverage check.
- Around line 127-132: The scoped Engram validation loop must check
noProjectTerminalPolicy within the current coordinator section rather than
across the entire content prefix. Update the section-scanning logic around
scopedEngramInvocation and firstContractOffset to track each model section and
require its own local project-resolution contract before accepting an
invocation, including independent contract occurrences in the generic
coordinator.
---
Outside diff comments:
In `@internal/assets/antigravity/sdd-orchestrator.md`:
- Around line 151-153: Apply the artifact-store gate to every Engram operation:
require an Engram-enabled store (engram or hybrid) in the SDD Init Guard,
strict-TDD, and apply/verify lookups. Update
internal/assets/antigravity/sdd-orchestrator.md lines 151-153 and 356;
internal/assets/codex/sdd-orchestrator.md lines 251-253 and 357-358;
internal/assets/cursor/sdd-orchestrator.md lines 154-156 and 369-370;
internal/assets/generic/sdd-orchestrator.md lines 136-138 and 387-388;
internal/assets/kiro/sdd-orchestrator.md lines 204-206 and 313-314;
internal/assets/qwen/sdd-orchestrator.md lines 130-136 and 332-333;
internal/assets/windsurf/sdd-orchestrator.md lines 135-137 and 415-416; and
internal/assets/claude/sdd-orchestrator-workflow.md lines 113, 287, and 289-291.
Ensure OpenSpec-only sessions skip project-scoped Engram initialization,
searches, and persistence.
- Line 149: Update the artifact-store routing around the native dispatcher in
internal/assets/antigravity/sdd-orchestrator.md:149-149,
internal/assets/cursor/sdd-orchestrator.md:152-152,
internal/assets/generic/sdd-orchestrator.md:134-134,
internal/assets/kiro/sdd-orchestrator.md:202-202, and
internal/assets/windsurf/sdd-orchestrator.md:133-133 to handle `none` separately
from `engram`. For `none`, preserve inline-only behavior: do not initialize or
persist through Engram and do not perform project-scoped Engram lookups; retain
the existing Engram routing only for `engram`.
In `@internal/assets/generic/sdd-orchestrator.md`:
- Around line 283-290: The model-small section must enforce the same SDD
execution gates as the full SDD flow. Add compact Native SDD Dispatcher Guard
and SDD Init Guard instructions within the model-small section, or update its
rendering path to always inject both guards before execution and Engram access.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ccaad0f-e90a-400a-907f-70a42a5d68ea
⛔ Files ignored due to path filters (9)
testdata/golden/combined-windsurf-global-rules.goldenis excluded by!testdata/**testdata/golden/sdd-antigravity-rulesmd.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-lowcost.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd-powerful.goldenis excluded by!testdata/**testdata/golden/sdd-codex-agentsmd.goldenis excluded by!testdata/**testdata/golden/sdd-cursor-rules.goldenis excluded by!testdata/**testdata/golden/sdd-kiro-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-vscode-instructions.goldenis excluded by!testdata/**testdata/golden/sdd-windsurf-global-rules.goldenis excluded by!testdata/**
📒 Files selected for processing (9)
internal/assets/antigravity/sdd-orchestrator.mdinternal/assets/assets_test.gointernal/assets/claude/sdd-orchestrator-workflow.mdinternal/assets/codex/sdd-orchestrator.mdinternal/assets/cursor/sdd-orchestrator.mdinternal/assets/generic/sdd-orchestrator.mdinternal/assets/kiro/sdd-orchestrator.mdinternal/assets/qwen/sdd-orchestrator.mdinternal/assets/windsurf/sdd-orchestrator.md
Contributor
|
One provider-parity blocker remains on the current |
Contributor
Author
|
Implemented in Gemini, Kimi, OpenCode, and the OpenCode The body now reports the live 646-line size and existing maintainer-approved |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked Issue
Closes #1903
🏷️ PR Type
type:bug- Bug fixtype:feature- New featuretype:docs- Documentation onlytype:refactor- Code refactoringtype:chore- Build, CI, or toolingtype:breaking-change- Breaking change📝 Summary
📂 Changes
internal/assets/*/sdd-orchestrator.mdand Claude workflowinternal/assets/opencode/commands/{sdd-continue,sdd-status}.mdinternal/assets/assets_test.gointernal/components/golden_test.gosdd-continueandsdd-statuscommand assets.testdata/golden/*🧪 Test Plan
gofmtcompleted for the changed Go files.internal/assetspackage passed.internal/componentspackage passed.git diff --checkpassed.🤖 Automated Checks
size:exceptionappliedCloses #1903links the approved issue.status:approvedstatus:approved.type:*Labeltype:bug.internal/assetsandinternal/componentschecks passed; the full repository suite was not run locally.gofmtcompleted for the changed Go files.✅ Contributor Checklist
status:approvedsize:exceptiondocumented abovetype:*label is applied:type:buggo test ./...) were not run locallyCo-Authored-Bytrailers💬 Notes for Reviewers
Receipt-driven development is globally disabled, so delivery is ordinary disabled/unmanaged.
Live PR size: 32 files, 596 additions, 50 deletions, 646 changed lines.
Maintainer-approved
size:exceptionrationale: Provider parity spans all 12 orchestrator surfaces and generated installation snapshots are necessary regression evidence; splitting this correction would separate the shared contract from its parity/golden proof.Please focus on the canonical no-project behavior, the Gemini/Kimi/OpenCode provider parity, and the separately injected OpenCode command assets.
Summary by CodeRabbit
New Features
Tests