feat(t3layer): replace private runtime client with stock T3 HTTP baseline - #4
Conversation
…line Phase 3 of the stock-T3 realignment: T3Layer becomes a channel-agnostic external orchestration overlay for an unmodified stock T3 server. - Remove @t3tools/runtime-client and all private-fork prerequisites; delete the hand-written RPC protocol module - Add narrow local stock HTTP contracts (descriptor/auth/shell/detail/ dispatch) with fail-closed validation at pinned upstream d3037064 - Add bounded adaptive shell/detail poller with strict cadence, rate, concurrency, and evidence budgets; injected clock throughout - Rewrite nativeRuntime/facade for two-stage HTTP spawn (thread.create -> bootstrap-free thread.turn.start), receipt-targeted causal wait, expiring send leases, exhaustive typed partial/pending outcomes, and the no-throw-after-durable-mutation invariant across all three stock mutations (project/thread/turn) - Caller-held project-create identity for cross-runtime idempotency; stock-compatible workspace-root ingress canonicalization - Isolated live-proof harness with exact-SHA stock worktree, forgery- rejecting receipts, and shell-hardened teardown; stock-only gate script - 222 tests / 727 assertions green at pinned Bun 1.3.11; typecheck clean; ShellCheck clean; exact-stock characterization green at d3037064 Nine adversarial review rounds plus boundary-convergence and final pre-commit gates; final independent verdict SHIP 9/10 with zero HIGH/CRITICAL/MEDIUM findings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
|
@codex review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_68f8e5ce-3d0d-4f0b-aa34-700d0c138a5f) |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesThe PR replaces the prototype runtime with a Stock T3 HTTP runtime. It adds typed contracts, HTTP transport, adaptive polling, identity reconciliation, leases, receipts, proof validation, live harnesses, canary drills, and release documentation. Stock T3 runtime
Stock proof and release operations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| set -euo pipefail | ||
|
|
||
| candidate_root=${STOCK_ONLY_CANDIDATE_ROOT:-$PWD} | ||
| historical_path=${STOCK_ONLY_HISTORICAL_PATH:-/Users/etanheyman/Gits/t3layer/test/p2-live-proof-runner.test.ts} |
There was a problem hiding this comment.
🟡 Medium scripts/check-stock-only.sh:5
historical_path defaults to /Users/etanheyman/Gits/t3layer/test/p2-live-proof-runner.test.ts, an absolute path in the author's home directory. Running bash scripts/check-stock-only.sh in any normal clone exits with "historical evidence file is missing" because that path does not exist, and the only way to override it is via the undocumented STOCK_ONLY_HISTORICAL_PATH environment variable. This makes the stock-only gate non-portable and unusable as shipped. Consider changing the default to a path relative to $candidate_root (e.g., test/p2-live-proof-runner.test.ts) so it resolves in any clone.
| historical_path=${STOCK_ONLY_HISTORICAL_PATH:-/Users/etanheyman/Gits/t3layer/test/p2-live-proof-runner.test.ts} | |
| historical_path=${STOCK_ONLY_HISTORICAL_PATH:-$candidate_root/test/p2-live-proof-runner.test.ts} |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/check-stock-only.sh around line 5:
`historical_path` defaults to `/Users/etanheyman/Gits/t3layer/test/p2-live-proof-runner.test.ts`, an absolute path in the author's home directory. Running `bash scripts/check-stock-only.sh` in any normal clone exits with `"historical evidence file is missing"` because that path does not exist, and the only way to override it is via the undocumented `STOCK_ONLY_HISTORICAL_PATH` environment variable. This makes the stock-only gate non-portable and unusable as shipped. Consider changing the default to a path relative to `$candidate_root` (e.g., `test/p2-live-proof-runner.test.ts`) so it resolves in any clone.
There was a problem hiding this comment.
This path is an explicit acceptance criterion, not a portable runtime input: revision-3 lines 471/484 require the exact external historical file and exact SHA-256. The gate currently passes that attestation, so no change is appropriate.
There was a problem hiding this comment.
Understood — I cannot access the revision-3 acceptance criteria to verify lines 471/484. Note that the hardcoded default path /Users/etanheyman/Gits/t3layer/... will fail on any machine where that absolute path does not exist (CI, other developers) unless STOCK_ONLY_HISTORICAL_PATH is explicitly set. If revision-3 requires this exact default for attestation purposes, the script's portability is constrained by that external requirement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/operations/stock-t3-first-release.md`:
- Around line 31-37: Update the --execute checksum logic in
scripts/stock-t3-canary-drill.sh to use a portable SHA-256 command, reusing the
fallback selection already implemented in scripts/check-stock-only.sh. Ensure
Linux runners with sha256sum and environments with shasum both work before the
first transition, or explicitly enforce the documented macOS-only prerequisite.
- Around line 9-17: Update scripts/stock-t3-canary-drill.sh to require
caller-supplied approved artifact and configuration digests/boundaries, validate
the derived T3_STOCK_ARTIFACT_PATH digest and other relevant values against
them, and fail closed on any mismatch before route-canary starts. Ensure the
drill cannot proceed using only the digest derived from T3_STOCK_ARTIFACT_PATH.
In `@package.json`:
- Line 10: The stock-only gate currently relies on an author-local default
evidence path. Update package.json line 10 to invoke scripts/check-stock-only.sh
with repository-relative evidence inputs or documented environment variables,
and update README.md line 172 to document that same portable invocation and
required inputs.
In `@README.md`:
- Around line 130-136: Update the README’s polling budget section to define
separate scopes for per-wait, per-environment, and per-client/global limits.
Correct the aggregate arithmetic to reflect that each environment independently
enforces the 32/30 shell-start caps and that detail reads can occur during shell
cycles; remove or revise the inconsistent eight-wait 64/62 ceiling. Add tests
covering each scope and their interactions.
In `@scripts/stock-proof-cli.ts`:
- Around line 8-21: Update the top-level argument destructuring near command,
source, first, and second to also capture the fifth argument as candidateSha,
then use that variable in the publish branch instead of indexing
process.argv[6]. Keep the existing required-argument validation and checksum
flow unchanged.
In `@scripts/stock-t3-canary-drill.sh`:
- Around line 68-94: Declare function-scoped variables with local in
verify_artifact, record_status, run_step, and verify_health. Localize
artifact_stage, current_digest, step_name, step_command, step_status, stage,
descriptor, and thread at their assignments, while preserving the existing
run_step and verify_health behavior.
- Around line 96-115: Clear or remove recovery_armed in the canary drill flow so
recover() no longer retains an always-true guard after recovery is no longer
needed. Update the code around the recovery completion path and preserve the
existing recover() behavior for incomplete drills.
- Around line 117-151: Quote the command expansions used to execute the
configured commands in verify_health and the cancellation flow: update
T3_STOCK_DESCRIPTOR_COMMAND, T3_STOCK_THREAD_READ_COMMAND, and
T3_STOCK_CANCEL_WAITS_COMMAND to preserve paths containing spaces and prevent
unintended splitting or glob expansion, matching the existing quoted invocation
pattern.
In `@scripts/stock-t3-exact-characterization.sh`:
- Around line 15-20: Update the setup around generated_path before the heredoc
creation to detect an existing file and exit with an error before cleanup can
remove it; preserve the current trap and generation flow when the path does not
exist.
In `@scripts/stock-t3-live-harness.sh`:
- Around line 5-6: Remove hardcoded /Users/etanheyman paths from
scripts/stock-t3-live-harness.sh lines 5-6 by sourcing stock_repo and
candidate_repo from required environment variables or deriving them relative to
the script location; preserve the candidate_repo usage in the test-mode copy at
lines 182-183. In scripts/check-stock-only.sh lines 4-15, remove the
historical_path default and execute the historical digest check at lines 52-60
only when the operator provides that path.
- Line 107: Update the staging permission assertion in the cleanup path to
explicitly test the result of the stat comparison and abort or return failure
when the mode is not 600. Ensure the check remains effective despite set +e and
prevents publication with incorrect permissions.
- Around line 273-293: Update the three curl requests assigning negative_status,
negative_shell_status, and negative_detail_status to use the existing 0600
curl_config via --config instead of exposing the bearer token in command-line
headers; write the negative payload to a file and send it with --data `@file`.
Preserve the current endpoints, methods, response-body files, and status
validation.
In `@src/adaptivePoller.ts`:
- Around line 115-128: Update defaultSleep to remove the abort listener when the
timer resolves, while retaining cleanup when cancellation occurs. Ensure both
completion paths detach the listener so repeated run cycles do not accumulate
closures on the shared AbortSignal.
In `@src/facade.ts`:
- Around line 23-41: Update createStockT3Facade to forward close, pollMetrics,
and httpObservations from the supplied T3NativeRuntime, preserving the existing
operation forwarding. Extend the facade’s exports to re-export the native
runtime result types, including TurnReceipt and StockRuntimeError, alongside the
existing project-identity types so consumers need not import nativeRuntime
directly.
In `@src/nativeRuntime.ts`:
- Around line 511-526: Update mapReceivedError to map unrecognized errors to a
distinct internal error class instead of transport_unavailable, while preserving
the original error message in the returned error’s evidence. Keep the existing
StockRuntimeError passthrough and recognized StockT3HttpError mappings
unchanged.
- Around line 547-565: Update canonical so undefined object properties are
omitted and undefined array elements use an explicit canonical marker rather
than relying on JSON.stringify or Array.join behavior. Preserve deterministic
sorting and recursive canonicalization, and version-bind the changed digest
format so existing persisted inputDigest records are not treated as compatible.
- Around line 1561-1563: Update workspaceMatches to defensively canonicalize
each row value and return false when workspaceComparisonKey cannot process it,
including blank or relative server roots; preserve matching for valid
canonicalizable roots and avoid allowing one invalid current.projects entry to
abort the spawn.
- Around line 280-294: Update canonicalizeWorkspaceRoot to detect when
options.platform differs from the local platform and no platform-specific cwd or
homeDirectory is provided; in that case, require workspace_root to be absolute
and reject both ~ expansion and relative input before path resolution. Preserve
existing tilde and relative-path behavior when platform semantics are local or
the corresponding cwd/homeDirectory options are explicitly supplied.
In `@src/stockProof.ts`:
- Around line 285-292: Update canonical to handle undefined explicitly before
JSON.stringify: either reject undefined values with a clear error or omit object
properties whose values are undefined. Ensure canonicalProofEnvelopeJson and
canonicalProofJson never emit the literal text "undefined" and continue
producing parseable JSON.
- Around line 241-275: Update canonicalProofBody to allow only the expected
top-level proof-body keys by checking Object.keys(input) against the established
schema fields before returning the clone. Reject any unknown or missing keys
with ProofReceiptError, while preserving validation of the existing known fields
and canonical output.
In `@src/stockT3Contracts.ts`:
- Line 1: Update the runtime options definition in nativeRuntime.ts to import
and use the existing ConnectionProfile type from stockT3Contracts.ts instead of
repeating the "local" | "relay" | "tunnel" union inline.
- Around line 365-367: Remove the redundant nullableOptional wrapper and export
optionalNullableString as the single public helper, preserving its existing
behavior and signature. Update any references in the file if needed so callers
use optionalNullableString directly.
In `@src/stockT3HttpClient.ts`:
- Around line 64-73: Update the request URL construction near the operation-path
resolution at line 180 to resolve operation paths relative to the normalized
pathname returned by normalizeBaseUrl, rather than as root-absolute paths.
Preserve the base path prefix, including its trailing-slash normalization, for
all requests made through the client.
- Line 111: Bound the endpointStatusTrace array declared in stockT3HttpClient by
retaining only the most recent entries. Update the append paths for both
successful requests and transport failures to trim the array immediately after
each addition, preserving newest-first retention and ensuring observations()
copies only the capped trace.
- Around line 161-179: Move acquireCapacity(boundary) before computing
timeoutMs, creating linkedAttemptSignal, and incrementing requestCount in the
request-attempt flow. Recompute the deadline budget after admission so queued
time does not consume the attempt timeout, then start the timer and abort
listener only for admitted requests. Ensure capacity is released and any newly
created attempt timer/listener is cleaned up when admission or subsequent setup
rejects.
- Around line 117-153: Update acquireCapacity so the queue waker claims the
freed slot by incrementing inFlight before resolving the resumed waiter,
matching the pumpSlots behavior in adaptivePoller. Ensure any waiter that
resumes but then fails the abort/deadline checks releases that claimed slot and
wakes the next waiter, preventing queue stalls while preserving the
MAX_HTTP_IN_FLIGHT limit and accurate peakInFlight tracking.
In `@test/adaptive-poller.test.ts`:
- Around line 113-116: Update the adaptive poller test’s final metrics assertion
to verify the invariant that peakHttpInFlight is greater than or equal to peak,
rather than requiring exact equality. Keep the existing concurrency limit and
completion assertions unchanged, and continue closing the poller afterward.
In `@test/r5-runtime-regressions.test.ts`:
- Around line 136-138: Replace wall-clock sleeps with deterministic
synchronization: in test/r5-runtime-regressions.test.ts lines 136-138, pass a
mutable clock to createStockT3NativeRuntime and advance it inside the detail
handler instead of sleeping 60 ms; in lines 225-239, reuse the injected clock
and advance it in dispatch before returning the sequence. In
test/boundary-convergence.test.ts lines 416-419, remove the 5 ms polling loop
and await the fixture’s deterministic signal before asserting
pollMetrics().activeWaits.
In `@test/r8-runtime-regressions.test.ts`:
- Around line 475-478: In the test fixture’s id assignment, remove the redundant
stage ternary because both branches call ids with the same arguments. Replace it
with a single ids("create-1", "thread-1", "turn-1", "message-1", "lease-1")
expression, preserving the existing identifiers.
In `@test/stock-only-gate.test.ts`:
- Around line 37-92: Add a test alongside the existing stock-only gate cases
that creates a valid historical file, supplies an intentionally incorrect
STOCK_ONLY_HISTORICAL_SHA256 to check-stock-only.sh, and asserts a nonzero exit
plus the historical SHA-256 mismatch message. Reuse the existing fixture,
digest, and run setup patterns without changing the other cases.
In `@test/stock-t3-live-harness.test.ts`:
- Around line 179-198: Set an explicit 60,000 ms timeout on the test containing
the 14-seam loop in “all setup/live fault seams preserve secret redaction and
clean the proof root,” matching the existing canary test’s timeout pattern.
In `@test/stock-t3-live.test.ts`:
- Around line 48-94: Increase the enclosing test timeout in the stock T3 live
test beyond the combined budgets of the two 120,000 ms waits, the 120,000 ms
send, and the 30,000 ms observe so the receipt-writing path after
runtime.observe can complete. Keep the existing per-operation timeouts unchanged
unless adjusting them is necessary to ensure the test-level timeout exceeds
their total.
- Around line 77-81: In the test setup that builds the live proof’s sequences,
keep the receipt acceptedSequence values in local variables and assert each is
non-null before constructing the sequences object. Use descriptive assertion
messages identifying the ambiguous receipt, then pass the narrowed numeric
values to validateLive().
In `@test/stock-t3-sequence.test.ts`:
- Around line 47-61: Move the /.well-known/t3/environment response in the
request handler behind the authorization check so descriptor requests validate
the Bearer token. Keep the existing descriptor payload unchanged, and ensure
invalid or missing credentials return the existing 401 response before reaching
that route.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eda6fda3-f92b-40cc-b650-b5d007b9dffc
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
README.mddocs/operations/stock-t3-first-release.mdpackage.jsonscripts/check-stock-only.shscripts/stock-proof-cli.tsscripts/stock-t3-canary-drill.shscripts/stock-t3-exact-characterization.shscripts/stock-t3-live-harness.shsrc/adaptivePoller.tssrc/facade.tssrc/nativeRuntime.tssrc/protocol.tssrc/stockProof.tssrc/stockT3Contracts.tssrc/stockT3HttpClient.tstest/adaptive-poller.test.tstest/boundary-convergence.test.tstest/facade.contract.test.tstest/facade.send.test.tstest/facade.spawn.test.tstest/facade.stock-http.test.tstest/facade.wait.test.tstest/native-runtime-adapter.test.tstest/protocol.test.tstest/r3-runtime-regressions.test.tstest/r4-runtime-regressions.test.tstest/r5-runtime-regressions.test.tstest/r6-runtime-regressions.test.tstest/r7-runtime-regressions.test.tstest/r8-runtime-regressions.test.tstest/r9-runtime-regressions.test.tstest/stock-only-gate.test.tstest/stock-t3-contracts.test.tstest/stock-t3-exact-stock-negative.test.tstest/stock-t3-http-client.test.tstest/stock-t3-live-harness.test.tstest/stock-t3-live.test.tstest/stock-t3-sequence.test.ts
💤 Files with no reviewable changes (6)
- test/facade.wait.test.ts
- test/facade.contract.test.ts
- test/protocol.test.ts
- test/facade.spawn.test.ts
- test/facade.send.test.ts
- src/protocol.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Macroscope - Correctness Check
🧰 Additional context used
🪛 ast-grep (0.45.0)
scripts/stock-t3-live-harness.sh
[warning] 41-41: set +e (or set +o errexit) disables the shell's errexit option, so the script keeps running after a command fails. This masks failures of security-critical operations (downloads, signature/checksum verification, permission changes, cleanup of secrets), letting the script proceed with a bad or insecure state. Leave errexit enabled (set -e / set -euo pipefail), or handle failures explicitly with if/|| and an explicit exit instead of globally turning off failure detection.
Context: set +e
Note: [CWE-754] Improper Check for Unusual or Exceptional Conditions.
(set-plus-e-error-masking-bash)
[warning] 210-210: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: http_token=test-mode-redacted
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
🪛 LanguageTool
README.md
[grammar] ~161-~161: Ensure spelling is correct
Context: ... a colliding old ref. Scoped old project-create evidence fails closed, while stable wor...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (73)
README.md (6)
3-61: LGTM!
63-126: LGTM!
137-162: LGTM!
164-171: LGTM!
173-199: LGTM!
201-216: LGTM!docs/operations/stock-t3-first-release.md (3)
1-7: LGTM!
18-30: LGTM!
39-55: LGTM!package.json (2)
3-9: LGTM!
11-14: LGTM!src/stockProof.ts (5)
1-104: LGTM!
106-144: LGTM!
220-239: LGTM!
294-317: LGTM!
169-207: 🗄️ Data Integrity & IntegrationNo change needed.
httpObservations().endpointStatusTraceincludes the exact response status for each request, while poll counters only count attempted poll starts. A 404 detail request appears in the trace, contributes torequests, and can be accepted as expected projection-lag evidence atexactHttpNegative.detailStatus.scripts/stock-proof-cli.ts (1)
22-27: LGTM!scripts/check-stock-only.sh (2)
17-50: LGTM!
52-63: LGTM!test/stock-only-gate.test.ts (2)
11-34: LGTM!
45-45: LGTM!test/stock-t3-exact-stock-negative.test.ts (2)
8-18: LGTM!
20-43: LGTM!scripts/stock-t3-exact-characterization.sh (2)
4-13: LGTM!
21-201: LGTM!scripts/stock-t3-live-harness.sh (3)
61-76: LGTM!
131-200: LGTM!
301-331: LGTM!scripts/stock-t3-canary-drill.sh (2)
4-54: LGTM!
153-217: LGTM!test/stock-t3-live-harness.test.ts (4)
1-33: LGTM!
35-77: LGTM!
222-363: LGTM!
391-391: No change needed forBun.file().stat().
BunFile.stat()is available for this declared Bun version and exposesmode, so this fixture permission check is valid.test/stock-t3-live.test.ts (1)
7-27: LGTM!test/stock-t3-sequence.test.ts (2)
6-41: LGTM!
163-200: LGTM!src/stockT3Contracts.ts (4)
12-57: LGTM!
59-155: LGTM!
157-216: LGTM!Also applies to: 244-319
321-363: LGTM!src/nativeRuntime.ts (8)
1-227: LGTM!
377-509: LGTM!Also applies to: 528-545
567-608: LGTM!
610-1013: LGTM!
1015-1540: LGTM!
1564-1825: LGTM!
1827-2444: LGTM!
2446-2492: LGTM!src/facade.ts (1)
1-21: LGTM!test/boundary-convergence.test.ts (1)
1-249: LGTM!Also applies to: 251-396, 438-922
test/facade.stock-http.test.ts (1)
1-891: LGTM!test/native-runtime-adapter.test.ts (1)
2-465: LGTM!test/r3-runtime-regressions.test.ts (1)
1-438: LGTM!test/r4-runtime-regressions.test.ts (1)
1-442: LGTM!test/r5-runtime-regressions.test.ts (1)
1-126: LGTM!Also applies to: 166-219, 251-338
test/r8-runtime-regressions.test.ts (1)
1-474: LGTM!Also applies to: 479-604
src/stockT3HttpClient.ts (4)
16-62: LGTM!
180-219: LGTM!
221-249: LGTM!
263-338: LGTM!src/adaptivePoller.ts (6)
4-113: LGTM!
130-210: LGTM!
212-289: LGTM!
291-353: LGTM!
355-474: LGTM!
476-571: LGTM!test/adaptive-poller.test.ts (1)
6-84: LGTM!Also applies to: 119-422
test/stock-t3-contracts.test.ts (1)
12-172: LGTM!test/stock-t3-http-client.test.ts (1)
8-243: LGTM!test/r6-runtime-regressions.test.ts (1)
19-399: LGTM!test/r7-runtime-regressions.test.ts (1)
18-466: LGTM!test/r9-runtime-regressions.test.ts (1)
9-664: LGTM!
| "packageManager": "bun@1.3.11", | ||
| "private": true, | ||
| "scripts": { | ||
| "check:stock-only": "bash scripts/check-stock-only.sh", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the stock-only gate independent of the author’s filesystem.
Both entry points invoke scripts/check-stock-only.sh without portable historical evidence inputs. Its default path is an author-local /Users/... path, so the documented command and package script fail on clean checkouts and CI.
package.json#L10-L10: expose a command that supplies repository-relative evidence or requires documented environment variables.README.md#L172-L172: document the same portable invocation and required evidence inputs.
📍 Affects 2 files
package.json#L10-L10(this comment)README.md#L172-L172
🤖 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 `@package.json` at line 10, The stock-only gate currently relies on an
author-local default evidence path. Update package.json line 10 to invoke
scripts/check-stock-only.sh with repository-relative evidence inputs or
documented environment variables, and update README.md line 172 to document that
same portable invocation and required inputs.
There was a problem hiding this comment.
Already resolved in the round-1 path cluster: the npm gate entry is repository-relative, while the two remaining absolute evidence paths are explicit revision-3 acceptance inputs.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- scripts/check-stock-only.sh ---'
nl -ba scripts/check-stock-only.sh | sed -n '1,180p'
printf '%s\n' '--- README.md relevant stock-only references ---'
rg -n -C 5 'check:stock-only|check-stock-only|evidence|revision-3' README.md
printf '%s\n' '--- package.json ---'
nl -ba package.jsonLength of output: 718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- scripts/check-stock-only.sh ---'
sed -n '1,180p' scripts/check-stock-only.sh
printf '%s\n' '--- README.md relevant stock-only references ---'
rg -n -C 5 'check:stock-only|check-stock-only|evidence|revision-3' README.md || true
printf '%s\n' '--- package.json ---'
sed -n '1,80p' package.jsonLength of output: 5599
@EtanHey, I verified the current files. The finding remains valid.
scripts/check-stock-only.sh still defaults STOCK_ONLY_HISTORICAL_PATH to:
/Users/etanheyman/Gits/t3layer/test/p2-live-proof-runner.test.ts
package.json does not provide STOCK_ONLY_HISTORICAL_PATH, and README Line 172 does not document it. Therefore, bun run check:stock-only fails on a clean checkout before the gate runs.
If revision-3 acceptance requires external absolute evidence, the package command must require or pass that input explicitly, and README must document the required variables.
You are interacting with an AI system.
| test("all setup/live fault seams preserve secret redaction and clean the proof root", async () => { | ||
| const seams = [ | ||
| "after-worktree-add", "after-stock-install", "after-stock-build", "after-archive-extract", | ||
| "after-candidate-install", "after-exact-characterization", "after-bearer-issue", | ||
| "after-secret-read", "after-server-launch", "after-readiness", "after-http-negative", | ||
| "after-live-test", "after-provisional-validation", "before-normal-exit", | ||
| ]; | ||
| for (const seam of seams) { | ||
| const result = await run(["bash", "scripts/stock-t3-live-harness.sh"], { | ||
| T3_STOCK_PROVIDER_SECRET_REF: "op://fixture/provider/key", | ||
| T3_STOCK_HARNESS_TEST_MODE: "1", | ||
| T3_STOCK_HARNESS_COMMAND_RUNNER: "/usr/bin/true", | ||
| T3_STOCK_FAIL_AT: seam, | ||
| }); | ||
| expect(result.exitCode, seam).not.toBe(0); | ||
| expect(result.stderr, seam).toContain(`injected failure: ${seam}`); | ||
| expect(result.stderr, seam).toContain("cleanup root_removed=true"); | ||
| expect(result.stderr, seam).not.toContain("op://fixture/provider/key"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Set an explicit timeout for the 14-seam loop.
This test spawns bash scripts/stock-t3-live-harness.sh 14 times in sequence. The default bun:test timeout is 5000 ms per test. The canary loop at Line 445 already passes 60_000 for the same reason. Add a timeout argument here to avoid a flaky failure.
💚 Proposed fix
- });
+ }, 60_000);Apply this to the test(...) call that ends at Line 198.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("all setup/live fault seams preserve secret redaction and clean the proof root", async () => { | |
| const seams = [ | |
| "after-worktree-add", "after-stock-install", "after-stock-build", "after-archive-extract", | |
| "after-candidate-install", "after-exact-characterization", "after-bearer-issue", | |
| "after-secret-read", "after-server-launch", "after-readiness", "after-http-negative", | |
| "after-live-test", "after-provisional-validation", "before-normal-exit", | |
| ]; | |
| for (const seam of seams) { | |
| const result = await run(["bash", "scripts/stock-t3-live-harness.sh"], { | |
| T3_STOCK_PROVIDER_SECRET_REF: "op://fixture/provider/key", | |
| T3_STOCK_HARNESS_TEST_MODE: "1", | |
| T3_STOCK_HARNESS_COMMAND_RUNNER: "/usr/bin/true", | |
| T3_STOCK_FAIL_AT: seam, | |
| }); | |
| expect(result.exitCode, seam).not.toBe(0); | |
| expect(result.stderr, seam).toContain(`injected failure: ${seam}`); | |
| expect(result.stderr, seam).toContain("cleanup root_removed=true"); | |
| expect(result.stderr, seam).not.toContain("op://fixture/provider/key"); | |
| } | |
| }); | |
| test("all setup/live fault seams preserve secret redaction and clean the proof root", async () => { | |
| const seams = [ | |
| "after-worktree-add", "after-stock-install", "after-stock-build", "after-archive-extract", | |
| "after-candidate-install", "after-exact-characterization", "after-bearer-issue", | |
| "after-secret-read", "after-server-launch", "after-readiness", "after-http-negative", | |
| "after-live-test", "after-provisional-validation", "before-normal-exit", | |
| ]; | |
| for (const seam of seams) { | |
| const result = await run(["bash", "scripts/stock-t3-live-harness.sh"], { | |
| T3_STOCK_PROVIDER_SECRET_REF: "op://fixture/provider/key", | |
| T3_STOCK_HARNESS_TEST_MODE: "1", | |
| T3_STOCK_HARNESS_COMMAND_RUNNER: "/usr/bin/true", | |
| T3_STOCK_FAIL_AT: seam, | |
| }); | |
| expect(result.exitCode, seam).not.toBe(0); | |
| expect(result.stderr, seam).toContain(`injected failure: ${seam}`); | |
| expect(result.stderr, seam).toContain("cleanup root_removed=true"); | |
| expect(result.stderr, seam).not.toContain("op://fixture/provider/key"); | |
| } | |
| }, 60_000); |
🤖 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/stock-t3-live-harness.test.ts` around lines 179 - 198, Set an explicit
60,000 ms timeout on the test containing the 14-seam loop in “all setup/live
fault seams preserve secret redaction and clean the proof root,” matching the
existing canary test’s timeout pattern.
| const first = await facade.wait(spawned.turnReceipt, { timeoutMs: 120_000 }); | ||
| const sent = await facade.send( | ||
| spawned.agentRef, | ||
| "Reply with exactly T3LAYER_STOCK_PROOF_FOLLOWUP_OK.", | ||
| { timeoutMs: 120_000 }, | ||
| ); | ||
| const second = await facade.wait(sent, { timeoutMs: 120_000 }); | ||
| const descriptor = await runtime.client.getDescriptor(); | ||
| const detail = await runtime.observe(spawned.agentRef, { timeoutMs: 30_000 }); | ||
| if (detail === undefined) throw new Error("live thread disappeared before receipt capture"); | ||
| const http = runtime.httpObservations(); | ||
| const polls = runtime.pollMetrics(); | ||
|
|
||
| const provisional = canonicalProvisionalProof({ | ||
| provisional: true, | ||
| success: false, | ||
| runId, | ||
| environmentId: descriptor.environmentId, | ||
| serverVersion: descriptor.serverVersion, | ||
| endpointStatusTrace: http.endpointStatusTrace, | ||
| ids: { | ||
| projectId: detail.thread.projectId, | ||
| threadId: spawned.agentRef.threadId, | ||
| createCommandId: spawned.createReceipt.commandId, | ||
| initialCommandId: spawned.turnReceipt.commandId, | ||
| initialMessageId: spawned.turnReceipt.messageId, | ||
| followupCommandId: sent.commandId, | ||
| followupMessageId: sent.messageId, | ||
| }, | ||
| sequences: { | ||
| create: spawned.createReceipt.acceptedSequence, | ||
| initial: spawned.turnReceipt.acceptedSequence, | ||
| followup: sent.acceptedSequence, | ||
| }, | ||
| terminalKinds: [first.kind, second.kind], | ||
| counters: { | ||
| requests: http.requestCount, | ||
| shellPolls: polls.shellStarts, | ||
| detailPolls: polls.detailStarts, | ||
| peakInFlight: Math.max(http.peakInFlight, polls.peakHttpInFlight), | ||
| }, | ||
| timestamps: { startedAt, completedAt: new Date().toISOString() }, | ||
| }, runId); | ||
| await Bun.write(receiptPath, `${JSON.stringify(provisional)}\n`); | ||
| expect(first.kind).toBe("completed"); | ||
| expect(second.kind).toBe("completed"); | ||
| }, 120_000); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Align the test timeout with the sum of the operation timeouts.
The flow uses two waits at 120_000 ms (Lines 48 and 54), a send at 120_000 ms (Line 52), and an observe at 30_000 ms (Line 56). The test timeout at Line 94 is also 120_000 ms. bun:test can abort the test before any operation reaches its own deadline. The receipt at Line 91 is then never written, and scripts/stock-t3-live-harness.sh fails at validate-provisional with no evidence. Raise the test timeout above the sum of the operation budgets, or lower the per-operation budgets.
💚 Proposed fix
- }, 120_000);
+ }, 420_000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const first = await facade.wait(spawned.turnReceipt, { timeoutMs: 120_000 }); | |
| const sent = await facade.send( | |
| spawned.agentRef, | |
| "Reply with exactly T3LAYER_STOCK_PROOF_FOLLOWUP_OK.", | |
| { timeoutMs: 120_000 }, | |
| ); | |
| const second = await facade.wait(sent, { timeoutMs: 120_000 }); | |
| const descriptor = await runtime.client.getDescriptor(); | |
| const detail = await runtime.observe(spawned.agentRef, { timeoutMs: 30_000 }); | |
| if (detail === undefined) throw new Error("live thread disappeared before receipt capture"); | |
| const http = runtime.httpObservations(); | |
| const polls = runtime.pollMetrics(); | |
| const provisional = canonicalProvisionalProof({ | |
| provisional: true, | |
| success: false, | |
| runId, | |
| environmentId: descriptor.environmentId, | |
| serverVersion: descriptor.serverVersion, | |
| endpointStatusTrace: http.endpointStatusTrace, | |
| ids: { | |
| projectId: detail.thread.projectId, | |
| threadId: spawned.agentRef.threadId, | |
| createCommandId: spawned.createReceipt.commandId, | |
| initialCommandId: spawned.turnReceipt.commandId, | |
| initialMessageId: spawned.turnReceipt.messageId, | |
| followupCommandId: sent.commandId, | |
| followupMessageId: sent.messageId, | |
| }, | |
| sequences: { | |
| create: spawned.createReceipt.acceptedSequence, | |
| initial: spawned.turnReceipt.acceptedSequence, | |
| followup: sent.acceptedSequence, | |
| }, | |
| terminalKinds: [first.kind, second.kind], | |
| counters: { | |
| requests: http.requestCount, | |
| shellPolls: polls.shellStarts, | |
| detailPolls: polls.detailStarts, | |
| peakInFlight: Math.max(http.peakInFlight, polls.peakHttpInFlight), | |
| }, | |
| timestamps: { startedAt, completedAt: new Date().toISOString() }, | |
| }, runId); | |
| await Bun.write(receiptPath, `${JSON.stringify(provisional)}\n`); | |
| expect(first.kind).toBe("completed"); | |
| expect(second.kind).toBe("completed"); | |
| }, 120_000); | |
| const first = await facade.wait(spawned.turnReceipt, { timeoutMs: 120_000 }); | |
| const sent = await facade.send( | |
| spawned.agentRef, | |
| "Reply with exactly T3LAYER_STOCK_PROOF_FOLLOWUP_OK.", | |
| { timeoutMs: 120_000 }, | |
| ); | |
| const second = await facade.wait(sent, { timeoutMs: 120_000 }); | |
| const descriptor = await runtime.client.getDescriptor(); | |
| const detail = await runtime.observe(spawned.agentRef, { timeoutMs: 30_000 }); | |
| if (detail === undefined) throw new Error("live thread disappeared before receipt capture"); | |
| const http = runtime.httpObservations(); | |
| const polls = runtime.pollMetrics(); | |
| const provisional = canonicalProvisionalProof({ | |
| provisional: true, | |
| success: false, | |
| runId, | |
| environmentId: descriptor.environmentId, | |
| serverVersion: descriptor.serverVersion, | |
| endpointStatusTrace: http.endpointStatusTrace, | |
| ids: { | |
| projectId: detail.thread.projectId, | |
| threadId: spawned.agentRef.threadId, | |
| createCommandId: spawned.createReceipt.commandId, | |
| initialCommandId: spawned.turnReceipt.commandId, | |
| initialMessageId: spawned.turnReceipt.messageId, | |
| followupCommandId: sent.commandId, | |
| followupMessageId: sent.messageId, | |
| }, | |
| sequences: { | |
| create: spawned.createReceipt.acceptedSequence, | |
| initial: spawned.turnReceipt.acceptedSequence, | |
| followup: sent.acceptedSequence, | |
| }, | |
| terminalKinds: [first.kind, second.kind], | |
| counters: { | |
| requests: http.requestCount, | |
| shellPolls: polls.shellStarts, | |
| detailPolls: polls.detailStarts, | |
| peakInFlight: Math.max(http.peakInFlight, polls.peakHttpInFlight), | |
| }, | |
| timestamps: { startedAt, completedAt: new Date().toISOString() }, | |
| }, runId); | |
| await Bun.write(receiptPath, `${JSON.stringify(provisional)}\n`); | |
| expect(first.kind).toBe("completed"); | |
| expect(second.kind).toBe("completed"); | |
| }, 420_000); |
🤖 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/stock-t3-live.test.ts` around lines 48 - 94, Increase the enclosing test
timeout in the stock T3 live test beyond the combined budgets of the two 120,000
ms waits, the 120,000 ms send, and the 30,000 ms observe so the receipt-writing
path after runtime.observe can complete. Keep the existing per-operation
timeouts unchanged unless adjusting them is necessary to ensure the test-level
timeout exceeds their total.
| if (url.pathname === "/.well-known/t3/environment") { | ||
| return Response.json({ | ||
| environmentId: "environment-fixture", | ||
| label: "fixture", | ||
| platform: { os: "darwin", arch: "arm64" }, | ||
| serverVersion: "d3037064-fixture", | ||
| capabilities: { repositoryIdentity: false }, | ||
| }); | ||
| } | ||
| if (request.headers.get("authorization") !== `Bearer ${auth}`) { | ||
| return Response.json( | ||
| { code: "auth_invalid", reason: "missing_credential", traceId: "redacted" }, | ||
| { status: 401 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Move the descriptor route behind the authorization check.
Line 47 answers /.well-known/t3/environment before the authorization check at Line 56. The fixture therefore never verifies that the runtime sends the bearer token on the descriptor request. A regression that drops the token from getDescriptor would still pass this test. If the stock server requires no credential on that route, add a comment that states it. Otherwise, place the route after the check.
🤖 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/stock-t3-sequence.test.ts` around lines 47 - 61, Move the
/.well-known/t3/environment response in the request handler behind the
authorization check so descriptor requests validate the Bearer token. Keep the
existing descriptor payload unchanged, and ensure invalid or missing credentials
return the existing 401 response before reaching that route.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4a1634e5-eef7-42d3-940c-1548b566df21) |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a7ca9833-08c0-4873-a873-314a847154cd) |
Replies to review threads that could not accept inline replies (HTTP 422 — thread outdated by fix commits)Comment 3696116457 (TRIVIAL — FIX): Fixed in Comment 3696116460 (MINOR — WONT_FIX (test-timeout style)): No change: the bounded seam loop passes consistently under the suite timeout; an additional per-test timeout would be redundant timing policy. Comment 3696116466 (MINOR — WONT_FIX (design cite: revision-3 line 458)): No change: revision-3 line 458 explicitly pins the live proof at 120,000 ms. The operation budgets are fail-safe ceilings, not an additive runtime allowance. Comment 3696116471 (MINOR — FIX): Fixed in Comment 3696116475 (TRIVIAL — WONT_FIX (design cite: revision-3 line 54)): No change: revision-3 line 54 defines descriptor auth as none, so the fixture intentionally serves it before bearer validation. Comment 3696128436 (MEDIUM — FIX): Fixed in Comment 3696128438 (HIGH — FIX): Fixed in Comment 3696128440 (MEDIUM — FIX): Fixed in Comment 3696128441 (HIGH — FIX): Fixed in Comment 3696128442 (HIGH — FIX): Fixed in All fixes are in |
|
@coderabbitai review |
|
@codex review |
Rate Limit Exceeded
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nativeRuntime.ts (1)
306-330: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject driveless POSIX absolute workspace roots when targeting
win32cross-platform.
path.win32.isAbsolute()accepts/tmp/projectand returnstrue, whilepath.win32.resolve('/Users/dev', '/tmp/project')maps it to\tmp\projectwithout a drive letter. Whenoptions.platform === "win32"(or"windows") andoptions.cwdis POSIX-only, a POSIX-style absoluteworkspaceRootshould be rejected before resolving so a Windows workspace root is never emitted without a drive letter.🤖 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/nativeRuntime.ts` around lines 306 - 330, Update canonicalizeWorkspaceRoot to reject POSIX-style absolute inputs when targeting win32 cross-platform and options.cwd is POSIX-only, before path.resolve runs. Add the identityConflict validation alongside the existing crossPlatform checks, while preserving valid drive-qualified Windows roots and existing home-directory handling.
🤖 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 `@test/adaptive-poller.test.ts`:
- Around line 210-241: Update the test “removes each resolved default-sleep
abort listener” so it observes the signal used by the repeated defaultSleep
calls rather than the per-cycle shell signal. Either instrument an accumulating
signal across sleep cycles or drive the rate-limit path through
reserveShellStart with repeated waits, then assert the listener count
demonstrates resolved sleep listeners are removed.
In `@test/stock-t3-http-client.test.ts`:
- Around line 231-232: Bound the microtask spin loops around the descriptor
admission checks: update the loop waiting for starts to reach 8 and the later
loop waiting for starts.length to reach 8 to use the same finite iteration limit
as the existing bounded spins near lines 279-281 and 291-293. Preserve the
current completion conditions while ensuring either loop exits so the test can
fail clearly when admission order is unexpected.
In `@test/stock-t3-live-harness.test.ts`:
- Around line 488-489: Replace the fixed 50 ms delay before child.kill in the
test with polling for observable drill readiness: wait until the canary recovery
marker is emitted or another existing readiness signal confirms recovery_armed
and trap installation, with a timeout to prevent hangs. Only send SIGINT after
readiness is observed, preserving the existing exit-code and output assertions.
---
Outside diff comments:
In `@src/nativeRuntime.ts`:
- Around line 306-330: Update canonicalizeWorkspaceRoot to reject POSIX-style
absolute inputs when targeting win32 cross-platform and options.cwd is
POSIX-only, before path.resolve runs. Add the identityConflict validation
alongside the existing crossPlatform checks, while preserving valid
drive-qualified Windows roots and existing home-directory handling.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 813762ba-7150-499f-88ed-991f9b9256d5
📒 Files selected for processing (21)
docs/operations/stock-t3-first-release.mdscripts/stock-proof-cli.tsscripts/stock-t3-canary-drill.shscripts/stock-t3-exact-characterization.shscripts/stock-t3-live-harness.shsrc/adaptivePoller.tssrc/facade.tssrc/nativeRuntime.tssrc/stockProof.tssrc/stockT3Contracts.tssrc/stockT3HttpClient.tstest/adaptive-poller.test.tstest/boundary-convergence.test.tstest/native-runtime-adapter.test.tstest/pr4-review-regressions.test.tstest/r8-runtime-regressions.test.tstest/stock-only-gate.test.tstest/stock-t3-exact-stock-negative.test.tstest/stock-t3-http-client.test.tstest/stock-t3-live-harness.test.tstest/stock-t3-live.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (37)
docs/operations/stock-t3-first-release.md (4)
1-24: LGTM!
26-33: LGTM!
42-58: LGTM!
34-40: 🗄️ Data Integrity & IntegrationNo wording change needed.
run_stepchecks the current artifact digest against the initial committedartifact_digestafter each stage, and allassert_artifact_unmodifiedcalls useverify_artifact. The receipt can also be updated to say the digest is compared with the commit hash rather than just recorded.> Likely an incorrect or invalid review comment.src/stockT3Contracts.ts (1)
44-44: LGTM!src/stockT3HttpClient.ts (2)
80-87: LGTM!Also applies to: 123-196
214-260: LGTM!test/adaptive-poller.test.ts (2)
2-2: LGTM!Also applies to: 83-84
243-296: LGTM!Also applies to: 298-343
src/adaptivePoller.ts (3)
67-68: LGTM!Also applies to: 105-105, 114-114, 123-133, 159-163
177-236: LGTM!Also applies to: 266-313, 328-343
407-413: LGTM!Also applies to: 427-431, 491-494, 505-512, 533-536, 589-589, 632-632, 660-660
test/stock-t3-http-client.test.ts (2)
96-116: LGTM!
252-301: LGTM!Also applies to: 303-312
src/stockProof.ts (1)
105-117: LGTM!Also applies to: 254-258, 301-311
scripts/stock-proof-cli.ts (1)
8-8: LGTM!Also applies to: 17-17
test/stock-only-gate.test.ts (1)
94-108: LGTM!scripts/stock-t3-canary-drill.sh (1)
33-44: LGTM!Also applies to: 67-72, 83-92, 98-104, 112-138, 140-175, 222-240
scripts/stock-t3-exact-characterization.sh (1)
15-28: LGTM!scripts/stock-t3-live-harness.sh (4)
22-48: LGTM!
224-237: LGTM!
310-334: LGTM!
136-144: 🩺 Stability & AvailabilityConfirm macOS is the supported harness OS, or make the mode read portable.
scripts/stock-t3-live-harness.shuses/usr/bin/stat -f '%Lp',/usr/sbin/lsof, and macOS-style/bin/ps -o lstart=paths, and README tooling references do not document Linux support. If Linux operators can run this script, replace the mode checks at lines 136 and 158 with a format selected byuname -sor another portablestatcall.test/stock-t3-exact-stock-negative.test.ts (1)
16-21: LGTM!test/stock-t3-live-harness.test.ts (4)
35-85: LGTM!
159-162: LGTM!Also applies to: 233-277
293-297: LGTM!
456-473: LGTM!test/stock-t3-live.test.ts (1)
60-63: LGTM!Also applies to: 82-84
src/nativeRuntime.ts (3)
250-291: LGTM!Also applies to: 547-564, 585-613, 643-661, 1618-1636
1-229: LGTM!Also applies to: 231-305, 413-545, 667-761, 763-882, 884-1016, 1018-1143, 1145-1182, 1184-1487, 1489-1597, 1599-1617, 1637-1898, 1900-2084, 2086-2202, 2342-2517, 2519-2565
2308-2336: 🩺 Stability & AvailabilityKeep the status-code heuristic behind the server contract.
The status-code retention behavior is covered for
send()intest/facade.stock-http.test.tsunderpreserves an ambiguous send receipt when its identical retry returns ...; the statuses themselves originate from the server-side dispatch error encoding, so the concern belongs to that contract, not this heuristic alone.src/facade.ts (1)
1-58: LGTM!test/boundary-convergence.test.ts (1)
295-313: LGTM!test/native-runtime-adapter.test.ts (1)
410-437: LGTM!test/pr4-review-regressions.test.ts (1)
1-52: LGTM!Also applies to: 54-68, 70-80, 131-179
test/r8-runtime-regressions.test.ts (1)
475-475: LGTM!
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b803e233-3ec4-490d-9f88-b32bb0d44cbf) |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5875ebca-710c-45f8-b733-7a4ce1ed59e5) |
| '{success:true,transitions:$transitions,artifactDigest:$digest,configDigestBefore:$before,configDigestAfter:$after,schema:"stock-http-v1",acceleration:"off",cancellation:$cancellation,commandStatuses:$statuses,descriptors:$descriptors,threadReadability:$threads,artifactChecks:$artifacts}' >"$body_staging" | ||
| checksum=$(sha256_file "$body_staging") | ||
| /usr/bin/jq -cS --arg checksum "$checksum" '. + {checksum:$checksum}' "$body_staging" >"$staging" | ||
| mv -f -- "$staging" "$T3_STOCK_DRILL_RECEIPT_PATH" |
There was a problem hiding this comment.
🟠 High scripts/stock-t3-canary-drill.sh:241
The script atomically installs the receipt at mv -f -- "$staging" "$T3_STOCK_DRILL_RECEIPT_PATH"" (line 241) with success:truealready baked into the body, but the required permission check, checksum reread, and validation all happen afterward (lines 242–255). Ifchmod, file_mode, jq, or the checksum comparison fails, the script exits without ever removing the receipt. The recovertrap only runs routing commands — it never deletes the installed file — so a failed drill leaves behind a receipt containingsuccess:truethat downstream release tooling can mistake for valid evidence. Ifchmod 600itself failed, the file may also retain an unsafe mode. Consider deferring themvuntil after all verification checks pass, or have the failure pathsrm -f` the receipt before exiting.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @scripts/stock-t3-canary-drill.sh around line 241:
The script atomically installs the receipt at `mv -f -- "$staging" "$T3_STOCK_DRILL_RECEIPT_PATH"" (line 241) with `success:true` already baked into the body, but the required permission check, checksum reread, and validation all happen afterward (lines 242–255). If `chmod`, `file_mode`, `jq`, or the checksum comparison fails, the script exits without ever removing the receipt. The `recover` trap only runs routing commands — it never deletes the installed file — so a failed drill leaves behind a receipt containing `success:true` that downstream release tooling can mistake for valid evidence. If `chmod 600` itself failed, the file may also retain an unsafe mode. Consider deferring the `mv` until after all verification checks pass, or have the failure paths `rm -f` the receipt before exiting.
1 participant
Summary
Phase 3 of the stock-T3 realignment (revision-3 design). T3Layer becomes a channel-agnostic external orchestration overlay for an unmodified stock T3 server:
@t3tools/runtime-clientis gone frompackage.json/bun.lock; the hand-written RPCprotocol.tsis deleted; a stock-only gate script (scripts/check-stock-only.sh) scans every tracked candidate path for forbidden private references and passes clean.src/stockT3Contracts.ts,src/stockT3HttpClient.ts): descriptor/auth/shell/thread-detail/dispatch schemas validated fail-closed at pinned upstreampingdotgg/t3code@d3037064; unknown additive fields tolerated; exact 400/401/403/500 dispatch error tuples.src/adaptivePoller.ts): one shell scheduler per environment, strict 250ms→2s cadence, rate/concurrency/evidence budgets, injected clock, cancellation, bounded backoff.src/nativeRuntime.ts,src/facade.ts):thread.create→ reconciliation → fresh empty-thread preflight → bootstrap-freethread.turn.start; receipt-targeted causalwaitwith exact prefix binding; expiring send leases; exhaustive typedSpawnResultunion; the no-throw-after-durable-mutation invariant across all three stock mutations; caller-held project-create identity for cross-runtime idempotency; stock-compatible workspace-root canonicalization.scripts/stock-t3-live-harness.sh,test/stock-t3-live.test.ts): exact-SHA disposable stock worktree, run-identity-bound forgery-rejecting receipts (18/18 forgery classes rejected), shell-hardened teardown.docs/operations/stock-t3-first-release.md,scripts/stock-t3-canary-drill.sh): dry-run correctly reportsrelease_blocked=truewithout a real deploy controller.Review provenance
Nine adversarial pre-commit review rounds (Opus + Codex Sol-xhigh), a boundary-convergence gate, a post-SHIP medium closure gate, and a final pre-commit gate. Final independent verdict: SHIP 9/10, zero HIGH / zero CRITICAL / zero MEDIUM. An independent read-only drift audit classified every consumed upstream surface unchanged/compatible across
a0419812..d3037064and authorized the re-pin.Test plan
bun testat pinned Bun 1.3.11: 222 pass / 2 authorized skips / 0 fail / 727 assertions / 19 filesbun run typecheckclean; ShellCheck clean;git diff --checkcleand3037064: green in disposable worktree, registry restoredtest/p2-live-proof-runner.test.tspreserved byte-for-byte (SHA-256 verified)🤖 Generated with Claude Code
Note
High Risk
Large architectural swap of orchestration, spawn/send/wait semantics, and error/receipt shapes; incorrect causal or lease handling could mis-attribute turn completion or block threads in production routing.
Overview
Replaces the
@t3tools/runtime-client/ Effect WebSocket stack with a stock-only HTTP orchestration layer against unmodified T3 public endpoints. Runtime dependencies drop to Bun/TypeScript dev tooling only;check:stock-onlyblocks forbidden private client references.Runtime surface:
createStockT3Facade/createStockT3NativeRuntimeexpose receipt-basedspawn,send,wait,observe, and lease management overstockT3HttpClient+adaptivePoller(coalesced shell polling, rate/concurrency caps, backoff). The old largecreateT3Facadeagent/subscription API is removed in favor of causalTurnReceiptwaits, caller-heldprojectCreateIdentity, two-stage HTTP spawn, and typed reconciliation/partial outcomes.Ops & proof: README documents the HTTP boundary, budgets, and causal contracts. Adds first-release runbook, canary drill (
off → canary → promoted → rollback → off), isolated live harness with checksum proof envelopes, and exact-stock SHA characterization at a pinned commit.Reviewed by Cursor Bugbot for commit b95c3ee. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Note
Replace private runtime client with stock T3 HTTP baseline in
nativeRuntime.ts@t3tools/runtime-clientandeffectas runtime dependencies, replacing the WebSocket/Effect-based orchestration with a new HTTP client (src/stockT3HttpClient.ts) and adaptive poller (src/adaptivePoller.ts).src/nativeRuntime.tsascreateStockT3NativeRuntime, exposingspawn,send,wait,observe,releaseReceipt,pollMetrics, andclosewith lease-based turn management and reconciliation for ambiguous dispatch outcomes.src/stockT3Contracts.tsand structured HTTP error handling with concurrency limits (8 in-flight max) in the new HTTP client.scripts/stock-t3-canary-drill.sh), a live harness (scripts/stock-t3-live-harness.sh), a stock-only gate (scripts/check-stock-only.sh), and a proof CLI (scripts/stock-proof-cli.ts).test/native-runtime-adapter.test.tswith stock runtime tests covering create state machine, read-only resume, deadline behavior, and regression scenarios (r3–r9).createT3NativeRuntimeis retained as an alias but all internal behavior has changed; consumers relying on WebSocket session semantics or Effect-based error types will break.Macroscope summarized b95c3ee.