feat(controller): ship the trace-aware inference control plane - #8
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:
📝 WalkthroughWalkthroughThe change adds deterministic TASC contracts, bounded parsing and work budgets, evaluator trust verification, evidence joining, paired assessment workflows, controller replay, atomic artifact packets, CLI v2 execution, runtime codecs and network controls, explicit public exports, extensive tests, and legacy v1 documentation updates. ChangesTASC control-plane platform
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 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 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/statistics.test.ts (1)
2-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
bootstrapGroupedWeightedMeanCIdirectly instead of casting the namespace toany.The function is exported from
../src/statistics.js, so the four(statistics as any)lookups (Lines 37, 62, 91, 111) discard type checking and would silently keep passing if the signature or export name changed.♻️ Proposed refactor
-import * as statistics from "../src/statistics.js"; import { bootstrapMeanCI, + bootstrapGroupedWeightedMeanCI, median, mulberry32, quantile, } from "../src/statistics.js";Then drop the four local aliases and call
bootstrapGroupedWeightedMeanCI(...)directly.Also applies to: 36-116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/statistics.test.ts` around lines 2 - 8, Update the imports in statistics.test.ts to include bootstrapGroupedWeightedMeanCI directly from ../src/statistics.js. Remove the four local aliases and replace each (statistics as any) lookup with direct bootstrapGroupedWeightedMeanCI calls, preserving the existing arguments and assertions.src/schema.ts (1)
322-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
arrayLengthLimitsoption.No tracked TypeScript call site passes
arrayLengthLimits, making the contextual limit branch inaccessible. Wire this up for the intended caller if needed, or remove the option until it is required.🤖 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/schema.ts` around lines 322 - 336, Remove the unused arrayLengthLimits option from snapshotPlainDataTree and stop forwarding it to snapshotPlainDataTreeAt, unless an existing intended caller is first wired to provide it. Keep the snapshot behavior and other traversal options unchanged.src/evidence.ts (1)
906-913: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional:
deepFreezeContractrecurses infinitely on a cyclic graph.The
Object.isFrozenshort-circuit doesn't protect against cycles because the parent is frozen only after its children are visited. Every in-repo caller passes acyclic, snapshot-validated values, so this is not currently reachable — but the function is exported, so aWeakSetof visited objects would make the public surface safe by construction.🤖 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/evidence.ts` around lines 906 - 913, Update deepFreezeContract to track already-visited objects with a WeakSet and stop recursion when an object has been seen, while preserving recursive freezing and the DeepReadonly return type for acyclic values.
🤖 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 `@README.md`:
- Around line 237-240: Align all six migration references with the
implementation plan: update README.md lines 237-240 and 274-276, docs/design.md
lines 17 and 227, and docs/operating-guide.md lines 185-188 and 288-291 to
identify the later milestone when controller assessment and CLI
production-recommendation support are complete, or describe it as a future
release; do not attribute this behavior to Task 5, which only delivers the
assessment engine and does not change production status.
In `@src/policy.ts`:
- Around line 131-133: Update isRegisteredLegacyCandidate to compare
policy.criticalSlices against normalizedCriticalSlices(spec.criticalSlices)
rather than the raw spec array, preserving element-wise matching against the
canonical sorted, deduplicated values.
---
Nitpick comments:
In `@src/evidence.ts`:
- Around line 906-913: Update deepFreezeContract to track already-visited
objects with a WeakSet and stop recursion when an object has been seen, while
preserving recursive freezing and the DeepReadonly return type for acyclic
values.
In `@src/schema.ts`:
- Around line 322-336: Remove the unused arrayLengthLimits option from
snapshotPlainDataTree and stop forwarding it to snapshotPlainDataTreeAt, unless
an existing intended caller is first wired to provide it. Keep the snapshot
behavior and other traversal options unchanged.
In `@tests/statistics.test.ts`:
- Around line 2-8: Update the imports in statistics.test.ts to include
bootstrapGroupedWeightedMeanCI directly from ../src/statistics.js. Remove the
four local aliases and replace each (statistics as any) lookup with direct
bootstrapGroupedWeightedMeanCI calls, preserving the existing arguments and
assertions.
🪄 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: 81ec5032-1ccf-477b-ac09-21a73cb5aa0b
📒 Files selected for processing (35)
.gitignoreREADME.mddocs/design.mddocs/operating-guide.mddocs/superpowers/plans/2026-07-27-trace-control-plane.mddocs/superpowers/specs/2026-07-27-trace-control-plane-design.mdexamples/synthetic/spec.jsonsrc/assessment-context.tssrc/determinism.tssrc/evaluate.tssrc/evaluator-trust.tssrc/evidence-join.tssrc/evidence.tssrc/index.tssrc/integrity.tssrc/legacy-work-budget.tssrc/policy.tssrc/report.tssrc/schema.tssrc/statistics.tssrc/work-budget.tstests/assessment-context.test.tstests/cli.test.tstests/determinism.test.tstests/evaluate.test.tstests/evaluator-trust.test.tstests/evidence-join-operations.test.tstests/evidence-join.test.tstests/evidence.test.tstests/fixtures/evidence.tstests/policy.test.tstests/public-api.test.tstests/schema.test.tstests/statistics.test.tstests/work-budget.test.ts
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 `@tests/window.test.ts`:
- Around line 239-250: Correct the golden-vector test around
deriveWindowMembershipBucket so the documented SHA-256 digest and its modulo
result match the asserted bucket. Recompute or update the stale digest/comment
and corresponding hexadecimal assertion, or change the bucket expectation if the
implementation’s output is correct; ensure both assertions validate the same
domain-separated selection rather than unrelated values.
🪄 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: 38602bc0-885b-439f-b2d4-a481a29972f3
📒 Files selected for processing (14)
src/assessment-contract.tssrc/assessment.tssrc/evidence-join.tssrc/evidence.tssrc/index.tssrc/policy.tssrc/statistics.tssrc/window.tstests/assessment-contract.test.tstests/assessment.test.tstests/evidence-join.test.tstests/evidence.test.tstests/fixtures/evidence.tstests/window.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/fixtures/evidence.ts
- tests/evidence-join.test.ts
- src/evidence.ts
- tests/evidence.test.ts
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add sealed P0 run plans, independent digest admission, authenticated resumable journals, exact target provenance, distinct dispatch and collector signatures, and the opt-in P1 runtime surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep loopback as the safe default while allowing one explicitly opted-in, pinned inference call with no deployment, retry, probe, or sensitive artifact persistence authority. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Require an independently supplied plan digest before reading P1 configuration, validate exact runtime and collector bindings, and expose bounded collection without widening controller authority. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ship reproducible signed fixtures, a deterministic offline replay, exact P0-to-P1 lineage checks, and a regeneration path that preserves the evidence-only authority boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document the trace-aware controller premise, P0/P1 boundary, runtime support evidence, authenticated recovery model, operational limits, migration contract, and residual same-UID risk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Require the P1 runtime entry point in exact release artifacts and compile a clean external TypeScript consumer with explicit Node type roots while preserving actionable subprocess diagnostics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Seal normalized invocation limits into P0 targets, CLI/P1 admission, signed collection provenance, and trace identity; reject substitution before effects and regenerate the signed study with deterministic public fixture-only keys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document exact P0 HTTP-limit lineage, pre-effect substitution rejection, migration requirements, and the explicitly public deterministic key material used only to reproduce the signed synthetic study. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exercise fail-closed report inputs, every evidence-driven experiment branch, and direct nominated artifact publication so the enforced coverage floor remains green without weakening it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/runtime/http.ts (1)
1476-1503: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBody idle timer includes consumer work, and the threat model claims it does not. One root cause:
resetBodyTimer()runs beforeyield chunk, so caller processing time is charged tobodyTimeoutMs.
src/runtime/http.ts#L1476-L1503: moveresetBodyTimer()to afteryield chunkresumes (keeping a reset on the empty-chunkcontinuepath) so the timer measures transport idle time only.docs/threat-model.md#L184-L184: keep the “body idle timing excludes bounded consumer work” control only once the timer placement is fixed; otherwise reword it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/http.ts` around lines 1476 - 1503, The body idle timer currently includes consumer processing because resetBodyTimer runs before yield in the response-body loop. In src/runtime/http.ts lines 1476-1503, keep the reset for empty chunks but move the non-empty-chunk reset until after yield chunk resumes, so it measures transport idle time only. In docs/threat-model.md line 184, retain the statement that bounded consumer work is excluded once this timer placement is corrected; otherwise reword it to match the implementation.
🧹 Nitpick comments (15)
src/evidence.ts (2)
631-655: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpread-based payload construction is fragile compared to the explicit field list used by
dispatchIntentSigningBytes....tracerelies ontraceEnvelopeBodySchema.strict()to keep the preimage raw-free; any future non-strict relaxation silently widens what gets signed. Consider mirroring the explicit projection style of Line 591-618 for symmetry and defense against later refactors.🤖 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/evidence.ts` around lines 631 - 655, The payload construction in collectorAttestationSigningBytes should avoid spreading the entire trace object. Mirror the explicit field projection used by dispatchIntentSigningBytes, listing only the approved trace fields and the selected collectorAttestation fields before parsing with collectorAttestationPayloadSchema; preserve the existing signing domain and canonicalization.
278-288: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
collectorAuthoritySchemaduplicatesdispatchAuthoritySchemaverbatim. Extract oneauthoritySchemaand reuse it; the distinctness rule already lives inassertProtocolSemantics.♻️ Proposed consolidation
-const dispatchAuthoritySchema = z.object({ - keyId: contractSlugSchema, - algorithm: z.literal("ed25519"), - publicKeySpki: canonicalBase64UrlSchema, -}).strict(); - -const collectorAuthoritySchema = z.object({ - keyId: contractSlugSchema, - algorithm: z.literal("ed25519"), - publicKeySpki: canonicalBase64UrlSchema, -}).strict(); +const authoritySchema = z.object({ + keyId: contractSlugSchema, + algorithm: z.literal("ed25519"), + publicKeySpki: canonicalBase64UrlSchema, +}).strict(); +const dispatchAuthoritySchema = authoritySchema; +const collectorAuthoritySchema = authoritySchema;🤖 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/evidence.ts` around lines 278 - 288, Replace the duplicate dispatchAuthoritySchema and collectorAuthoritySchema definitions with one shared authoritySchema containing the common strict object shape, then reuse it for both authority validations. Keep the existing distinctness handling in assertProtocolSemantics unchanged.src/runtime/probe.ts (1)
520-589: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd an exhaustiveness guard in
canaryBody.
bodyis leftundefinedifroute.wireProtocolis ever outside the seven handled cases;JSON.stringify(undefined)then makesBuffer.from(undefined, "utf8")throw a rawTypeErrorinstead of aRuntimeProbeInputError. Adefault: fail()(orneverassignment) keeps a future route addition failing closed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/probe.ts` around lines 520 - 589, Add an exhaustiveness guard to the switch in canaryBody for any unhandled route.wireProtocol, using the established fail mechanism or a never assignment so execution cannot continue with body undefined. Preserve the existing request bodies and ensure unsupported future protocols produce RuntimeProbeInputError rather than reaching Buffer.from.src/artifacts.ts (1)
1763-1783: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueForward
optionsto the snaphotted artifact writer
writeSnapshottedArtifactPacketstill receivesOptionsby name, but this path only passes{ filesystem }. WithArtifactWriterOptionscurrently containing onlyfilesystem, the behavior is functionally equivalent, but this loses future writer options just as easily as a larger options interface.🤖 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/artifacts.ts` around lines 1763 - 1783, Update the writeSnapshottedArtifactPacket call in the surrounding artifact-writing flow to forward the complete options object instead of constructing an object containing only filesystem. Preserve the existing filesystem fallback behavior while ensuring future ArtifactWriterOptions fields in options reach the writer.src/cli-args.ts (1)
424-441: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the capability/effect validators from single source-of-truth tuples.
RUNTIME_PROBE_CAPABILITIES/RUNTIME_PROBE_EFFECTSrestate the literals ofRuntimeProbeCapability/RuntimeProbeObservationEffect(Lines 113-128). Adding a capability in one place silently diverges from the other, and theascasts inparseRuntimeCommandhide it from the compiler.♻️ Derive types from the runtime tuples
-const RUNTIME_PROBE_CAPABILITIES: ReadonlySet<string> = new Set([ - "modelDiscovery", - "liveness", - "readiness", - "prometheusMetrics", - "jsonMetrics", - "chatCompletions", - "completions", - "responses", - "nativeChat", - "nativeGenerate", -]); - -const RUNTIME_PROBE_EFFECTS: ReadonlySet<string> = new Set([ - "non-mutating", - "inference-canary", - "consumptive", -]); +const RUNTIME_PROBE_CAPABILITY_VALUES = Object.freeze([ + "modelDiscovery", + "liveness", + "readiness", + "prometheusMetrics", + "jsonMetrics", + "chatCompletions", + "completions", + "responses", + "nativeChat", + "nativeGenerate", +] as const); + +const RUNTIME_PROBE_EFFECT_VALUES = Object.freeze([ + "non-mutating", + "inference-canary", + "consumptive", +] as const); + +const RUNTIME_PROBE_CAPABILITIES: ReadonlySet<string> = new Set( + RUNTIME_PROBE_CAPABILITY_VALUES, +); +const RUNTIME_PROBE_EFFECTS: ReadonlySet<string> = new Set( + RUNTIME_PROBE_EFFECT_VALUES, +);Then define the exported unions as
typeof RUNTIME_PROBE_CAPABILITY_VALUES[number](Lines 113-128) and replace theascasts with narrowing type guards.🤖 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/cli-args.ts` around lines 424 - 441, Make the runtime probe value tuples the single source of truth: define capability and observation-effect tuples, derive RuntimeProbeCapability and RuntimeProbeObservationEffect from their indexed element types, and build RUNTIME_PROBE_CAPABILITIES/RUNTIME_PROBE_EFFECTS from those tuples. In parseRuntimeCommand, replace the unsafe as casts with type guards that narrow values by membership in the corresponding readonly sets.tests/control-plane-demo.test.ts (1)
114-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
expect(result.stderr).toBe("")is a likely cross-version flake source.Running the child with
--import tsxand adata:loader can emit NodeExperimentalWarninglines on some Node 22.x patch releases, which would fail this assertion for reasons unrelated to the demo. Consider filtering known warning lines instead of demanding an exactly empty stderr.♻️ Proposed tolerance for benign Node warnings
- expect(result.stderr).toBe(""); + const unexpectedStderr = result.stderr + .split("\n") + .filter((line) => + line.trim() !== "" + && !/^\(node:\d+\) (?:Experimental)?Warning:/u.test(line) + && !/^\s+\(Use `node --trace-warnings/u.test(line) + ); + expect(unexpectedStderr).toEqual([]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/control-plane-demo.test.ts` around lines 114 - 119, Update the stderr assertion in the control-plane demo test to tolerate known benign Node ExperimentalWarning lines emitted by the --import tsx/data loader, while still failing on unexpected stderr output. Filter only the documented warning pattern before asserting that the remaining stderr is empty; keep the result.error, status, and artifact-root checks unchanged.scripts/live-smoke.ts (1)
322-335: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
generationignoresrouteand always emits a prompt-shaped request.Fine today because the loopback matrix only uses
completions/nativeGenerate, but adding a chat route to the matrix would silently produce an invalid shape rejected deep insidenormalizeInvocation. Either drop the parameter or branch on it the wayoperatorGeneration(Line 441-465) does.🤖 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 `@scripts/live-smoke.ts` around lines 322 - 335, Update generation to stop ignoring its route parameter: either remove route from the function and all call sites, or branch on route like operatorGeneration so chat routes produce the appropriate chat request shape while completions/nativeGenerate retain the existing prompt request.tests/runtime-invoke.test.ts (1)
804-838: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe clock spy depends on an internal function name appearing in
new Error().stack.Renaming or inlining
pinAuthorizedCollectorRequest(or any stack-depth/trace-limit change) breaks this test in a way that looks like a behavior regression rather than a test-coupling issue. Consider driving the advance from an observable seam instead — e.g. a counted number ofbigint()calls, or an injectedsignal/limits hook — so the intent ("time advances between pin and contact") survives refactors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/runtime-invoke.test.ts` around lines 804 - 838, Update the clock control in this test around prepareRuntimeInvocation and dispatchPreparedRuntimeInvocation to avoid matching pinAuthorizedCollectorRequest in Error.stack. Trigger the nanosecond advance through a stable observable seam, such as a precisely counted process.hrtime.bigint call or an existing injected signal/limits hook, while preserving the assertion that time advances between pinning and contact and the existing expiration expectations.tests/shadow-runner.test.ts (1)
406-413: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead conditional: both ternary branches are
"support-routing-study".♻️ Proposed simplification
const terminalOutputIdentity = createStudyPayloadIdentity( - prepared.requestIdentity.keyId === "shadow-key" - ? "support-routing-study" - : "support-routing-study", + "support-routing-study", "shadow-key",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/shadow-runner.test.ts` around lines 406 - 413, Update the terminalOutputIdentity construction to remove the redundant conditional expression and pass the constant "support-routing-study" value directly to createStudyPayloadIdentity. Preserve the remaining arguments and identity-generation behavior unchanged.scripts/package-smoke.mjs (1)
83-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnvironment isolation covers userconfig but not the global/builtin npm config.
npm_config_userconfigis redirected into the temp dir, yet a machine-level/usr/etc/npmrc(globalconfig/builtin) can still inject registry, proxy, or lifecycle settings and make this smoke non-hermetic. Consider also pinningnpm_config_globalconfigto the temp config directory.🔒️ Proposed hardening
npm_config_userconfig: join(configDirectory, ".npmrc"), + npm_config_globalconfig: join(configDirectory, "global-npmrc"),🤖 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 `@scripts/package-smoke.mjs` around lines 83 - 117, Update safeEnvironment to set npm_config_globalconfig to a configuration file under configDirectory, alongside npm_config_userconfig, so npm cannot read machine-level global or builtin configuration during the smoke test. Preserve the existing environment isolation and immutable return value.src/shadow-plan.ts (1)
578-589: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
isShadowRunPlanMemberre-validates the whole plan on every membership decision.
parseShadowRunPlanre-runs bounded snapshotting, protocol normalization, policy matching, target re-derivation, and a full canonical digest recomputation. Since membership is evaluated per case × replicate, this turns a cheap seeded hash into a heavyweight revalidation per call. Consider validating once at the call site (or memoizing onplanDigest) and keeping this helper to the membership rule only.♻️ Sketch
export function isShadowRunPlanMember( planInput: ShadowRunPlan, caseId: string, replicateId: string, ): boolean { - const plan = parseShadowRunPlan(planInput); + // Callers admit the plan once (CLI/runner already verify expectedPlanDigest). + const plan = verifiedPlanCache.get(planInput) ?? cacheVerified(planInput); return isWindowMembershipSelected( plan.window.membershipRule, caseId, replicateId, ); }🤖 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/shadow-plan.ts` around lines 578 - 589, Update isShadowRunPlanMember so it does not call parseShadowRunPlan for each membership decision; use the already validated ShadowRunPlan input and pass its window.membershipRule directly to isWindowMembershipSelected. Ensure validation remains performed once by the calling flow, preserving the helper’s existing caseId and replicateId behavior..github/workflows/ci.yml (1)
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTen copies of an off-lockfile global npm pin across CI and release (zizmor
adhoc-packages). Every job repeatsnpm install --global npm@11.6.1, so the toolchain version is duplicated instead of declared once, and each install resolves outside any lockfile.
.github/workflows/ci.yml#L32-L33: replace the per-job pin with a single declared source (e.g.packageManagerinpackage.jsonplusactions/setup-node, or a reusable composite step) and reuse it in thetypecheck,build,test,package-smoke,demo,live-smoke,coverage, andauditjobs..github/workflows/release.yml#L34-L35: drop the duplicated pins in thecandidateandpublishjobs and consume the same shared definition so release and CI cannot drift.🤖 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 @.github/workflows/ci.yml around lines 32 - 33, Centralize the npm version currently installed by the CI and release workflows instead of repeating global npm pins. Update .github/workflows/ci.yml lines 32-33 and reuse the shared definition across the typecheck, build, test, package-smoke, demo, live-smoke, coverage, and audit jobs; update .github/workflows/release.yml lines 34-35 to remove the candidate and publish pins and consume the same definition, using a lockfile-aware packageManager/setup-node approach or reusable composite step so both workflows cannot drift.Source: Linters/SAST tools
tests/fixtures/evidence.ts (1)
528-537: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the unsigned-evidence parameter instead of
Record<string, unknown>.Using the fixture's own shape (
ReturnType<typeof unsignedEvaluatorEvidence>) keeps field renames caught at compile time and removes theas anymutations consumers currently need (seeverificationForintests/assessment.test.ts).♻️ Proposed change
-export const signEvaluatorEvidence = ( - privateKey: KeyObject, - unsigned: Record<string, unknown> = unsignedEvaluatorEvidence(), -) => ({ +export const signEvaluatorEvidence = ( + privateKey: KeyObject, + unsigned: ReturnType<typeof unsignedEvaluatorEvidence> = unsignedEvaluatorEvidence(), +) => ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/fixtures/evidence.ts` around lines 528 - 537, Update signEvaluatorEvidence so its unsigned parameter uses ReturnType<typeof unsignedEvaluatorEvidence> instead of Record<string, unknown>. Preserve the existing default and signing behavior, allowing consumers such as verificationFor to pass the strongly typed unsigned-evidence shape without any casts.src/index.ts (2)
252-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResidual wildcard re-exports undercut the enumerated surface.
./integrity.js,./report.js, and./work-budget.js(plus./determinism.jsat Line 1) still leak anything newly added in those modules, which is the failure mode the rest of this file was rewritten to prevent. Consider enumerating them too, or add a public-API snapshot test asserting the full export list.Also applies to: 272-272, 375-375
🤖 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/index.ts` at line 252, Replace the remaining wildcard exports for integrity.js, report.js, work-budget.js, and determinism.js in the main barrel with explicit named exports, preserving only the intended public API and preventing future module additions from leaking automatically.
56-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose the artifact filesystem contract in the root barrel export.
ArtifactVerificationOptionsandArtifactWriterOptionsexpose afilesystemmember, butArtifactFilesystem,ArtifactFileHandle, andnodeArtifactFilesystemare only reachable viasrc/artifacts.jsimports. Add these tosrc/index.tsso consumers can type and override the filesystem option through the published package surface.♻️ Proposed addition
export { ARTIFACT_MANIFEST_FILENAME, MAX_ARTIFACT_FILES, MAX_ARTIFACT_FILE_BYTES, MAX_ARTIFACT_TOTAL_BYTES, NO_DEPLOYMENT_AUTHORITY, PURE_NODE_NAMESPACE_LIMITATION, + nodeArtifactFilesystem, readArtifactPacketIfPresent, verifyArtifactPacket, writeArtifactPacket, writeArtifactPacketOrVerifyIdentical, } from "./artifacts.js"; export type { ArtifactDurability, ArtifactDurabilityLimitation, + ArtifactFileHandle, + ArtifactFilesystem, ArtifactManifest,🤖 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/index.ts` around lines 56 - 71, Update the root barrel export in src/index.ts to also re-export ArtifactFilesystem, ArtifactFileHandle, and nodeArtifactFilesystem alongside the existing artifact symbols, making the filesystem contract and implementation available through the published package surface.
🤖 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 `@examples/control-plane/protocol.json`:
- Around line 156-196: Update the protocol fixture’s minimumCriticalSliceGroups
value to 1 so it matches the single criticalSlices entry, account-recovery. Keep
the existing criticalSlices and related protocol configuration unchanged.
In `@src/runtime/invoke.ts`:
- Around line 964-971: Update the endpoint-binding validation around
fingerprintCollectorEndpointBinding to catch malformed snapshot.policy or other
fingerprinting failures and normalize them through inputFail(). Reuse the parsed
CollectorTrustPolicy to pass maximumRequestDurationMs into the subsequent
deadline check, while preserving the existing digest mismatch handling.
In `@tests/cli.test.ts`:
- Around line 517-520: Extend the redaction assertions in tests/cli.test.ts at
lines 517-520 to verify malformed.stdout omits secret, and at lines 560-584 to
verify both stdout and stderr omit sourceSecret and pathSecret for rejected,
successful, and repeated invocations. Keep the existing status, stderr, and
output-directory assertions unchanged.
In `@tests/live-smoke-operator.test.ts`:
- Around line 187-190: Update the test cases in
tests/live-smoke-operator.test.ts that use PROCESS_TIMEOUT_MS so their Vitest
timeout is explicitly greater than the 15-second subprocess budget, either
through each relevant it(...) call or the applicable configuration. Preserve the
existing timedOut handling and ensure Vitest does not terminate these tests
before the subprocess timeout callback runs.
In `@tests/report-diagnostics.test.ts`:
- Around line 98-112: Canonicalize the temporary root before constructing the
test output path. In the test setup around mkdtemp, resolve tmpdir() with
realpath (as done in control-plane-demo.ts) so the root has no symlinked
components, while preserving the existing artifact creation and verification
assertions.
In `@vitest.config.ts`:
- Around line 12-15: Update the coverage-rationale comment in vitest.config.ts
to reference the final 788-test suite and its reported 85.65% statement
coverage, along with the corresponding final branch, function, and line
percentages. Preserve the existing explanation about integer floors and
instrumentation margin.
---
Duplicate comments:
In `@src/runtime/http.ts`:
- Around line 1476-1503: The body idle timer currently includes consumer
processing because resetBodyTimer runs before yield in the response-body loop.
In src/runtime/http.ts lines 1476-1503, keep the reset for empty chunks but move
the non-empty-chunk reset until after yield chunk resumes, so it measures
transport idle time only. In docs/threat-model.md line 184, retain the statement
that bounded consumer work is excluded once this timer placement is corrected;
otherwise reword it to match the implementation.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 32-33: Centralize the npm version currently installed by the CI
and release workflows instead of repeating global npm pins. Update
.github/workflows/ci.yml lines 32-33 and reuse the shared definition across the
typecheck, build, test, package-smoke, demo, live-smoke, coverage, and audit
jobs; update .github/workflows/release.yml lines 34-35 to remove the candidate
and publish pins and consume the same definition, using a lockfile-aware
packageManager/setup-node approach or reusable composite step so both workflows
cannot drift.
In `@scripts/live-smoke.ts`:
- Around line 322-335: Update generation to stop ignoring its route parameter:
either remove route from the function and all call sites, or branch on route
like operatorGeneration so chat routes produce the appropriate chat request
shape while completions/nativeGenerate retain the existing prompt request.
In `@scripts/package-smoke.mjs`:
- Around line 83-117: Update safeEnvironment to set npm_config_globalconfig to a
configuration file under configDirectory, alongside npm_config_userconfig, so
npm cannot read machine-level global or builtin configuration during the smoke
test. Preserve the existing environment isolation and immutable return value.
In `@src/artifacts.ts`:
- Around line 1763-1783: Update the writeSnapshottedArtifactPacket call in the
surrounding artifact-writing flow to forward the complete options object instead
of constructing an object containing only filesystem. Preserve the existing
filesystem fallback behavior while ensuring future ArtifactWriterOptions fields
in options reach the writer.
In `@src/cli-args.ts`:
- Around line 424-441: Make the runtime probe value tuples the single source of
truth: define capability and observation-effect tuples, derive
RuntimeProbeCapability and RuntimeProbeObservationEffect from their indexed
element types, and build RUNTIME_PROBE_CAPABILITIES/RUNTIME_PROBE_EFFECTS from
those tuples. In parseRuntimeCommand, replace the unsafe as casts with type
guards that narrow values by membership in the corresponding readonly sets.
In `@src/evidence.ts`:
- Around line 631-655: The payload construction in
collectorAttestationSigningBytes should avoid spreading the entire trace object.
Mirror the explicit field projection used by dispatchIntentSigningBytes, listing
only the approved trace fields and the selected collectorAttestation fields
before parsing with collectorAttestationPayloadSchema; preserve the existing
signing domain and canonicalization.
- Around line 278-288: Replace the duplicate dispatchAuthoritySchema and
collectorAuthoritySchema definitions with one shared authoritySchema containing
the common strict object shape, then reuse it for both authority validations.
Keep the existing distinctness handling in assertProtocolSemantics unchanged.
In `@src/index.ts`:
- Line 252: Replace the remaining wildcard exports for integrity.js, report.js,
work-budget.js, and determinism.js in the main barrel with explicit named
exports, preserving only the intended public API and preventing future module
additions from leaking automatically.
- Around line 56-71: Update the root barrel export in src/index.ts to also
re-export ArtifactFilesystem, ArtifactFileHandle, and nodeArtifactFilesystem
alongside the existing artifact symbols, making the filesystem contract and
implementation available through the published package surface.
In `@src/runtime/probe.ts`:
- Around line 520-589: Add an exhaustiveness guard to the switch in canaryBody
for any unhandled route.wireProtocol, using the established fail mechanism or a
never assignment so execution cannot continue with body undefined. Preserve the
existing request bodies and ensure unsupported future protocols produce
RuntimeProbeInputError rather than reaching Buffer.from.
In `@src/shadow-plan.ts`:
- Around line 578-589: Update isShadowRunPlanMember so it does not call
parseShadowRunPlan for each membership decision; use the already validated
ShadowRunPlan input and pass its window.membershipRule directly to
isWindowMembershipSelected. Ensure validation remains performed once by the
calling flow, preserving the helper’s existing caseId and replicateId behavior.
In `@tests/control-plane-demo.test.ts`:
- Around line 114-119: Update the stderr assertion in the control-plane demo
test to tolerate known benign Node ExperimentalWarning lines emitted by the
--import tsx/data loader, while still failing on unexpected stderr output.
Filter only the documented warning pattern before asserting that the remaining
stderr is empty; keep the result.error, status, and artifact-root checks
unchanged.
In `@tests/fixtures/evidence.ts`:
- Around line 528-537: Update signEvaluatorEvidence so its unsigned parameter
uses ReturnType<typeof unsignedEvaluatorEvidence> instead of Record<string,
unknown>. Preserve the existing default and signing behavior, allowing consumers
such as verificationFor to pass the strongly typed unsigned-evidence shape
without any casts.
In `@tests/runtime-invoke.test.ts`:
- Around line 804-838: Update the clock control in this test around
prepareRuntimeInvocation and dispatchPreparedRuntimeInvocation to avoid matching
pinAuthorizedCollectorRequest in Error.stack. Trigger the nanosecond advance
through a stable observable seam, such as a precisely counted
process.hrtime.bigint call or an existing injected signal/limits hook, while
preserving the assertion that time advances between pinning and contact and the
existing expiration expectations.
In `@tests/shadow-runner.test.ts`:
- Around line 406-413: Update the terminalOutputIdentity construction to remove
the redundant conditional expression and pass the constant
"support-routing-study" value directly to createStudyPayloadIdentity. Preserve
the remaining arguments and identity-generation behavior unchanged.
🪄 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: 9fb0c502-d80a-4b6c-8fa4-e12d491b3baa
⛔ Files ignored due to path filters (3)
benchmarks/mlx/build-requirements.lockis excluded by!**/*.lockbenchmarks/mlx/requirements.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (97)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/release.yml.gitignoreCHANGELOG.mdCONTRIBUTING.mdREADME.mdSECURITY.mdbenchmarks/mlx/README.mdbenchmarks/mlx/build-requirements.txtbenchmarks/mlx/requirements.txtbenchmarks/mlx/run_benchmarks.pybenchmarks/mlx/test_run_benchmarks.pydocs/design.mddocs/migration-v2.mddocs/operating-guide.mddocs/runtime-support.mddocs/superpowers/plans/2026-07-27-trace-control-plane.mddocs/superpowers/specs/2026-07-27-trace-control-plane-design.mddocs/threat-model.mdexamples/control-plane/README.mdexamples/control-plane/collector-trust.jsonexamples/control-plane/controller-snapshot.jsonexamples/control-plane/development-context.jsonexamples/control-plane/development-evidence.ndjsonexamples/control-plane/development-traces.ndjsonexamples/control-plane/experiment-budget.jsonexamples/control-plane/experiment-history.jsonexamples/control-plane/online-context.jsonexamples/control-plane/online-evidence.ndjsonexamples/control-plane/online-traces.ndjsonexamples/control-plane/protocol.jsonexamples/control-plane/shadow-profiles.jsonexamples/control-plane/shadow-run-plan.jsonexamples/control-plane/trust-snapshot.jsonexamples/control-plane/window-manifest.jsonexamples/control-plane/work-budget.jsonpackage.jsonscripts/control-plane-demo.tsscripts/live-smoke-config.tsscripts/live-smoke.tsscripts/package-smoke.mjsscripts/regenerate-control-plane-fixtures.mjsscripts/release-artifact.mjssrc/artifacts.tssrc/bounded-input.tssrc/cli-args.tssrc/cli-v2.tssrc/cli.tssrc/evidence-join.tssrc/evidence.tssrc/experiments.tssrc/index.tssrc/runtime-http-limits.tssrc/runtime/http.tssrc/runtime/index.tssrc/runtime/invoke.tssrc/runtime/metrics.tssrc/runtime/ndjson.tssrc/runtime/network-policy.tssrc/runtime/orchestration.tssrc/runtime/probe.tssrc/runtime/profiles.tssrc/runtime/shadow.tssrc/runtime/sse.tssrc/runtime/types.tssrc/shadow-plan.tstests/artifacts.test.tstests/assessment.test.tstests/bounded-input.test.tstests/cli-args.test.tstests/cli-runtime.test.tstests/cli-v2.test.tstests/cli.test.tstests/control-plane-demo.test.tstests/controller.test.tstests/evidence-join.test.tstests/evidence.test.tstests/experiments.test.tstests/fixtures/cli-v2.tstests/fixtures/evidence.tstests/live-smoke-config.test.tstests/live-smoke-operator.test.tstests/network-policy.test.tstests/package-lock.test.tstests/public-api.test.tstests/report-diagnostics.test.tstests/runtime-codecs.test.tstests/runtime-http-limits.test.tstests/runtime-http.test.tstests/runtime-invoke.test.tstests/runtime-probe.test.tstests/runtime-profiles.test.tstests/shadow-plan.test.tstests/shadow-runner.test.tstests/window.test.tsvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- .gitignore
- tests/public-api.test.ts
- tests/cli-args.test.ts
- src/runtime/orchestration.ts
- docs/superpowers/specs/2026-07-27-trace-control-plane-design.md
- tests/runtime-profiles.test.ts
- src/bounded-input.ts
- tests/evidence-join.test.ts
- docs/superpowers/plans/2026-07-27-trace-control-plane.md
- tests/assessment.test.ts
- tests/fixtures/cli-v2.ts
- src/runtime/types.ts
- src/cli.ts
- src/runtime/sse.ts
- src/runtime/profiles.ts
- tests/experiments.test.ts
- src/runtime/metrics.ts
- tests/window.test.ts
- src/runtime/network-policy.ts
- tests/cli-v2.test.ts
- tests/controller.test.ts
- src/runtime/ndjson.ts
- tests/runtime-codecs.test.ts
- src/experiments.ts
- src/evidence-join.ts
- tests/network-policy.test.ts
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
Ships TASC as an out-of-band, trace-aware inference control plane for better evaluators and routing logic.
This is deliberately not an LLM-as-judge package. Evaluator outputs enter TASC as independently produced, provenance-bound evidence. TASC owns trace integrity, deterministic evidence joins, offline and sealed-window policy assessment, bounded shadow collection, controller decisions, and falsifiable experiment proposals. It never gains inline production-routing or deployment authority.
Authority boundary
@rachittshah/tasc/runtimeexport that can contact pre-authorized inference endpoints only after P0 has sealed the complete plan and the operator independently supplies its expected digest.Before any external effect, P1 must exactly match the sealed runtime, backend, model, endpoint, route, transport, authentication reference, and canonical HTTP-limit fingerprint. Collection is bounded by request, token, byte, time, retry, concurrency, and cancellation budgets. Its resumable journal is HMAC-authenticated and lineage-bound; lease claims are unique; dispatch and collector receipts use separate Ed25519 signing domains; ambiguous sends become
sent_unknownand are never replayed automatically.What changed
Runtime integration matrix
0.26.00.5.161.2.1b101560.32.53.3.70.4.10.31.32.56.10.13.1rc1Ray and SkyPilot integrations record operator-supplied orchestration provenance around an already authorized endpoint. TASC does not use their SDKs or create, discover, scale, promote, roll back, or otherwise mutate deployments.
Real inference validation
One real, bounded Ollama call was run against the pinned
ollama/ollama:0.32.5image usingsmollm2:135m-instruct-q4_0on Linux/amd64 CPU:lengthfinish reasonvLLM, SGLang, TensorRT-LLM, llama.cpp, TGI, LM Studio, and MLX-LM are covered by pinned source review, contract tests, codec tests, and loopback live-smoke servers. They were not claimed as real GPU/server validations in this PR.
Security and operational limits
PASSmeans that the evidence contract passed; it is not production deployment approval.Non-goals
Verification
Final local verification on
afab1e39df661adb3fd465827f8e4894d7b8eab3:npm run typecheck— cleannpm test— 788/788 passed across 40 filesnpm run coverage— 85.65% statements, 80.06% branches, 96.98% functions, 87.15% linesnpm run build— cleannpm run demo—NOMINATED / DEMO_ONLYnpm run demo:control-plane— 0 network/model calls; 2/2 targets, 8/8 plan-lineage checks, 16/16 signatures, 4/4 packetsnpm run live:smoke— vLLM, Ollama NDJSON, and TGI loopback paths passed; 6 contactsnpm pack --dry-run— 128 files, 436.0 kB packed, 2.5 MB unpackednpm audit --package-lock-only --audit-level=high— 0 vulnerabilitiesnpm publish ./release-candidate/package.tgz --dry-run --access public --provenance=false— passed40f324e2690f2c8d6c3baf552478481924b95deb2ef5498815df1c31695056fcb709cb4ea86d3080dcbe554ebb972c9db6dcaac33f9c8934e67a0c85cf40f25bThe pushed SHA is additionally gated by the repository CI, release, CodeQL, dependency-review, secret-scan, GitGuardian, and CodeRabbit checks.
Design records
docs/superpowers/specs/2026-07-27-trace-control-plane-design.mddocs/superpowers/plans/2026-07-27-trace-control-plane.mddocs/threat-model.mddocs/runtime-support.mddocs/operating-guide.mdMerge context
main