Resolve priority-next runtime boundary batch#664
Merged
Conversation
Closes #535.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds new CLI commands, MCP protocol handling, query planning and diff surfaces, sync paging and auth-scheme support, strategy-based materialization and strand flows, merge-inspection models, and compatibility/validation boundaries. It also updates docs, exports, tests, packaging, and release checks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Graph
participant Planner
participant Materializer
participant Sync
participant MCP
User->>CLI: run command
CLI->>Graph: open runtime/graph
Graph->>Planner: build plan or request surface
Planner->>Materializer: materialize / diff / traverse
CLI->>Sync: sync or serve request
CLI->>MCP: stdio JSON-RPC message
MCP->>Graph: read tools/resources
Graph-->>MCP: structured result
sequenceDiagram
participant Client
participant SyncHTTP
participant SyncAuth
participant SyncCore
participant Policy
Client->>SyncHTTP: request with auth + page
SyncHTTP->>SyncAuth: sign / verify scheme
SyncAuth->>Policy: apply retry/timeout policy
Policy->>SyncCore: execute sync request
SyncCore-->>SyncHTTP: page + metrics response
SyncHTTP-->>Client: JSON payload
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@codex Code Lawyer discovery for PR #664
|
Member
Author
|
@codex Code Lawyer Activity Summary for PR #664
Local verification after fixes:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (9)
src/domain/services/controllers/MaterializeLiveStrategy.ts (2)
21-21: 💤 Low valueSimplify the null/undefined check.
Since
loadCheckpoint()returnsCheckpointData | null, the explicit!== undefinedcheck is redundant. You can simplify this to:if (checkpoint !== null && isCurrentCheckpointSchema(checkpoint.schema)) {or use:
if (checkpoint != null && isCurrentCheckpointSchema(checkpoint.schema)) {🤖 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/domain/services/controllers/MaterializeLiveStrategy.ts` at line 21, The null/undefined check in the condition at line 21 is redundant because loadCheckpoint() returns CheckpointData | null, not CheckpointData | undefined. Remove the explicit !== undefined check from the if statement condition that validates the checkpoint object before calling isCurrentCheckpointSchema(checkpoint.schema). You can either keep checkpoint !== null alone or use the loose inequality checkpoint != null, both of which will correctly handle null values and allow the schema check to proceed safely.
47-50: Remove the redundanttypeofcheck;streamPatchesSinceis guaranteed to exist on allPatchCollectorinstances.The
patchesfield is typed asPatchCollector, which definesstreamPatchesSinceas a concrete async generator method (not abstract). Thetypeofcheck at line 47 is therefore unnecessary—it will never fail at runtime. The fallback code (lines 51–53) duplicates the internal logic ofPatchCollector.streamPatchesSince, introducing dead code duplication. Either call the method directly or remove the check entirely.🤖 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/domain/services/controllers/MaterializeLiveStrategy.ts` around lines 47 - 50, The `typeof` check for `streamPatchesSince` in the MaterializeLiveStrategy file is redundant because `streamPatchesSince` is a concrete method defined on the `PatchCollector` interface and will always exist at runtime. Remove the unnecessary conditional check and the fallback code block that duplicates the internal logic, and instead call `this.runtime.deps.patches.streamPatchesSince(checkpoint)` directly followed by the return statement. This eliminates the dead code and makes the intention clearer.test/unit/domain/crdt/CrdtShimRetirement.test.ts (1)
33-36: ⚡ Quick winConsider checking for alternative export syntaxes.
The test only checks for
export function ${shim}andexport const ${shim}patterns. Alternative export syntaxes likeexport { createVersionVector }orexport { createVersionVector as vvCreate }would not be caught.🔍 Enhanced pattern matching
Consider adding checks for re-export patterns:
for (const shim of FORBIDDEN_VERSION_VECTOR_SHIMS) { expect(source).not.toContain(`export function ${shim}`); expect(source).not.toContain(`export const ${shim}`); + // Also catch re-exports: export { createVersionVector } + expect(source).not.toMatch(new RegExp(`export\\s*{[^}]*\\b${shim}\\b[^}]*}`)); }🤖 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 `@test/unit/domain/crdt/CrdtShimRetirement.test.ts` around lines 33 - 36, The test loop checking for forbidden shims only validates `export function` and `export const` patterns but misses alternative export syntaxes. Add additional expect statements in the loop iterating over FORBIDDEN_VERSION_VECTOR_SHIMS to also check for re-export patterns including `export { ${shim} }` and `export { ${shim} as` to catch cases where shims might be re-exported using destructuring syntax or with aliases.src/domain/services/query/GraphTraversal.ts (1)
140-140: ⚡ Quick winExtract abort-check cadence into a named constant.
The
1000literal appears twice in traversal loops. Pull it into a shared constant to avoid magic numbers and keep cadence changes atomic.♻️ Proposed refactor
+const ABORT_CHECK_INTERVAL = 1000; + private async *_bfsStream(run: PrimitiveTraversalRun): AsyncGenerator<string> { @@ - if (run.visited.size % 1000 === 0) { checkAborted(run.signal, 'bfs'); } + if (run.visited.size % ABORT_CHECK_INTERVAL === 0) { checkAborted(run.signal, 'bfs'); } @@ private async *_dfsStream(run: PrimitiveTraversalRun): AsyncGenerator<string> { @@ - if (run.visited.size % 1000 === 0) { checkAborted(run.signal, 'dfs'); } + if (run.visited.size % ABORT_CHECK_INTERVAL === 0) { checkAborted(run.signal, 'dfs'); }As per coding guidelines, use named constants instead of magic strings or numbers in TypeScript code.
Also applies to: 185-185
🤖 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/domain/services/query/GraphTraversal.ts` at line 140, The magic number 1000 for the abort-check cadence appears multiple times in the traversal logic (in the condition checking run.visited.size % 1000 === 0 at the location shown and another similar occurrence). Extract this value into a named constant at the top of the GraphTraversal class or file (e.g., something like ABORT_CHECK_CADENCE or TRAVERSAL_ABORT_CHECK_INTERVAL) and replace all occurrences of the literal 1000 with this constant reference. This eliminates magic numbers and ensures that any future changes to the cadence only need to be updated in one place.Source: Coding guidelines
src/domain/warp/RuntimeHostBoot.ts (1)
196-197: ⚡ Quick winReplace magic defaults with named constants in
minimal().Line 196 and Line 197 hardcode string defaults. Please lift them into named constants for consistency with
src/**/*.{ts,tsx}standards.Suggested refactor
+const DEFAULT_OPEN_GRAPH_NAME = 'default'; +const DEFAULT_OPEN_WRITER_ID = 'local'; + static minimal(options: { persistence: CorePersistence & Partial<RuntimeStorageCapabilityPort>; graphName?: string; writerId?: string; }): WarpOpenOptions { return new WarpOpenOptions({ persistence: options.persistence, - graphName: options.graphName ?? 'default', - writerId: options.writerId ?? 'local', + graphName: options.graphName ?? DEFAULT_OPEN_GRAPH_NAME, + writerId: options.writerId ?? DEFAULT_OPEN_WRITER_ID, }); }As per coding guidelines,
src/**/*.{ts,tsx}requires named constants instead of magic strings or numbers.🤖 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/domain/warp/RuntimeHostBoot.ts` around lines 196 - 197, The `minimal()` function contains hardcoded string defaults 'default' for graphName and 'local' for writerId on lines 196-197. Replace these magic strings by defining named constants at the top of the file or in an appropriate constants section, then use those constant references instead of the literal strings in the graphName and writerId property assignments.Source: Coding guidelines
src/domain/services/coordinate/SubstrateCoordinateBoundary.ts (1)
48-66: 💤 Low valueRemove the unnecessary
includesNamehelper.The
includesNamefunction is a trivial wrapper aroundArray.includes()that adds no value. Inline the calls directly to reduce indirection.♻️ Proposed refactor
-function includesName(names: readonly string[], name: string): boolean { - return names.includes(name); -} - export default class SubstrateCoordinateBoundary { readonly laneKinds = SUBSTRATE_LANE_KINDS; readonly coordinateKinds = SUBSTRATE_COORDINATE_KINDS; readonly substrateCapabilities = SUBSTRATE_CAPABILITY_NAMES; readonly sessionPolicyCapabilities = SESSION_POLICY_CAPABILITY_NAMES; authorityFor(capabilityName: string): SubstrateCapabilityAuthority | null { - if (includesName(SUBSTRATE_CAPABILITY_NAMES, capabilityName)) { + if (SUBSTRATE_CAPABILITY_NAMES.includes(capabilityName)) { return 'substrate'; } - if (includesName(SESSION_POLICY_CAPABILITY_NAMES, capabilityName)) { + if (SESSION_POLICY_CAPABILITY_NAMES.includes(capabilityName)) { return 'session-policy'; } return null; } }🤖 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/domain/services/coordinate/SubstrateCoordinateBoundary.ts` around lines 48 - 66, Remove the trivial includesName helper function that wraps Array.includes() and replace its two usages in the authorityFor method with direct calls to the includes method. Change includesName(SUBSTRATE_CAPABILITY_NAMES, capabilityName) to SUBSTRATE_CAPABILITY_NAMES.includes(capabilityName) and includesName(SESSION_POLICY_CAPABILITY_NAMES, capabilityName) to SESSION_POLICY_CAPABILITY_NAMES.includes(capabilityName) to eliminate unnecessary indirection.test/unit/domain/services/query/BoundedSupportRule.test.ts (1)
16-37: ⚡ Quick winSimplify empty generator functions.
The
nodes()andneighbors()generators create empty arrays and then iterate over them, which is unnecessarily verbose. Since the arrays are empty, the loops never execute.♻️ Proposed simplification
class NoopQueryReadModelProvider implements QueryReadModelProvider { async openQueryReadModel(_request?: QueryReadModelOpenRequest): Promise<QueryReadModel> { return { stateHash: 'unused', async *nodes() { - const nodes: QueryNodeSnapshot[] = []; - for (const node of nodes) { - yield node; - } + // Empty generator - no nodes to yield }, async *neighbors() { - const neighbors: QueryNeighborEntry[] = []; - for (const neighbor of neighbors) { - yield neighbor; - } + // Empty generator - no neighbors to yield }, async nodeProps() { return null; }, }; } }🤖 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 `@test/unit/domain/services/query/BoundedSupportRule.test.ts` around lines 16 - 37, The `nodes()` and `neighbors()` generator methods in the NoopQueryReadModelProvider class are creating empty arrays and then iterating over them with for-loops that never execute. Simplify both of these generator methods by removing the unnecessary empty array declarations and for-loop constructs, leaving just the generator function definitions with no yield statements since they should return empty generators.src/domain/services/controllers/MaterializeCoordinateStrategy.ts (1)
66-68: ⚡ Quick winReplace magic number with named constant.
The literal
0at line 67 is a magic number. Extract it to a named constant to improve readability and maintainability.♻️ Suggested refactor
+const MIN_INCLUSIVE_CEILING = 1; + export default class MaterializeCoordinateStrategy { private readonly runtime: MaterializeStrategyRuntime; constructor(runtime: MaterializeStrategyRuntime) { this.runtime = runtime; } // ... other methods ... private ceilingExcludesAll(ceiling: number | null): boolean { - return ceiling !== null && ceiling <= 0; + return ceiling !== null && ceiling < MIN_INCLUSIVE_CEILING; }As per coding guidelines: "Use named constants instead of magic strings or numbers in TypeScript code" for files in
src/**/*.{ts,tsx}.🤖 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/domain/services/controllers/MaterializeCoordinateStrategy.ts` around lines 66 - 68, The ceilingExcludesAll method contains a magic number (0) that should be extracted to a named constant for improved readability and maintainability. Define a private or class-level constant (such as MINIMUM_CEILING_THRESHOLD or ZERO_BOUNDARY) at the top of the MaterializeCoordinateStrategy class and replace the literal 0 in the ceilingExcludesAll method's comparison (ceiling <= 0) with this named constant reference.Source: Coding guidelines
src/domain/services/merge/MergeClassifier.ts (1)
10-32: ⚡ Quick winReplace inline reason-code literals with named constants.
These branches hardcode reason strings in multiple places, which makes code drift easier as the taxonomy evolves. Please centralize the reason codes into a typed constant map and reuse it in all return paths.
♻️ Suggested refactor
+const MERGE_REASON = Object.freeze({ + SHARED_PRECURSOR: 'shared-precursor', + MISSING_SHARED_PRECURSOR: 'missing-shared-precursor', + OVERLAPPING_FOOTPRINTS: 'overlapping-footprints', + DISJOINT_FOOTPRINTS: 'disjoint-footprints', + CANDIDATE_JOIN: 'candidate-join', + LOWERING_WITNESS: 'lowering-witness', + OBSTRUCTION_WITNESS: 'obstruction-witness', + NO_CANDIDATE_JOIN: 'no-candidate-join', + POLICY_REQUIREMENT: 'policy-requirement', +} as const); + function precursorReason(evidence: MergeClassificationEvidence): string { - return evidence.sharedPrecursor ? 'shared-precursor' : 'missing-shared-precursor'; + return evidence.sharedPrecursor ? MERGE_REASON.SHARED_PRECURSOR : MERGE_REASON.MISSING_SHARED_PRECURSOR; }As per coding guidelines,
src/**/*.{ts,tsx}requires: “Use named constants instead of magic strings or numbers in TypeScript code”.Also applies to: 38-39
🤖 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/domain/services/merge/MergeClassifier.ts` around lines 10 - 32, The functions precursorReason, footprintReason, and classifyWithoutPolicy contain hardcoded reason code strings such as 'shared-precursor', 'missing-shared-precursor', 'overlapping-footprints', 'disjoint-footprints', 'candidate-join', 'lowering-witness', 'obstruction-witness', 'projection', 'semantic', and 'no-candidate-join' scattered throughout the code. Create a named constants object at the top of the file that maps descriptive constant names to these reason code values, then replace all hardcoded string literals in precursorReason, footprintReason, and classifyWithoutPolicy with references to these constants to ensure consistency and prevent code drift as the taxonomy evolves.Source: Coding guidelines
🤖 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 `@bin/cli/commands/mcp/McpProtocol.ts`:
- Around line 190-191: The isRequestId function currently accepts all numeric
types including NaN and Infinity when checking typeof value === 'number', but
these are not valid JSON-RPC IDs and can break request/response correlation.
Modify the numeric validation check in isRequestId to use Number.isFinite() to
ensure only finite numbers are accepted as valid request IDs, while keeping the
existing checks for undefined, null, and regular strings intact.
In `@bin/cli/commands/serve.ts`:
- Line 23: The `.nonnegative()` method in Zod 3.24.1 requires the error message
to be passed as an object property, not as a direct string argument. In the port
field definition, change the `.nonnegative()` call from accepting a string
parameter directly to wrapping the message in an object with a message property
(e.g., { message: 'port must be a non-negative integer' }). This will align the
code with the correct Zod API syntax and the pattern used consistently
throughout the codebase.
In `@src/domain/services/OpStrategyRegistryValidation.ts`:
- Around line 29-33: The code in the registry validation loop assumes each
strategy value is a valid object, but malformed entries (null or undefined) will
cause a raw TypeError when accessing strategy[methodName] instead of throwing a
PatchError, breaking the validator's error contract. Add guard checks before
accessing strategy properties in both requireStrategyMethod and
requireReceiptName functions to validate that strategy is a valid object, and
throw a PatchError with a descriptive message if the strategy is null,
undefined, or not an object, ensuring consistent error handling throughout the
validator.
In `@src/domain/services/query/CausalIndexPlan.ts`:
- Around line 109-117: The validation helpers requireNonEmptyString and
freezeStringList are duplicated across CausalIndexPlan.ts and
SupportFragmentPlan.ts with the same implementations. Create a new shared
utility module at src/domain/services/query/queryValidation.ts and move both
requireNonEmptyString and freezeStringList implementations there, ensuring they
support the trim-and-return pattern. Update both CausalIndexPlan.ts and
SupportFragmentPlan.ts to import these helpers from the new shared utility
module instead of maintaining local copies, passing the errorCode parameter as
needed (E_QUERY_CAUSAL_INDEX_PLAN for CausalIndexPlan and the appropriate code
for SupportFragmentPlan).
In `@src/domain/services/strand/StrandCoordinator.ts`:
- Around line 315-317: The materialize() method now delegates to
materializeReadState() which implements live/pinned auto-selection behavior for
determining the parent basis, but the patch listing methods getPatchEntries()
and patchesFor() still default to using pinned basis only. To align the
behavior, update both getPatchEntries() and patchesFor() methods to use the same
parent-basis auto-selection logic from materializeReadState() instead of
defaulting to pinned basis, ensuring that materialize() and patch listing
operations produce consistent results for the same strand.
In `@src/domain/services/sync/SyncPayloadSchema.ts`:
- Around line 103-106: The syncRequestPageSchema function currently only
validates maxPatches with a minimum constraint using z.number().int().min(1),
but lacks a maximum constraint, allowing clients to request arbitrarily large
values and causing excessive server-side memory allocation. Add a
.max(limits.maxPatches) constraint to the maxPatches field validation in the
syncRequestPageSchema function to match the response schema validation and
prevent this resource amplification DoS vector.
In `@src/domain/utils/canonicalStringify.ts`:
- Around line 18-25: The sortedReplacer function in
src/domain/utils/canonicalStringify.ts violates the boundary typing rule by
using unknown type outside of adapter code. Replace both instances of unknown in
the function signature with an explicit JSON value type. The value parameter and
the return type should use a defined JSON value type (such as JsonValue or
similar) that represents valid JSON-serializable values instead of the generic
unknown type.
In `@src/domain/warp/RuntimeHostBoot.ts`:
- Line 138: The gcPolicy property in RuntimeHostBoot is being assigned a direct
reference to the caller's options.gcPolicy object. This allows external
mutations of the original object to affect the RuntimeHostBoot instance even
though Object.freeze is applied (which only provides shallow freezing). Create a
snapshot or deep copy of options.gcPolicy before assigning it to this.gcPolicy
to prevent external mutations from affecting the stored policy configuration.
In `@test/unit/infrastructure/adapters/CasPayloadPointer.test.ts`:
- Line 45: The assertion at line 45 uses toBe(bytes) which checks for reference
identity, causing the test to fail if readPayloadBlob() returns an equivalent
defensive copy of the bytes object. Replace the toBe matcher with toEqual or
toStrictEqual to perform value equality comparison instead, ensuring the test
validates the actual byte content rather than object identity.
---
Nitpick comments:
In `@src/domain/services/controllers/MaterializeCoordinateStrategy.ts`:
- Around line 66-68: The ceilingExcludesAll method contains a magic number (0)
that should be extracted to a named constant for improved readability and
maintainability. Define a private or class-level constant (such as
MINIMUM_CEILING_THRESHOLD or ZERO_BOUNDARY) at the top of the
MaterializeCoordinateStrategy class and replace the literal 0 in the
ceilingExcludesAll method's comparison (ceiling <= 0) with this named constant
reference.
In `@src/domain/services/controllers/MaterializeLiveStrategy.ts`:
- Line 21: The null/undefined check in the condition at line 21 is redundant
because loadCheckpoint() returns CheckpointData | null, not CheckpointData |
undefined. Remove the explicit !== undefined check from the if statement
condition that validates the checkpoint object before calling
isCurrentCheckpointSchema(checkpoint.schema). You can either keep checkpoint !==
null alone or use the loose inequality checkpoint != null, both of which will
correctly handle null values and allow the schema check to proceed safely.
- Around line 47-50: The `typeof` check for `streamPatchesSince` in the
MaterializeLiveStrategy file is redundant because `streamPatchesSince` is a
concrete method defined on the `PatchCollector` interface and will always exist
at runtime. Remove the unnecessary conditional check and the fallback code block
that duplicates the internal logic, and instead call
`this.runtime.deps.patches.streamPatchesSince(checkpoint)` directly followed by
the return statement. This eliminates the dead code and makes the intention
clearer.
In `@src/domain/services/coordinate/SubstrateCoordinateBoundary.ts`:
- Around line 48-66: Remove the trivial includesName helper function that wraps
Array.includes() and replace its two usages in the authorityFor method with
direct calls to the includes method. Change
includesName(SUBSTRATE_CAPABILITY_NAMES, capabilityName) to
SUBSTRATE_CAPABILITY_NAMES.includes(capabilityName) and
includesName(SESSION_POLICY_CAPABILITY_NAMES, capabilityName) to
SESSION_POLICY_CAPABILITY_NAMES.includes(capabilityName) to eliminate
unnecessary indirection.
In `@src/domain/services/merge/MergeClassifier.ts`:
- Around line 10-32: The functions precursorReason, footprintReason, and
classifyWithoutPolicy contain hardcoded reason code strings such as
'shared-precursor', 'missing-shared-precursor', 'overlapping-footprints',
'disjoint-footprints', 'candidate-join', 'lowering-witness',
'obstruction-witness', 'projection', 'semantic', and 'no-candidate-join'
scattered throughout the code. Create a named constants object at the top of the
file that maps descriptive constant names to these reason code values, then
replace all hardcoded string literals in precursorReason, footprintReason, and
classifyWithoutPolicy with references to these constants to ensure consistency
and prevent code drift as the taxonomy evolves.
In `@src/domain/services/query/GraphTraversal.ts`:
- Line 140: The magic number 1000 for the abort-check cadence appears multiple
times in the traversal logic (in the condition checking run.visited.size % 1000
=== 0 at the location shown and another similar occurrence). Extract this value
into a named constant at the top of the GraphTraversal class or file (e.g.,
something like ABORT_CHECK_CADENCE or TRAVERSAL_ABORT_CHECK_INTERVAL) and
replace all occurrences of the literal 1000 with this constant reference. This
eliminates magic numbers and ensures that any future changes to the cadence only
need to be updated in one place.
In `@src/domain/warp/RuntimeHostBoot.ts`:
- Around line 196-197: The `minimal()` function contains hardcoded string
defaults 'default' for graphName and 'local' for writerId on lines 196-197.
Replace these magic strings by defining named constants at the top of the file
or in an appropriate constants section, then use those constant references
instead of the literal strings in the graphName and writerId property
assignments.
In `@test/unit/domain/crdt/CrdtShimRetirement.test.ts`:
- Around line 33-36: The test loop checking for forbidden shims only validates
`export function` and `export const` patterns but misses alternative export
syntaxes. Add additional expect statements in the loop iterating over
FORBIDDEN_VERSION_VECTOR_SHIMS to also check for re-export patterns including
`export { ${shim} }` and `export { ${shim} as` to catch cases where shims might
be re-exported using destructuring syntax or with aliases.
In `@test/unit/domain/services/query/BoundedSupportRule.test.ts`:
- Around line 16-37: The `nodes()` and `neighbors()` generator methods in the
NoopQueryReadModelProvider class are creating empty arrays and then iterating
over them with for-loops that never execute. Simplify both of these generator
methods by removing the unnecessary empty array declarations and for-loop
constructs, leaving just the generator function definitions with no yield
statements since they should return empty generators.
🪄 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
Run ID: e438d299-0b42-4a96-abb9-1b193d48b6db
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (246)
CHANGELOG.mdREADME.mdbin/cli/commands/checkpoint.tsbin/cli/commands/fork.tsbin/cli/commands/gc.tsbin/cli/commands/mcp.tsbin/cli/commands/mcp/McpJsonValue.tsbin/cli/commands/mcp/McpProtocol.tsbin/cli/commands/mcp/McpProtocolError.tsbin/cli/commands/mcp/McpToolCatalog.tsbin/cli/commands/registry.tsbin/cli/commands/serve.tsbin/cli/commands/sync.tsbin/cli/commands/watch.tsbin/cli/infrastructure.tsdocs/ADVANCED_GUIDE.mddocs/API_REFERENCE.mddocs/ARCHITECTURE.mddocs/CLI_GUIDE.mddocs/CONCEPTUAL_OVERVIEW.mddocs/DOCTRINE_RUNTIME_ALIGNMENT.mddocs/GLOSSARY.mddocs/GUIDE.mddocs/README.mddocs/VISION.mddocs/archive/retrospectives/2026-04-01-tsc-zero-and-joinreducer-strategy.mddocs/audit/2026-06-20_tsc-zero-agent-merge-audit.mddocs/audits/WARP_DOCTRINE_RUNTIME_ALIGNMENT.mddocs/audits/WARP_DRIFT.mddocs/checklists/adr-2-go-no-go-checklist.mddocs/migrations/v17.0.0.mddocs/specs/CONTENT_ATTACHMENT.mddocs/specs/LANE_COORDINATE_CAPABILITY_BOUNDARY.mdindex.tspackage.jsonpatches/@git-stunts+trailer-codec+2.1.1.patchpatches/README.mdscripts/migrations/v17.0.0/SubstrateMigrationCompatibilityPolicy.tsscripts/source-size-gate.tsscripts/source-version-name-policy.tssrc/continuumExports.tssrc/domain/RuntimeHost.tssrc/domain/WarpApp.tssrc/domain/WarpCore.tssrc/domain/WarpGraph.tssrc/domain/WarpWorldline.tssrc/domain/WarpWorldlineCoordinate.tssrc/domain/capabilities/ComparisonCapability.tssrc/domain/capabilities/DetachedGraphFactory.tssrc/domain/capabilities/MaterializeCapability.tssrc/domain/capabilities/PatchCollector.tssrc/domain/capabilities/QueryCapability.tssrc/domain/capabilities/SyncCapability.tssrc/domain/continuum/GitWarpReceiptEnvelopeBoundary.tssrc/domain/continuum/GitWarpWitnessedSuffixAdmissionOutcome.tssrc/domain/continuum/GitWarpWitnessedSuffixAdmissionShell.tssrc/domain/crdt/ORSet.tssrc/domain/crdt/VersionVector.tssrc/domain/errors/OperationPolicyExhaustedError.tssrc/domain/errors/OperationPolicyTimeoutError.tssrc/domain/errors/index.tssrc/domain/runtimeHelpers.tssrc/domain/services/OpStrategies.tssrc/domain/services/OpStrategyRegistryValidation.tssrc/domain/services/PatchBuilder.tssrc/domain/services/PatchCommitter.tssrc/domain/services/ProjectionHandle.tssrc/domain/services/codec/MessageCodecInternal.tssrc/domain/services/codec/MessageSchemaDetector.tssrc/domain/services/comparison/GraphDiff.tssrc/domain/services/controllers/ComparisonController.tssrc/domain/services/controllers/ComparisonEngine.tssrc/domain/services/controllers/MaterializeCeilingStrategy.tssrc/domain/services/controllers/MaterializeCheckpointStrategy.tssrc/domain/services/controllers/MaterializeController.tssrc/domain/services/controllers/MaterializeCoordinateStrategy.tssrc/domain/services/controllers/MaterializeLiveStrategy.tssrc/domain/services/controllers/MaterializePatchStreamReducer.tssrc/domain/services/controllers/MaterializePatchSummary.tssrc/domain/services/controllers/MaterializeStrategyRuntime.tssrc/domain/services/controllers/QueryController.tssrc/domain/services/controllers/StrandController.tssrc/domain/services/controllers/SyncController.tssrc/domain/services/coordinate/SubstrateCoordinateBoundary.tssrc/domain/services/merge/MergeClassification.tssrc/domain/services/merge/MergeClassificationEvidence.tssrc/domain/services/merge/MergeClassificationKind.tssrc/domain/services/merge/MergeClassifier.tssrc/domain/services/merge/TtdMergeBranch.tssrc/domain/services/merge/TtdMergeFootprint.tssrc/domain/services/merge/TtdMergeInspection.tssrc/domain/services/merge/TtdMergeInspectionDomain.tssrc/domain/services/merge/TtdMergeInspector.tssrc/domain/services/merge/TtdMergeLoweringSurface.tssrc/domain/services/merge/TtdMergeLoweringWitness.tssrc/domain/services/merge/TtdMergeObstructionWitness.tssrc/domain/services/merge/TtdMergePolicyRequirement.tssrc/domain/services/merge/TtdMergeValidation.tssrc/domain/services/optic/StreamingCheckpointBasisBuilder.tssrc/domain/services/query/BoundedSupportRule.tssrc/domain/services/query/CausalIndexPlan.tssrc/domain/services/query/GraphTraversal.tssrc/domain/services/query/LogicalTraversal.tssrc/domain/services/query/Observer.tssrc/domain/services/query/ObserverAccumulation.tssrc/domain/services/query/ObserverBasis.tssrc/domain/services/query/ObserverEmission.tssrc/domain/services/query/ObserverPlan.tssrc/domain/services/query/ObserverReadingEnvelope.tssrc/domain/services/query/QueryBuilder.tssrc/domain/services/query/QueryReadModelProvider.tssrc/domain/services/query/QueryRunner.tssrc/domain/services/query/SupportFragmentPlan.tssrc/domain/services/strand/ConflictAnalyzerService.tssrc/domain/services/strand/ConflictCandidateCollector.tssrc/domain/services/strand/ConflictFrameLoader.tssrc/domain/services/strand/ConflictPipelineContext.tssrc/domain/services/strand/ConflictTraceAssembler.tssrc/domain/services/strand/StrandCoordinator.tssrc/domain/services/strand/StrandMaterializer.tssrc/domain/services/strand/conflictCandidateAnalysis.tssrc/domain/services/strand/conflictTargetIdentity.tssrc/domain/services/sync/SyncAuthService.tssrc/domain/services/sync/SyncPayloadSchema.tssrc/domain/services/sync/SyncProtocol.tssrc/domain/services/sync/SyncResponsePaging.tssrc/domain/services/sync/syncRequestResponse.tssrc/domain/trust/schemas.tssrc/domain/types/Aperture.tssrc/domain/types/DeliveryObservation.tssrc/domain/types/EffectEmission.tssrc/domain/types/TickReceipt.tssrc/domain/types/WarpPersistence.tssrc/domain/utils/canonicalStringify.tssrc/domain/utils/scalarValidation.tssrc/domain/warp/RuntimeHostBoot.tssrc/domain/warp/RuntimeHostProduct.tssrc/domain/warp/RuntimePatchCollector.tssrc/domain/warp/WarpCoreRuntimeProduct.tssrc/domain/warp/WarpGraphRuntimeBridge.tssrc/domain/warp/WarpGraphRuntimeProduct.tssrc/domain/warp/Writer.tssrc/globals.d.tssrc/infrastructure/adapters/AlfredOperationPolicyAdapter.tssrc/infrastructure/adapters/CasBlobAdapter.tssrc/infrastructure/adapters/CasContentEncryptionPolicy.tssrc/infrastructure/adapters/CasIndexStorageAdapter.tssrc/infrastructure/adapters/CasPayloadPointer.tssrc/infrastructure/adapters/CasSeekCacheAdapter.tssrc/infrastructure/adapters/CborCheckpointStoreAdapter.tssrc/infrastructure/adapters/CborIndexStoreAdapter.tssrc/infrastructure/adapters/CborPatchJournalAdapter.tssrc/infrastructure/adapters/FetchSyncHttpClientAdapter.tssrc/infrastructure/adapters/GitGraphAdapter.tssrc/infrastructure/adapters/GitRecursiveTreeOidReaderAdapter.tssrc/infrastructure/adapters/GitTrustChainAdapter.tssrc/infrastructure/adapters/NoopOperationPolicyAdapter.tssrc/infrastructure/adapters/SubstrateCompatibilityPolicy.tssrc/infrastructure/adapters/gitErrorClassification.tssrc/ports/OperationPolicyPort.tssrc/ports/SyncHttpClientPort.tssrc/ports/WarpKernelPort.tstest/benchmark/MergeConflictCorpus.benchmark.tstest/conformance/queryReadModelSeam.test.tstest/fixtures/mergeConflictCorpus.tstest/type-check/consumer.tstest/type-check/trailer-codec.d.tstest/type-check/tsconfig.jsontest/unit/cli/commands/mcp.test.tstest/unit/domain/RuntimePatchCollector.stream.test.tstest/unit/domain/WarpGraph.encryption.test.tstest/unit/domain/WarpGraph.materializeSlice.test.tstest/unit/domain/WarpGraph.strands.compare.test.tstest/unit/domain/WarpGraph.strands.test.tstest/unit/domain/WarpGraph.test.tstest/unit/domain/WarpGraph.traverse.stream.test.tstest/unit/domain/WarpWorldline.capabilities.test.tstest/unit/domain/WarpWorldline.test.tstest/unit/domain/WarpWorldlineCoordinate.test.tstest/unit/domain/continuum/GitWarpReceiptEnvelopeBoundary.test.tstest/unit/domain/continuum/GitWarpWitnessedSuffixAdmissionShell.test.tstest/unit/domain/crdt/CrdtShimRetirement.test.tstest/unit/domain/crdt/ORSet.test.tstest/unit/domain/crdt/VersionVector.test.tstest/unit/domain/errors/index.test.tstest/unit/domain/index.exports.test.tstest/unit/domain/publicReadingSurface.behavior.test.tstest/unit/domain/services/EdgePropSetWireMigrationGate.test.tstest/unit/domain/services/GraphTraversal.stream.test.tstest/unit/domain/services/MessageCodecModules.test.tstest/unit/domain/services/OpStrategyRegistryValidation.test.tstest/unit/domain/services/ProjectionHandle.test.tstest/unit/domain/services/SchemaCompat.test.tstest/unit/domain/services/SyncAuthService.test.tstest/unit/domain/services/WarpMessageCodec.v3.test.tstest/unit/domain/services/comparison/GraphDiff.test.tstest/unit/domain/services/controllers/MaterializeController.snapshotCache.test.tstest/unit/domain/services/controllers/MaterializeController.stateSession.test.tstest/unit/domain/services/controllers/MaterializeController.test.tstest/unit/domain/services/controllers/MaterializePatchStreamReducer.test.tstest/unit/domain/services/controllers/QueryController.test.tstest/unit/domain/services/controllers/StrandController.host-interface.test.tstest/unit/domain/services/coordinate/SubstrateCoordinateBoundary.test.tstest/unit/domain/services/merge/MergeClassifier.test.tstest/unit/domain/services/merge/TtdMergeInspector.test.tstest/unit/domain/services/query/BoundedSupportRule.test.tstest/unit/domain/services/query/CausalIndexPlan.test.tstest/unit/domain/services/query/ObserverStructural.test.tstest/unit/domain/services/query/SupportFragmentPlan.test.tstest/unit/domain/services/strand/ConflictAnalyzerService.test.tstest/unit/domain/services/sync/SyncResponsePagingMetrics.test.tstest/unit/domain/specCompliance.test.tstest/unit/domain/trust/schemas.test.tstest/unit/domain/utils/canonicalStringify.test.tstest/unit/domain/utils/scalarValidation.test.tstest/unit/domain/warp/PatchSession.test.tstest/unit/domain/warp/WarpOpenOptions.test.tstest/unit/domain/warp/Writer.sameWriterRace.test.tstest/unit/fixtures/mergeConflictCorpus.test.tstest/unit/index.exports.test.tstest/unit/infrastructure/adapters/CasBlobAdapter.test.tstest/unit/infrastructure/adapters/CasContentEncryptionPolicy.test.tstest/unit/infrastructure/adapters/CasPayloadPointer.test.tstest/unit/infrastructure/adapters/CborPatchJournalAdapter.test.tstest/unit/infrastructure/adapters/GitGraphAdapter.gitCasPersistence.test.tstest/unit/infrastructure/adapters/OperationPolicyAdapter.test.tstest/unit/scripts/architecture-doc-shape.test.tstest/unit/scripts/audit-ambient-time-ratchet.test.tstest/unit/scripts/cli-command-gap-closeout.test.tstest/unit/scripts/cli-command-registry.test.tstest/unit/scripts/cli-help-shape.test.tstest/unit/scripts/conflict-pipeline-context.test.tstest/unit/scripts/dependency-hygiene.test.tstest/unit/scripts/docs-runtime-convergence-ratchet.test.tstest/unit/scripts/lane-coordinate-boundary.test.tstest/unit/scripts/non-ts-tail-shape.test.tstest/unit/scripts/observer-first-guide.test.tstest/unit/scripts/op-hydration-boundary-ratchet.test.tstest/unit/scripts/runtime-noun-doc-graph.test.tstest/unit/scripts/source-size-inventory-command.test.tstest/unit/scripts/tsc-zero-agent-audit.test.tstest/unit/scripts/v17-public-doc-read-contract.test.tstest/unit/scripts/v18-content-property-closeout-audit.test.tstest/unit/scripts/v18-package-surface-audit.test.tstest/unit/scripts/warp-doctrine-runtime-alignment.test.tstest/unit/scripts/warpserve-boundary-ratchet.test.ts
💤 Files with no reviewable changes (6)
- test/unit/scripts/non-ts-tail-shape.test.ts
- test/type-check/trailer-codec.d.ts
- src/globals.d.ts
- test/unit/domain/WarpGraph.materializeSlice.test.ts
- test/unit/domain/WarpGraph.test.ts
- src/domain/services/optic/StreamingCheckpointBasisBuilder.ts
Member
Author
|
@codex Code Lawyer Activity Summary for PR #664 follow-up
Review-thread resolution:
Local validation after fixes:
|
Member
Author
Code Lawyer Activity Summary@codex second opinion requested on the self-discovered CI coverage blocker and closure evidence.
Local verification after the latest fix:
Current PR thread state: 0 unresolved review threads after refresh. |
Member
Author
Code Lawyer Activity Summary Update@codex second opinion requested on the final CI preflight fix.
Verification:
Current PR thread state after refresh: 0 unresolved review threads. |
Release Preflight
If you tag this commit as |
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
Issues
Closes #555
Closes #556
Closes #557
Closes #558
Closes #559
Closes #563
Closes #564
Closes #561
Closes #554
Closes #562
Closes #560
Closes #152
Closes #154
Closes #166
Closes #177
Closes #181
Closes #183
Closes #196
Closes #222
Closes #234
Validation
git diff --check origin/main...HEADnpm run lintnpm run typechecknpm run lint:mdnpm run lint:md:codenpm run typecheck:policynpm run lint:sludgenpm run lint:quarantine-graduatenpm run test:localSummary by CodeRabbit
Release Notes
New Features
checkpoint,gc,sync(with paging),serve,fork,watch, plus MCP server support over stdio.graph.diff).bfsStream/dfsStream).API Changes
ProjectionHandle; graph/core now exposediff.x-warp-auth-scheme.Documentation
Breaking Changes
@git-stunts/plumbing:Plumbingnamed export removed; use defaultGitPlumbing.