refactor(agent): establish runtime ownership boundaries - #2023
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughDeepChat runtime ownership is reorganized around lifecycle scopes, centralized status and hook handling, durable pending-input claims, and coordinator-specific responsibilities. Runtime policies, tests, architecture documentation, enforcement scripts, baseline inventories, and ACP registry metadata are also updated. ChangesDeepChat runtime ownership
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DeepChatRuntimeCoordinator
participant PendingInputAdmissionCoordinator
participant PendingInputPump
participant TurnCoordinator
participant RunLifecycleCoordinator
Client->>DeepChatRuntimeCoordinator: queue or steer input
DeepChatRuntimeCoordinator->>PendingInputAdmissionCoordinator: admit pending input
PendingInputAdmissionCoordinator->>PendingInputPump: schedule or start drain
PendingInputPump->>TurnCoordinator: start claimed turn
TurnCoordinator->>RunLifecycleCoordinator: settle run and status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
src/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts (2)
383-387: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRecovery paths bypass the redacting logger. Both modules standardize on
logger+redactRuntimeErrorForLog, but the claim release/restore failure paths fall back to rawconsole.*with the unredacted error object, which loses structured logging and can leak error details.
src/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts#L383-L387: replaceconsole.errorwithlogger.error(..., redactRuntimeErrorForLog(restoreError)); apply the same change to theconsole.errorat Line 278.src/main/agent/deepchat/runtime/pendingInputPump.ts#L456-L467: replaceconsole.warnwithlogger.warn(..., redactRuntimeErrorForLog(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/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts` around lines 383 - 387, Replace raw recovery-path console logging with the established structured redacting logger: in pendingInputAdmissionCoordinator.ts at lines 383-387 and 278, use logger.error with redactRuntimeErrorForLog for the restore and claim-release errors; in pendingInputPump.ts at lines 456-467, use logger.warn with redactRuntimeErrorForLog(error) instead of console.warn.
197-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated lane-key format. Line 197 hardcodes
steer:${sessionId}whileacquireAttachmentAcceptanceLanebuilds${lane}:${sessionId}at Line 428. Extract a privatelaneKey(lane, sessionId)helper so the fast-path check can't silently diverge.🤖 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/pendingInputAdmissionCoordinator.ts` around lines 197 - 201, Extract a private laneKey helper that builds the shared lane/session key format, and use it in both the hasPriorSteerAcceptance check and acquireAttachmentAcceptanceLane. Remove the duplicated template literal while preserving the existing fast-path behavior.test/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.ts (1)
202-326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering
sendQueuedMessageand lane serialization. The suite exercises the steer paths well, but the send-admission path (capacity rejection,needs_user_actionshort-circuit before queueing, abort after preparation) and theattachmentAcceptanceTailsserialization guarantee are untested — both are newly introduced behavior in this coordinator.🤖 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/runtime/pendingInputAdmissionCoordinator.test.ts` around lines 202 - 326, Extend the PendingInputAdmissionCoordinator suite to cover sendQueuedMessage admission behavior: capacity rejection, returning the durable needs_user_action result without queueing, and aborting after attachment preparation. Add a lane-serialization test that exercises attachmentAcceptanceTails and verifies overlapping admissions are serialized in order. Reuse the existing createHarness and mocks, and assert the relevant queueing, settlement, and result behavior.test/main/agent/deepchat/runtime/pendingInputPump.test.ts (1)
260-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFixed microtask hops make settlement assertions flaky.
launchchains.then→.finally(async …)→.catch, plus awaits insidescheduleNextIfReady, so three hops isn't a guaranteed quiescence point. Prefervi.waitFor(...)on the actual expectation (as done at Line 548) instead offlushPromises().🤖 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/runtime/pendingInputPump.test.ts` around lines 260 - 264, Replace the fixed three-hop flushPromises() usage in the pending-input settlement assertions with vi.waitFor(...) around the actual expectation, matching the established pattern near line 548. Ensure the wait observes the intended settled state rather than relying on a fixed number of microtask turns, and remove flushPromises if no longer used.test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts (2)
756-768: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider proxy-based delegation to avoid drift with
MemoryRuntimePort.The hand-written member list must be updated whenever the port gains a method; otherwise the new member silently bypasses the installed override in tests.
♻️ Optional simplification
-function createDelegatingMemoryRuntimePort(getTarget: () => MemoryRuntimePort): MemoryRuntimePort { - return { - isEnabled: (...args) => getTarget().isEnabled(...args), - captureExecutionToken: (...args) => getTarget().captureExecutionToken(...args), - canContinueExecution: (...args) => getTarget().canContinueExecution(...args), - buildInjection: (...args) => getTarget().buildInjection(...args), - recordInjectionAccess: (...args) => getTarget().recordInjectionAccess(...args), - extractAndStore: (...args) => getTarget().extractAndStore(...args), - maybeReflect: (...args) => getTarget().maybeReflect(...args), - maybeEvolvePersona: (...args) => getTarget().maybeEvolvePersona(...args), - observeExtractionQueue: (...args) => getTarget().observeExtractionQueue?.(...args) - } -} +function createDelegatingMemoryRuntimePort(getTarget: () => MemoryRuntimePort): MemoryRuntimePort { + return new Proxy({} as MemoryRuntimePort, { + get: (_target, key: keyof MemoryRuntimePort) => { + const value = getTarget()[key] + return typeof value === 'function' + ? (...args: unknown[]) => (value as (...a: unknown[]) => unknown).apply(getTarget(), args) + : 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 `@test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts` around lines 756 - 768, Replace the hand-written delegation object in createDelegatingMemoryRuntimePort with proxy-based delegation that forwards any MemoryRuntimePort member to getTarget(), while preserving the installed override behavior and optional-member handling.
7240-7247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSingle microtask flush makes this assertion timing-sensitive.
await Promise.resolve()only drains one microtask; if the fenced-claim admission path awaits more than once, the "still pending / not re-driven" assertions could pass or fail depending on internal await depth. Prefer a bounded settle (e.g.await vi.waitFor(...)on the pending list plus theprocessStreamcount) so intent is asserted rather than tick timing.🤖 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/runtime/deepChatRuntimeCoordinator.test.ts` around lines 7240 - 7247, Update the test around queuePendingInput and the processStream assertion to use a bounded settle mechanism such as vi.waitFor, waiting until the pending input remains present and processStream has the expected invocation count. Remove the single await Promise.resolve() timing dependency while preserving the assertions that the fenced claim is not re-driven and the input remains pending.test/main/agent/deepchat/runtime/runLifecycleCoordinator.test.ts (1)
289-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLine exceeds the 100-column limit.
♻️ Proposed change
- const message = createMessage('message-paused', [createPendingAction('tool-1')], 1, '{"runId":"run-1"}') + const message = createMessage( + 'message-paused', + [createPendingAction('tool-1')], + 1, + '{"runId":"run-1"}' + )As per coding guidelines, "Use Oxfmt formatting: single quotes, no semicolons, and a line width of 100."
🤖 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/runtime/runLifecycleCoordinator.test.ts` at line 289, Reformat the createMessage call in the affected lifecycle coordinator test so it stays within the 100-column Oxfmt line width, preserving its single-quote style and behavior.Source: Coding guidelines
src/main/agent/deepchat/runtime/runLifecycleCoordinator.ts (1)
426-435: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared
loggerinstead ofconsole.warn.This file already imports
logger(Line 1) and uses it at Line 381;console.warnhere bypasses the shared logging transport/redaction. Note the assertions intest/main/agent/deepchat/runtime/runLifecycleCoordinator.test.ts(Lines 329, 353, 366-369) would need updating.♻️ Proposed change
- console.warn( + logger.warn( `[DeepChatAgent] Failed to cancel ACP permission request ${permission.requestId}:`, - error + redactRuntimeErrorForLog(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/agent/deepchat/runtime/runLifecycleCoordinator.ts` around lines 426 - 435, Update cancelProviderPermissions to replace console.warn with the file’s existing shared logger, preserving the current message and error details; update the affected runLifecycleCoordinator tests to assert against the shared logger instead of console.warn.src/main/agent/deepchat/runtime/sessionSettingsCoordinator.ts (1)
164-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate project-dir mutation logic between
setProjectDirandresolveProjectDir.Both methods repeat the same normalize → compute-previous → set → conditionally-invalidate sequence, and use two different invalidation paths (
invalidateToolProfile(sessionId)helper vs. directexpectedInstance.invalidateToolProfileCache()). Consider extracting a shared private helper that both call, keeping a single invalidation path.♻️ Proposed refactor
+ private applyProjectDir( + sessionId: string, + instance: DeepChatAgentInstance, + projectDir?: string | null + ): string | null { + const normalized = this.normalizeProjectDir(projectDir) + const previous = instance.hasProjectDir() + ? instance.getProjectDir() + : this.resolvePersistedProjectDir(sessionId) + instance.setProjectDir(normalized) + if (previous !== normalized) { + instance.invalidateToolProfileCache() + } + return normalized + } + setProjectDir(sessionId: string, projectDir: string | null): void { - const normalized = this.normalizeProjectDir(projectDir) - const instance = this.deps.getInstance(sessionId) - const previous = instance.hasProjectDir() - ? instance.getProjectDir() - : this.resolvePersistedProjectDir(sessionId) - instance.setProjectDir(normalized) - if (previous !== normalized) { - this.invalidateToolProfile(sessionId) - } + this.applyProjectDir(sessionId, this.deps.getInstance(sessionId), projectDir) } resolveProjectDir( sessionId: string, incoming?: string | null, expectedInstance = this.deps.getInstance(sessionId) ): string | null { this.deps.assertCurrent(sessionId, expectedInstance) if (incoming !== undefined) { - const normalized = this.normalizeProjectDir(incoming) - const previous = expectedInstance.hasProjectDir() - ? expectedInstance.getProjectDir() - : this.resolvePersistedProjectDir(sessionId) - expectedInstance.setProjectDir(normalized) - if (previous !== normalized) { - expectedInstance.invalidateToolProfileCache() - } - return normalized + return this.applyProjectDir(sessionId, expectedInstance, incoming) }🤖 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/sessionSettingsCoordinator.ts` around lines 164 - 206, Extract the duplicated normalize, previous-value comparison, project-directory assignment, and conditional invalidation logic from setProjectDir and resolveProjectDir into one shared private helper. Have both methods call that helper and use a single invalidation path, preserving the existing behavior for the session instance and expectedInstance.
🤖 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/runtime/pendingInputPump.ts`:
- Around line 305-354: The drain guard in drain must use an atomic
check-and-start operation before the first await, replacing the separate
canDrain check and later markPendingQueueDrainStarted call. Update the relevant
runLifecycle/scope instance API so concurrent drains for the same session cannot
both proceed, and ensure every failure or completion path releases the gate only
after its own drain finishes.
In `@src/main/agent/deepchat/runtime/runLifecycleCoordinator.ts`:
- Around line 177-179: Update clearFirstTurnReady in the lifecycle coordinator
to use getHydratedScope instead of getOrCreateScope, matching
clearDeferredToolController and ensuring clearing the flag does not recreate an
evicted or disposed session.
- Around line 280-297: The cancellation path in refreshPendingInteractions
currently terminalizes only pendingInteractions[0].messageId. Before
replacePendingInteractions([]), iterate over each distinct pending interaction
messageId, parse and stamp its metadata as aborted with user_stop, and call
settleAbortedTurn for each message while preserving the existing pending-input
drain behavior.
In `@src/main/agent/deepchat/runtime/turnCoordinator.ts`:
- Around line 748-765: Track whether the pending input turn was rolled back in
the failure path around claimedInput.settle, and after rollback clear both the
user and assistant message identifiers (or otherwise mark the turn as rolled
back). Gate the subsequent terminal-persistence block—setMessageError,
emitMessageRefresh, and chat.stream.failed—so it is skipped for rolled-back
turns while preserving existing behavior for abort, stale-instance, and other
failures.
---
Nitpick comments:
In `@src/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts`:
- Around line 383-387: Replace raw recovery-path console logging with the
established structured redacting logger: in pendingInputAdmissionCoordinator.ts
at lines 383-387 and 278, use logger.error with redactRuntimeErrorForLog for the
restore and claim-release errors; in pendingInputPump.ts at lines 456-467, use
logger.warn with redactRuntimeErrorForLog(error) instead of console.warn.
- Around line 197-201: Extract a private laneKey helper that builds the shared
lane/session key format, and use it in both the hasPriorSteerAcceptance check
and acquireAttachmentAcceptanceLane. Remove the duplicated template literal
while preserving the existing fast-path behavior.
In `@src/main/agent/deepchat/runtime/runLifecycleCoordinator.ts`:
- Around line 426-435: Update cancelProviderPermissions to replace console.warn
with the file’s existing shared logger, preserving the current message and error
details; update the affected runLifecycleCoordinator tests to assert against the
shared logger instead of console.warn.
In `@src/main/agent/deepchat/runtime/sessionSettingsCoordinator.ts`:
- Around line 164-206: Extract the duplicated normalize, previous-value
comparison, project-directory assignment, and conditional invalidation logic
from setProjectDir and resolveProjectDir into one shared private helper. Have
both methods call that helper and use a single invalidation path, preserving the
existing behavior for the session instance and expectedInstance.
In `@test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts`:
- Around line 756-768: Replace the hand-written delegation object in
createDelegatingMemoryRuntimePort with proxy-based delegation that forwards any
MemoryRuntimePort member to getTarget(), while preserving the installed override
behavior and optional-member handling.
- Around line 7240-7247: Update the test around queuePendingInput and the
processStream assertion to use a bounded settle mechanism such as vi.waitFor,
waiting until the pending input remains present and processStream has the
expected invocation count. Remove the single await Promise.resolve() timing
dependency while preserving the assertions that the fenced claim is not
re-driven and the input remains pending.
In `@test/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.ts`:
- Around line 202-326: Extend the PendingInputAdmissionCoordinator suite to
cover sendQueuedMessage admission behavior: capacity rejection, returning the
durable needs_user_action result without queueing, and aborting after attachment
preparation. Add a lane-serialization test that exercises
attachmentAcceptanceTails and verifies overlapping admissions are serialized in
order. Reuse the existing createHarness and mocks, and assert the relevant
queueing, settlement, and result behavior.
In `@test/main/agent/deepchat/runtime/pendingInputPump.test.ts`:
- Around line 260-264: Replace the fixed three-hop flushPromises() usage in the
pending-input settlement assertions with vi.waitFor(...) around the actual
expectation, matching the established pattern near line 548. Ensure the wait
observes the intended settled state rather than relying on a fixed number of
microtask turns, and remove flushPromises if no longer used.
In `@test/main/agent/deepchat/runtime/runLifecycleCoordinator.test.ts`:
- Line 289: Reformat the createMessage call in the affected lifecycle
coordinator test so it stays within the 100-column Oxfmt line width, preserving
its single-quote style and behavior.
🪄 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 Plus
Run ID: f58e4f57-e0db-40cf-b037-f711823daa73
📒 Files selected for processing (41)
docs/architecture/baselines/agent-system-layered-runtime-baseline.jsondocs/architecture/deepchat-agent-harness-boundaries/plan.mddocs/architecture/deepchat-agent-harness-boundaries/spec.mddocs/architecture/deepchat-agent-harness-boundaries/tasks.mdresources/acp-registry/registry.jsonscripts/agent-cleanup-guard.mjsscripts/generate-architecture-baseline.mjssrc/main/agent/deepchat/instance/deepChatAgentRuntime.tssrc/main/agent/deepchat/runtime/abortErrors.tssrc/main/agent/deepchat/runtime/compactionRuntimeCoordinator.tssrc/main/agent/deepchat/runtime/contextBudgetPolicy.tssrc/main/agent/deepchat/runtime/deepChatLoopRunner.tssrc/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.tssrc/main/agent/deepchat/runtime/interactionCoordinator.tssrc/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.tssrc/main/agent/deepchat/runtime/pendingInputContracts.tssrc/main/agent/deepchat/runtime/pendingInputPump.tssrc/main/agent/deepchat/runtime/preStreamWatchdog.tssrc/main/agent/deepchat/runtime/providerInputCapabilities.tssrc/main/agent/deepchat/runtime/providerPermissionCoordinator.tssrc/main/agent/deepchat/runtime/providerPermissionResolution.tssrc/main/agent/deepchat/runtime/runLifecycleCoordinator.tssrc/main/agent/deepchat/runtime/runtimeHookSink.tssrc/main/agent/deepchat/runtime/sessionSettingsCoordinator.tssrc/main/agent/deepchat/runtime/sessionStatusPublisher.tssrc/main/agent/deepchat/runtime/turnCoordinator.tssrc/main/agent/manager/deepChatAgentBackend.tssrc/main/session/data/pendingInputs.tstest/main/agent/deepchat/instance/deepChatAgentRuntime.test.tstest/main/agent/deepchat/runtime/abortErrors.test.tstest/main/agent/deepchat/runtime/compactionRuntimeCoordinator.test.tstest/main/agent/deepchat/runtime/contextBudgetPolicy.test.tstest/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.tstest/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.tstest/main/agent/deepchat/runtime/pendingInputPump.test.tstest/main/agent/deepchat/runtime/preStreamWatchdog.test.tstest/main/agent/deepchat/runtime/providerInputCapabilities.test.tstest/main/agent/deepchat/runtime/runLifecycleCoordinator.test.tstest/main/agent/deepchat/runtime/runtimeHookSink.test.tstest/main/agent/deepchat/runtime/sessionStatusPublisher.test.tstest/main/scripts/agentCleanupGuard.test.ts
💤 Files with no reviewable changes (1)
- src/main/agent/manager/deepChatAgentBackend.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.ts (1)
202-208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
createDeferredis duplicated verbatim intest/main/agent/deepchat/runtime/pendingInputPump.test.ts.Consider moving it to a shared test utility so both runtime pump/admission suites use one implementation.
🤖 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/runtime/pendingInputAdmissionCoordinator.test.ts` around lines 202 - 208, Move the duplicated createDeferred helper into a shared test utility, then update pendingInputAdmissionCoordinator.test.ts and pendingInputPump.test.ts to import and reuse that single implementation, removing their local definitions.src/main/agent/deepchat/runtime/pendingInputPump.ts (1)
278-290: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDistinguish lease contention from claim-adoption failure in the log.
Losing the drain lease race is an expected, recoverable outcome (the claim is released and a drain is rescheduled), yet it is logged at
errorwithstage=adopt-claim, the same signature used for genuine stale-instance failures indrain. This makes the log misleading and noisy under normal concurrency.♻️ Suggested logging split
const drainLease = scope?.instance.tryAcquirePendingQueueDrain() ?? null if (!scope || !drainLease) { const released = this.tryRelease(claim) - logger.error( - `[DeepChatAgent] pending input start rejected session=${record.sessionId} stage=adopt-claim` - ) + const stage = scope ? 'drain-lease-unavailable' : 'adopt-claim' + const log = scope ? logger.info : logger.error + log( + `[DeepChatAgent] pending input start rejected session=${record.sessionId} stage=${stage} released=${released}` + ) if (released) { this.schedule(record.sessionId, 'enqueue') } return }🤖 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/pendingInputPump.ts` around lines 278 - 290, The pending-input rejection path currently conflates missing scope, drain-lease contention, and claim-adoption failure. Update the logic around tryAcquirePendingQueueDrain and tryRelease so expected lease contention uses a distinct non-error log signature, while genuine missing-scope or stale-instance adoption failures retain the existing error logging and stage=adopt-claim context; preserve release and enqueue rescheduling 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.
Nitpick comments:
In `@src/main/agent/deepchat/runtime/pendingInputPump.ts`:
- Around line 278-290: The pending-input rejection path currently conflates
missing scope, drain-lease contention, and claim-adoption failure. Update the
logic around tryAcquirePendingQueueDrain and tryRelease so expected lease
contention uses a distinct non-error log signature, while genuine missing-scope
or stale-instance adoption failures retain the existing error logging and
stage=adopt-claim context; preserve release and enqueue rescheduling behavior.
In `@test/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.ts`:
- Around line 202-208: Move the duplicated createDeferred helper into a shared
test utility, then update pendingInputAdmissionCoordinator.test.ts and
pendingInputPump.test.ts to import and reuse that single implementation,
removing their local definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4c68f29-14f3-44cf-b322-8e3ad1f019ff
📒 Files selected for processing (17)
docs/architecture/baselines/agent-system-layered-runtime-baseline.jsondocs/architecture/deepchat-agent-harness-boundaries/plan.mddocs/architecture/deepchat-agent-harness-boundaries/spec.mddocs/architecture/deepchat-agent-harness-boundaries/tasks.mdscripts/agent-cleanup-guard.mjssrc/main/agent/deepchat/instance/deepChatAgentInstance.tssrc/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.tssrc/main/agent/deepchat/runtime/pendingInputPump.tssrc/main/agent/deepchat/runtime/runLifecycleCoordinator.tssrc/main/agent/deepchat/runtime/sessionSettingsCoordinator.tssrc/main/agent/deepchat/runtime/turnCoordinator.tstest/main/agent/deepchat/instance/deepChatAgentRuntime.test.tstest/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.tstest/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.tstest/main/agent/deepchat/runtime/pendingInputPump.test.tstest/main/agent/deepchat/runtime/runLifecycleCoordinator.test.tstest/main/scripts/agentCleanupGuard.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- test/main/scripts/agentCleanupGuard.test.ts
- docs/architecture/deepchat-agent-harness-boundaries/tasks.md
- test/main/agent/deepchat/runtime/runLifecycleCoordinator.test.ts
- test/main/agent/deepchat/runtime/pendingInputPump.test.ts
- docs/architecture/baselines/agent-system-layered-runtime-baseline.json
- src/main/agent/deepchat/runtime/sessionSettingsCoordinator.ts
- src/main/agent/deepchat/runtime/runLifecycleCoordinator.ts
- scripts/agent-cleanup-guard.mjs
- src/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts
- docs/architecture/deepchat-agent-harness-boundaries/spec.md
- src/main/agent/deepchat/runtime/turnCoordinator.ts
- test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts
This was referenced Jul 25, 2026
This was referenced Jul 27, 2026
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
This PR decomposes
DeepChatRuntimeCoordinatorinto explicit runtime owners while preserving existing public APIs, durable queue semantics, Tape/Memory behavior, provider behavior, and visible-turn semantics except for the documented corrections below.SessionRuntimeScopeidentity and stale-instance fencing.RunLifecycleCoordinator.PendingInputAdmissionCoordinator.PendingInputPump.The design and implementation stages are documented in:
docs/architecture/deepchat-agent-harness-boundaries/spec.mddocs/architecture/deepchat-agent-harness-boundaries/plan.mddocs/architecture/deepchat-agent-harness-boundaries/tasks.mdIncluded Runtime Corrections
This refactor intentionally retains three explicit fixes and five ownership corrections because subsequent extraction rewrote or relocated the same control flow:
Each correction is pinned by focused regression coverage, including an explicit draining + question-follow-up overlap test.
Reliability And Diagnostics
Summary by CodeRabbit