Repository navigation
feat: add bounded T3 agent facade - #1
Conversation
|
@coderabbitai review @codex review Independent local adversarial gate: SHIP_SLICE after replaying the final four counterexamples. Full local evidence: Bun 1.3.11 81/81, typecheck, formatting, diff hygiene, and Gitleaks all clean on head 2a740ab. |
📝 WalkthroughWalkthroughAdds a typed T3 facade for project/thread spawning, turn dispatch, ambiguous-operation recovery, lifecycle waiting, typed transport errors, evidence limits, and identity validation, with comprehensive tests and an exact Bun type dependency pin. ChangesT3 facade runtime integration
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant T3Facade
participant NativeRuntime
Caller->>T3Facade: spawn or reuse project and thread
T3Facade->>NativeRuntime: create project or start thread
NativeRuntime-->>T3Facade: thread snapshot
Caller->>T3Facade: send turn
T3Facade->>NativeRuntime: dispatch turn
NativeRuntime-->>T3Facade: receipt or ambiguous outcome
Caller->>T3Facade: wait for lifecycle completion
T3Facade->>NativeRuntime: lookup and subscribe to thread
NativeRuntime-->>T3Facade: snapshots and thread events
T3Facade-->>Caller: agent events or typed terminal error
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Action performedReview finished.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/facade.ts`:
- Around line 701-714: Enforce monotonic observation ordering in the iteration
block before replacing lastSnapshot or yielding an event: compare
next.value.sequence with the previously accepted sequence (including
afterSequence), reject or skip duplicates and out-of-order snapshots according
to the existing transport-unavailable behavior, and only update lastSnapshot and
emit when sequence advances. Use the visible next, lastSnapshot, and
afterSequence flow without changing evidence accounting for accepted events.
- Around line 279-328: Update the lifecycle classification in the snapshot
status logic so interrupted and error states are evaluated before
pendingApproval/pendingInput, ensuring failed turns are not reported as
awaiting_input; preserve awaiting_input for non-failed snapshots. Also align the
assistantContent inclusion condition with the completed branch by requiring
trimmed assistant content to be non-empty before attaching it.
In `@test/facade.send.test.ts`:
- Around line 63-74: Add a send evidence case in the relevant facade test using
a multibyte message and assert that messageBytes equals its encoded byte length,
not its character count; update the expected message content and byte value
consistently while preserving the existing evidence shape and secret-exclusion
assertion. Apply the same multibyte coverage in the spawn facade tests so both
messageBytes paths pin byte-length semantics.
- Around line 223-229: Update the recovered receipt assertions and related
receipt contract to expose the recovery sequence through a stable field instead
of snapshotSequence. Adjust TurnReceipt and the recovered/dispatch
discriminators to use sequenceSource ("dispatch" or "projection"), ensuring
recovered receipts provide the stable sequence field while preserving direct
dispatch behavior.
In `@test/facade.spawn.test.ts`:
- Around line 11-49: The runtime doubles are duplicated across both facade test
suites instead of using a shared configurable factory. Add a shared
createRuntimeDouble helper with inert defaults and per-test overrides, then
replace the inline stubs in test/facade.spawn.test.ts lines 11-49 and all
remaining tests in that file, including overrides for listProjects, startThread,
and getThread; update test/facade.send.test.ts lines 8-36 to use the same helper
with only startTurn and getThread overridden as needed.
In `@test/facade.wait.test.ts`:
- Around line 76-98: Extract the shared six-method runtime stub used by the
tests into a factory, adding the required NativeRuntime type import from
../src/facade. Keep the unused listProjects, createProject, startThread, and
startTurn implementations in the factory, while allowing each test to provide
its varying getThread and subscribeThread behavior; replace the repeated inline
runtime objects with this helper.
🪄 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: ecf9b8f9-0d9e-46bb-8a1b-ce22137b0872
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
package.jsonsrc/facade.tstest/facade.send.test.tstest/facade.spawn.test.tstest/facade.wait.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: Macroscope - Correctness Check
🔇 Additional comments (10)
src/facade.ts (7)
1-216: LGTM!
218-274: LGTM!
333-357: LGTM!
529-626: LGTM!
628-700: LGTM!Also applies to: 717-729
514-526: 🩺 Stability & AvailabilityConfirm
startThreadatomically materializes the firstlatestTurn.userMessageId.This path rejects
transport_unavailableunlesssnapshot.latestTurn?.userMessageId === messageId. Since this repository has no native runtime projection or tests showing thatgetThread()populateslatestTurnbefore the returned thread sequence is observable, confirm that contract; otherwise gate only onthreadId/projectIdand resolveuserMessageIdwith a bounded follow-up read.
393-425: 🗄️ Data Integrity & IntegrationManual verification needed: native
createProjectidempotency contractThe retry on
AmbiguousDispatchErrorreplays the sameprojectCommand, so duplicate-project avoidance depends on native-side deduplication keyed bycommandId/workspaceRoot. That contract is not established in the available evidence.test/facade.wait.test.ts (1)
12-39: LGTM!Also applies to: 126-273, 275-478, 480-646, 648-803, 805-959
package.json (1)
11-11: 📐 Maintainability & Code QualityNo change needed for
@types/bun@1.3.11.The pinned package is published, no
peerDependenciesortypesVersionsconstraints block the lockfile TypeScript pin, and it does not conflict with the existing toolchain.test/facade.spawn.test.ts (1)
292-375: LGTM!Also applies to: 377-443, 445-514, 516-567, 569-656, 658-745
|
@coderabbitai review @codex review Fix head: d1980c9. Accepted pending-approval/input finding is closed with two regression tests. Full Bun 1.3.11 suite: 83 passed, 0 failed; typecheck, formatting, diff hygiene, and Gitleaks clean. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR diff summary ==='
git diff --stat origin/main...HEAD || git diff --stat HEAD~1...HEAD
echo '=== Recent commits ==='
git log --oneline -5
echo '=== Files changed by the fix commit ==='
git show --format=fuller --name-status --find-renames d1980c9
echo '=== Facade structure ==='
ast-grep outline src/facade.ts --items all --view expanded
echo '=== Relevant lifecycle and approval/input references ==='
rg -n -i -C 5 'pending|approval|input|wait\s*\(' src/facade.ts test/facade.wait.test.ts
echo '=== Fix commit patch ==='
git show --format= --find-renames --unified=80 d1980c9 -- src/facade.ts test/facade.wait.test.tsLength of output: 49360 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Exact regression-test changes in d1980c9 ==='
git diff d1980c9^ d1980c9 -- test/facade.send.test.ts
echo '=== send implementation ==='
sed -n '520,635p' src/facade.ts
echo '=== Test structure and pending-state test contexts ==='
ast-grep outline test/facade.send.test.ts --items all --view expanded
rg -n -i -C 18 'pendingApproval|pendingInput|startTurn' test/facade.send.test.ts
echo '=== All runtime dispatch call sites in the facade ==='
rg -n -C 4 'runtime\.(startTurn|startThread)\s*\(' src/facade.tsLength of output: 24592
The preflight guard now rejects both ✅ Action performedReview finished.
|
|
@coderabbitai review @codex review Final bounded fix head: 482a468. All five accepted round-two findings are closed; the two broad test-factory refactors remain intentionally rejected. Independent root verification: exact Bun 1.3.11 95/95, typecheck, Prettier, diff hygiene, and Gitleaks clean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata ==='
gh pr view 1 --json number,headRefName,headRefOid,baseRefName,commits,files --jq '{number, headRefName, headRefOid, baseRefName, commits: [.commits[] | {oid,messageHeadline}], files: [.files[] | {path,additions,deletions}]}'
echo '=== Commits after prior reviewed head ==='
git log --oneline d1980c9..HEAD
echo '=== Changed files after prior reviewed head ==='
git diff --stat d1980c9...HEAD
git diff --name-status d1980c9...HEAD
echo '=== Focused final-fix diff ==='
git diff --find-renames --unified=60 d1980c9...HEAD -- src/facade.ts test/facade.send.test.ts test/facade.spawn.test.ts test/facade.wait.test.ts package.json
echo '=== Facade map ==='
ast-grep outline src/facade.ts --items all --view expanded
echo '=== Test maps ==='
ast-grep outline test/facade.send.test.ts --items all --view expanded
ast-grep outline test/facade.spawn.test.ts --items all --view expanded
ast-grep outline test/facade.wait.test.ts --items all --view expandedLength of output: 64015 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Package and public entrypoints ==='
cat -n package.json
fd -t f -a '^(index|facade)\.(ts|tsx|js|mjs|cjs)$|^tsconfig.*\.json$' . | sort
echo '=== Complete facade implementation (bounded ranges) ==='
sed -n '1,260p' src/facade.ts
sed -n '260,560p' src/facade.ts
sed -n '560,820p' src/facade.ts
echo '=== Receipt/observation usage outside facade ==='
rg -n -C 4 --glob '!bun.lock' \
'(snapshotSequence|sequenceSource|TurnReceipt|NativeThreadObservation|afterSequence|subscribeThread)' \
.
echo '=== Reconciliation and stream regression tests ==='
rg -n -C 20 \
'(ambiguous|reconcili|does not retry|different thread|duplicates the initial|regresses after|failedState|sequenceSource)' \
test/facade.send.test.ts test/facade.spawn.test.ts test/facade.wait.test.ts
echo '=== Diff check for non-test source changes in final commit ==='
git diff --check d1980c9...HEADLength of output: 125599 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Package and public entrypoints ==='
cat -n package.json
fd -t f -a '^(index|facade)\.(ts|tsx|js|mjs|cjs)$|^tsconfig.*\.json$' . | sort
echo '=== Complete facade implementation (bounded ranges) ==='
sed -n '1,260p' src/facade.ts
sed -n '260,560p' src/facade.ts
sed -n '560,820p' src/facade.ts
echo '=== Receipt/observation usage outside facade ==='
rg -n -C 4 --glob '!bun.lock' \
'(snapshotSequence|sequenceSource|TurnReceipt|NativeThreadObservation|afterSequence|subscribeThread)' \
.
echo '=== Reconciliation and stream regression tests ==='
rg -n -C 20 \
'(ambiguous|reconcili|does not retry|different thread|duplicates the initial|regresses after|failedState|sequenceSource)' \
test/facade.send.test.ts test/facade.spawn.test.ts test/facade.wait.test.ts
echo '=== Diff check for non-test source changes in final commit ==='
git diff --check d1980c9...HEADLength of output: 125598 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Exact wait observation validation block ==='
nl -ba src/facade.ts | sed -n '700,742p'
echo '=== Observation fixtures: envelope sequence versus snapshot sequence ==='
rg -n -U -C 3 \
'yield \{ sequence: [0-9]+, snapshot: [A-Za-z_][A-Za-z0-9_]* \};' \
test/facade.wait.test.ts
echo '=== Existing tests mentioning snapshot/observation mismatch ==='
rg -n -i -C 4 \
'(snapshotSequence.*sequence|sequence.*snapshotSequence|mismatch|monotonic|duplicate|regress)' \
test/facade.wait.test.ts src/facade.ts
echo '=== TypeScript configuration ==='
cat -n tsconfig.jsonLength of output: 1004 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Exact wait observation validation block ==='
awk 'NR >= 700 && NR <= 742 { printf "%6d %s\n", NR, $0 }' src/facade.ts
echo '=== Observation fixtures: envelope sequence versus snapshot sequence ==='
rg -n -U -C 3 \
'yield \{ sequence: [0-9]+, snapshot: [A-Za-z_][A-Za-z0-9_]* \};' \
test/facade.wait.test.ts || true
echo '=== Existing tests mentioning snapshot/observation mismatch ==='
rg -n -i -C 4 \
'(snapshotSequence.*sequence|sequence.*snapshotSequence|mismatch|monotonic|duplicate|regress)' \
test/facade.wait.test.ts src/facade.ts || true
echo '=== TypeScript configuration ==='
cat -n tsconfig.jsonLength of output: 8652
Fail closed unless the observation sequence and snapshot sequence agree (and therefore the emitted snapshot sequence advances), and add a regression covering an advancing envelope with a duplicate/regressing You are interacting with an AI system. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 482a468. Configure here.
|
Fixed the remaining stream-integrity issue in the new head. @coderabbitai review @codex review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR and incremental commits ==='
gh pr view 1 --json headRefOid,baseRefName --jq '{headRefOid, baseRefName}'
git log --oneline 482a468..HEAD
echo '=== Incremental changed files ==='
git diff --name-status 482a468...HEAD
git diff --check 482a468...HEAD
echo '=== Facade structure ==='
ast-grep outline src/facade.ts --items all --view expanded
echo '=== wait() sequence validation and event emission ==='
rg -n -C 14 \
'(lastObservationSequence|snapshotSequence|observation.*sequence|sequence.*snapshot|AgentEvent|subscribeThread)' \
src/facade.ts
echo '=== Stream-integrity regression tests ==='
rg -n -i -C 22 \
'(duplicate.*initial|post.*advance|snapshot.*regress|regress.*snapshot|envelope.*sequence|observation.*sequence|snapshotSequence)' \
test/facade.wait.test.ts
echo '=== Exact incremental patch ==='
git diff --find-renames --unified=80 482a468...HEAD -- src/facade.ts test/facade.wait.test.tsLength of output: 62201
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
@coderabbitai review @codex review All known review threads are now dispositioned. Latest bounded fix preserves pending terminal state ahead of the empty-assistant invariant; root verification is Bun 1.3.11 99/99 plus typecheck, Prettier, diff hygiene, and Gitleaks. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR head and commits since prior reviewed revision ==='
gh pr view 1 --json headRefOid,baseRefName,commits --jq \
'{headRefOid, baseRefName, commits: [.commits[] | {oid, messageHeadline}]}'
git log --oneline e39d8fe..HEAD
echo '=== Incremental files and whitespace validation ==='
git diff --name-status e39d8fe...HEAD
git diff --check e39d8fe...HEAD
echo '=== Incremental patch ==='
git diff --find-renames --unified=100 e39d8fe...HEAD -- src/facade.ts test/facade.wait.test.ts test/facade.send.test.ts test/facade.spawn.test.ts package.json
echo '=== Facade structure ==='
ast-grep outline src/facade.ts --items all --view expanded
echo '=== Lifecycle / terminal / empty-assistant handling ==='
rg -n -i -C 18 \
'(pending|terminal|empty.*assistant|assistant.*empty|completion|wait\s*\(|FacadeError)' \
src/facade.ts test/facade.wait.test.ts
echo '=== All wait() regressions and relevant fixture definitions ==='
ast-grep outline test/facade.wait.test.ts --items all --view expanded
rg -n -i -C 16 \
'(pending.*terminal|terminal.*pending|empty.*assistant|assistant.*empty|pendingApproval|pendingInput|stopped|interrupted|failed)' \
test/facade.wait.test.tsLength of output: 138477
✅ Action performedReview finished.
|
1 participant
Problem
T3Layer needs a thin, deterministic control-plane facade before it can replace UI-driven orchestration. The facade must fail closed across ambiguous native dispatches, identity mismatches, lifecycle transitions, timeouts, and bounded evidence.
Solution
spawn,send, andwaitmappings over a supplied native T3 runtimeVerification
Full P2 is intentionally not claimed complete: immutable runtime-client distribution, the production adapter, remaining three operations, and the live programmatic proof are separate follow-up slices.
Implemented with GPT-5.6 Codex in T3 Code.
Note
Medium Risk
New orchestration layer with subtle idempotency and identity checks that will gate all agent control-plane traffic; risk is mitigated by heavy test coverage and no live runtime adapter in this PR.
Overview
Introduces
createT3Facadeinsrc/facade.ts, a control-plane wrapper over an injectableNativeRuntimewithspawn,send, andwaitfor programmatic agent orchestration.spawnresolves or creates a project by workspace root (without creating the workspace directory), preallocates stable IDs, callsstartThreadfor the first turn, and onAmbiguousDispatchErrorreconciles viagetThread/listProjectswith at most one identical retry—rejecting thread/project/message identity mismatches.sendpreflights withrequireSendableThread(blocks in-flight turns, pending approval/input), dispatchesstartTurn, and returns aTurnReceiptthat marks whether the sequence came from dispatch or recovered projection.waitstreamsAgentEventlifecycle updates from snapshots andsubscribeThread, enforces deadline timeouts andmaxEvidenceBytes, validates monotonic observation sequences, maps native state to lifecycles (includingawaiting_inputandempty_assistant_response), and surfacesFacadeErrorwith allowlisted structural snapshots (no raw assistant text or hostile session fields in errors). Optionalevidencehooks record operation metadata and byte counts without embedding user messages.Also adds
bun.lock, pins@types/bunto1.3.11, and adds broad Bun tests for spawn/send/wait adversarial paths (ambiguous dispatch, identity collisions, timeouts, buffer caps, credential leakage).Reviewed by Cursor Bugbot for commit d5e5d73. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests
Note
Add
createT3Facadewith spawn, send, and wait API over a native agent runtimesrc/facade.tsexportingcreateT3Facade(runtime, options), a high-level facade with three operations:spawn(create project and start thread),send(dispatch a turn with preflight validation), andwait(streamAgentEvents via async iterator until a terminal lifecycle state).NativeRuntime,NativeThreadSnapshot,AgentEvent,AgentLifecycle,FacadeError, etc.) for all runtime interactions, withFacadeErrorcarrying structured error codes (transport_unavailable,turn_error,empty_assistant_response,timeout,buffer_exhausted) and a sanitizedstructuralSnapshot.spawnandsendby retrying with identical pre-allocated IDs and reconciling viagetThreadidentity checks, throwingAmbiguousDispatchErroron collisions.waitenforces a configurable deadline andmaxEvidenceBytescap, validates snapshot sequence monotonicity, and fails closed on identity mismatches or stalled subscriptions.test/facade.spawn.test.ts,test/facade.send.test.ts, andtest/facade.wait.test.tscovering normal paths, multibyte evidence counting, blocked-state rejection, and all failure modes.Macroscope summarized d5e5d73.