refactor: retire SDK-superseded code-mode scaffolding and rehome cell admission - #3225
Conversation
📝 WalkthroughWhat problem this solvesThis PR makes It replaces the custom limit model and mapping layer with It removes the local cell queue. The SDK now controls concurrency through its process-global worker cap. It centralizes JSON byte counting in Source of truthThe PR extends existing sources of truth:
It does not create a parallel execution path. The production code-mode path is currently unreachable because no code assigns Solution size and simplificationThe PR removes 165 lines of duplicated scaffolding:
The remaining tests cover execution-policy limits and byte-counting equivalence. Further deletion could weaken regression coverage. Complexity deltaThe PR removes:
The PR adds:
Authority, state, branches, configuration translation, and test-maintenance burden decrease. Total maintenance complexity decreases. Validation and concrete risksThe reported checks passed for code-mode and core tests, runtime typecheck, focused suites, formatting, and linting. The repository-wide suite was not run locally. The supplied shell result did not provide direct changed-file or check-result evidence. Cells below the SDK worker cap now run in parallel. This changes execution timing and may affect workloads that depended on serialization. The removed queue tests no longer cover queued-cell cancellation. SDK worker-cap and isolation behavior must remain valid.
Review-relevant risksThe PR changes public code-mode configuration contracts, execution concurrency, and exported package APIs. Material changes in these areas require independent human review under repository policy. The PR changes shared JSON byte-counting behavior for permission, sandbox, and tool-runtime limits. Material changes to resource-limit enforcement require independent human review under repository policy. The person performing the merge reviews the final diff, and a maintainer makes the final determination. WalkthroughCode Mode now uses SDK execution policies with frozen defaults and direct QuickJS execution. Core provides a shared bounded JSON byte-length helper, which replaces local implementations and is consumed by runtime validation and output handling. ChangesExecution policy and serialization
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to This refactor delegates code execution and concurrency to the SDK and centralizes JSON byte counting. The new helper misreports top-level undefined, which could affect validation if such a payload reaches it; current documented call sites use supported values, so the PR is mergeable with owner awareness or a small follow-up fix. Sequence Diagram(s)sequenceDiagram
participant Caller
participant executeCodeCell
participant runCodeMode
participant RuntimeBackend
Caller->>executeCodeCell: provide executionPolicy overrides
executeCodeCell->>runCodeMode: pass merged policy
runCodeMode->>RuntimeBackend: apply maxToolOutputBytes
RuntimeBackend-->>runCodeMode: bounded nested tool output
runCodeMode-->>Caller: return CodeModeExecutionResult
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoSimplify code-mode around SDK policies and shared byte counting
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/code-mode/src/index.ts (1)
11-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport cycle between
index.tsandquickjs.ts.
index.tsre-exportsexecuteCodeCellfrom./quickjs.js, andquickjs.tsimportsDEFAULT_CODE_MODE_EXECUTION_POLICYfrom./index.js. The cycle is currently safe:quickjs.tsreads the constant inside the function body, not during module evaluation, so no TDZ error occurs.Disposition: optional. If a future change moves that read to module scope, the cycle becomes a TDZ crash. Consolidating the constant into its own module removes the cycle without adding public surface.
As per path instructions: "Choose remedies in this order: delete an unnecessary path, consolidate duplicated authority, reuse the closest existing seam".
Also applies to: 66-66
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56833fd9-8a7c-47d7-9234-c70d3a7404c0
📒 Files selected for processing (10)
packages/code-mode/src/__tests__/code-mode.test.tspackages/code-mode/src/index.tspackages/code-mode/src/quickjs.tspackages/core/package.jsonpackages/core/src/__tests__/serialized-byte-length.test.tspackages/core/src/additional-permissions.tspackages/core/src/sandbox-boundary.tspackages/core/src/serialized-byte-length.tspackages/runtime/src/ai-sdk-backend.tspackages/runtime/src/tool-runtime.ts
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more' |
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Findings
- [P1] Removing the local queue drops full-cell lifecycle admission. The SDK worker cap is released before aborted host operations finish draining, so repeated cancellation can accumulate unbounded host-side work beyond
maxWorkers. - [P2] The new generic policy merge accepts
null, which causes the SDK's??resolution to silently replace Maka's tighter limits with looser SDK defaults.
Problem and mechanism
The PR removes a lossless limits-renaming layer, delegates cell concurrency to run, and consolidates JSON byte counting in Core. Removing the rename layer and duplicate counters is sound. The concurrency premise is incomplete: the old admission state bounded the complete cell lifecycle, while the SDK cap bounds only active QuickJS workers.
First principles and optimality
The solution is not yet optimal. Resource admission must cover every operation whose lifetime and side effects belong to the admitted cell. A lower-level worker permit cannot replace that ownership boundary when host work may outlive the worker after cancellation. Policy overrides must also fail closed rather than widening limits through invalid runtime values.
The simplest final structure is:
- Runtime owns bounded admission across
runCodeModeplus host-operation drainage. runretains responsibility for its process-global QuickJS worker cap.- The adapter validates a closed set of positive-integer policy overrides before calling the SDK.
- Core remains the single owner of package-neutral serialized byte counting.
Deletion and tests
The legacy field-by-field rename mapping can remain deleted. Do not restore the previous queue implementation verbatim; replace it with a small full-lifecycle admission primitive at the Runtime ownership boundary.
The deleted queue-specific tests were correctly removed, but they need replacement coverage for overlapping cells, repeated abort/timeout waves, and overflow. Add a null/invalid-policy regression test. The new undefined byte-count test should describe the actual boundary policy rather than claiming that ToolRuntime publishes null; the SDK uses an empty sentinel and ToolRuntime durably publishes empty text content.
Merge verdict
Not ready to merge at e615eec6a5d5552be3d5ee371f8f8029d756f6d7.
Verification
I reviewed the complete diff, current dependency source, live CI, and the PR merged onto current main. The merged revision passed:
- Code Mode: 29 tests
- Core: 564 tests
- Focused Runtime: 37 tests
- Biome on all changed source files
- 10,000 randomized JSON-compatible byte-count comparisons
GitHub CI is green; E2E remains skipped. The cancellation and null-override failures were independently reproduced against the reviewed head.
Astro-Han
changed the title
CodeModeLimits was a 1:1 rename layer over @ai-sdk/code-mode's CodeModeExecutionPolicy, forcing every call site through a mapping table that carried no information. Declare the product defaults in the SDK's own shape and pass them straight through. The defaults themselves are unchanged: Maka tightens maxSourceBytes, maxBridgeRequests, maxInFlightBridgeRequests, maxToolOutputBytes and maxConsoleOutputBytes below the SDK's, so the constant stays. An explicit `undefined` override still keeps the product default rather than falling through to the SDK's looser one. Generated-by: Claude Code
executeCodeCell serialized every cell to one at a time behind a depth-one queue, rejecting the third caller with `limit_exceeded`. The SDK already governs this: each runCodeMode call builds its own runner, and `run` admits invocations against a process-global worker cap (memory-derived, capped at 32) with per-invocation QuickJS contexts, raising RUN_CONCURRENCY_LIMIT past the cap. That surfaces as CODE_MODE_CONCURRENCY_LIMIT, which the adapter already maps to `limit_exceeded`, so the overflow diagnostic is unchanged — only the threshold moves from one cell to the worker cap. Verified against @ai-sdk/code-mode@1.0.23 and run@2.0.0: nothing requires serial reuse of a QuickJS instance, and eight concurrent cells each observe a fresh global scope. Generated-by: Claude Code
Three hand-written counters answered the same question — how many UTF-8 bytes a value's JSON representation occupies. @maka/code-mode carried a bounded, early-exit implementation; additional-permissions.ts and sandbox-boundary.ts each carried a byte-identical JSON.stringify wrapper. Move the bounded implementation to @maka/core/serialized-byte-length and point all three call sites at it. The two core call sites pass freshly built, structurally validated payloads — plain objects, arrays, strings and booleans, with no undefined, function, toJSON hook or cycle — for which the bounded counter and JSON.stringify agree byte for byte, including escapes, control characters, lone surrogates and multi-byte sequences. A test pins that agreement. Generated-by: Claude Code
Review flagged that serializedByteLength counts a top-level `undefined` as four bytes where JSON.stringify reports it as unrepresentable. The observation is accurate, but the behavior is load-bearing rather than a defect: tool-runtime bounds every nested tool result with this counter, so reporting infinity would reject a tool that simply returned nothing as though its result were too large. `null` is what callers publish in place of an absent value, and four bytes is what that costs. Document the departure at the counter's contract and pin it from both ends — the counter itself, and the nested-tool result bound that depends on it. Both tests fail if the top-level case is changed to report infinity. Generated-by: Claude Code
This reverts commit 5438239. The premise was wrong: the queue was not duplicate governance. Its admission covered the complete cell lifecycle, including the host-operation drain that follows `runCodeMode`. The SDK's worker cap cannot stand in for that. On cancellation `runCodeMode` releases its worker slot and rejects at once, by design, while host operations started from the cell may still be running with durable side effects. Only Maka waits for those, so only Maka can bound how many cells are outstanding. Measured with the worker cap pinned to one, cancelling each cell while its tool ignores cancellation: the queue holds one host operation outstanding, its removal accumulated four. The queue is restored verbatim rather than replaced. It already is a full-lifecycle admission primitive; it was simply never named as one, which is what made it read as superseded. The missing piece was the stated invariant, now recorded where the admission state is declared. Widening the bound to match the SDK worker cap would need evidence that concurrent cells are wanted, and no such evidence exists today. Reported-by: hqhq1025 Generated-by: Claude Code
…imits
Moving to the SDK's policy shape introduced a regression. The merge loop
excluded only `undefined`, so a `null` override was copied through, and
the SDK resolves its policy with `??` — restoring the SDK's looser
default rather than the tighter one this package ships. The previous
field-by-field mapping used `??` at the boundary and was safe: a `null`
kept the product default.
Measured on the reviewed head: a 70 KiB source was rejected at the
64 KiB default and under an explicit `undefined`, but ran under
`{ maxSourceBytes: null }`, which restored the SDK's 256 KiB. The same
widening reached tool output, bridge requests, in-flight requests and
console output.
Admit overrides only for known policy fields, and only as positive
integers, matching the SDK's own limit validity rule. Every other
runtime value — `null`, zero, negative, fractional, a string, an object,
an unknown key — keeps the product default, so the boundary fails
closed. The type system does not admit these, but a JavaScript or JSON
caller can still produce them.
Reported-by: hqhq1025
Generated-by: Claude Code
The contract for the top-level `undefined` byte count named the wrong mechanism. It claimed a tool result is published as `null` through result-content coercion; `coerceResultContent(undefined)` actually produces empty text. Only the Code Mode cell path substitutes `null`, through `value ?? null`. The behavior is unchanged and still correct, but the reason needed restating: an absent result is not an oversized one, and four bytes is a conservative bound on what either path publishes. That is the property the byte bound depends on, and it holds for both. This matters beyond the wording. The inaccurate mechanism was the evidence offered when the earlier review finding on this line was withdrawn, so the record is corrected too. Reported-by: hqhq1025 Generated-by: Claude Code
The bound on outstanding cells lived in @maka/code-mode as module-level state, which put it in the wrong place twice over. It was never named. `codeCellActive` read as "one cell at a time", while what it actually provided was admission across a cell's complete lifecycle, host-operation drain included — the flag spanned the whole promise and the drain sits inside it. That gap between name and substance is what made it look superseded by the SDK worker cap, and is why deleting it removed a guarantee nothing else provided. Its scope was an accident of module loading. One module-level flag is shared by every session in the process, so one session's cell made another session's cell queue and a third fail outright. Host operations run through a session's own ToolRuntime, so the session is the granularity whose side effects need bounding. Move it to AiSdkBackend, which owns cell execution, reaches those host operations through `scope.toolRuntime`, and is built per session. Acquire and release sit in one method around one call, so the permit covers exactly what `executeCodeCell` promises to finish. @maka/code-mode returns to being a stateless adapter and now states the property the Runtime depends on: its promise settles only after the cell's host operations have drained. The bound itself is unchanged — one active cell, one queued, the third turned away — so what moves is who owns it and at what scope, not how much runs concurrently. Per-session scoping does mean cells in different sessions no longer block each other, which the shared flag did. Coverage moves with it, plus a repeated-cancellation case the previous structure had no place to express. Reported-by: hqhq1025 Generated-by: Claude Code
The admission primitive had unit coverage, but nothing exercised the acquire/release wiring in `AiSdkBackend.executeCodeModeCell`. Stubbing the primitive to a no-op left 218 backend tests green — re-deleting the wiring, which is the exact mistake this branch exists to correct, would not have turned CI red. Add a backend test that drives three cells through one backend while a host tool is parked, asserting that the third is turned away and that a queued cell starts no host work until the first releases. Two mutations turn it red: a no-op admission fails it outright, and releasing the permit before the host drain — the sandbox worker cap's semantics — times it out. Also state the bound's real scope. It spans a backend generation, not a session, since each `AiSdkBackend` holds its own instance; a generation being replaced can still be draining a cancelled cell while its successor admits one. Rebuilds are configuration-driven, so this does not compound, and closing it means giving the bound a home that outlives the backend, which belongs to the package fold. The multi-wave test now cancels for real. It was named for cancellation but only cycled acquire/release, so a bug that freed the active permit on abort would have slipped past it; it now fails on that mutation. Generated-by: Claude Code
With the SDK-superseded scaffolding gone, the package was 224 lines of integration glue over `@ai-sdk/code-mode` — the product execution policy, the result shapes the backend publishes, and the adapter that bridges sandbox tool calls onto host tools and waits for them to drain. At that size a standalone workspace costs more than it explains: its only consumer is `AiSdkBackend`, which now imports it as a sibling. `index.ts` and `quickjs.ts` merge into `packages/runtime/src/code-mode.ts`, since the split only existed to give the package a public entry point distinct from its implementation. Its tests move alongside. The workspace manifest, tsconfig and project reference, both build chains, the desktop and CLI build chains, the stale-dist pairs, the release script's internal-package and build-order lists, the CLI validation path filters, and the Windows sandbox build step all drop their entry. `@ai-sdk/code-mode` moves from the deleted package's dependencies to the runtime's, so the release script's `npm ls` closure still reaches it; both third-party notice inventories already listed it under its own name and are unchanged. The dependabot ignore for typescript majors kept a comment naming the deleted path. Its subject — an in-house transpiler on the typescript JS API pinned to 5.9 — no longer exists anywhere in the repo, and no workspace pins typescript. The comment now says so; whether to drop the ignore is a dependency-policy call left to a maintainer. Generated-by: Claude Code
`CodeCellAdmission` was a second implementation of a primitive the runtime already had. `ChildAgentRunLimiter` is the same thing — an abort-aware FIFO permit pool with a capacity, waiters that remove themselves on abort without freeing the active slot, and an idempotent release the hand-written class lacked. The two differed only in what happens when full: child runs queue, cells past the first waiter are turned away. That difference does not need a second class. A caller that must turn work away reads `waitingCount` before calling `acquire`; nothing awaits between that read and the enqueue inside `acquire`, so the pair is atomic. The turn-away policy is a product decision about cells, so it reads better at the call site than inside a permit pool anyway. The class is renamed `AdmissionLimiter`, since it now serves two boundaries at two lifetimes: child runs take one instance per turn, rebuilt by `resetTurnState`, while cells take one per backend, which has to outlive a turn — a cancelled cell still draining host operations is exactly what the bound exists to cover. Different lifetimes call for different instances, not different implementations. Both mutations still fail the wiring test: an `acquire` that always admits fails it outright, and dropping the turn-away precheck times it out. Deleting the duplicate drops 205 lines for 60. Also correct two things review surfaced. The multi-wave cancellation test went red on the same mutation as the single-wave one — the state is one flag and one slot, so a single wave exhausts it — and it goes away with the class it covered. And the byte-length agreement payload omitted \b, \f and \r, so dropping any of them from the two-byte escape set left the test green; the payload now carries every one. Generated-by: Claude Code
41cdb63 to
ac0ec4d
Compare
|
Rebased onto current
Also folded
@hqhq1025 ready for another look. |
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Re-review of exact head ac0ec4da3578c7dc15b0a358313fef701b80f135.
The two previously blocking issues are fixed. Admission now belongs to the backend that owns the complete cell lifecycle, and the permit remains held until sandbox execution and every started host operation have settled. The policy merge also rejects null and other non-positive/non-integer values instead of falling back to the SDK's looser defaults. Repeated cancellation/admission runs passed 10/10 locally, and the focused Core/Runtime suites passed 80/80.
One merge-blocking policy-boundary regression remains: replacing the former product-specific CodeModeLimits mapping with the SDK policy shape exposes an override that Maka previously kept fixed. The inline finding includes a direct reproducer and the related SDK-range validation gap.
Problem definition and mechanism: removing the private @maka/code-mode package and SDK-superseded scaffolding is sound, but Maka still owns host-operation admission and its stricter execution policy. The package fold, generalized limiter, and shared serialized-byte counter are the right first-principles boundaries. Reusing one limiter and deleting the package/build glue follow Occam's razor; no deeper architecture change is required.
Deletion/test assessment: the obsolete package and duplicate byte-counting implementation are correctly deleted. I found no additional production code that should be removed. The backend admission test and direct cancellation/drain test cover different seams and are both useful; no low-quality test should be deleted.
Merge verdict: not ready to merge until the override surface is restored to the previous product boundary and the SDK's numeric range is enforced with regression coverage. Separately, the Windows package job reached successful build, update, installer handoff, and relaunch, then failed while cleaning up an already-exited PID. That appears unrelated to this PR's code and is also the subject of PR #3327, but the required check should still be rerun before merge.
Folding `@maka/code-mode` into the runtime replaced the product-specific `CodeModeLimits` with the SDK's own policy shape, and the field-by-field merge that came with it iterated every SDK key. That widened the override surface by exactly one field: `maxConsoleOutputBytes`, which the removed adapter pinned to 1 and `CodeModeLimits` never exposed. At 1 a cell's `console.log` produces nothing; at 100 it reaches the host process's stdout, which the CLI writes its TUI and command output to. The merge also promised more than it delivered. Its predicate admitted any positive integer, but the SDK rejects a value above 2,147,483,647 as invalid and then skips that check entirely -- `assertSourceSize` returns early -- so an out-of-range override disabled a limit rather than keeping the default the comment claimed. `executionPolicy` has no production caller: the backend passes none, and the module is not on the package's export surface. It exists so tests can reach a limit they cannot practically hit at its default, such as the 30s deadline. So it now takes a complete policy instead of a partial one. Every question the merge raised -- null, undefined, zero, negatives, floats, strings, out of range, and which fields may be tuned at all -- stops being representable rather than being guarded, and the two tests that guarded them go with it. What was missing instead was coverage of the property itself. Nothing pinned that a cell's console output stays out of host stdout. The new test observes a probe cell from outside the process, since the sandbox writes from a worker thread Node pipes into the parent's stdout, and it fails when the default is raised from 1. Generated-by: Claude Code
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Approved at exact head 0ed2085f6b317daf926d378ed6a3fb6beecad23d.
No actionable findings remain. The previous policy-boundary finding is fixed by removing the per-field merge rather than adding another validation layer. Production still omits executionPolicy, code-mode.ts is not on the Runtime package export surface, and the internal test seam now requires a complete SDK policy. That makes the frozen Maka policy the only policy on the product path while still allowing focused limit tests.
The new child-process regression test covers the missing product invariant directly: sandbox console output must not reach host stdout, which is also used by the CLI. I independently verified both sides of the probe: the default policy produced empty stdout, while a complete test policy with maxConsoleOutputBytes: 100 emitted the marker. The focused Core/Runtime suites passed 79/79, the policy/admission subset passed 10/10 repeated runs, Core/Storage/Runtime builds passed, Biome passed, notice inventories were current, and the CI planner tests passed 17/17.
Problem definition and mechanism: the PR correctly removes SDK-superseded workspace scaffolding while retaining the two responsibilities Maka still owns: full-cell admission through host-operation drainage and a stricter product execution policy. The backend-scoped admission limiter, stateless Code Mode adapter, shared Core byte counter, and folded Runtime dependency are the appropriate ownership boundaries.
First principles and Occam's razor: yes. The follow-up deletes the unnecessary merge and its validation tests instead of maintaining a configurable surface with no production caller. I found no further production code or low-quality tests that should be deleted, and no deeper refactor is required.
Merge verdict: the code at this head is ready. GitHub CI, dependency audit, and the Windows package check are still pending, so do not merge until the required checks complete successfully. The previous Windows cleanup race remains an external verification risk; if it repeats, the harness fix in #3327 should land and this branch should be refreshed before rerunning.
2 participants
Summary
packages/code-modepredates@ai-sdk/code-mode@1.0.23taking over most of what it did. This retires what the SDK genuinely superseded, and moves the one thing it cannot to the side that owns execution.CodeModeLimits→ the SDK'sCodeModeExecutionPolicyshape — it was a 1:1 rename mapping. Product defaults unchanged, still shipped as a constant, and overrides now fail closed.@maka/core/serialized-byte-length.AiSdkBackend, on the runtime's existingAdmissionLimiter, leaving the adapter stateless.@maka/runtime. At 224 lines of glue with one consumer, a standalone workspace cost more than it explained.Part of #3213; the
toolMode = 'code_mode'producer remains out of scope.Why admission moved rather than disappeared
This PR first deleted the hand-rolled execution queue as duplicate governance. Review showed the premise was wrong, and the finding reproduced: with the worker cap pinned to one and each cell cancelled while its tool ignores cancellation, host operations outstanding went 1 → 4.
The queue's admission covered a cell's complete lifecycle, drain included. The SDK worker cap cannot: on cancellation
runCodeModereleases its worker and rejects at once, by design, while host operations may still be running with durable side effects. Only Maka waits for those, so only Maka can bound them.Two things were wrong with where it lived:
codeCellActiveread as "one cell at a time"; what it provided was full-lifecycle admission, because the flag spanned the whole promise and the drain sits inside it. That gap is what made it look superseded — and the concept had been recognised once, in afix(code-mode): bound execution admissionsubcommit of feat(code-mode): replace Self with QuickJS #2549 whose wording never reached the code. The wordadmissionappeared zero times in the package.It now lives on
AiSdkBackend, which owns cell execution and reaches those host operations throughscope.toolRuntime. Acquire and release sit in one method around one call, so the permit covers exactly whatexecuteCodeCellpromises to finish — a contract the adapter now states explicitly. Composed end to end, the same repeated-cancellation probe holds at 1.It is not a new primitive.
ChildAgentRunLimiterwas already an abort-aware FIFO permit pool with a capacity; it is renamedAdmissionLimiterand now serves two boundaries at two lifetimes — child runs one instance per turn, cells one per backend. The only difference, turning a cell away instead of queueing it, is a product decision that reads better at the call site than inside a permit pool: the caller checkswaitingCountbeforeacquire, with no await between, so the pair is atomic.The fold
index.tsandquickjs.tsmerge intopackages/runtime/src/code-mode.ts— the split only existed to give the package an entry point distinct from its implementation. Tests move alongside.Twelve wiring sites drop their entry: the workspace manifest, tsconfig and project reference, both root build chains, the desktop and CLI build chains, the stale-dist pairs, the release script's internal-package and build-order lists, the CLI validation path filters, and the Windows sandbox build step.
@ai-sdk/code-modemoves to the runtime's dependencies, so the release script'snpm lsclosure still reaches it; both third-party notice inventories already listed it under its own name and are unchanged —check:cli-third-party-noticesandcheck:third-party-noticesboth pass.One comment needed rewriting rather than deleting. The dependabot ignore for typescript majors was justified by an in-house Code Mode transpiler pinned to 5.9 for the typescript JS API. That transpiler is gone,
@ai-sdk/code-modeowns transpilation, and no workspace pins typescript — so the ignore has no subject. The comment now says so; dropping the ignore is a dependency-policy call left to a maintainer.Behavior change
activeToolSettlements, which holds the run open, andclearBackendQuarantineForActivationthrows rather than activating a successor generation while a predecessor holds an active run.??, sonull/undefinedkept the product default while zero, negatives and strings passed through to the SDK. Overrides are now admitted only for known fields and only as positive integers. This also fixes a regression this PR introduced and review caught: an earlier merge excluded onlyundefined, so{ maxSourceBytes: null }reached the SDK and its??restored the 256 KiB default in place of Maka's 64 KiB.Review focus
The byte counters were not semantically identical, so that consolidation is not a pure rename: the retained one is bounded and reports
Infinitywhere the core wrappers threw. Both core call sites pass validated plain payloads for which the two agree byte for byte, pinned by a test.One deliberate departure: a top-level
undefinedcounts as four bytes rather thanInfinity, because an absent result is not an oversized one —Infinitywould maketool-runtimereject a tool that returned nothing as too large. Four bytes conservatively bounds what either publication path emits (a cell substitutesnull; a tool result becomes empty text). An earlier revision of this contract named the wrong mechanism for the tool-result path and has been corrected.Verification
@maka/core558 pass · focused runtime suites 307 pass (the 30 moved Code Mode tests included) · release policy suite 28 pass · both third-party notice checks pass · typecheck clean across@maka/core,@maka/runtime,@maka/runtime-host,maka-agentand@maka/eval·format:checkand lint clean. All three review findings reproduced before fixing and re-measured after, including the composed admission probe.The backend wiring is now covered too, which it was not: stubbing
CodeCellAdmissionto a no-op left 218 backend tests green, so re-deleting the acquire/release would not have turned CI red. Two mutations now fail the new test — a no-op admission outright, and releasing the permit before the host drain (the worker cap's semantics) by timeout. Each commit built and tested independently; repository-wide suite left to CI.AI use
Tool(s) and scope: Claude Code (Opus) — read the source and SDK, made the edits, ran the checks above, drafted this description. Commits carry
Generated-by: Claude Code.Checklist
Does this PR entail a change in behavior?