feat(tape): add contract lineage enforcement - #2107
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:
📝 WalkthroughWalkthroughThis change introduces immutable TaskContract and ExecutionContract lineage, provenance-aware prompt assemblies, contract-bound View manifests, runtime authority checks, Tape persistence, live-delegation evaluation, recovery handling, and schema migrations. ChangesCanonical contracts and evaluations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 11
🧹 Nitpick comments (17)
src/main/tape/domain/executionContract.ts (1)
1103-1118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing
isDeepChatExecutionContractinsideverifyExecutionContractHash.
verifyExecutionContractHashrepeats the schema, hash version, hash format, and hash recomputation checks thatisDeepChatExecutionContractalready performs. The two functions can drift when the hash rules change. A thin wrapper keeps one source of truth for hash verification.🤖 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/main/tape/domain/executionContract.ts` around lines 1103 - 1118, Update verifyExecutionContractHash to reuse isDeepChatExecutionContract as the single validation path, removing its duplicated schema, hash-version, format, and recomputation checks while preserving the boolean verification contract.src/main/tape/domain/taskContract.ts (2)
139-162: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAdd explicit
typeofguards before the SHA-256 tests.
normalizeEvaluationRefreceives values that originate from untyped restore paths, for example thecontract.taskConfig.predecessorEvaluationRefread inisDeepChatTaskContract.SHA_256_PATTERN.test(value.tapeIdentity)coerces a non-string operand instead of rejecting it, and the returned object then copies the raw value. Addtypeofchecks so only strings pass.♻️ Proposed guard
if ( value?.schemaVersion !== 1 || + typeof value.tapeIdentity !== 'string' || + typeof value.evaluationHash !== 'string' || !SHA_256_PATTERN.test(value.tapeIdentity) || !SHA_256_PATTERN.test(value.evaluationHash) ) {🤖 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/main/tape/domain/taskContract.ts` around lines 139 - 162, Update normalizeEvaluationRef to require typeof value.tapeIdentity === 'string' and typeof value.evaluationHash === 'string' before applying SHA_256_PATTERN.test. Keep invalid schema or non-string/hash-mismatched values on the existing TaskContractError path, and only return validated string values.
419-444: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNote the cost of the rebuild-based type guard.
isDeepChatTaskContractrebuilds the whole contract, including the bounded JSON-schema walk, on every call. Callers invoke it on hot paths, for examplenormalizeTaskContractRefinsrc/main/tape/domain/executionContract.tsfor each provider request and row matching insrc/main/tape/application/taskEvaluationService.ts. The work is bounded byMAX_TASK_CONTRACT_BYTESandMAX_RESULT_SCHEMA_NODES, so it is safe. If profiling shows this guard in a hot path, add a fast pre-check oncontractHashbefore the rebuild.🤖 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/main/tape/domain/taskContract.ts` around lines 419 - 444, If profiling identifies isDeepChatTaskContract as a hot-path cost, add an early contractHash consistency check before the buildTaskContract rebuild and bounded schema walk, returning false on mismatch while preserving the existing canonical comparison for matching hashes. Keep the change scoped to isDeepChatTaskContract and retain all current validation behavior.src/main/tape/domain/workspacePath.ts (1)
8-18: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider stripping a trailing separator during normalization.
path.normalizekeeps a trailing separator, so/work/repo/and/work/reponormalize to different strings.workspacePathsMatchandisWorkspacePathWithintreat them as equal because they userelative, so authority checks stay correct. The stored contract string still differs, which produces two different contract hashes for one directory. If you want one canonical string per directory, strip the trailing separator here.🤖 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/main/tape/domain/workspacePath.ts` around lines 8 - 18, Update normalizeAbsoluteWorkspacePath to strip trailing separators from the normalized POSIX and Windows paths, while preserving filesystem roots such as "/" and drive roots. Ensure equivalent directory inputs like "/work/repo/" and "/work/repo" produce the same canonical contract path.src/main/tape/domain/viewManifest.ts (2)
243-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one form for the schema-5 comparison.
Line 244 uses
Number(manifest.schemaVersion) === 5. Line 248 usesmanifest.schemaVersion !== 5. The two forms test the same property in adjacent branches of one guard. Use the direct comparison in both places, and add a short comment if a cast is required for type narrowing.🤖 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/main/tape/domain/viewManifest.ts` around lines 243 - 249, Standardize the schema-5 checks in the manifest hash-version branches by using the direct manifest.schemaVersion comparison consistently, updating the check in the TAPE_VIEW_MANIFEST_LEGACY_HASH_VERSION branch to match the existing comparison in the TAPE_VIEW_MANIFEST_HASH_VERSION branch. Add a brief narrowing comment only if the type system requires a cast.
154-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one contract-to-manifest consistency rule. Both modules independently assert that a schema-5 manifest and its embedded
ExecutionContractagree on session, message, request sequence, provider, model, prompt hash, and tool-definitions hash. Adding a provenance field later requires two edits, and a missed edit weakens one of the two validation paths.
src/main/tape/domain/viewManifest.ts#L154-L166: export the field-by-field consistency check fromexecutionContractMatchesManifestas a shared helper.src/main/tape/domain/replay.ts#L141-L167: call that shared helper fromhasExecutionContractForSchemaand keep only the replay-specific checks, that is thehashVersioncheck and theviewIdderivation.🤖 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/main/tape/domain/viewManifest.ts` around lines 154 - 166, In src/main/tape/domain/viewManifest.ts lines 154-166, export executionContractMatchesManifest as the shared field-by-field consistency helper. In src/main/tape/domain/replay.ts lines 141-167, update hasExecutionContractForSchema to call that helper and retain only the replay-specific hashVersion validation and viewId derivation checks, removing its duplicated contract-to-manifest comparisons.src/main/tape/domain/taskEvaluation.ts (1)
322-323: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDrop the section identity from the schema cache key.
The cache key combines the section identity and the schema hash. Two requirements that use the same schema on different sections compile and validate separately. The validation result depends on the parsed section value, so the section identity must stay in the key for result caching. If you want to reuse compiled validators, cache the compiled function by schema hash and keep the result cache keyed by section and 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/main/tape/domain/taskEvaluation.ts` around lines 322 - 323, Update the caching logic around schemaEvaluation to separate compiled-validator reuse from validation-result caching: key compiled validators only by hashJsonData(requirement.schema), while retaining sectionIdentity in the key used for parsed-section validation results. Ensure result caching remains isolated per section and schema.test/main/session/data/tapeViewManifest.test.ts (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate the empty
toolsfixture.TypeScript infers
never[]forconst tools = []. The empty array then satisfies every array parameter, so the fixture no longer checks that the tool list matches the manifest and contract element type. The file already importsMCPToolDefinitionBaseat line 4. Add the annotation.♻️ Proposed typing fix
- const tools = [] + const tools: MCPToolDefinitionBase[] = []🤖 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/main/session/data/tapeViewManifest.test.ts` at line 39, Annotate the empty tools fixture in the test setup with MCPToolDefinitionBase[], using the existing MCPToolDefinitionBase import so TypeScript validates the fixture against the manifest’s tool contract.src/main/orchestration/liveDelegationService.ts (1)
1544-1547: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare one project-directory resolution.
resolveParentProjectDirat lines 1624-1627 repeats the expression inresolveCurrentSafetyat lines 1544-1547. Both derive the workspace that is frozen into the TaskContract. If one changes later, the spawn path and the follow-up path can freeze different workspaces for the same delegation. CallresolveParentProjectDirfromresolveCurrentSafety.♻️ Proposed refactor
- const projectDir = - (await this.options.sessions.resolveConversationWorkdir(parent.sessionId)) || - parent.projectDir || - null + const projectDir = await this.resolveParentProjectDir(parent) return { parent, projectDir }Also applies to: 1624-1627
🤖 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/main/orchestration/liveDelegationService.ts` around lines 1544 - 1547, Update resolveCurrentSafety to obtain the project directory by calling resolveParentProjectDir instead of duplicating the resolveConversationWorkdir/parent.projectDir fallback expression. Preserve the existing resolved-or-null behavior and use the shared value in both spawn and follow-up paths.test/main/orchestration/liveDelegationService.test.ts (1)
2348-2363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the accepted-answer fixture from
LIVE_DELEGATION_REQUIRED_RESULT_SECTIONS. Both suites hard-code the same six-section Markdown answer. The shared root cause is that the required-section list is copied as literals instead of derived from the exported constant. If a section is added or renamed, one copy can drift and that suite silently stops producing an accepted verdict while still passing.
test/main/orchestration/liveDelegationService.test.ts#L2348-L2363: build the answer by mappingLIVE_DELEGATION_REQUIRED_RESULT_SECTIONSto## <section>plus a body line, and export the helper from a shared test module.test/main/orchestration/liveDelegationRepository.test.ts#L114-L129: delete the local copy and import the shared helper.As per path instructions, "Keep committed tests lean and focused on project reliability, stability, and observable contracts".
🤖 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/main/orchestration/liveDelegationService.test.ts` around lines 2348 - 2363, Derive the accepted-answer fixture from LIVE_DELEGATION_REQUIRED_RESULT_SECTIONS instead of duplicating section literals. In test/main/orchestration/liveDelegationService.test.ts:2348-2363, update completeAcceptedAnswer to map each required section to its heading and body, then export it from a shared test module. In test/main/orchestration/liveDelegationRepository.test.ts:114-129, remove the local fixture and import the shared helper.Source: Path instructions
src/main/tape/application/taskEvaluationService.ts (1)
258-272: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument why
serializeEvaluationRefis called for its side effect.Line 270 discards the return value. The call exists to assert that the reference serializes, and it throws when the reference is malformed. A future reader can remove it as dead code and silently drop that validation. Add a short comment.
♻️ Proposed comment
+ // Assert the reference is serializable before it becomes part of a receipt. serializeEvaluationRef(ref) return Object.freeze(ref)🤖 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/main/tape/application/taskEvaluationService.ts` around lines 258 - 272, In buildEvaluationRef, add a short comment immediately before the discarded serializeEvaluationRef(ref) call explaining that it intentionally validates serialization and throws for malformed references; preserve the call and its existing placement before Object.freeze(ref).test/main/tape/executionContract.test.ts (1)
659-666: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce coupling to the internal hashing recipe.
The test rebuilds
internalExecutionPolicyHashashashJsonData(stored.ceilings)andcontractHashashashJsonData(draft). This reproduces the exact internal hashing recipe. If the recipe changes without a behavior change, this test fails.Prefer a helper exported from the domain module that re-seals a mutated contract, or build the Windows-path contract through
buildExecutionContractand assertisDeepChatExecutionContracton the serialized round trip.As per coding guidelines: "Keep committed tests lean and focused on project reliability, stability, and observable contracts; remove temporary checks that only test implementation internals before handoff."
🤖 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/main/tape/executionContract.test.ts` around lines 659 - 666, Update the Windows-path validation test around isDeepChatExecutionContract to avoid manually recomputing internalExecutionPolicyHash and contractHash. Prefer constructing the contract through buildExecutionContract and validating its serialized round trip, or use an exported domain helper that re-seals the mutated contract, while preserving the assertion that the canonical Windows path is accepted independently of the replay host platform.Source: Coding guidelines
src/main/tool/index.ts (2)
938-953: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider one generic definition lookup.
getAgentDefinitionduplicatesgetMcpDefinitionexactly, including theglobalMapperConversationId !== nullfallback rule. A single private helper that takes the conversation map and the global map would keep the two lookups in sync.♻️ Proposed refactor
+ private lookupDefinition( + toolName: string, + conversationMaps: Map<string, Map<string, MCPToolDefinition>>, + globalMap: Map<string, MCPToolDefinition>, + conversationId?: string + ): MCPToolDefinition | undefined { + const normalizedConversationId = conversationId?.trim() + if (normalizedConversationId) { + const definitions = conversationMaps.get(normalizedConversationId) + if (definitions) return definitions.get(toolName) + if (this.globalMapperConversationId !== null) return undefined + } + return globalMap.get(toolName) + }🤖 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/main/tool/index.ts` around lines 938 - 953, The getAgentDefinition and getMcpDefinition lookups duplicate the same conversation/global resolution logic. Extract one private generic helper that accepts the relevant conversation-definition map and global-definition map, including the existing globalMapperConversationId fallback behavior, then update both methods to delegate to it.
755-757: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDerive dispatch subagent depth from
currentDefinition.
src/main/tool/index.ts:756currently hardcodes1forLIVE_DELEGATION_AGENT_TOOL_NAME, while the task contract layer computes depth from the tool definition and checks each tool againstmaxSubagentDepth. Move the shared rule intosrc/main/tool/index.tsor export a reusable helper, then pass that value toassertExecutionContractAllowsDispatch; this keeps runtime tool dispatch and task-contract filtering in sync if another tool gains subagent depth later.🤖 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/main/tool/index.ts` around lines 755 - 757, Replace the hardcoded requestedSubagentDepth logic near assertExecutionContractAllowsDispatch with a shared computation derived from currentDefinition, matching the task contract layer’s maxSubagentDepth rules. Reuse or export the existing helper from the task contract implementation, then pass its result for the current tool so dispatch and contract filtering remain synchronized as definitions gain subagent depth.src/main/tool/runtimePorts.ts (1)
72-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the tri-state
enabledMcpServerIds.
string[] | nullplus the optional marker gives three states for two meanings. The consumer insrc/main/tool/index.tsline 732 usesArray.isArray(...), soundefinedandnullbehave identically. UseenabledMcpServerIds: string[] | nullto make "no restriction" a single value.🤖 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/main/tool/runtimePorts.ts` at line 72, Update the enabledMcpServerIds declaration in the runtime ports type to remove the optional marker, keeping it as string[] | null. Ensure all construction and usage sites provide either an array for restricted IDs or null for no restriction, preserving the consumer’s Array.isArray behavior.test/main/agent/deepchat/loop/loopRun.test.ts (1)
108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIsolate the stale-sequence rejection.
Line 108 passes both a wrong
requestSeqand anullcontract. The assertion cannot show which input caused the rejection. Pass a valid contract with the wrong sequence to pin the stale-sequence rule. Thecontractfactory on line 97 already supportssessionIdandmessageIdoverrides, so a mismatched session case costs one more line.💚 Proposed test change
- expect(() => bindActiveRequestContract(run, requestSeq + 1, null)).toThrow(/request sequence/) + expect(() => bindActiveRequestContract(run, requestSeq + 1, contract())).toThrow( + /request sequence/ + ) expect(() => bindActiveRequestContract(run, requestSeq, contract({ runId: 'other' }))).toThrow( /Loop Run/ ) + expect(() => + bindActiveRequestContract(run, requestSeq, contract({ sessionId: 'other' })) + ).toThrow()🤖 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/main/agent/deepchat/loop/loopRun.test.ts` around lines 108 - 111, Update the stale-sequence assertion around bindActiveRequestContract to pass a valid contract instead of null, isolating the rejection to requestSeq + 1. Reuse the existing contract factory and its sessionId/messageId override support to add a separate mismatched-session case, while preserving the existing wrong-run assertion.src/main/agent/deepchat/runtime/deepChatLoopRunner.ts (1)
472-482: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the effective-system-prompt read.
The same expression appears three times: here, at lines 736-740, and in
src/main/agent/deepchat/runtime/process.tslines 1315-1319. A small shared helper in@/agent/deepchat/resources/promptAssemblywould keep the three sites consistent.♻️ Proposed helper
// src/main/agent/deepchat/resources/promptAssembly.ts export function readEffectiveSystemPrompt(messages: readonly ChatMessage[]): string { const first = messages[0] return first?.role === 'system' && typeof first.content === 'string' ? first.content : '' }🤖 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/main/agent/deepchat/runtime/deepChatLoopRunner.ts` around lines 472 - 482, Extract the repeated effective-system-prompt expression into a shared readEffectiveSystemPrompt helper in promptAssembly, accepting readonly ChatMessage[] and returning the first string system message or ''. Replace the local expressions in deepChatLoopRunner and process.ts with this helper, preserving existing behavior and importing it from the shared module.
🤖 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 `@src/main/agent/deepchat/resources/promptAssembly.ts`:
- Around line 76-79: Update the assembly return paths in promptAssembly to
snapshot section objects before returning them, rather than only freezing the
sections array. Clone and freeze each section, and clone and freeze its
degradationCodes collection, so later caller mutations to content, contentHash,
or degradationCodes cannot alter returned metadata. Apply the same change to all
assembly boundaries, including the paths corresponding to the other noted return
blocks.
In `@src/main/agent/deepchat/resources/systemPromptBuilder.ts`:
- Around line 10-17: Apply Oxfmt formatting to the changed code in
systemPromptBuilder.ts, including the imports and the referenced ranges. Use
single quotes, remove semicolons, and enforce the 100-column width without
altering behavior.
In `@src/main/agent/deepchat/runtime/process.ts`:
- Around line 1086-1091: Make contract absence fail closed across both sites: in
src/main/agent/deepchat/runtime/process.ts lines 1086-1091, require
run.activeRequestContract when the session must carry a contract and fail the
round instead of assigning a null executionContract; in
src/main/agent/deepchat/runtime/deepChatLoopRunner.ts lines 725-782, update
onBuildError to either abort with a contract-specific terminal reason or
explicitly record the degradation, so the later binding check at lines 839-846
does not misclassify a build failure as lost binding.
In `@src/main/orchestration/liveDelegationRepository.ts`:
- Around line 1053-1096: The projection validation errors in toTurn() currently
throw plain Error instances, preventing runWithAdmission() from quarantining
corrupt stored rows. Replace the incomplete, conflicting, and misbound
TaskContract/evaluation projection throws with LiveDelegationTaskContractError
while preserving their existing validation conditions and messages.
- Around line 1258-1285: Update parseTaskContract, parseTaskContractRef,
parseTaskEvaluation, and parseEvaluationRef to perform JSON.parse inside a guard
and convert parse failures into each helper’s existing “is malformed.” domain
error; preserve the current null handling and restore-function validation
behavior.
In `@src/main/tape/domain/taskContract.ts`:
- Around line 239-248: Extend the key validation in assertBoundedJsonSchema to
reject $dynamicRef, $recursiveRef, $id, and $schema alongside the existing $ref
and $async checks. Keep these failures as invalid_input TaskContractError
results before recursively processing descriptor.value or invoking Ajv.
In `@src/main/tape/infrastructure/sqlite/tapeEntryStore.ts`:
- Around line 377-383: Align the SQL contract-name exclusion predicates in the
projection and search queries with isContractTapeReservedName: exclude every
name beginning with the contract/ namespace rather than only the literal
contract/* pattern. Apply the same matching behavior in both query locations, or
instead update isContractTapeReservedName so its domain behavior exactly matches
the existing SQL predicate.
In `@src/shared/types/task-contract.ts`:
- Line 246: Bound both reasonCodes projections in
src/shared/types/task-contract.ts at lines 246-246 and 266-266 by applying
.max(DEEPCHAT_TASK_EVALUATION_REASON_CODES.length) to each array schema,
including the parent-facing summary.
In `@test/main/agent/deepchat/harness/deepChatAgentHarness.test.ts`:
- Around line 5262-5266: Update the refreshSystemPrompt handling at both
referenced call sites to treat the result as a DeepChatPromptAssembly
unconditionally: remove the typeof string fallback and read its prompt property
directly. This should assert the documented assembly contract and preserve the
existing refreshedSystemPrompt assignment.
In `@test/main/agent/deepchat/resources/systemPromptBuilder.test.ts`:
- Around line 160-165: Update the configured_prompt assertion in the assembly
test to verify the exact SHA-256 contentHash for “BASE PROMPT” instead of only
matching a 64-character hexadecimal format. Keep the existing inclusion
assertion and add only this focused regression expectation.
In `@test/main/agent/deepchat/runtime/process.test.ts`:
- Around line 1022-1030: In the test around finalPauseCall, assert that
finalPauseCall is defined before accessing its blocks or parsing the execution
contract binding, matching the sibling test’s guard. Keep the existing
permissionBlock lookup and binding equality assertion unchanged after this
validation.
---
Nitpick comments:
In `@src/main/agent/deepchat/runtime/deepChatLoopRunner.ts`:
- Around line 472-482: Extract the repeated effective-system-prompt expression
into a shared readEffectiveSystemPrompt helper in promptAssembly, accepting
readonly ChatMessage[] and returning the first string system message or ''.
Replace the local expressions in deepChatLoopRunner and process.ts with this
helper, preserving existing behavior and importing it from the shared module.
In `@src/main/orchestration/liveDelegationService.ts`:
- Around line 1544-1547: Update resolveCurrentSafety to obtain the project
directory by calling resolveParentProjectDir instead of duplicating the
resolveConversationWorkdir/parent.projectDir fallback expression. Preserve the
existing resolved-or-null behavior and use the shared value in both spawn and
follow-up paths.
In `@src/main/tape/application/taskEvaluationService.ts`:
- Around line 258-272: In buildEvaluationRef, add a short comment immediately
before the discarded serializeEvaluationRef(ref) call explaining that it
intentionally validates serialization and throws for malformed references;
preserve the call and its existing placement before Object.freeze(ref).
In `@src/main/tape/domain/executionContract.ts`:
- Around line 1103-1118: Update verifyExecutionContractHash to reuse
isDeepChatExecutionContract as the single validation path, removing its
duplicated schema, hash-version, format, and recomputation checks while
preserving the boolean verification contract.
In `@src/main/tape/domain/taskContract.ts`:
- Around line 139-162: Update normalizeEvaluationRef to require typeof
value.tapeIdentity === 'string' and typeof value.evaluationHash === 'string'
before applying SHA_256_PATTERN.test. Keep invalid schema or
non-string/hash-mismatched values on the existing TaskContractError path, and
only return validated string values.
- Around line 419-444: If profiling identifies isDeepChatTaskContract as a
hot-path cost, add an early contractHash consistency check before the
buildTaskContract rebuild and bounded schema walk, returning false on mismatch
while preserving the existing canonical comparison for matching hashes. Keep the
change scoped to isDeepChatTaskContract and retain all current validation
behavior.
In `@src/main/tape/domain/taskEvaluation.ts`:
- Around line 322-323: Update the caching logic around schemaEvaluation to
separate compiled-validator reuse from validation-result caching: key compiled
validators only by hashJsonData(requirement.schema), while retaining
sectionIdentity in the key used for parsed-section validation results. Ensure
result caching remains isolated per section and schema.
In `@src/main/tape/domain/viewManifest.ts`:
- Around line 243-249: Standardize the schema-5 checks in the manifest
hash-version branches by using the direct manifest.schemaVersion comparison
consistently, updating the check in the TAPE_VIEW_MANIFEST_LEGACY_HASH_VERSION
branch to match the existing comparison in the TAPE_VIEW_MANIFEST_HASH_VERSION
branch. Add a brief narrowing comment only if the type system requires a cast.
- Around line 154-166: In src/main/tape/domain/viewManifest.ts lines 154-166,
export executionContractMatchesManifest as the shared field-by-field consistency
helper. In src/main/tape/domain/replay.ts lines 141-167, update
hasExecutionContractForSchema to call that helper and retain only the
replay-specific hashVersion validation and viewId derivation checks, removing
its duplicated contract-to-manifest comparisons.
In `@src/main/tape/domain/workspacePath.ts`:
- Around line 8-18: Update normalizeAbsoluteWorkspacePath to strip trailing
separators from the normalized POSIX and Windows paths, while preserving
filesystem roots such as "/" and drive roots. Ensure equivalent directory inputs
like "/work/repo/" and "/work/repo" produce the same canonical contract path.
In `@src/main/tool/index.ts`:
- Around line 938-953: The getAgentDefinition and getMcpDefinition lookups
duplicate the same conversation/global resolution logic. Extract one private
generic helper that accepts the relevant conversation-definition map and
global-definition map, including the existing globalMapperConversationId
fallback behavior, then update both methods to delegate to it.
- Around line 755-757: Replace the hardcoded requestedSubagentDepth logic near
assertExecutionContractAllowsDispatch with a shared computation derived from
currentDefinition, matching the task contract layer’s maxSubagentDepth rules.
Reuse or export the existing helper from the task contract implementation, then
pass its result for the current tool so dispatch and contract filtering remain
synchronized as definitions gain subagent depth.
In `@src/main/tool/runtimePorts.ts`:
- Line 72: Update the enabledMcpServerIds declaration in the runtime ports type
to remove the optional marker, keeping it as string[] | null. Ensure all
construction and usage sites provide either an array for restricted IDs or null
for no restriction, preserving the consumer’s Array.isArray behavior.
In `@test/main/agent/deepchat/loop/loopRun.test.ts`:
- Around line 108-111: Update the stale-sequence assertion around
bindActiveRequestContract to pass a valid contract instead of null, isolating
the rejection to requestSeq + 1. Reuse the existing contract factory and its
sessionId/messageId override support to add a separate mismatched-session case,
while preserving the existing wrong-run assertion.
In `@test/main/orchestration/liveDelegationService.test.ts`:
- Around line 2348-2363: Derive the accepted-answer fixture from
LIVE_DELEGATION_REQUIRED_RESULT_SECTIONS instead of duplicating section
literals. In test/main/orchestration/liveDelegationService.test.ts:2348-2363,
update completeAcceptedAnswer to map each required section to its heading and
body, then export it from a shared test module. In
test/main/orchestration/liveDelegationRepository.test.ts:114-129, remove the
local fixture and import the shared helper.
In `@test/main/session/data/tapeViewManifest.test.ts`:
- Line 39: Annotate the empty tools fixture in the test setup with
MCPToolDefinitionBase[], using the existing MCPToolDefinitionBase import so
TypeScript validates the fixture against the manifest’s tool contract.
In `@test/main/tape/executionContract.test.ts`:
- Around line 659-666: Update the Windows-path validation test around
isDeepChatExecutionContract to avoid manually recomputing
internalExecutionPolicyHash and contractHash. Prefer constructing the contract
through buildExecutionContract and validating its serialized round trip, or use
an exported domain helper that re-seals the mutated contract, while preserving
the assertion that the canonical Windows path is accepted independently of the
replay host platform.
🪄 Autofix
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 Plus
Run ID: 5ea6b8a6-28c3-40e8-811f-0ae73acbe20f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (92)
docs/architecture/proactive-multi-agent-orchestration/spec.mddocs/architecture/tape-contract-lineage/plan.mddocs/architecture/tape-contract-lineage/spec.mddocs/architecture/tape-contract-lineage/tasks.mddocs/architecture/tape-system.mdpackage.jsonsrc/main/agent/deepchat/harness/createDeepChatAgentHarness.tssrc/main/agent/deepchat/harness/runtimeServices.tssrc/main/agent/deepchat/instance/deepChatAgentInstance.tssrc/main/agent/deepchat/loop/contextCoordinator.tssrc/main/agent/deepchat/loop/loopRun.tssrc/main/agent/deepchat/loop/ports.tssrc/main/agent/deepchat/resources/promptAssembly.tssrc/main/agent/deepchat/resources/systemEnvPromptBuilder.tssrc/main/agent/deepchat/resources/systemPromptBuilder.tssrc/main/agent/deepchat/runtime/deepChatLoopRunner.tssrc/main/agent/deepchat/runtime/deferredExecutionContract.tssrc/main/agent/deepchat/runtime/deferredToolExecutor.tssrc/main/agent/deepchat/runtime/dispatch.tssrc/main/agent/deepchat/runtime/interactionCoordinator.tssrc/main/agent/deepchat/runtime/process.tssrc/main/agent/deepchat/runtime/promptAssemblyService.tssrc/main/agent/deepchat/runtime/sessionIdentityService.tssrc/main/agent/deepchat/runtime/taskContractCapability.tssrc/main/agent/deepchat/runtime/turnCoordinator.tssrc/main/agent/deepchat/runtime/types.tssrc/main/app/composition.tssrc/main/app/startupMigrations/sessionDataMigrations.tssrc/main/data/schemaCatalog.tssrc/main/orchestration/data/tables/liveDelegationEvents.tssrc/main/orchestration/data/tables/liveDelegationTurns.tssrc/main/orchestration/data/tables/liveDelegations.tssrc/main/orchestration/liveDelegationRepository.tssrc/main/orchestration/liveDelegationService.tssrc/main/orchestration/liveDelegationTaskContract.tssrc/main/session/data/database.tssrc/main/tape/application/forkService.tssrc/main/tape/application/lineageService.tssrc/main/tape/application/taskContractService.tssrc/main/tape/application/taskEvaluationService.tssrc/main/tape/domain/canonicalJson.tssrc/main/tape/domain/contractFacts.tssrc/main/tape/domain/effectiveView.tssrc/main/tape/domain/executionContract.tssrc/main/tape/domain/replay.tssrc/main/tape/domain/tapeIdentity.tssrc/main/tape/domain/taskContract.tssrc/main/tape/domain/taskEvaluation.tssrc/main/tape/domain/viewManifest.tssrc/main/tape/domain/workspacePath.tssrc/main/tape/infrastructure/sqlite/tapeEntryStore.tssrc/main/tape/ports/storage.tssrc/main/tool/index.tssrc/main/tool/runtimePorts.tssrc/shared/chat.d.tssrc/shared/orchestration/liveDelegation.tssrc/shared/orchestration/liveDelegationMarkdown.tssrc/shared/types/agent-interface.d.tssrc/shared/types/core/chat.tssrc/shared/types/execution-contract.tssrc/shared/types/prompt-assembly.tssrc/shared/types/tape-view-manifest.tssrc/shared/types/task-contract.tssrc/shared/types/tool.d.tstest/main/agent/deepchat/harness/deepChatAgentHarness.test.tstest/main/agent/deepchat/loop/contextCoordinator.test.tstest/main/agent/deepchat/loop/loopRun.test.tstest/main/agent/deepchat/resources/promptAssembly.test.tstest/main/agent/deepchat/resources/systemEnvPromptBuilder.test.tstest/main/agent/deepchat/resources/systemPromptBuilder.test.tstest/main/agent/deepchat/runtime/deferredExecutionContract.test.tstest/main/agent/deepchat/runtime/deferredToolExecutor.test.tstest/main/agent/deepchat/runtime/process.test.tstest/main/agent/deepchat/runtime/promptAssemblyService.test.tstest/main/agent/deepchat/runtime/sessionIdentityService.test.tstest/main/app/startupMigrations/sessionDataMigrations.sqlite.test.tstest/main/app/startupMigrations/sessionDataMigrations.test.tstest/main/data/mainDatabase.test.tstest/main/orchestration/liveDelegationMigration.test.tstest/main/orchestration/liveDelegationRepository.test.tstest/main/orchestration/liveDelegationService.test.tstest/main/scheduler/schedulerService.test.tstest/main/session/data/tapeFork.test.tstest/main/session/data/tapeViewManifest.test.tstest/main/session/runtimeIntegration.test.tstest/main/tape/canonicalJson.test.tstest/main/tape/executionContract.test.tstest/main/tape/taskContract.test.tstest/main/tape/taskContractPersistence.test.tstest/main/tape/taskEvaluation.test.tstest/main/tool/agentTools/agentToolDependencies.tstest/main/tool/toolService.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main/agent/deepchat/resources/systemPromptBuilder.ts`:
- Around line 197-205: Update the active-skill filtering in the system prompt
builder around normalizeSkillMetadata so metadata lookup failure does not remove
requested skills; distinguish successful empty metadata from a failed lookup,
retain the pinned_skill_unavailable degradation, and allow loadSkillContent to
attempt the requested skills when metadata is unavailable. Add a focused Vitest
regression covering this user-visible failure path.
🪄 Autofix
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 Plus
Run ID: b267d442-4f2e-439a-8f1e-eb1ed5229aa0
📒 Files selected for processing (13)
src/main/agent/deepchat/resources/promptAssembly.tssrc/main/agent/deepchat/resources/systemPromptBuilder.tssrc/main/orchestration/liveDelegationRepository.tssrc/main/orchestration/liveDelegationService.tssrc/main/tape/domain/taskContract.tssrc/shared/types/task-contract.tstest/main/agent/deepchat/resources/promptAssembly.test.tstest/main/agent/deepchat/resources/systemPromptBuilder.test.tstest/main/agent/deepchat/runtime/process.test.tstest/main/orchestration/liveDelegationRepository.test.tstest/main/orchestration/liveDelegationService.test.tstest/main/tape/taskContract.test.tstest/main/tape/taskEvaluation.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- test/main/agent/deepchat/resources/systemPromptBuilder.test.ts
- test/main/tape/taskContract.test.ts
- test/main/agent/deepchat/runtime/process.test.ts
- src/main/orchestration/liveDelegationService.ts
- src/main/tape/domain/taskContract.ts
- src/shared/types/task-contract.ts
- src/main/orchestration/liveDelegationRepository.ts
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)
src/main/app/composition.ts (2)
1146-1151: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse one fail-closed subagent policy predicate.
At Line 1149,
!== falsetreats an unsetsubagentEnabledvalue as enabled. The session projection uses=== trueat Line 1180. A missing or legacy setting can therefore pass execution authority checks while the session reports subagent capability as disabled. Normalize this setting once and use the same value in both paths.Proposed fix
- agentPolicyEnabled: agentConfig.subagentEnabled !== false, + agentPolicyEnabled: agentConfig.subagentEnabled === 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 `@src/main/app/composition.ts` around lines 1146 - 1151, Normalize the subagent-enabled setting once in the surrounding composition flow using a fail-closed predicate (`=== true`), then reuse that normalized value for both `resolveDeepChatSubagentCapability` and the session capability projection. Replace the inline `agentConfig.subagentEnabled !== false` check while preserving the existing `=== true` behavior and keeping both paths consistent.
1497-1500: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDo not write the stable session identifier to the error log.
The new warning records
update.sessionIdin application logs. Remove this field or replace it with a redacted correlation identifier.Proposed fix
logger.warn('[YoBrowser] Failed to release inactive preview', { - sessionId: update.sessionId, error })🤖 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/main/app/composition.ts` around lines 1497 - 1500, Remove update.sessionId from the logger.warn payload in the releaseInactivePreview error handler, or replace it with an approved redacted correlation identifier while preserving the error details and warning context.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 `@src/main/app/composition.ts`:
- Around line 1146-1151: Normalize the subagent-enabled setting once in the
surrounding composition flow using a fail-closed predicate (`=== true`), then
reuse that normalized value for both `resolveDeepChatSubagentCapability` and the
session capability projection. Replace the inline `agentConfig.subagentEnabled
!== false` check while preserving the existing `=== true` behavior and keeping
both paths consistent.
- Around line 1497-1500: Remove update.sessionId from the logger.warn payload in
the releaseInactivePreview error handler, or replace it with an approved
redacted correlation identifier while preserving the error details and warning
context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71128fbd-7174-44cf-b49d-2257f4d0c154
📒 Files selected for processing (11)
src/main/agent/deepchat/harness/createDeepChatAgentHarness.tssrc/main/agent/deepchat/loop/ports.tssrc/main/agent/deepchat/runtime/deferredToolExecutor.tssrc/main/agent/deepchat/runtime/dispatch.tssrc/main/agent/deepchat/runtime/interactionCoordinator.tssrc/main/agent/deepchat/runtime/turnCoordinator.tssrc/main/app/composition.tssrc/main/tool/index.tssrc/shared/types/agent-interface.d.tstest/main/agent/deepchat/harness/deepChatAgentHarness.test.tstest/main/tool/toolService.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/agent/deepchat/harness/createDeepChatAgentHarness.ts
- src/shared/types/agent-interface.d.ts
- test/main/tool/toolService.test.ts
- src/main/agent/deepchat/runtime/deferredToolExecutor.ts
- src/main/agent/deepchat/runtime/interactionCoordinator.ts
- src/main/agent/deepchat/loop/ports.ts
- src/main/tool/index.ts
- src/main/agent/deepchat/runtime/dispatch.ts
- test/main/agent/deepchat/harness/deepChatAgentHarness.test.ts
- src/main/agent/deepchat/runtime/turnCoordinator.ts
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
Requesting changes because the implementation scope is substantially wider than the product contract that exists today.
-
The only production TaskContract producer,
createLiveDelegationTaskContractInput(), always emits one fixedrequired_sectionsrequirement for six Markdown headings. There is nosrc/caller that creates aresult_schemarequirement; all current references are inside the domain/type implementation. Please remove the dormantresult_schemabranch, the Ajv runtime dependency, the regex/schema safety machinery, and their test matrix from this PR. Reintroduce them with the concrete user/API/configuration path that can actually supply such a requirement. -
The current evaluator marks a result
passed/acceptedwhen those six sections merely have non-empty bodies. That proves output shape, not completion of the delegated task. Do not expose this as a task-success verdict. Either narrow the terminology/contract to format validation, or add a real task-specific acceptance input before assigningpassed/accepted. -
Although strict manifest admission is limited to subagents,
DeepChatLoopRunnerbuilds schema-v5 ExecutionContracts for every ordinary interactive View, and every successfully built contract is carried into ToolService dispatch checks. This puts prompt/tool hashing, manifest persistence, workdir/tool identity checks, and deferred bindings on the normal-chat hot path for a delegation feature. Restrict generation and dispatch enforcement to contract-bearing child sessions in V1, or split global interactive-View attestation into a separately motivated PR with its own failure/performance analysis.
After reducing the scope, prune tests that only prove the unused schema/canonicalization machinery. Keep the observable contracts: delegation settlement, atomic persistence/recovery, and live authority revocation. Also move the explicitly noted scheduler/startup-migration baseline repairs out of this feature PR unless a direct dependency is documented.
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/main/tape/domain/taskEvaluation.ts (1)
61-87: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHash and size the raw terminal candidate.
Lines 61-67 hash the trimmed candidate. Two outputs that differ only in leading or trailing bytes then have the same candidate hash, byte count, and evaluation idempotency identity. Preserve trimming only for blank detection and section evaluation. Hash and measure the original non-empty input.
Proposed fix
- const candidateResult = input.candidateResult?.trim() || null + const rawCandidateResult = input.candidateResult ?? '' + const candidateResult = rawCandidateResult.trim() || null const candidate = candidateResult ? { kind: 'answer' as const, - sha256: createHash('sha256').update(candidateResult, 'utf8').digest('hex'), - utf8Bytes: Buffer.byteLength(candidateResult, 'utf8') + sha256: createHash('sha256').update(rawCandidateResult, 'utf8').digest('hex'), + utf8Bytes: Buffer.byteLength(rawCandidateResult, 'utf8') }🤖 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/main/tape/domain/taskEvaluation.ts` around lines 61 - 87, Update candidate construction in the task evaluation flow to retain the original non-empty input for SHA-256 hashing and UTF-8 byte measurement, rather than using the trimmed value. Continue using the trimmed candidate only for blank detection and evaluateRequirements, preserving the existing absent-candidate behavior.
🤖 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/architecture/tape-system.md`:
- Around line 234-241: Clarify the terminal evaluation contract in the
architecture section by distinguishing live delegation state “idle” from
evaluation disposition “parked.” Add an explicit mapping table covering
completed, failed, cancelled, interrupted, and indeterminate outcomes, defining
their verdict and disposition; enforce that only passed maps to accepted and
every other outcome maps to parked, so terminal consumers apply one settlement
rule.
---
Outside diff comments:
In `@src/main/tape/domain/taskEvaluation.ts`:
- Around line 61-87: Update candidate construction in the task evaluation flow
to retain the original non-empty input for SHA-256 hashing and UTF-8 byte
measurement, rather than using the trimmed value. Continue using the trimmed
candidate only for blank detection and evaluateRequirements, preserving the
existing absent-candidate behavior.
🪄 Autofix
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 Plus
Run ID: 33f513bd-a486-4d67-b97e-c7046160179a
📒 Files selected for processing (19)
docs/architecture/proactive-multi-agent-orchestration/spec.mddocs/architecture/tape-contract-lineage/plan.mddocs/architecture/tape-contract-lineage/spec.mddocs/architecture/tape-contract-lineage/tasks.mddocs/architecture/tape-system.mdsrc/main/agent/deepchat/runtime/deepChatLoopRunner.tssrc/main/agent/deepchat/runtime/turnCoordinator.tssrc/main/orchestration/liveDelegationService.tssrc/main/orchestration/liveDelegationTaskContract.tssrc/main/tape/domain/taskContract.tssrc/main/tape/domain/taskEvaluation.tssrc/shared/types/task-contract.tstest/main/agent/deepchat/harness/deepChatAgentHarness.test.tstest/main/orchestration/liveDelegationRepository.test.tstest/main/orchestration/liveDelegationService.test.tstest/main/tape/executionContract.test.tstest/main/tape/taskContract.test.tstest/main/tape/taskContractPersistence.test.tstest/main/tape/taskEvaluation.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- test/main/tape/taskContractPersistence.test.ts
- src/main/tape/domain/taskContract.ts
- test/main/orchestration/liveDelegationRepository.test.ts
- docs/architecture/tape-contract-lineage/plan.md
- docs/architecture/proactive-multi-agent-orchestration/spec.md
- src/main/agent/deepchat/runtime/turnCoordinator.ts
- test/main/tape/executionContract.test.ts
- test/main/orchestration/liveDelegationService.test.ts
- src/main/orchestration/liveDelegationService.ts
- src/shared/types/task-contract.ts
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
The previous scope/design blockers are resolved, and the current CI plus focused local validation pass. One persisted-data compatibility issue still blocks merge.
2 participants
Summary
Add end-to-end contract lineage for delegated DeepChat execution, from a frozen TaskContract through contract-bearing provider Views, tool dispatch, Handoff format evaluation, and parent-visible settlement.
V1 is intentionally scoped to DeepChat child execution: ordinary interactive chat and ACP compatibility remain on schema-v4 ViewManifests without ExecutionContract construction or dispatch enforcement.
What changed
inspect,wait,read_result, and Handoff envelopes.Scope clarifications from review
result_schemabranch, direct Ajv dependency, regex/schema safety machinery, and tests because V1 has no production producer for those requirements.passed/failedplusaccepted/parkedwithevaluationKind=handoff_formatandformatStatus=valid|invalid|indeterminate.taskHarness.acceptanceremains the stored field name, and a golden digest locks the required-sections-only identity.Reliability and compatibility
Validation
Passed:
pnpm run format:check— 2,713 filespnpm run i18npnpm run lintpnpm run typecheckpnpm run test:maincompleted with 568 files / 6,952 tests passing, 1 file / 5 tests skipped, and 3 files / 11 tests failing after restoring the five unrelated baseline owner files toorigin/dev. The failures are eight stale MainDatabase API fixtures, one scheduler provider-config fixture, and two startup-migration missing-table fixtures; those owner files are absent from this PR's final diff.Summary by CodeRabbit
New Features
Bug Fixes