Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 80 additions & 4 deletions .agents/upstream-review.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
remote: t3code-upstream
branch: main
reviewed-through: "a850895f6833b99d90fc6c50192b5eaa4966d5c7"
reviewed-through-date: "2026-08-19"
reviewed-through: "beab6886f45bf42906d0bd01aefe5dfe9e66a867"
reviewed-through-date: "2026-08-20"
---

# T3 upstream review log
Expand Down Expand Up @@ -1871,6 +1871,81 @@ unioned; the `legacy-plan-mode` settings-search id is registered against
F6 and H1 stay in their original batch tables as historical record — they
describe what was decided then. This entry is what supersedes them.

## 2026-08-20 — `a850895f6833b99d90fc6c50192b5eaa4966d5c7..beab6886f45bf42906d0bd01aefe5dfe9e66a867`

Eight upstream commits, four change sets. **Six adopted** onto
`upstream/2026-08-20-batch`; **two deferred** (see DEF-5). `git cherry` reported every
one absent from Pylon. The register was empty going in and gains one entry.

**`#7602` closes the canary follow-up opened by the second 2026-08-19 batch.** It moves
`@clerk/electron` to stable `0.0.34`, `@clerk/electron-passkeys` back to `0.0.3`, and
deletes all four platform-specific native canary entries from `minimumReleaseAgeExclude`.
Verified against the condition the ledger recorded: `npm view @clerk/electron dist-tags`
now reports `latest: 0.0.34`. Pylon no longer ships prerelease native binaries, and the
"only darwin-arm64 was ever exercised" risk retires with them.

**`#7150` and `#7152` are deferred, not skipped.** `#7150` rewrites 1084 lines of
`ChatComposer.tsx` — a file with 50 Pylon commits, structurally divergent at 4837 lines
against upstream's 2824. A full pass was made: all 11 conflict blocks were classified and
individually resolved, keeping Pylon's settle loop, `resolvedRuntimeMode`, the 169 lines
of Quick question and session-resource controls, and the ThreadHandoffTab wrapper, while
taking upstream's drawer measurement and its 335-line drawer structure. The result did
not compile — 8 JSX errors — because upstream flattens the fragment and nested
provider-frame divs into one `<form>` and its added tree cannot be hosted by Pylon's
layers. The two are ends of one restructure. `#7152` then conflicts on
`MessagesTimeline` because it genuinely depends on `#7150`.

**One conflict resolution leaked a secret and Pylon's own test caught it.** Adopting
`#7151` as a plain union put upstream's unconditional `toolCallId: event.itemId` beside
Pylon's `primeAgentTool` gating rather than behind it. A Prime tool's itemId is a
canonical filesystem path, so the projection began carrying
`canonical-prime-tool-/private/native-secret` — exactly what the gating exists to
withhold. Fixed in `0d70d9aec`; the guarding assertion lives in
`ProviderRuntimeIngestion.test.ts`.

Conflicts and adaptations, all resolved Pylon-first:

| File | Conflict | Resolution |
| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts` | `#7151` adds `toolCallId`, `status`, and `data` spreads to three tool payloads; Pylon gates the same payloads on `primeAgentTool` | `toolCallId` placed behind Pylon's gate; upstream's `status` kept for non-Prime alongside Pylon's derived Prime status; `tool.started` regained upstream's `data`. A later review collapsed the status ternary — both arms were identical. |

Review and validation:

- 4 changed test files, **115 tests, 0 failures**; the projection suite is 20 after the
test added below. Typecheck clean across 7 packages, each confirmed to have run.
- Validated live in the web client against a seeded copy of real data: pairing, thread
view, the Usage page including the "Hourly cost by provider" chart, and Settings →
Appearance with its theme list and Import theme. No error surfaces anywhere.
**Not verified:** `#7595`'s chronological hour ordering — the chart exposes no hour
labels to the DOM, so ordering could not be asserted from the browser.
- An xhigh review produced 13 findings. Two were fixed in `2cf95d544`. The rest are
upstream design decisions adopted as-is; the ones worth acting on later are below.

