fix: make startup and cross-layer retrieval answer-ready - #566
Merged
Merged
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds obligation-aware cross-layer retrieval, evidence-aware context-pack compaction, strict MCP context-pack controls, workspace-scoped Codex configuration, graph startup reuse, bounded readiness waits, and expanded regression and acceptance coverage. ChangesRetrieval and evidence
Strict MCP and native-agent contracts
Startup and Codex integration
Documentation and acceptance artifacts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Jul 17, 2026
Owner
Author
|
Pushed This PR now includes the activation fixes found in the live OpenStatus trial:
Verification completed:
The final automated Codex-CLI receipt remains blocked by a host issue in Codex CLI 0.144.5: it performs MCP initialize and tools/list, then cancels a tool call before it sends |
mohanagy
marked this pull request as ready for review
July 17, 2026 22:22
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
src/runtime/retrieve.ts (1)
6481-6523: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRetain every node supporting an emitted claim.
Only
claim.node_labels[0]is pinned. Other referenced nodes may be culled, their relationships filtered out, and the unsupported claim still emitted. Pin all claim labels atomically; if they cannot fit, omit that claim or adjust the cap.🤖 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 `@src/runtime/retrieve.ts` around lines 6481 - 6523, Update the claim/node selection logic around claims, pinnedNodes, and matchedNodeCap so every emitted claim retains all nodes referenced by claim.node_labels, not only the first label. Treat each claim’s referenced nodes atomically: if all cannot fit within the node cap, omit that claim (and avoid emitting it in claims) or otherwise adjust selection so no emitted claim is unsupported, while preserving relationship filtering for retained nodes.
🟡 Minor comments (7)
docs/integrations/compatibility.md-18-18 (1)
18-18: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the configured Codex home consistently.
These summaries hardcode
~/.codex/config.toml, while the installer supports$CODEX_HOME/config.toml. Document the environment-variable path with the default fallback at every verification surface.
docs/integrations/compatibility.md#L18-L18: update the Codex generated-files/config column.docs/reference/cli-and-mcp.md#L17-L17: update the agent-install matrix.docs/release.md#L49-L49: resolve the configured path during the smoke check.🤖 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 `@docs/integrations/compatibility.md` at line 18, Update the Codex config references to consistently resolve $CODEX_HOME/config.toml, falling back to ~/.codex/config.toml when unset. Apply this to the generated-files/config column in docs/integrations/compatibility.md:18-18 and the agent-install matrix in docs/reference/cli-and-mcp.md:17-17; update the smoke check in docs/release.md:49-49 to resolve the same configured path before validation.CHANGELOG.md-13-13 (1)
13-13: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the readiness contract across release documentation.
The documents currently describe both “wait and complete the same request” and “return a retryable response while reconciling.” Define the bounded-wait timeout behavior once and use it consistently.
CHANGELOG.md#L13-L13: state whether retry occurs only after the bounded wait expires.README.md#L76-L76: align the startup/readiness wording with that contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` at line 13, Align the readiness contract in CHANGELOG.md line 13 and README.md line 76: document that graph-backed requests wait during the bounded transient reconciliation window and return the same request once ready, with a retryable response occurring only if that wait expires. Update both documents’ startup/readiness wording consistently without changing the described rebuild or fail-closed behavior.docs/auto-refresh.md-39-40 (1)
39-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the 25-second guarantee with queue behavior.
Later queued requests can currently wait multiple 25-second windows before receiving a response. Keep this wording only after enforcing the deadline from request arrival.
🤖 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 `@docs/auto-refresh.md` around lines 39 - 40, Update the auto-refresh request queue and readiness-wait handling so the 25-second timeout is measured from each request’s arrival time, not restarted when a queued request begins processing. Ensure queued requests exceeding that deadline return the documented madar_graph_not_ready response with the existing retry metadata, then retain this documentation wording.tests/unit/retrieve-cross-layer-flow.test.ts-3-3 (1)
3-3: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove the wall-clock SLA out of the unit test.
The 750 ms assertion depends on runner load and can fail without a behavior regression. Measure latency in a benchmark suite instead.
Proposed fix
-import { performance } from 'node:perf_hooks' ... - const started = performance.now() const result = retrieveContext(buildCrossLayerMonitorFlowFixture(), { ... - const elapsedMs = performance.now() - started ... - expect(elapsedMs).toBeLessThan(750)Also applies to: 35-35, 41-41, 85-85
🤖 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 `@tests/unit/retrieve-cross-layer-flow.test.ts` at line 3, Remove the performance-based timing measurement and 750 ms SLA assertion from the unit test, including the node:perf_hooks import and related references in the cross-layer flow test. Keep the functional assertions intact, and move latency measurement to the project’s benchmark suite if one exists.tests/unit/retrieval-gate.test.ts-323-323 (1)
323-323: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the execution-owner branch specifically.
The regex accepts any “behavior slice” reason, so runtime-generation classification would also pass.
Proposed fix
- expect(decision.reason).toMatch(/runtime flow|behavior slice/i) + expect(decision.reason).toBe('runtime flow question — behavior slice retrieval')🤖 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 `@tests/unit/retrieval-gate.test.ts` at line 323, Strengthen the assertion for decision.reason in the execution-owner branch test so it matches the specific expected execution-owner reason rather than the broad “runtime flow|behavior slice” alternatives. Keep the assertion case-insensitive while ensuring runtime-generation classifications cannot satisfy it.tests/unit/install.test.ts-1250-1250 (1)
1250-1250: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope shared Codex-config assertions to the workspace under test.
The user-level config can contain unrelated managed blocks, so file existence, any strict profile string, or any
madar_<suffix>section can produce false positives.
tests/unit/install.test.ts#L1250-L1250: validate the canonical block forprojectDir.tests/unit/install-compatibility.test.ts#L450-L451: replace the existence-only check with workspace-specific content validation.tests/unit/install.test.ts#L1941-L1941: retain the content for workspace-specific validation.tests/unit/install.test.ts#L1950-L1954: replace the generic server regex withisMadarCodexMcpConfig(firstCodexConfig, projectDir).🤖 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 `@tests/unit/install.test.ts` at line 1250, Scope all Codex configuration assertions to the tested workspace: in tests/unit/install.test.ts lines 1250 and 1941, validate the canonical projectDir-specific block and retain content needed for that validation; in tests/unit/install-compatibility.test.ts lines 450-451, replace the existence-only assertion with workspace-specific content validation; and in tests/unit/install.test.ts lines 1950-1954, replace the generic server regex with isMadarCodexMcpConfig(firstCodexConfig, projectDir).tests/setup.ts-1-11 (1)
1-11: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReset the Codex home between test files.
tests/setup.tsreuses one PID-scopedCODEX_HOME, so anyconfig.tomlwritten by a suite can bleed into the next file or a watch rerun. Remove the directory before recreating it.Proposed fix
-import { mkdirSync } from 'node:fs' +import { mkdirSync, rmSync } from 'node:fs' ... const testCodexHome = join(tmpdir(), `madar-vitest-codex-home-${process.pid}`) +rmSync(testCodexHome, { recursive: true, force: true }) mkdirSync(testCodexHome, { recursive: true })🤖 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 `@tests/setup.ts` around lines 1 - 11, Update the test setup around testCodexHome to remove the existing PID-scoped directory before recreating it, then continue setting process.env.CODEX_HOME to that freshly initialized path. Preserve recursive directory creation so setup remains safe when the path does not exist.
🧹 Nitpick comments (1)
tests/unit/stdio-tool-profile.test.ts (1)
586-672: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise authorization through the real
context_packrequest path.This test calls the transformer directly and then manually seeds the same handle into session state. It cannot catch a regression where
context_packadvertises a handle but fails to register it. Extract the handle from an actual pack response and expand it using that same session.🤖 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 `@tests/unit/stdio-tool-profile.test.ts` around lines 586 - 672, The test should exercise authorization through the real context_pack request path instead of calling constrainStrictContextPackPayload and manually seeding contextPackHandles. Within the existing strict session, invoke context_pack, extract the advertised strict-verify-target handle from the response, then use that handle for context_expand and the repeated unauthorized expansion assertion; retain the checks that the response advertises the expected evidence class and focus file.
🤖 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 `@docs/benchmarks/2026-07-17-openstatus-activation/receipt.share-safe.json`:
- Around line 67-70: Regenerate the benchmark receipt from the canonical source
mappings rather than editing JSON manually: map triggerNotifications() to
apps/workflows/src/checker/utils.ts, computeEffectiveStatus() to
packages/api/src/router/external-service/effective-status.ts, and the incident
schema to packages/db/src/schema/incidents.ts. Then update
docs/benchmarks/2026-07-17-openstatus-activation/README.md lines 18-21 to
republish the 7/7 precision result only after the corrected receipt is
generated.
In `@src/infrastructure/compare.ts`:
- Around line 2810-2819: Update the strict profile guidance around the
context_pack instructions so a ready or ready_with_caveat pack is terminal only
for read-only tasks. For strict implementation tasks, explicitly permit the
required native repository edits and validation commands after context_pack
while continuing to forbid additional MCP calls. Apply the same policy
consistently at the related strict guidance occurrences.
- Around line 5112-5115: Update the prompt comparison logic around
traceContextPackPrompts so a null or otherwise unavailable traced prompt is
classified as not_measured rather than adding the byte-mismatch evidence. Only
emit the existing strict context_pack mismatch evidence when an actual traced
prompt is available and differs from report.question.
In `@src/infrastructure/context-pack-command.ts`:
- Around line 1808-1814: Update the reconciliation logic using existingTargets
so it reuses only verification targets relevant to the lost obligations, rather
than treating any existing target as sufficient. Ensure canVerify and the
resulting answerability state remain monotonic: preserve prior low,
insufficient, and explore_with_caution restrictions and never upgrade them to
medium or broaden verification requirements during reconciliation.
In `@src/infrastructure/install-skill-templates.ts`:
- Around line 328-337: Update the generated Codex strict profile instructions
near the existing Madar routing guidance to remove references to unavailable
tools such as retrieve and relevant_files. Replace that guidance with a strict
routing table limited to the installed context_pack and context_expand tools,
preserving the existing answerability rules and terminal behavior.
In `@src/infrastructure/install.ts`:
- Around line 2289-2330: Serialize Codex config mutations in
installCodexMcpServer and the corresponding uninstall flow by acquiring a lock
for configPath before reading, re-reading the file while locked, and performing
the complete managed-block update within that critical section. Replace direct
writeFileSync updates with an atomic temporary-file replacement that preserves
the existing config permissions, and ensure the lock and temporary resources are
cleaned up on success or failure.
In `@src/runtime/context-pack-recovery.ts`:
- Around line 195-203: Update the recovery gating logic around
conceptualObligationsCovered and recoveryAllowed so a recovered plan does not
disable evidence recovery solely when all conceptual obligations are covered.
Keep the existing answerability/readiness check authoritative, or limit
suppression to duplicate conceptual targets while still allowing missing
relationship and verification evidence recovery.
In `@src/runtime/context-pack.ts`:
- Around line 390-417: Update buildInputProvenanceClaims in
src/runtime/context-pack.ts (lines 390-417) so source_file-scoped snippets do
not attribute router-output consumption to node.label; emit file-level
provenance or require symbol-scoped evidence. Update the serialization logic in
src/infrastructure/context-pack-command.ts (lines 745-752) to preserve
snippet_line_number and snippet_scope, without substituting line_number as the
citation anchor.
In `@src/runtime/retrieve.ts`:
- Around line 4527-4530: Update the Gin inference expression near explicitGin so
the bare “go” token cannot activate Gin-specific filtering or boosts; retain
explicitGin and require an explicit “gin”, “golang”, or an unambiguous
Go-framework phrase before setting gin.
In `@src/runtime/retrieve/conceptual-fallback.ts`:
- Line 121: Update QUERY_EVIDENCE_DELIVERY_OPERATION_PATTERN to include notify*
and trigger* alongside the existing delivery verbs, covering both bare function
calls and member-method calls so normalized `@delivery` operations such as
notifyIncident and triggerIncident satisfy delivery evidence.
- Around line 777-785: Update runtimeScopeForSource so its repository-scope
match in runtimeScopeForSource recognizes paths beginning with apps/ or
packages/ as well as paths containing those segments after a slash. Preserve the
existing lowercase scope format and fallback behavior for paths that do not
match.
- Around line 1597-1617: Update the recovery telemetry around basePlan and
obligationRecoveryNeeded to use the total prompt-obligation count, including
obligations without anchors, rather than diversified.preferredByObligation.size.
Preserve the existing covered-count calculation, but ensure
query_obligations.total and the missing_query_obligations decision reflect every
prompt obligation.
In `@src/runtime/stdio-server.ts`:
- Around line 1231-1244: Bound each auto-refresh request’s readiness wait from
arrival time rather than dequeue time. In src/runtime/stdio-server.ts lines
1231-1244, capture and queue an arrival deadline for each request; in the
polling logic around lines 1154-1179, use only the remaining budget and report
queue time in waited_ms. Update docs/auto-refresh.md lines 39-40 only if needed
to retain the 25-second guarantee once enforced end-to-end.
In `@src/runtime/stdio/definitions.ts`:
- Around line 243-254: Update the shared context-pack tool definitions returned
by activeMcpTools('full') so their descriptions remain full-capable and do not
require prompt/task-only usage or forbid retrieve expansions. Move strict-only
wording and field restrictions into strictToolSchema(), including the related
definition at the additional location, while preserving the advertised budget,
freshness, delta-session, and verbose options in full mode.
In `@src/runtime/stdio/tools.ts`:
- Around line 903-946: Update the authorization construction around the returned
target so authorization.entry is a bounded clone using the same focus_files and
focus_ranges lists exposed by target, each limited to five items. Preserve the
existing entry metadata and avoid retaining the original unbounded follow_up
arrays, ensuring context_expand cannot access scope outside the listed
verification target.
In `@src/shared/discovery-safety.ts`:
- Line 127: Update ENVIRONMENT_CONFIG_INTENT_PATTERN to recognize
credential-related questions, including terms such as credential(s), cloud
credentials, and authentication keys, while preserving the existing .env,
configuration, deployment, environment, runtime-variable, secret, and settings
matches. Ensure the expanded pattern drives the same environment-file exclusion
and confidence handling used by existing matches.
---
Outside diff comments:
In `@src/runtime/retrieve.ts`:
- Around line 6481-6523: Update the claim/node selection logic around claims,
pinnedNodes, and matchedNodeCap so every emitted claim retains all nodes
referenced by claim.node_labels, not only the first label. Treat each claim’s
referenced nodes atomically: if all cannot fit within the node cap, omit that
claim (and avoid emitting it in claims) or otherwise adjust selection so no
emitted claim is unsupported, while preserving relationship filtering for
retained nodes.
---
Minor comments:
In `@CHANGELOG.md`:
- Line 13: Align the readiness contract in CHANGELOG.md line 13 and README.md
line 76: document that graph-backed requests wait during the bounded transient
reconciliation window and return the same request once ready, with a retryable
response occurring only if that wait expires. Update both documents’
startup/readiness wording consistently without changing the described rebuild or
fail-closed behavior.
In `@docs/auto-refresh.md`:
- Around line 39-40: Update the auto-refresh request queue and readiness-wait
handling so the 25-second timeout is measured from each request’s arrival time,
not restarted when a queued request begins processing. Ensure queued requests
exceeding that deadline return the documented madar_graph_not_ready response
with the existing retry metadata, then retain this documentation wording.
In `@docs/integrations/compatibility.md`:
- Line 18: Update the Codex config references to consistently resolve
$CODEX_HOME/config.toml, falling back to ~/.codex/config.toml when unset. Apply
this to the generated-files/config column in
docs/integrations/compatibility.md:18-18 and the agent-install matrix in
docs/reference/cli-and-mcp.md:17-17; update the smoke check in
docs/release.md:49-49 to resolve the same configured path before validation.
In `@tests/setup.ts`:
- Around line 1-11: Update the test setup around testCodexHome to remove the
existing PID-scoped directory before recreating it, then continue setting
process.env.CODEX_HOME to that freshly initialized path. Preserve recursive
directory creation so setup remains safe when the path does not exist.
In `@tests/unit/install.test.ts`:
- Line 1250: Scope all Codex configuration assertions to the tested workspace:
in tests/unit/install.test.ts lines 1250 and 1941, validate the canonical
projectDir-specific block and retain content needed for that validation; in
tests/unit/install-compatibility.test.ts lines 450-451, replace the
existence-only assertion with workspace-specific content validation; and in
tests/unit/install.test.ts lines 1950-1954, replace the generic server regex
with isMadarCodexMcpConfig(firstCodexConfig, projectDir).
In `@tests/unit/retrieval-gate.test.ts`:
- Line 323: Strengthen the assertion for decision.reason in the execution-owner
branch test so it matches the specific expected execution-owner reason rather
than the broad “runtime flow|behavior slice” alternatives. Keep the assertion
case-insensitive while ensuring runtime-generation classifications cannot
satisfy it.
In `@tests/unit/retrieve-cross-layer-flow.test.ts`:
- Line 3: Remove the performance-based timing measurement and 750 ms SLA
assertion from the unit test, including the node:perf_hooks import and related
references in the cross-layer flow test. Keep the functional assertions intact,
and move latency measurement to the project’s benchmark suite if one exists.
---
Nitpick comments:
In `@tests/unit/stdio-tool-profile.test.ts`:
- Around line 586-672: The test should exercise authorization through the real
context_pack request path instead of calling constrainStrictContextPackPayload
and manually seeding contextPackHandles. Within the existing strict session,
invoke context_pack, extract the advertised strict-verify-target handle from the
response, then use that handle for context_expand and the repeated unauthorized
expansion assertion; retain the checks that the response advertises the expected
evidence class and focus file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a7c75a04-a507-458d-91c7-57daa936578e
📒 Files selected for processing (59)
CHANGELOG.mdREADME.mddocs/auto-refresh.mddocs/benchmarks/2026-07-17-openstatus-activation/README.mddocs/benchmarks/2026-07-17-openstatus-activation/receipt.share-safe.jsondocs/integrations/compatibility.mddocs/reference/cli-and-mcp.mddocs/release.mddocs/security/mcp-threat-model.mddocs/tutorials/agent-quickstarts.mdsrc/contracts/context-pack.tssrc/contracts/retrieval-plan.tssrc/infrastructure/compare.tssrc/infrastructure/context-pack-command.tssrc/infrastructure/doctor.tssrc/infrastructure/indexing-manifest.tssrc/infrastructure/install-skill-templates.tssrc/infrastructure/install.tssrc/infrastructure/watch.tssrc/runtime/context-pack-recovery.tssrc/runtime/context-pack.tssrc/runtime/freshness.tssrc/runtime/mcp-response-evidence.tssrc/runtime/retrieval-gate.tssrc/runtime/retrieve.tssrc/runtime/retrieve/conceptual-fallback.tssrc/runtime/retrieve/expansion.tssrc/runtime/retrieve/pipeline.tssrc/runtime/retrieve/slicing.tssrc/runtime/stdio-server.tssrc/runtime/stdio/definitions.tssrc/runtime/stdio/tools.tssrc/shared/discovery-safety.tstests/fixtures/cross-layer-monitor-flow.tstests/setup.tstests/unit/background-auto-refresh.test.tstests/unit/compare-native-agent.test.tstests/unit/compare.test.tstests/unit/context-pack-adapter-gating.test.tstests/unit/context-pack-command.test.tstests/unit/context-pack-recovery.test.tstests/unit/context-pack.test.tstests/unit/doctor.test.tstests/unit/freshness-surfaces.test.tstests/unit/indexing-completeness.test.tstests/unit/install-compatibility.test.tstests/unit/install-docs.test.tstests/unit/install-templates.test.tstests/unit/install.test.tstests/unit/mcp-response-evidence.test.tstests/unit/mcp-schema-budget.test.tstests/unit/retrieval-gate.test.tstests/unit/retrieve-conceptual-fallback.test.tstests/unit/retrieve-cross-layer-flow.test.tstests/unit/retrieve.test.tstests/unit/stdio-server.test.tstests/unit/stdio-tool-profile.test.tstests/unit/watch.test.tsvitest.config.ts
mohanagy
force-pushed
the
agent/openstatus-activation
branch
from
July 17, 2026 23:33
d3a0755 to
41fe1bb
Compare
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)
src/infrastructure/compare.ts (1)
5131-5159: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the implementation exception to the later broad-exploration check.
Line 5131 permits required native implementation work, but Lines 5149-5160 still classify any resulting broad exploration as
not_measuredorviolated. Skip that generic check fortask === 'implement', while continuing to reject later MCP calls.Proposed fix
- if (report.madar_trace.broad_exploration_tool_call_count > 0) { + if ( + report.task !== 'implement' + && report.madar_trace.broad_exploration_tool_call_count > 0 + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infrastructure/compare.ts` around lines 5131 - 5159, Update the later broad-exploration check guarded by report.madar_trace.broad_exploration_tool_call_count to bypass its not_measured/violated classification when report.task === 'implement'. Preserve the existing generic behavior for other tasks and continue rejecting later MCP calls through the earlier validation.
🤖 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 `@tests/unit/retrieve.test.ts`:
- Around line 3248-3250: Update the claim-integrity assertions in the
compact-result test so they first verify the expected retained claim count or
claim identity is present, preventing an empty claims array from passing
vacuously. Keep the existing node-label anchor validation for each retained
claim after this assertion.
---
Outside diff comments:
In `@src/infrastructure/compare.ts`:
- Around line 5131-5159: Update the later broad-exploration check guarded by
report.madar_trace.broad_exploration_tool_call_count to bypass its
not_measured/violated classification when report.task === 'implement'. Preserve
the existing generic behavior for other tasks and continue rejecting later MCP
calls through the earlier validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 25bf4ca9-91c8-4b0f-8014-3199f562e695
📒 Files selected for processing (15)
src/infrastructure/compare.tssrc/infrastructure/context-pack-command.tssrc/infrastructure/install-skill-templates.tssrc/infrastructure/install.tssrc/runtime/context-pack.tssrc/runtime/retrieve.tssrc/runtime/retrieve/conceptual-fallback.tssrc/runtime/stdio-server.tssrc/runtime/stdio/definitions.tssrc/runtime/stdio/tools.tssrc/shared/discovery-safety.tstests/unit/install-templates.test.tstests/unit/install.test.tstests/unit/retrieve.test.tstests/unit/stdio-slice-surface.test.ts
💤 Files with no reviewable changes (1)
- src/infrastructure/install-skill-templates.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/unit/install-templates.test.ts
- src/shared/discovery-safety.ts
- src/runtime/stdio/definitions.ts
- src/runtime/stdio-server.ts
- src/infrastructure/install.ts
- src/infrastructure/context-pack-command.ts
- src/runtime/stdio/tools.ts
- src/runtime/retrieve.ts
- src/runtime/retrieve/conceptual-fallback.ts
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.
Summary
Addresses #564.
Addresses #565.
Verification
npm run typecheck --silentnpm run build --silentnpm run release:verifynpm run registry:validatenpm run verify:pack-paritygit diff --checkThe monolithic Vitest run exhausted fork startup handshakes on this workstation. Every file that failed to start was rerun in its own process and passed; there were no remaining assertion failures.
Packed OpenStatus acceptance
Tested from a clean detached OpenStatus worktree using a fresh install of the local
@lubab/madar@0.31.3tarball (sha256:ea71918c1e74fd7b73609f3054d2f90619f117d9d41bbb57cc167bfca525a645):context_packresponse: 3,893 msanswer_from_packnot_neededAcceptance boundary
This PR provides implementation acceptance, not a new public performance claim. Three consecutive Claude trials, three consecutive Codex trials, and human semantic comparison against the direct-search answer remain pending. Keep #565 open until those gates pass.
Summary by CodeRabbit
New Features
context_packandcontext_expand, with one-time verification controls.Bug Fixes
Documentation