Open questions raised by this batch:

- **The snapshot dedupe widened.** `toolLifecycleIdentity` now prefers a payload-level
`toolCallId`, and `#7151` sets one on every tool activity, so identity is `id:<itemId>`
where it used to fall back to itemType/title/detail. The clients read only
`data.toolCallId` (`session-logic.ts` `extractToolCallId`, mobile's
`deriveToolLifecycleCollapseKey`), so they cannot mirror it: in-flight rows with
differing details render live and collapse after a reload. A test now pins the
behavior; whether it is the behavior Pylon wants is undecided. The doc comment on
`dropSupersededToolUpdatedActivities` still asserts an invariant measured under the old
identity.
- ~~`toolLifecycleIdentity`'s fallback joins with no separator.~~ **Withdrawn — false
positive.** The separator is a unit-separator character, which is invisible in source
output, so the call reads as `join("")` in a terminal and in review tooling; `cat -v`
shows the real `join("^_")`. Pylon and upstream are byte-identical here. Recorded so
the same illusion does not get re-reported.
- **`tool.started` now persists the provider's full unprojected `data`.** The
`item.updated` branch wraps in `projectActivityPayload`; `tool.started` does not, so a
large Write stores the whole body — and both clients skip `tool.started` rows entirely.
- **`#7642` doubles `MAX_UNCOMPRESSED_BYTES` to 100 MB** and raises `MAX_ZIP_ENTRIES`, a
relaxation of an anti-zip-bomb guard, to accommodate extensions shipping `node_modules`
the importer never reads. Bounding only the theme payload would fix the class.
- **`#7595` made the Past-24h empty state unreachable**: `hours` is always 24 entries, so
an idle day renders 24 `$0.00` rows instead of "No activity in this window."

## Deferred register

_The register is currently empty. DEF-1 and DEF-2 were adopted on 2026-08-11
Expand All @@ -1888,5 +1963,6 @@ Every review must read this register before reporting new candidates,
re-evaluate each `Revisit when` against the current upstream head, and report
the outcome. See Phase 2.5 of the `review-t3-upstream` skill.

| ID | Upstream | Deferred on | Revisit when | Why deferred |
| --- | -------- | ----------- | ------------ | ------------ |
| ID | Upstream | Deferred on | Revisit when | Why deferred |
| ----- | ----------------------------------------- | ----------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| DEF-5 | `#7150` `792a1404f` + `#7152` `4a9edff4c` | 2026-08-20 | Whenever the composer is next worked on deliberately. This is scheduled work, not a bet on upstream changing, so there is no waiting condition to poll. Before starting, check `git log --oneline 792a1404f..t3code-upstream/main -- apps/web/src/components/chat/ChatComposer.tsx` for follow-ups that should land with it. | `#7150` rewrites 1084 lines of `ChatComposer.tsx` against 50 Pylon commits and a 4837-vs-2824 line divergence. A full block-by-block resolution compiled to 8 JSX errors: upstream flattens the fragment and provider-frame divs into one `<form>`, so its 335-line drawer tree cannot be hosted by Pylon's layers. Needs the composer rebuilt on upstream's structure with Pylon's ThreadHandoffTab, provider frame classes, settle loop, `resolvedRuntimeMode`, and Quick question / session-resource controls grafted back. `#7152` depends on it. |
41 changes: 40 additions & 1 deletion apps/server/src/orchestration/ActivityPayloadProjection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function activity(payload: Record<string, unknown>): OrchestrationThreadActivity
* If slimming ever moves to an allowlist over the whole payload, these
* assertions are the tripwire.
*/
describe("projectActivityPayload agent-field survival", () => {
describe("projectActivityPayload", () => {
it("preserves tool attribution (agentId/parentToolUseId) through data slimming", () => {
const projected = projectActivityPayload(
activity({
Expand Down Expand Up @@ -97,6 +97,45 @@ describe("projectActivityPayload agent-field survival", () => {
expect(JSON.stringify(acp.payload).length).toBeLessThan(500);
});

it("normalizes Claude and OpenCode command inputs before slimming provider data", () => {
const claude = projectActivityPayload(
activity({
itemType: "command_execution",
toolCallId: "claude-call-1",
data: {
toolName: "Bash",
input: { command: "vp test run" },
result: { content: "x".repeat(5_000) },
},
}),
);
const openCode = projectActivityPayload(
activity({
itemType: "command_execution",
toolCallId: "opencode-call-1",
data: {
tool: "bash",
state: {
status: "running",
input: { command: "vp lint" },
output: "x".repeat(5_000),
},
},
}),
);

expect(claude.payload).toMatchObject({
toolCallId: "claude-call-1",
data: { command: "vp test run" },
});
expect(openCode.payload).toMatchObject({
toolCallId: "opencode-call-1",
data: { command: "vp lint" },
});
expect(JSON.stringify(claude.payload).length).toBeLessThan(200);
expect(JSON.stringify(openCode.payload).length).toBeLessThan(200);
});

it("slims Codex-shaped mcp_tool_call items to rendered fields plus a result summary", () => {
const projected = projectActivityPayload(
activity({
Expand Down
36 changes: 27 additions & 9 deletions apps/server/src/orchestration/ActivityPayloadProjection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,24 @@ function projectCommandData(data: Record<string, unknown>): Record<string, unkno
return Object.keys(projectedItem).length > 0 ? projectedItem : undefined;
}

function projectCommandValue(data: Record<string, unknown>): unknown {
if (data.command !== undefined) {
return data.command;
}

const input = asRecord(data.input);
if (input?.command !== undefined) {
return input.command;
}

const stateInput = asRecord(asRecord(data.state)?.input);
if (stateInput?.command !== undefined) {
return stateInput.command;
}

return undefined;
}

function summarizeToolTextOutput(value: string): string | null {
const lines: string[] = [];
for (const rawLine of value.split(/\r?\n/u)) {
Expand Down Expand Up @@ -339,8 +357,9 @@ export function projectActivityPayload(
if (item) {
projectedData.item = item;
}
if ("command" in data) {
projectedData.command = data.command;
const command = projectCommandValue(data);
if (command !== undefined) {
projectedData.command = command;
}

const changedFiles: string[] = [];
Expand Down Expand Up @@ -418,20 +437,19 @@ function dropStaleContextWindowActivities(
}

/**
* Identity both clients use to fold a tool lifecycle row into the call it
* belongs to (`deriveToolLifecycleCollapseKey` in web's `session-logic` and
* mobile's `threadActivity`): an explicit `data.toolCallId` when the adapter
* emits one, otherwise the itemType/title/detail triple. Returns null for rows
* with no identity at all — those never collapse on the client either, so they
* must not be dropped here.
* Identity used to retain only the newest lifecycle row for each call in a
* thread snapshot. Prefer the runtime item id, then the legacy nested id, and
* finally the itemType/title/detail triple. Rows without any identity remain
* untouched.
*/
function toolLifecycleIdentity(activity: OrchestrationThreadActivity): string | null {
const payload = asRecord(activity.payload);
if (!payload) {
return null;
}

const toolCallId = asTrimmedString(asRecord(payload.data)?.toolCallId);
const toolCallId =
asTrimmedString(payload.toolCallId) ?? asTrimmedString(asRecord(payload.data)?.toolCallId);
if (toolCallId) {
return `id:${toolCallId}`;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3312,11 +3312,16 @@ describe("ProviderRuntimeIngestion", () => {
createdAt: now,
threadId: asThreadId("thread-1"),
turnId: asTurnId("turn-9"),
itemId: asItemId("tool-call-9"),
payload: {
itemType: "command_execution",
status: "in_progress",
title: "Read file",
detail: "/tmp/file.ts",
status: "inProgress",
title: "Command run",
detail: "Bash: vp test run",
data: {
toolName: "Bash",
input: { command: "vp test run" },
},
},
});

Expand All @@ -3331,11 +3336,20 @@ describe("ProviderRuntimeIngestion", () => {
);

expect(thread.session?.status).toBe("ready");
expect(
thread.activities.some(
(activity: ProviderRuntimeTestActivity) => activity.kind === "tool.started",
),
).toBe(true);
const activity = thread.activities.find(
(entry: ProviderRuntimeTestActivity) => entry.kind === "tool.started",
);
const payload = activity?.payload as Record<string, unknown> | undefined;
expect(payload).toMatchObject({
itemType: "command_execution",
toolCallId: "tool-call-9",
status: "inProgress",
detail: "Bash: vp test run",
data: {
toolName: "Bash",
input: { command: "vp test run" },
},
});
});

it("consumes P1 runtime events into thread metadata, diff checkpoints, and activities", async () => {
Expand Down Expand Up @@ -3453,6 +3467,7 @@ describe("ProviderRuntimeIngestion", () => {
expect(toolUpdate?.kind).toBe("tool.updated");
expect(toolUpdatePayload?.itemType).toBe("command_execution");
expect(toolUpdatePayload?.status).toBe("in_progress");
expect(toolUpdatePayload?.toolCallId).toBe("item-p1-tool");

const warning = thread.activities.find(
(activity: ProviderRuntimeTestActivity) => activity.id === "evt-runtime-warning",
Expand Down
16 changes: 14 additions & 2 deletions apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,9 @@ export function runtimeEventToActivities(
summary: toolLifecycleActivityTitle(event, "Tool updated"),
payload: {
itemType: event.payload.itemType,
// A Prime tool's itemId is a canonical path, so it stays behind the
// same gate as the rest of the native detail.
...(primeAgentTool || event.itemId === undefined ? {} : { toolCallId: event.itemId }),
...(event.payload.status ? { status: event.payload.status } : {}),
...(primeAgentTool || !event.payload.detail
? {}
Expand Down Expand Up @@ -1395,7 +1398,10 @@ export function runtimeEventToActivities(
summary: toolLifecycleActivityTitle(event, "Tool"),
payload: {
itemType: event.payload.itemType,
...(primeAgentTool ? toolLifecycleActivityStatus(event) : {}),
// A Prime tool's itemId is a canonical path, so it stays behind the
// same gate as the rest of the native detail.
...(primeAgentTool || event.itemId === undefined ? {} : { toolCallId: event.itemId }),
...toolLifecycleActivityStatus(event),
...(primeAgentTool || !event.payload.detail
? {}
: { detail: truncateDetail(event.payload.detail) }),
Expand Down Expand Up @@ -1455,10 +1461,16 @@ export function runtimeEventToActivities(
summary: `${toolLifecycleActivityTitle(event, "Tool")} started`,
payload: {
itemType: event.payload.itemType,
...(primeAgentTool ? toolLifecycleActivityStatus(event) : {}),
// A Prime tool's itemId is a canonical path, so it stays behind the
// same gate as the rest of the native detail.
...(primeAgentTool || event.itemId === undefined ? {} : { toolCallId: event.itemId }),
...toolLifecycleActivityStatus(event),
...(primeAgentTool || !event.payload.detail
? {}
: { detail: truncateDetail(event.payload.detail) }),
...(primeAgentTool || event.payload.data === undefined
? {}
: { data: event.payload.data }),
...(primeAgentTool || !event.payload.agentId ? {} : { agentId: event.payload.agentId }),
...(primeAgentTool || !event.payload.parentToolUseId
? {}
Expand Down
Loading
Loading