starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-06 00:02:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/main' into orchestration-v3

Browse Source 
# Conflicts:
#	config/scripts/skill-description-length.test.mjs
#	resources/skills/current-manifest.json
#	resources/skills/snapshot-registry.json
#	skill-guides/orchestration.md
#	skills/orchestration/SKILL.md
#	src/cli/bundled-skill-guides.ts
This commit is contained in:
Jinwoo-H
2026-09-04 14:28:08 -04:00
parent d571ab154f 3e4fd4a7af
commit 9806af4258
77 changed files with 3092 additions and 260 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+2 -2
View File
@@ -238,9 +238,9 @@ Pair with your desktop app to monitor and steer your agents from your phone.
- **Discord:** Join the community on **[Discord](https://discord.gg/fzjDKHxv8Q)**.
- **Twitter / X:** Follow **[@orca_build](https://x.com/orca_build)** for updates and announcements.
- **WeChat:** Scan to join the Orca community WeChat group 8.
- **WeChat:** Scan to join the Orca community WeChat group 8. Group 8 may be full; if so, scan the Group 9 QR code instead.
<img src="docs/assets/wechat-qr-group8.jpg" alt="WeChat group 8 QR code for the Orca community" width="160" />
<img src="docs/assets/wechat-qr-group8.jpg" alt="WeChat group 8 QR code for the Orca community" width="160" />&nbsp;&nbsp;<img src="docs/assets/wechat-qr-group9.jpg" alt="WeChat group 9 QR code for the Orca community" width="160" />
- **Feedback &amp; Ideas:** We ship fast. Missing something? [Request a new feature](https://github.com/stablyai/orca/issues).
- **Privacy:** See the [privacy &amp; telemetry docs](https://www.onorca.dev/docs/telemetry) for what anonymous usage data Orca collects and how to opt out.
cloud/apps/relay/src/assignment-connection-headroom-postgres.test.ts
+6
View File
@@ -44,6 +44,12 @@ describePostgres('PostgreSQL assignment connection headroom', () => {
`DELETE FROM relay_assignments
WHERE user_id LIKE 'connection-headroom-postgres-%'`
)
// A snapshot left by an aborted run rejects the replayed watermark
// with stale_connection_snapshot.
await database.query(
`DELETE FROM relay_cell_connection_snapshots WHERE cell_id = ?`,
[cell.id]
)
await database.query(
`DELETE FROM relay_cell_connection_runtime WHERE cell_id = ?`,
[cell.id]
cloud/apps/relay/src/assignment-control-supersession-postgres.test.ts
+4
View File
@@ -38,6 +38,10 @@ describePostgres('PostgreSQL control supersession', () => {
[identity.userId]
)
await database.query(`DELETE FROM relay_assignments WHERE user_id = ?`, [identity.userId])
// A snapshot left by an aborted run rejects the replayed watermark with stale_connection_snapshot.
await database.query(`DELETE FROM relay_cell_connection_snapshots WHERE cell_id = ?`, [
cell.id
])
await database.query(`DELETE FROM relay_cell_connection_runtime WHERE cell_id = ?`, [cell.id])
await database.query(`DELETE FROM relay_cell_connection_limits WHERE cell_id = ?`, [cell.id])
await database.query(`DELETE FROM relay_cell_runtime WHERE cell_id = ?`, [cell.id])
cloud/apps/relay/src/assignment-store.ts
+24 -8
View File
@@ -3202,8 +3202,7 @@ export class RelayAssignmentStore {
)
const requestDelta = ACTIVITY_REQUEST_UNITS[kind] * (after - before)
if (requestDelta !== 0) {
await this.lockCellInventory(transaction, 'request')
await this.adjustCellReservation(transaction, text(row, 'cell_id'), requestDelta)
await this.adjustCellReservationAtomically(transaction, text(row, 'cell_id'), requestDelta)
}
})
})
@@ -3263,9 +3262,12 @@ export class RelayAssignmentStore {
}
const units = ACTIVITY_REQUEST_UNITS[input.kind]
if (existing) {
await this.lockCellInventory(transaction, 'request')
// Why: a client-chosen activity id can move between cells, so lock the
// one or two rows this path touches in cell_id order, the same order
// placement takes the inventory in, and no cycle can form.
await this.lockCellRows(transaction, [text(existing, 'cell_id'), input.cellId])
await this.removeActivityLease(transaction, identity, existing, now)
await this.adjustCellReservation(transaction, input.cellId, units)
await this.adjustCellReservationAtomically(transaction, input.cellId, units)
}
await this.adjustActivityCount(transaction, identity, input.kind, 1, expiresAt, now)
await transaction.query(
@@ -3580,8 +3582,7 @@ export class RelayAssignmentStore {
)
await this.touchAssignment(transaction, identity, expiresAt, now)
} else {
await this.lockCellInventory(transaction, 'request')
await this.adjustCellReservation(transaction, input.cellId, 1)
await this.adjustCellReservationAtomically(transaction, input.cellId, 1)
await this.adjustActivityCount(transaction, identity, 'control', 1, expiresAt, now)
await transaction.query(
`INSERT INTO relay_assignment_activity_leases
@@ -6954,6 +6955,19 @@ export class RelayAssignmentStore {
return rows
}
// Per-connection paths touch one or two cells. Locking exactly those rows,
// in the same ascending order the inventory lock uses (ORDER BY fixes the
// row-lock order), keeps them off the fleet-wide lock without a cycle.
private async lockCellRows(database: RelayDatabase, cellIds: string[]): Promise<SqlRow[]> {
const distinct = [...new Set(cellIds)]
return await database.queryLocked(
`SELECT * FROM relay_cells WHERE cell_id IN (${distinct.map(() => '?').join(', ')})
ORDER BY cell_id ASC`,
distinct,
{ lockTimeoutMs: CELL_INVENTORY_LOCK_TIMEOUT_MS }
)
}
private async lockGeneralCellInventory(
database: RelayDatabase,
mode: CellInventoryLockMode
@@ -7590,7 +7604,10 @@ export class RelayAssignmentStore {
) {
throw new Error('activity_lease_shape_mismatch')
}
const cells = await this.lockCellInventory(database, 'request')
// Why: this recomputes one cell's reservation from its leases, so only that
// row needs to be held; the 23-row inventory lock here serialised every
// desktop control rebind in the fleet behind every other one.
const cellRow = (await this.lockCellRows(database, [cellId]))[0]
await database.query(
`DELETE FROM relay_assignment_activity_leases
WHERE user_id = ? AND relay_host_id = ? AND activity_kind = 'control'
@@ -7611,7 +7628,6 @@ export class RelayAssignmentStore {
[cellId]
)
)[0]!
const cellRow = cells.find((cell) => text(cell, 'cell_id') === cellId)
const cellUnits = integer(cellUnitsRow, 'request_units')
if (!cellRow) throw new Error('assigned_cell_missing')
if (cellUnits > integer(cellRow, 'capacity_requests')) {
cloud/apps/relay/src/cell-inventory-lock-census.test.ts
+67 -6
View File
@@ -25,10 +25,12 @@ const CENSUS: CensusEntry[] = [
{ method: 'assignOnce', mode: 'nowait', reach: 'both' },
{ method: 'assignOnce', mode: 'nowait', reach: 'both' },
{ method: 'refreshDrainMigrationLeasesOnce', mode: 'request', reach: 'request' },
// Reachable from neither: changeActivity has no production callers, only tests.
{ method: 'changeActivity', mode: 'request', reach: 'orphan' },
{ method: 'acquireActivity', mode: 'request', reach: 'request' },
{ method: 'activateControl', mode: 'request', reach: 'request' },
// changeActivity, acquireActivity, activateControl and
// removeSupersededSameCellControls no longer take the inventory: they lock
// only the one or two cell rows they touch, in cell_id order (lockCellRows),
// so they cannot cycle with placement's ordered inventory lock, and the
// 23-row lock there had serialised every reconnect in the fleet behind every
// other one.
{ method: 'startEvacuation', mode: 'request', reach: 'request' },
{ method: 'completeEvacuationFromDeadSourceOnce', mode: 'request', reach: 'request' },
{ method: 'completeEvacuationFromDeadSourceOnce', mode: 'nowait', reach: 'request' },
@@ -48,8 +50,31 @@ const CENSUS: CensusEntry[] = [
{ method: 'releaseExpiredActivityLeases', mode: 'nowait', reach: 'sweep' },
{ method: 'releaseExpiredActivity', mode: 'nowait', reach: 'sweep' },
{ method: 'reconcileReservationAccounting', mode: 'pool-default', reach: 'both' },
{ method: 'leastLoadedCell', mode: 'pool-default', reach: 'both' },
{ method: 'removeSupersededSameCellControls', mode: 'request', reach: 'request' }
{ method: 'leastLoadedCell', mode: 'pool-default', reach: 'both' }
]
// Every inline `FROM relay_cells ... FOR UPDATE` outside the named lock helpers,
// in source order: whole-table locks in reconciliation and sticky placement,
// and single-row locks for a cell the method is already scoped to (heartbeat,
// fence, drain generation, configuration, or a reservation adjust that runs
// under a lock its caller already holds). A new inline lock fails the census
// below until it is listed here; per-connection paths that touch more than one
// cell go through lockCellRows so the order is fixed.
const NAMED_LOCK_HELPERS = ['lockCellInventory', 'lockGeneralCellInventory', 'lockCellRows']
const INLINE_CELL_LOCK_SITES = [
'reconcileCellsWithOptions',
'assignStickyOnce',
'recordCellHeartbeat',
'attestCellFence',
'adoptLegacyCellFence',
'commitLegacyCellFenceAdoption',
'prepareCellFenceAttempt',
'attestCellFenceAttempt',
'attestCellFenceAttempt',
'configureCell',
'assertDrainCellGeneration',
'adjustCellReservation'
]
// The background sweeps, and nothing else. A method reachable from one of these
@@ -151,6 +176,42 @@ describe('cell inventory lock call-site census', () => {
)
})
// Why: the census only sees lockCellInventory calls, so a hand-written
// `relay_cells ... FOR UPDATE` would escape classification entirely.
it('routes every relay_cells row lock through a named lock helper', () => {
const lines = storeSource()
const rawSites: string[] = []
// Whole statements, not a fixed window: a wide column list or a raw
// FOR UPDATE inside query() must not slip past.
const source = lines.join('\n')
const bounds: { name: string; start: number }[] = []
lines.forEach((line, index) => {
const declaration = DECLARATION.exec(line)
if (declaration) bounds.push({ name: declaration[1]!, start: index })
})
const methodAt = (offset: number): string => {
const lineIndex = source.slice(0, offset).split('\n').length - 1
let name = '<module>'
for (const bound of bounds) if (bound.start <= lineIndex) name = bound.name
return name
}
const tick = String.fromCharCode(96)
const statementCall = new RegExp(
'\\.(queryLocked|query)\\(\\s*' + tick + '([^' + tick + ']*)' + tick,
'g'
)
for (const call of source.matchAll(statementCall)) {
const statement = call[2]!
if (!/\bFROM\s+relay_cells\b/.test(statement)) continue
const locks = call[1] === 'queryLocked' || /\bFOR\s+UPDATE\b/.test(statement)
if (!locks) continue
const method = methodAt(call.index)
if (NAMED_LOCK_HELPERS.includes(method)) continue
rawSites.push(method)
}
expect(rawSites).toEqual(INLINE_CELL_LOCK_SITES)
})
it('leaves no call site taking the inventory without naming a mode', () => {
const source = readFileSync(new URL('./assignment-store.ts', import.meta.url), 'utf8')
const unclassified = source
cloud/apps/relay/src/control-rebind-inventory-lock-postgres.test.ts
+260
View File
@@ -0,0 +1,260 @@
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
import { RelayAssignmentStore } from './assignment-store.js'
import { openRelayDatabase, type RelayDatabase } from './database.js'
const databaseUrl = process.env.ORCA_RELAY_TEST_POSTGRES_URL
const describePostgres = databaseUrl ? describe : describe.skip
// Three cells: the inventory lock covers more than the rows a move touches, and
// a high-to-low move exposes any lock taken out of cell_id order.
const cells = [
{
id: 'rebind-inventory-postgres-a',
url: 'https://rebind-inventory-postgres-a.example.com',
capacityRequests: 1_000,
connectionHardCap: 600 as const,
connectionUnobservedBound: 50
},
{
id: 'rebind-inventory-postgres-b',
url: 'https://rebind-inventory-postgres-b.example.com',
capacityRequests: 1_000,
connectionHardCap: 600 as const,
connectionUnobservedBound: 50
},
{
id: 'rebind-inventory-postgres-c',
url: 'https://rebind-inventory-postgres-c.example.com',
capacityRequests: 1_000,
connectionHardCap: 600 as const,
connectionUnobservedBound: 50
}
]
const identity = { userId: 'rebind-inventory-postgres-user', relayHostId: 'rebindinvhost001' }
function heartbeat(cell: (typeof cells)[number]) {
return {
cellId: cell.id,
cellUrl: cell.url,
cellIncarnation: '11111111-1111-4111-8111-111111111111',
startedAt: 50,
ready: true,
observedRequests: 0,
totalConnections: 0,
inFlightConnections: 0,
reservedConnectionUnits: 0,
enforcedConnectionUnits: 0,
connectionInclusionWatermark: 1,
connectionHardCap: 600 as const,
connectionUnobservedBound: 50
}
}
// Why: every desktop control rebind used to take the fleet-wide relay_cells
// FOR UPDATE lock, so a rebind on one cell queued behind whatever held any
// other cell's row, until COMMIT (55P03 at the request bound). A rebind only
// touches its own cell row, so it must proceed while another cell's row is
// held elsewhere.
describePostgres('PostgreSQL control rebind under a held cell row', () => {
const databases: RelayDatabase[] = []
beforeAll(async () => {
databases.push(
await openRelayDatabase({ databaseUrl, dataDir: '' }),
await openRelayDatabase({ databaseUrl, dataDir: '' })
)
})
async function removeTestRows(database: RelayDatabase): Promise<void> {
await database.query(
`DELETE FROM relay_control_connection_reservations WHERE user_id = ?`,
[identity.userId]
)
for (const table of [
'relay_assignment_activity_leases',
'relay_post_drain_migration_pins',
'relay_assignment_migration_incarnations',
'relay_assignment_migrations',
'relay_assignments'
]) {
await database.query(`DELETE FROM ${table} WHERE user_id = ?`, [identity.userId])
}
for (const cell of cells) {
for (const table of [
'relay_cell_connection_snapshots',
'relay_cell_connection_runtime',
'relay_cell_connection_limits',
'relay_cell_runtime',
'relay_cells'
]) {
await database.query(`DELETE FROM ${table} WHERE cell_id = ?`, [cell.id])
}
}
}
afterAll(async () => {
if (databases[0]) await removeTestRows(databases[0])
for (const connection of databases) await connection.close()
})
it("rebinds and supersedes a control while another cell's row is held", async () => {
// A prior aborted run leaves connection snapshots that reject a replayed watermark.
await removeTestRows(databases[0]!)
const store = new RelayAssignmentStore(databases[0]!, () => 100)
await store.reconcileCells(cells)
for (const cell of cells) await store.recordCellHeartbeat(heartbeat(cell))
// Pin the host to cell A so placement is deterministic.
await store.setCellEnabled(cells[1]!.id, false)
await store.setCellEnabled(cells[2]!.id, false)
const assignment = await store.assign(identity)
expect(assignment.cellId).toBe(cells[0]!.id)
await store.setCellEnabled(cells[1]!.id, true)
await store.setCellEnabled(cells[2]!.id, true)
await store.activateControl(identity, {
cellId: cells[0]!.id,
assignmentEpoch: assignment.assignmentEpoch,
generation: 1,
connectionInclusionWatermark: 10
})
// Hold only cell B's row on a second connection, the way a rebind on B
// does, for longer than the request-path lock bound.
let releaseInventory!: () => void
const inventoryReleased = new Promise<void>((resolve) => {
releaseInventory = resolve
})
let inventoryHeld!: () => void
const inventoryHeldPromise = new Promise<void>((resolve) => {
inventoryHeld = resolve
})
const holder = databases[1]!.transaction(async (transaction) => {
await transaction.queryLocked(`SELECT * FROM relay_cells WHERE cell_id = ?`, [cells[1]!.id])
inventoryHeld()
await inventoryReleased
})
await inventoryHeldPromise
// A generation-2 rebind on cell A supersedes generation 1. It must not
// wait on cell B's row.
const startedAt = Date.now()
const blockedStatement = async (): Promise<string> => {
const rows = await databases[1]!.query(
`SELECT left(query, 160) AS q FROM pg_stat_activity
WHERE datname = current_database() AND wait_event_type = 'Lock'`
)
return rows.map((row) => String(row.q)).join(' | ')
}
const timeout = new Promise<never>((_, reject) =>
setTimeout(
() =>
void blockedStatement().then((statement) =>
reject(new Error(`rebind on cell A blocked behind cell B's row: ${statement}`))
),
2_000
)
)
const rebound = await Promise.race([
store.activateControl(identity, {
cellId: cells[0]!.id,
assignmentEpoch: assignment.assignmentEpoch,
generation: 2,
connectionInclusionWatermark: 11
}),
timeout
])
const elapsedMs = Date.now() - startedAt
releaseInventory()
await holder
expect(rebound).toBe(`control:${cells[0]!.id}:2`)
expect(elapsedMs).toBeLessThan(2_000)
const controls = await databases[0]!.query(
`SELECT activity_id FROM relay_assignment_activity_leases
WHERE user_id = ? AND activity_kind = 'control' ORDER BY activity_id`,
[identity.userId]
)
expect(controls).toEqual([{ activity_id: `control:${cells[0]!.id}:2` }])
const reserved = await databases[0]!.query(
`SELECT reserved_requests FROM relay_cells WHERE cell_id = ?`,
[cells[0]!.id]
)
expect(Number(reserved[0]!.reserved_requests)).toBe(1)
}, 15_000)
// Why: a phone's activity id is client-chosen and can follow the host across
// a migration, so acquireActivity may touch two cell rows. Moving from the
// higher cell to the lower one is where an unordered lock cycles with
// placement's ascending inventory lock (reproduced live before this fix).
it('moves an activity from a higher cell to a lower one in cell_id order', async () => {
await removeTestRows(databases[0]!)
const [cellA, cellB, cellC] = cells as [typeof cells[0], typeof cells[0], typeof cells[0]]
const store = new RelayAssignmentStore(databases[0]!, () => 100)
await store.reconcileCells(cells)
for (const cell of cells) await store.recordCellHeartbeat(heartbeat(cell))
await store.setCellEnabled(cellA.id, false)
await store.setCellEnabled(cellB.id, false)
const assignment = await store.assign(identity)
expect(assignment.cellId).toBe(cellC.id)
await store.setCellEnabled(cellA.id, true)
await store.setCellEnabled(cellB.id, true)
const activityId = 'splice:rebind-inventory-postgres'
await store.acquireActivity(identity, { activityId, kind: 'splice', cellId: cellC.id })
// The migration makes B authoritative; the lease still sits on C.
const migration = await store.startEvacuation(identity, cellB.id)
expect(migration.targetCellId).toBe(cellB.id)
// Hold B elsewhere. An ordered move locks B first and queues here holding
// nothing else. Locking C first (the old lease's row, as an unordered move
// does) or the whole inventory (which takes A) shows up as a held row.
let releaseRow!: () => void
const rowReleased = new Promise<void>((resolve) => {
releaseRow = resolve
})
let rowHeld!: () => void
const rowHeldPromise = new Promise<void>((resolve) => {
rowHeld = resolve
})
const heldWhileMoverWaits: string[] = []
const holder = databases[1]!.transaction(async (transaction) => {
await transaction.queryLocked(`SELECT * FROM relay_cells WHERE cell_id = ?`, [cellB.id])
rowHeld()
await rowReleased
for (const cell of [cellA, cellC]) {
try {
await transaction.queryLocked(`SELECT * FROM relay_cells WHERE cell_id = ?`, [cell.id], {
failIfUnavailable: true
})
} catch {
heldWhileMoverWaits.push(cell.id)
}
}
})
await rowHeldPromise
const move = store.acquireActivity(identity, { activityId, kind: 'splice', cellId: cellB.id })
let moved = false
void move.then(() => {
moved = true
})
await new Promise((resolve) => setTimeout(resolve, 250))
expect(moved).toBe(false)
releaseRow()
await holder
await move
expect(heldWhileMoverWaits).toEqual([])
const reservations = await databases[0]!.query(
`SELECT cell_id, reserved_requests FROM relay_cells
WHERE cell_id IN (?, ?, ?) ORDER BY cell_id ASC`,
[cellA.id, cellB.id, cellC.id]
)
const reserved = reservations.map((row) => [String(row.cell_id), Number(row.reserved_requests)])
expect(reserved).toEqual([
[cellA.id, 0],
// Migration grant plus the moved splice, as in the SQLite origin-scoped
// reservation case: the lock change did not alter accounting.
[cellB.id, 6],
// The sticky grant stays on the source until the migration completes.
[cellC.id, 1]
])
}, 15_000)
})
config/patches/node-pty@1.1.0.patch
+149 -21
View File
@@ -603,7 +603,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..2ae787c5bd4f3eba470584dc658a01a5
}
#endif
diff --git a/src/win/conpty.cc b/src/win/conpty.cc
index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a6a4082ce 100644
index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c97209248e 100644
--- a/src/win/conpty.cc
+++ b/src/win/conpty.cc
@@ -18,6 +18,7 @@
@@ -614,7 +614,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
#include <vector>
#include <Windows.h>
#include <strsafe.h>
@@ -44,12 +45,29 @@ struct pty_baton {
@@ -44,12 +45,39 @@ struct pty_baton {
HANDLE hOut;
HPCON hpc;
@@ -630,22 +630,32 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
+ // refused to create or assign one (an outer job without breakaway rights),
+ // in which case callers fall back to their pre-job behaviour.
+ HANDLE hJob = nullptr;
+
+ // Orca: teardown needs BOTH the shell's death and an explicit kill() before
+ // the baton can be freed, so each side records that it has run. Whichever
+ // arrives second frees it. Freeing on the shell's death alone -- what this
+ // file did before -- destroyed the only record of `hpc` while
+ // ClosePseudoConsole was still owed, which is why a self-exiting shell
+ // leaked its pseudoconsole and the console host it reaps (#18601 / F24).
+ bool shellExited = false;
+ bool consoleClosed = false;
pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {};
};
static std::vector<std::unique_ptr<pty_baton>> ptyHandles;
+// Orca: guards the job accessors below against the exit watcher thread. It does
+// NOT make the whole table safe -- PtyResize/PtyClear/PtyKill read it unlocked,
+// as they always have -- but it closes the window this patch opened, where the
+// watcher can close hShell/hJob and free the baton between a lookup and its use.
+// Orca: guards the job accessors below, and PtyKill, against the exit watcher
+// thread. It does NOT make the whole table safe -- PtyResize and PtyClear still
+// read it unlocked, as they always have -- but it closes the window this patch
+// opened, where the watcher can close hShell/hJob and free the baton between a
+// lookup and its use.
+// Handle VALUES are recycled aggressively, so an unguarded read could pass the
+// shell-pid check against an unrelated process and terminate the wrong job.
+static std::mutex ptyJobMutex;
static volatile LONG ptyCounter;
static pty_baton* get_pty_baton(int id) {
@@ -102,8 +120,27 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) {
@@ -102,8 +130,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) {
// Get process exit code.
GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code));
// Clean up handles
@@ -665,9 +675,13 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
+ // Why inside the lock: erasing frees the baton the job accessors hold a
+ // pointer to. Note remove_pty_baton must not be an assert() argument --
+ // NDEBUG would compile the call away and leak every baton.
+ const bool removed = remove_pty_baton(baton->id);
+ assert(removed);
+ (void)removed;
+ baton->shellExited = true;
+ if (baton->consoleClosed) {
+ const bool removed = remove_pty_baton(baton->id);
+ assert(removed);
+ (void)removed;
+ }
+ // Else PtyKill has not run yet and still owns hpc. It frees the baton.
+ }
+ // Why the lock ends here: BlockingCall below waits on the JS thread, and the
+ // JS thread can be waiting on ptyJobMutex inside PtyTerminateJob. Holding
@@ -675,7 +689,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
auto status = tsfn.BlockingCall(exit_event, callback); // In main thread
switch (status) {
@@ -409,6 +446,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
@@ -409,6 +460,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
throw errorWithCode(info, "UpdateProcThreadAttribute failed");
}
@@ -691,7 +705,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
PROCESS_INFORMATION piClient{};
fSuccess = !!CreateProcessW(
nullptr,
@@ -416,7 +462,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
@@ -416,7 +476,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
nullptr, // lpProcessAttributes
nullptr, // lpThreadAttributes
false, // bInheritHandles VERY IMPORTANT that this is false
@@ -703,7 +717,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
envArg, // lpEnvironment
mutableCwd.get(), // lpCurrentDirectory
&siEx.StartupInfo, // lpStartupInfo
@@ -426,8 +475,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
@@ -426,8 +489,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
throw errorWithCode(info, "Cannot create process");
}
@@ -753,7 +767,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
if (useConptyDll && fLoadedDll)
{
PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress(
@@ -440,6 +528,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
@@ -440,6 +542,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
// Update handle
handle->hShell = piClient.hProcess;
@@ -762,7 +776,91 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
// Close the thread handle to avoid resource leak
CloseHandle(piClient.hThread);
@@ -567,6 +657,143 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
@@ -544,29 +648,215 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
int id = info[0].As<Napi::Number>().Int32Value();
const bool useConptyDll = info[1].As<Napi::Boolean>().Value();
- const pty_baton* handle = get_pty_baton(id);
+ // Orca: resolve the DLL BEFORE touching any baton state, for the same reason
+ // PtyConnect does it before creating anything. LoadConptyDll throws when
+ // conpty.dll is missing, and a throw after consoleClosed was set would strand
+ // the pseudoconsole permanently: the retry would find the work already
+ // claimed and do nothing. Only the useConptyDll path can throw here; the
+ // other returns kernel32.
+ HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
+ PFNCLOSEPSEUDOCONSOLE pfnClosePseudoConsole = nullptr;
+ if (hLibrary != nullptr) {
+ pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress(
+ (HMODULE)hLibrary,
+ useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
+ }
- if (handle != nullptr) {
- HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
- bool fLoadedDll = hLibrary != nullptr;
- if (fLoadedDll)
- {
- PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress(
- (HMODULE)hLibrary,
- useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
- if (pfnClosePseudoConsole)
- {
- pfnClosePseudoConsole(handle->hpc);
+ // Orca: the baton now outlives the shell, so this runs on a self-exited pty
+ // too -- that is the whole point. Take what we need under the lock: the
+ // watcher thread nulls hShell the moment the shell dies, and TerminateProcess
+ // on a handle it just closed is an invalid-handle operation. Duplicating
+ // rather than reordering keeps upstream's close-then-terminate sequence.
+ HPCON hpc = nullptr;
+ HANDLE hShellDup = nullptr;
+ bool owed = false;
+ {
+ std::lock_guard<std::mutex> guard(ptyJobMutex);
+ pty_baton* handle = get_pty_baton(id);
+ // Why the consoleClosed check: a second kill() would otherwise close the
+ // same pseudoconsole twice. Upstream relied on the baton being gone.
+ if (handle != nullptr && !handle->consoleClosed) {
+ hpc = handle->hpc;
+ owed = true;
+ handle->consoleClosed = true;
+ // Null hShell means a self-exited pty, where there is nothing to kill.
+ if (useConptyDll && handle->hShell != nullptr) {
+ if (!DuplicateHandle(GetCurrentProcess(), handle->hShell, GetCurrentProcess(),
+ &hShellDup, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ // Why terminate here instead of skipping: a failed duplication leaves
+ // hShellDup null, which is indistinguishable from the self-exit case,
+ // and skipping would leave the shell RUNNING after its pane closed --
+ // a worse outcome than the leak this all exists to fix. hShell is
+ // valid under this lock and TerminateProcess does not block, so the
+ // only cost is that this rare path kills before the console closes.
+ hShellDup = nullptr;
+ TerminateProcess(handle->hShell, 1);
+ }
+ }
+ if (handle->shellExited) {
+ const bool removed = remove_pty_baton(id);
+ assert(removed);
+ (void)removed;
}
+ // Else the shell is still running and the watcher frees the baton.
}
- if (useConptyDll) {
- TerminateProcess(handle->hShell, 1);
+ }
+
+ // Why outside the lock: ClosePseudoConsole blocks until the conout side has
+ // drained, and the watcher must be able to take the lock while it does.
+ if (owed) {
+ if (pfnClosePseudoConsole)
+ {
+ pfnClosePseudoConsole(hpc);
+ }
+ if (hShellDup != nullptr) {
+ TerminateProcess(hShellDup, 1);
+ CloseHandle(hShellDup);
}
}
return env.Undefined();
}
@@ -808,9 +906,11 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
+ * Orca: the pids still alive in this pty's tree, straight from the kernel.
+ *
+ * Descendant liveness for a tree that is still tracked, including children that
+ * detached from the console. Once the shell exits the baton is gone, so this
+ * returns null rather than an empty list -- null means "no answer", never
+ * "they died". Also returns null when no job was assigned.
+ * detached from the console. Once the shell exits the watcher nulls hJob, which
+ * ownsShell rejects, so this returns null rather than an empty list -- null
+ * means "no answer", never "they died". (The baton itself now outlives the
+ * shell, until kill() runs; hJob is what makes the answer null.) Also returns
+ * null when no job was assigned.
+ *
+ * Does not include the ConPTY console host: CreatePseudoConsole spawns it
+ * before this job exists, so it is not a member and ClosePseudoConsole is what
@@ -906,7 +1006,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
/**
* Init
*/
@@ -577,6 +804,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
@@ -577,6 +867,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
exports.Set("resize", Napi::Function::New(env, PtyResize));
exports.Set("clear", Napi::Function::New(env, PtyClear));
exports.Set("kill", Napi::Function::New(env, PtyKill));
@@ -917,7 +1017,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a
};
diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js
index a358ffb..fb3a96f 100644
index a358ffb177357e177661033c1b092f9c9d0e5f5a..26c2a4c58799ce649f5113131e4c52f7ed2d87ad 100644
--- a/lib/windowsPtyAgent.js
+++ b/lib/windowsPtyAgent.js
@@ -136,6 +136,9 @@ var WindowsPtyAgent = /** @class */ (function () {
@@ -930,6 +1030,20 @@ index a358ffb..fb3a96f 100644
this._outSocket.readable = false;
this._getConsoleProcessList().then(function (consoleProcessList) {
consoleProcessList.forEach(function (pid) {
@@ -154,9 +157,10 @@ var WindowsPtyAgent = /** @class */ (function () {
// Close the input write handle to signal the end of session.
this._inSocket.destroy();
this._ptyNative.kill(this._pty, this._useConptyDll);
- this._outSocket.on('data', function () {
- _this._conoutSocketWorker.dispose();
- });
+ // Orca: dispose unconditionally, as the non-DLL branch above does.
+ // Waiting for another 'data' event leaks the conout worker on every
+ // self-exiting shell, because no more data ever arrives (F24).
+ this._conoutSocketWorker.dispose();
}
}
else {
diff --git a/lib/windowsTerminal.js b/lib/windowsTerminal.js
index 3c38f89..e20b3e6 100644
--- a/lib/windowsTerminal.js
@@ -1015,7 +1129,7 @@ index 3c38f89..e20b3e6 100644
\ No newline at end of file
+//# sourceMappingURL=windowsTerminal.js.map
diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts
index d705444..ce611b8 100644
index d7054449516f0c9a62af351c2caa17331206d530..0c28a32e2e1db2b3f208ddde8443cd4e67bb1ad6 100644
--- a/src/windowsPtyAgent.ts
+++ b/src/windowsPtyAgent.ts
@@ -143,6 +143,9 @@ export class WindowsPtyAgent {
@@ -1028,6 +1142,20 @@ index d705444..ce611b8 100644
this._outSocket.readable = false;
this._getConsoleProcessList().then(consoleProcessList => {
consoleProcessList.forEach((pid: number) => {
@@ -159,9 +162,10 @@ export class WindowsPtyAgent {
// Close the input write handle to signal the end of session.
this._inSocket.destroy();
(this._ptyNative as IConptyNative).kill(this._pty, this._useConptyDll);
- this._outSocket.on('data', () => {
- this._conoutSocketWorker.dispose();
- });
+ // Orca: dispose unconditionally, as the non-DLL branch above does.
+ // Waiting for another 'data' event leaks the conout worker on every
+ // self-exiting shell, because no more data ever arrives (F24).
+ this._conoutSocketWorker.dispose();
}
} else {
// Because pty.kill closes the handle, it will kill most processes by itself.
diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts
index 13f6c6d..eda63c8 100644
--- a/src/windowsTerminal.ts
config/relay-assets/node-pty-1.1.0-windows-pty-teardown-patch.cjs
+158
View File
@@ -0,0 +1,158 @@
const { createHash } = require('node:crypto')
const { readFileSync, renameSync, rmSync, writeFileSync } = require('node:fs')
const { join, resolve } = require('node:path')
/**
* Release the ConPTY teardown handles a relay's npm-installed node-pty never releases.
*
* Two files, and the ORDER of one of the edits is the whole fix.
*
* `windowsPtyAgent.js` -- `kill()` flips `readable` on both sockets and destroys neither.
* `_cleanUpProcess` destroys `_outSocket`, so the conout handle comes back; nothing ever destroys
* `_inSocket`, and it wraps a real Windows named-pipe handle from `fs.openSync(term.conin, 'w')`.
* Every terminal leaks one File handle for the life of the host process.
*
* The obvious fix -- and the one the desktop patch ships -- releases it at the TOP of the branch,
* before `_getConsoleProcessList()` forks and before the native kill. That is measurably worse than
* leaving the leak alone: teardown aborts partway, the forked console-list agent is never reaped,
* and both pipe handles stay alive instead of one. This asset releases it at the END of the branch
* instead, after the fork and the kill have already happened.
*
* Measured on a Windows SSH host, 20 spawn/kill cycles, handles bucketed by NT object type
* (identical numbers standalone and through a real relay):
*
* published node-pty File +1/terminal, Process flat
* desktop patch placement File +2/terminal, Process +1/terminal <-- 3x WORSE
* released last (here) File flat, Process flat
*
* `windowsTerminal.js` carries the desktop's error-listener hunks verbatim. The conin listener is
* what keeps a pipe error retiring one terminal instead of the host -- its own comment names the
* failure mode: "Without a listener, Node promotes errors such as write EAGAIN to uncaughtException".
* It is not what fixes the leak (adding it changed nothing on its own), but it is the guard that
* makes destroying conin safe at all.
*
* Why this ships as a relay asset rather than only in config/patches/node-pty@1.1.0.patch: pnpm
* patches do not cross the SSH boundary -- a relay host runs the tree `npm install` put there.
*
* DELIBERATE DIVERGENCE FROM THE DESKTOP: the desktop patch has the early placement and therefore
* the +2 File / +1 Process regression, measured against its exact installed tree. Correcting it
* there is a separate change with its own verification, so the two trees differ on this one hunk on
* purpose, and the test pins that so a future "sync the patches" does not copy the bug back.
*
* NOT ADDRESSED, AND A SEPARATE DEFECT THAT IS STILL OPEN: a terminal that exits on its own is
* still torn down through `kill()` -- both hosts call `destroy()` on natural exit and
* `WindowsTerminal.destroy()` is `kill()` -- but the shell is already gone by then, and the
* ordering this patch relies on does not hold. Measured over 20 self-exit cycles with that
* `destroy()` issued: published +3 File/+1 Process per terminal, desktop-patched +2/+1, this tree
* +2/+1. So this patch does not close it and the desktop patch does not either. It is reachable
* for every Windows user, local and relay, on every terminal closed by typing `exit`.
*/
const EXPECTED_NODE_PTY_VERSION = '1.1.0'
/** Each entry is one published file, its patched form, and the edits between them. */
const PATCH_TARGETS = [
{
relativePath: ['lib', 'windowsPtyAgent.js'],
originalSha256: '8636d16b38266112204061a22b135734177c242837982fd3a4055be726efa64a',
patchedSha256: '1e23ef480569e73706e3ab4f5482c7e553c76f51414ae8e7b0bdcc2fd75f7280',
replacements: [
[
' this._ptyNative.kill(this._pty, this._useConptyDll);\n this._conoutSocketWorker.dispose();\n',
' this._ptyNative.kill(this._pty, this._useConptyDll);\n this._conoutSocketWorker.dispose();\n // Orca: released AFTER the console-list fork and the native kill, not before them.\n // Destroying conin first aborts teardown partway -- measured on a Windows SSH relay\n // as +2 File and +1 Process handles per terminal, against +1 File unpatched.\n this._inSocket.destroy();\n'
]
]
},
{
relativePath: ['lib', 'windowsTerminal.js'],
originalSha256: 'c3a65716f53fed0135a8a633373d5f9c2ab092544d651f27ef0a67096dd3bcd9',
patchedSha256: '8247ecd69be8b18257050fb026b290024612c5ffc6d492ff1d46f81e613be2cf',
replacements: [
[
' _this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor);\n _this._socket = _this._agent.outSocket;\n // Not available until `ready` event emitted.\n _this._pid = _this._agent.innerPid;',
" _this._agent = new windowsPtyAgent_1.WindowsPtyAgent(file, args, parsedEnv, cwd, _this._cols, _this._rows, false, opt.useConpty, opt.useConptyDll, opt.conptyInheritCursor);\n _this._socket = _this._agent.outSocket;\n // Attach before readiness so a broken ConPTY output pipe cannot be unhandled.\n _this._socket.on('error', function (err) {\n var code = err && err.code;\n // PTY output can report EPIPE before `_close()` wins the race.\n _this._close();\n if (code === 'EPIPE' || code === 'ERR_STREAM_PUSH_AFTER_EOF' || code === 'ERR_STREAM_DESTROYED') {\n return;\n }\n // EIO, happens when someone closes our child process: the only process\n // in the terminal.\n // node < 0.6.14: errno 5\n // node >= 0.6.14: read EIO\n if (typeof code === 'string') {\n if (~code.indexOf('errno 5') || ~code.indexOf('EIO'))\n return;\n }\n // Throw anything else.\n if (_this.listeners('error').length < 2) {\n throw err;\n }\n });\n // Not available until `ready` event emitted.\n _this._pid = _this._agent.innerPid;"
],
[
" }\n });\n // Shutdown if `error` event is emitted.\n _this._socket.on('error', function (err) {\n // Close terminal session.\n _this._close();\n // EIO, happens when someone closes our child process: the only process\n // in the terminal.\n // node < 0.6.14: errno 5\n // node >= 0.6.14: read EIO\n if (err.code) {\n if (~err.code.indexOf('errno 5') || ~err.code.indexOf('EIO'))\n return;\n }\n // Throw anything else.\n if (_this.listeners('error').length < 2) {\n throw err;\n }\n });\n // Cleanup after the socket is closed.\n _this._socket.on('close', function () {",
" }\n });\n // Cleanup after the socket is closed.\n _this._socket.on('close', function () {"
],
[
' _this._readable = true;\n _this._writable = true;\n _this._forwardEvents();\n return _this;',
" _this._readable = true;\n _this._writable = true;\n // A ConPTY input-pipe error must retire only this terminal. Without a listener, Node promotes\n // errors such as write EAGAIN to uncaughtException and kills every PTY in the daemon.\n _this._agent.inSocket.on('error', function () {\n if (!_this._writable) {\n return;\n }\n _this._close();\n try {\n _this._agent.kill();\n }\n catch (_a) {\n // The failing pipe may have raced process exit; the terminal is already unwritable.\n }\n });\n _this._forwardEvents();\n return _this;"
],
[
'exports.WindowsTerminal = WindowsTerminal;\n//# sourceMappingURL=windowsTerminal.js.map',
'exports.WindowsTerminal = WindowsTerminal;\n//# sourceMappingURL=windowsTerminal.js.map\n'
]
]
}
]
function inspectTarget(relayDir, target) {
const nodePtyDir = resolve(relayDir, 'node_modules', 'node-pty')
const packageJson = JSON.parse(readFileSync(join(nodePtyDir, 'package.json'), 'utf8'))
if (packageJson.version !== EXPECTED_NODE_PTY_VERSION) {
throw new Error(
`Refusing to patch node-pty ${packageJson.version}; expected ${EXPECTED_NODE_PTY_VERSION}`
)
}
const filePath = join(nodePtyDir, ...target.relativePath)
return { filePath, source: readFileSync(filePath, 'utf8') }
}
function assertPatchedNodePtyWindowsTeardown(relayDir = process.cwd()) {
for (const target of PATCH_TARGETS) {
const inspected = inspectTarget(relayDir, target)
if (sourceSha256(inspected.source) !== target.patchedSha256) {
throw new Error(
`node-pty ConPTY teardown release is not installed in ${target.relativePath.join('/')}`
)
}
}
}
function patchNodePtyWindowsTeardown(relayDir = process.cwd()) {
for (const target of PATCH_TARGETS) {
const inspected = inspectTarget(relayDir, target)
const sourceHash = sourceSha256(inspected.source)
if (sourceHash === target.patchedSha256) {
continue
}
if (sourceHash !== target.originalSha256) {
throw new Error(
`Refusing to patch unexpected node-pty source in ${target.relativePath.join('/')}`
)
}
let patchedSource = inspected.source
for (const [from, to] of target.replacements) {
// Why the count check: an anchor that matched twice would patch the wrong site silently, and
// the hash below would then reject a tree this script had already rewritten.
if (patchedSource.split(from).length - 1 !== 1) {
throw new Error(`Refusing to patch ${target.relativePath.join('/')}; anchor is not unique`)
}
patchedSource = patchedSource.replace(from, to)
}
const temporaryPath = `${inspected.filePath}.orca-patch-${process.pid}`
// Why: a terminated remote install must leave either known source version recoverable on reconnect.
try {
writeFileSync(temporaryPath, patchedSource)
renameSync(temporaryPath, inspected.filePath)
} finally {
rmSync(temporaryPath, { force: true })
}
}
assertPatchedNodePtyWindowsTeardown(relayDir)
}
function sourceSha256(source) {
return createHash('sha256').update(source).digest('hex')
}
if (require.main === module) {
patchNodePtyWindowsTeardown()
}
module.exports = {
assertPatchedNodePtyWindowsTeardown,
patchNodePtyWindowsTeardown
}
config/scripts/build-relay.mjs
+11
View File
@@ -57,6 +57,13 @@ const NODE_PTY_CONSOLE_LIST_PATCH_SOURCE = join(
'relay-assets',
NODE_PTY_CONSOLE_LIST_PATCH_FILENAME
)
const NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME = 'node-pty-1.1.0-windows-pty-teardown-patch.cjs'
const NODE_PTY_WINDOWS_TEARDOWN_PATCH_SOURCE = join(
ROOT,
'config',
'relay-assets',
NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME
)
const NODE_PTY_MASTER_CLOEXEC_PATCH_FILENAME = 'node-pty-1.1.0-master-cloexec-patch.cjs'
const NODE_PTY_MASTER_CLOEXEC_PATCH_SOURCE = join(
ROOT,
@@ -132,6 +139,10 @@ for (const platform of RELAY_BUILD_PLATFORMS) {
NODE_PTY_CONSOLE_LIST_PATCH_SOURCE,
join(outDir, NODE_PTY_CONSOLE_LIST_PATCH_FILENAME)
)
copyFileSync(
NODE_PTY_WINDOWS_TEARDOWN_PATCH_SOURCE,
join(outDir, NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME)
)
}
copyFileSync(
NODE_PTY_MASTER_CLOEXEC_PATCH_SOURCE,
config/scripts/node-pty-windows-pty-teardown-patch.test.mjs
+213
View File
@@ -0,0 +1,213 @@
// The relay's copy of the ConPTY teardown release, and the guard that keeps it in lockstep with the
// desktop's own node-pty patch. pnpm patches do not cross the SSH boundary, so a relay runs the tree
// `npm install` put there; the desktop had this fix and the relay did not, and every terminal on a
// Windows SSH host leaked one File handle for the life of the relay process.
//
// The ORDER of the conin release is the fix. Releasing it at the top of the branch -- what the
// desktop patch does -- was measured at 3x WORSE than shipping nothing (File +2/terminal and a new
// Process +1/terminal); releasing it after the console-list fork and the native kill is flat.
import { createRequire } from 'node:module'
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { join, resolve } from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
const require = createRequire(import.meta.url)
const {
assertPatchedNodePtyWindowsTeardown,
patchNodePtyWindowsTeardown
} = require('../relay-assets/node-pty-1.1.0-windows-pty-teardown-patch.cjs')
const projectDir = resolve(import.meta.dirname, '..', '..')
const cleanupDirs = []
const PATCHED_FILES = ['windowsPtyAgent.js', 'windowsTerminal.js']
/** The hunks config/patches/node-pty@1.1.0.patch adds to the installed desktop tree. */
const DESKTOP_HUNKS = {
'windowsPtyAgent.js': [
[
[
' this._inSocket.readable = false;',
' // The non-DLL path previously only flipped `readable`, leaving the',
' // conin PipeWrap alive until the host exited (#947).',
' this._inSocket.destroy();',
' this._outSocket.readable = false;',
''
].join('\n'),
[
' this._inSocket.readable = false;',
' this._outSocket.readable = false;',
''
].join('\n')
],
// The useConptyDll branch, which only the DESKTOP runs -- the relay takes the
// non-DLL branch above, where the dispose is already unconditional. Listed here
// so un-applying still yields published; the relay asset needs no counterpart.
[
[
' // Orca: dispose unconditionally, as the non-DLL branch above does.',
" // Waiting for another 'data' event leaks the conout worker on every",
' // self-exiting shell, because no more data ever arrives (F24).',
' this._conoutSocketWorker.dispose();',
''
].join('\n'),
[
" this._outSocket.on('data', function () {",
' _this._conoutSocketWorker.dispose();',
' });',
''
].join('\n')
]
],
'windowsTerminal.js': [
[
' // Attach before readiness so a broken ConPTY output pipe cannot be unhandled.',
null
],
[' // A ConPTY input-pipe error must retire only this terminal.', null]
]
}
function desktopPath(file) {
return join(projectDir, 'node_modules', 'node-pty', 'lib', file)
}
afterEach(() => {
for (const dir of cleanupDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true })
}
})
describe('Windows SSH relay node-pty ConPTY teardown patch', () => {
// Why reconstruct rather than vendor upstream: the installed tree IS the published file plus the
// desktop's hunks, so un-applying them yields upstream exactly -- and pinning that against this
// asset's own hashes is what fails loudly if either side of the pair moves.
it('takes the desktop error listeners verbatim', () => {
const fixture = writeNodePtyFixture('1.1.0')
patchNodePtyWindowsTeardown(fixture.root)
expect(readFileSync(join(fixture.libDir, 'windowsTerminal.js'), 'utf8')).toBe(
readFileSync(desktopPath('windowsTerminal.js'), 'utf8')
)
})
// The one hunk that must NOT match the desktop, and the reason is measured, not stylistic:
// releasing conin before `_getConsoleProcessList()` forks aborts teardown partway.
it('releases conin after the console-list fork, not before it like the desktop patch', () => {
const fixture = writeNodePtyFixture('1.1.0')
patchNodePtyWindowsTeardown(fixture.root)
const patched = readFileSync(join(fixture.libDir, 'windowsPtyAgent.js'), 'utf8')
const branch = patched.slice(
patched.indexOf('if (!this._useConptyDll) {'),
patched.indexOf('else {', patched.indexOf('if (!this._useConptyDll) {'))
)
expect(branch).toContain('this._inSocket.destroy();')
expect(branch.indexOf('this._inSocket.destroy();')).toBeGreaterThan(
branch.indexOf('this._conoutSocketWorker.dispose();')
)
expect(branch.indexOf('this._inSocket.destroy();')).toBeGreaterThan(
branch.indexOf('this._getConsoleProcessList()')
)
// Pinned so a future "sync the relay asset to config/patches" cannot copy the regression back.
expect(patched).not.toBe(readFileSync(desktopPath('windowsPtyAgent.js'), 'utf8'))
})
it('installs and verifies idempotently', () => {
const fixture = writeNodePtyFixture('1.1.0')
patchNodePtyWindowsTeardown(fixture.root)
const once = PATCHED_FILES.map((file) => readFileSync(join(fixture.libDir, file), 'utf8'))
for (const file of PATCHED_FILES) {
expect(existsSync(`${join(fixture.libDir, file)}.orca-patch-${process.pid}`)).toBe(false)
}
expect(() => assertPatchedNodePtyWindowsTeardown(fixture.root)).not.toThrow()
patchNodePtyWindowsTeardown(fixture.root)
expect(PATCHED_FILES.map((file) => readFileSync(join(fixture.libDir, file), 'utf8'))).toEqual(
once
)
})
it('refuses a different package version or unexpected source', () => {
const wrongVersion = writeNodePtyFixture('1.2.0-beta.11')
expect(() => patchNodePtyWindowsTeardown(wrongVersion.root)).toThrow('expected 1.1.0')
for (const file of PATCHED_FILES) {
const drifted = writeNodePtyFixture('1.1.0')
const path = join(drifted.libDir, file)
writeFileSync(path, `${readFileSync(path, 'utf8')}\n// drift`)
expect(() => patchNodePtyWindowsTeardown(drifted.root)).toThrow('unexpected node-pty')
}
})
it('refuses a half-applied tree, so one file cannot pass for both', () => {
for (const file of PATCHED_FILES) {
const partial = writeNodePtyFixture('1.1.0')
const fixture = writeNodePtyFixture('1.1.0')
patchNodePtyWindowsTeardown(fixture.root)
writeFileSync(join(partial.libDir, file), readFileSync(join(fixture.libDir, file), 'utf8'))
expect(() => assertPatchedNodePtyWindowsTeardown(partial.root)).toThrow('is not installed')
}
})
})
/** A published node-pty tree, rebuilt by un-applying the desktop hunks from the installed one. */
function writeNodePtyFixture(version) {
const root = mkdtempSync(join(projectDir, '.node-pty-teardown-patch-test-'))
cleanupDirs.push(root)
const libDir = join(root, 'node_modules', 'node-pty', 'lib')
mkdirSync(libDir, { recursive: true })
writeFileSync(join(root, 'node_modules', 'node-pty', 'package.json'), JSON.stringify({ version }))
for (const file of PATCHED_FILES) {
const desktop = readFileSync(desktopPath(file), 'utf8')
for (const [marker] of DESKTOP_HUNKS[file]) {
expect(desktop).toContain(marker)
}
writeFileSync(join(libDir, file), unapplyDesktopHunks(file, desktop))
}
return { root, libDir }
}
/**
* Reverse of the published-to-desktop transform.
*
* `windowsTerminal.js` is taken verbatim from the desktop, so the asset's own replacement table is
* the transform and reversing it is exact. `windowsPtyAgent.js` deliberately diverges, so its
* published form is rebuilt from the desktop hunk instead -- which is also what makes this file the
* place that notices if the desktop hunk itself ever moves.
*/
function unapplyDesktopHunks(file, desktop) {
if (file === 'windowsPtyAgent.js') {
let published = desktop
for (const [patched, original] of DESKTOP_HUNKS[file]) {
expect(published.split(patched).length - 1).toBe(1)
published = published.replace(patched, original)
}
return published
}
const asset = readFileSync(
join(projectDir, 'config', 'relay-assets', 'node-pty-1.1.0-windows-pty-teardown-patch.cjs'),
'utf8'
)
const { PATCH_TARGETS } = loadPatchTargets(asset)
const target = PATCH_TARGETS.find((entry) => entry.relativePath.at(-1) === file)
expect(target).toBeDefined()
let published = desktop
for (const [from, to] of target.replacements.toReversed()) {
expect(published.split(to).length - 1).toBe(1)
published = published.replace(to, from)
}
return published
}
function loadPatchTargets(assetSource) {
const module = { exports: {} }
const factory = new Function(
'module',
'exports',
'require',
`${assetSource}\nmodule.exports.PATCH_TARGETS = PATCH_TARGETS`
)
factory(module, module.exports, require)
return module.exports
}
config/scripts/skill-description-length.test.mjs
+28 -25
View File
@@ -1,36 +1,39 @@
import { readFileSync, readdirSync } from 'node:fs'
import { readdirSync, readFileSync } from 'node:fs'
import { join, resolve } from 'node:path'
import { parse } from 'yaml'
import { describe, expect, it } from 'vitest'
import { parse } from 'yaml'
// Why: the Agent Skills spec caps `description` at 1,024 characters after YAML folding, and
// conforming installers reject a skill over it (#17935). Every skills/*/SKILL.md must fit.
const SKILLS_ROOT = resolve(import.meta.dirname, '../../skills')
const skillsDir = resolve(import.meta.dirname, '../../skills')
// Why: the Agent Skills spec caps `description` at 1024 chars and conforming installers
// reject the whole skill (#17935); the frontmatter is what the installer parses, so check it.
const MAX_DESCRIPTION_LENGTH = 1024
function readDescription(skillText) {
const frontmatter = /^---\n([\s\S]*?)\n---\n/u.exec(skillText)?.[1] ?? ''
return parse(frontmatter)?.description ?? ''
function readDescription(skillName) {
const skillMarkdown = readFileSync(join(skillsDir, skillName, 'SKILL.md'), 'utf8')
const frontmatter = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/u.exec(skillMarkdown)?.[1]
expect(frontmatter, `${skillName}: missing frontmatter`).toBeDefined()
return parse(frontmatter ?? '').description
}
describe('bundled skill descriptions', () => {
it('stay within the Agent Skills 1024-character limit', () => {
const skills = readdirSync(SKILLS_ROOT, { withFileTypes: true }).filter((entry) =>
entry.isDirectory()
)
expect(skills.length).toBeGreaterThan(0)
const skillNames = readdirSync(skillsDir, { withFileTypes: true })
.filter((entry) => entry.isDirectory())
.map((entry) => entry.name)
const violations = skills.flatMap((entry) => {
const description = readDescription(
readFileSync(join(SKILLS_ROOT, entry.name, 'SKILL.md'), 'utf8')
)
if (!description.trim()) {
return [`${entry.name}: missing description`]
}
return description.length > MAX_DESCRIPTION_LENGTH
? [`${entry.name}: ${description.length} chars (limit ${MAX_DESCRIPTION_LENGTH})`]
: []
})
expect(violations).toEqual([])
it('discovers the bundled skills', () => {
expect(skillNames).toContain('orchestration')
})
it.each(skillNames)('%s keeps description within the Agent Skills spec limit', (name) => {
const description = readDescription(name)
expect(typeof description, `${name}: description must be a string`).toBe('string')
expect(description.trim().length, `${name}: description is empty`).toBeGreaterThan(0)
expect(
description.length,
`${name}: description is ${description.length} chars`
).toBeLessThanOrEqual(MAX_DESCRIPTION_LENGTH)
})
})
docs/assets/readme-downloads.svg
+4 -4
View File
@@ -1,5 +1,5 @@
<svg xmlns="http://www.w3.org/2000/svg" width="106" height="20" role="img" aria-label="downloads: 38m">
<title>downloads: 38m</title>
<svg xmlns="http://www.w3.org/2000/svg" width="106" height="20" role="img" aria-label="downloads: 39m">
<title>downloads: 39m</title>
<linearGradient id="s" x2="0" y2="100%">
<stop offset="0" stop-color="#bbb" stop-opacity=".1"/>
<stop offset="1" stop-opacity=".1"/>
@@ -15,7 +15,7 @@
<g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="11">
<text x="37" y="15" fill="#010101" fill-opacity=".3">downloads</text>
<text x="37" y="14">downloads</text>
<text x="90" y="15" fill="#010101" fill-opacity=".3">38m</text>
<text x="90" y="14">38m</text>
<text x="90" y="15" fill="#010101" fill-opacity=".3">39m</text>
<text x="90" y="14">39m</text>
</g>
</svg>
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 935 B

After

Width:  |  Height:  |  Size: 935 B

docs/assets/wechat-qr-group9.jpg
BIN
View File
Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 386 KiB

docs/readme/README.fr.md
+2 -2
View File
@@ -243,9 +243,9 @@ Associez-la à l'app de bureau pour surveiller et piloter vos agents depuis votr
- **Discord :** Rejoignez la communauté sur **[Discord](https://discord.gg/fzjDKHxv8Q)**.
- **Twitter / X :** Suivez **[@orca_build](https://x.com/orca_build)** pour les news et annonces.
- **WeChat :** Scannez pour rejoindre le groupe WeChat 8 de la communauté Orca.
- **WeChat :** Scannez pour rejoindre le groupe WeChat 8 de la communauté Orca. Le groupe 8 est peut-être complet ; dans ce cas, scannez plutôt le QR code du groupe 9.
<img src="../assets/wechat-qr-group8.jpg" alt="QR code WeChat groupe 8 de la communauté Orca" width="160" />
<img src="../assets/wechat-qr-group8.jpg" alt="QR code WeChat groupe 8 de la communauté Orca" width="160" />&nbsp;&nbsp;<img src="../assets/wechat-qr-group9.jpg" alt="QR code WeChat groupe 9 de la communauté Orca" width="160" />
- **Feedback &amp; idées :** On ship vite. Il manque quelque chose ? [Demandez une feature](https://github.com/stablyai/orca/issues).
- **Confidentialité :** Voir la [doc confidentialité &amp; télémétrie](https://www.onorca.dev/docs/telemetry) pour ce qu'Orca collecte en anonyme et comment désactiver la télémétrie.
docs/readme/README.ko.md
+2 -2
View File
@@ -238,9 +238,9 @@ yay -S stably-orca-bin
- **Discord:** **[Discord](https://discord.gg/fzjDKHxv8Q)** 커뮤니티에 참여하세요.
- **Twitter / X:** 업데이트와 공지는 **[@orca_build](https://x.com/orca_build)** 를 팔로우하세요.
- **WeChat:** QR 코드를 스캔해 Orca 커뮤니티 WeChat 그룹 8에 참여하세요.
- **WeChat:** QR 코드를 스캔해 Orca 커뮤니티 WeChat 그룹 8에 참여하세요. 그룹 8이 가득 찼을 수 있으니, 그런 경우 그룹 9 QR 코드를 스캔하세요.
<img src="../assets/wechat-qr-group8.jpg" alt="Orca 커뮤니티 WeChat 그룹 8 QR 코드" width="160" />
<img src="../assets/wechat-qr-group8.jpg" alt="Orca 커뮤니티 WeChat 그룹 8 QR 코드" width="160" />&nbsp;&nbsp;<img src="../assets/wechat-qr-group9.jpg" alt="Orca 커뮤니티 WeChat 그룹 9 QR 코드" width="160" />
- **피드백과 아이디어:** 우리는 빠르게 출시합니다. 필요한 기능이 있나요? [새 기능을 요청](https://github.com/stablyai/orca/issues)하세요.
- **개인정보 보호:** Orca가 수집하는 익명 사용 데이터와 수집 거부 방법은 [개인정보 및 텔레메트리 문서](https://www.onorca.dev/docs/telemetry)를 참고하세요.
docs/readme/README.zh-CN.md
+2 -2
View File
@@ -235,9 +235,9 @@ yay -S stably-orca-bin
- **Discord:** 加入 **[Discord](https://discord.gg/fzjDKHxv8Q)** 社区。
- **Twitter / X:** 关注 **[@orca_build](https://x.com/orca_build)** 获取更新和公告。
- **微信:** 扫码加入 Orca 社区微信第 8 群。
- **微信:** 扫码加入 Orca 社区微信第 8 群。第 8 群可能已满，如遇这种情况请扫描第 9 群二维码。
<img src="../assets/wechat-qr-group8.jpg" alt="Orca 社区微信第 8 群二维码" width="160" />
<img src="../assets/wechat-qr-group8.jpg" alt="Orca 社区微信第 8 群二维码" width="160" />&nbsp;&nbsp;<img src="../assets/wechat-qr-group9.jpg" alt="Orca 社区微信第 9 群二维码" width="160" />
- **反馈与想法:** 我们发布很快。缺少什么功能？[提交功能请求](https://github.com/stablyai/orca/issues)。
- **隐私:** 查看[隐私与遥测文档](https://www.onorca.dev/docs/telemetry)，了解 Orca 收集哪些匿名使用数据以及如何退出。
pnpm-lock.yaml
Generated
+3 -3
View File
@@ -116,7 +116,7 @@ patchedDependencies:
'@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e
'@xterm/xterm@6.1.0-beta.303': 98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d
lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
node-pty@1.1.0: e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa
node-pty@1.1.0: 7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1
importers:
@@ -157,7 +157,7 @@ importers:
version: 3.3.1
node-pty:
specifier: ^1.1.0
version: 1.1.0(patch_hash=e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa)
version: 1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1)
posthog-node:
specifier: ^5.33.3
version: 5.33.3
@@ -12195,7 +12195,7 @@ snapshots:
node-int64@0.4.0: {}
node-pty@1.1.0(patch_hash=e262847f57a1d4d3f2287a843822f7dcf3c9d8655892b07a69eba464e1317eaa):
node-pty@1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1):
dependencies:
node-addon-api: 7.1.1
src/main/ipc/pty/ipc/inspect.ts
+12 -5
View File
@@ -170,7 +170,10 @@ export function installPtyInspectIpcHandlers(deps: {
ipcMain.handle(
'pty:inspectProcess',
async (_event, args: { id: string; expectedIncarnationId?: string }) => {
async (
_event,
args: { id: string; expectedIncarnationId?: string; scanChildProcesses?: boolean }
) => {
// Why: same routing hazard as pty:hasPty — an unroutable id must read as client-only unverifiable, not as a local-provider answer or a raised IPC error.
if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) {
return clientOnlyUnverifiableInspection('terminal_gone')
@@ -182,10 +185,14 @@ export function installPtyInspectIpcHandlers(deps: {
if (!hasPtyProviderForInspection(args.id)) {
return clientOnlyUnverifiableInspection('terminal_gone')
}
return args.expectedIncarnationId
? inspectPtyProviderProcessForRenderer(getProviderForPty(args.id), args.id, {
expectedIncarnationId: args.expectedIncarnationId
})
const options = {
...(args.expectedIncarnationId
? { expectedIncarnationId: args.expectedIncarnationId }
: {}),
...(args.scanChildProcesses === true ? { scanChildProcesses: true } : {})
}
return Object.keys(options).length > 0
? inspectPtyProviderProcessForRenderer(getProviderForPty(args.id), args.id, options)
: inspectPtyProviderProcessForRenderer(getProviderForPty(args.id), args.id)
}
)
src/main/ipc/pty/runtime/queried-host-kinds.test.ts
+60
View File
@@ -0,0 +1,60 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { parseExecutionHostId } from '../../../../shared/execution-host'
import { sshProviders } from '../provider/registry'
import { listProcessesWithHostScopeFromRuntimeController } from './inventory-operations'
import type { PtyRuntimeControllerDeps } from './controller-deps'
/**
* `hostScopeCensusIsComplete` discounts a `runtime:` host in `omittedHostIds` on the strength of
* one fact about this process: it has no paired-runtime PTY provider, so it never queried that
* host and never owed it coverage. This file pins the producer side of that fact.
*
* What it catches: a new branch here that spells a queried host `runtime:`. Every id this
* function emits is built by `toSshExecutionHostId` or is `LOCAL_EXECUTION_HOST_ID`, so a third
* shape is the observable form of "a runtime host can now answer an inventory" — at which point
* the client predicate would start calling a genuine gap complete.
*
* What it does NOT catch, so do not lean on it: a runtime-backed transport registered under an
* SSH connection id still reports as `ssh:` and passes, which is fine — the predicate only
* discounts the `runtime:` spelling. The consolidation moving the SSH path onto orcad is expected
* to look exactly like that. The other route into `queriedHostIds` is separately fenced to
* `kind === 'ssh'` in `orca-runtime-refresh-pty-worktree-records-with-controller-inventory.ts`.
*/
describe('the hosts a PTY inventory can report having queried', () => {
afterEach(() => {
sshProviders.clear()
})
it('emits only local and ssh spellings, never a paired-runtime one', async () => {
const listProcesses = vi.fn(async () => [])
sshProviders.set('box-1', { listProcesses } as never)
// A connection id shaped like an environment uuid still has to come back `ssh:`; the spelling
// is what the gate keys on, so a `runtime:` id appearing here is the breakage that matters.
sshProviders.set('a2478221-1d5c-4603-b8bf-b6b728eac9df', { listProcesses } as never)
const { hostIds } = await listProcessesWithHostScopeFromRuntimeController({
runtime: null
} as unknown as PtyRuntimeControllerDeps)
expect(hostIds).toContain('ssh:a2478221-1d5c-4603-b8bf-b6b728eac9df')
expect(new Set(hostIds.map((hostId) => parseExecutionHostId(hostId)?.kind))).toEqual(
new Set(['local', 'ssh'])
)
})
it('drops a provider that threw rather than reporting its host as queried', async () => {
sshProviders.set('box-live', { listProcesses: vi.fn(async () => []) } as never)
sshProviders.set('box-down', {
listProcesses: vi.fn(async () => {
throw new Error('relay unavailable')
})
} as never)
const { hostIds } = await listProcessesWithHostScopeFromRuntimeController({
runtime: { markPtyLivenessUnverifiable: vi.fn() }
} as unknown as PtyRuntimeControllerDeps)
expect(hostIds).toContain('ssh:box-live')
expect(hostIds).not.toContain('ssh:box-down')
})
})
src/main/ipc/worktrees/listing/detected-provider-listing.ts
+2 -5
View File
@@ -26,8 +26,7 @@ import {
type DetectedWorktreeSideEffectToken
} from './detected-worktree-scan-cache'
import { loggedWorktreeListFailures, warnOnce } from './worktree-listing-diagnostics'
import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta'
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
import { readAllWorktreeMetaForRepo } from '../../../persistence/host-qualified-worktree-meta'
export async function listDetectedWorktreesForCapturedRepo(
store: Store,
@@ -40,9 +39,7 @@ export async function listDetectedWorktreesForCapturedRepo(
providerAbort?.signal.aborted
? ({ providerAbortStatus: providerAbort.status() } as const)
: undefined
const allMeta = isFolderRepo(repo)
? undefined
: readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo))
const allMeta = isFolderRepo(repo) ? undefined : readAllWorktreeMetaForRepo(store, repo)
// Why: only the disconnected fallbacks read this, so keep parseWorktreeId over the whole host snapshot
// off the connected path entirely.
let cachedSshWorktreeMetaIndex: SshWorktreeMetaIndex | undefined
src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts
+6 -5
View File
@@ -23,7 +23,10 @@ import {
warnOnce
} from './worktree-listing-diagnostics'
import type { WorktreeIpcContext } from '../worktree-ipc-context'
import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta'
import {
readAllWorktreeMetaForHost,
readAllWorktreeMetaForRepo
} from '../../../persistence/host-qualified-worktree-meta'
import type { WorktreeMeta } from '../../../../shared/worktree/meta-types'
const WORKTREE_LIST_ALL_CONCURRENCY = 8
@@ -174,9 +177,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
if (!repo) {
return []
}
const allMeta = repo.connectionId
? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo))
: undefined
const allMeta = repo.connectionId ? readAllWorktreeMetaForRepo(store, repo) : undefined
const sshWorktreeMetaIndex = repo.connectionId
? createSshWorktreeMetaIndex(Object.entries(allMeta ?? {}))
: new Map()
@@ -226,7 +227,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
})
}
loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`)
const metadata = allMeta ?? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo))
const metadata = allMeta ?? readAllWorktreeMetaForRepo(store, repo)
return buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata)
.filter((worktree) => worktree.visible)
.map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata))
src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts
+2 -2
View File
@@ -9,7 +9,7 @@ import type { GitWorktreeInfo, DetectedWorktree, Worktree } from '../../../../sh
import type { Store } from '../../../persistence/loading-store/store'
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
import {
readWorktreeMetaForHost,
readWorktreeMetaForRepo,
writeWorktreeMetaForHost
} from '../../../persistence/host-qualified-worktree-meta'
import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership'
@@ -159,7 +159,7 @@ export function buildDetectedGitWorktrees(
const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined
const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {})
const meta =
readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) ??
readWorktreeMetaForRepo(store, worktreeId, repo) ??
getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount)
const worktree = mergeWorktree(repo.id, gitWorktree, meta, repo.displayName)
const detected = toDetectedWorktree({
src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts
+2 -2
View File
@@ -4,7 +4,7 @@ import type { WorktreeMeta } from '../../../../shared/worktree/meta-types'
import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-lookup'
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
import {
readWorktreeMetaForHost,
readWorktreeMetaForRepo,
writeWorktreeMetaForHost
} from '../../../persistence/host-qualified-worktree-meta'
import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership'
@@ -44,7 +44,7 @@ export function resolveWorktreeMetaWithDiscoveryBackfill(
// Why: the locator-keyed row is only a stand-in for a missing snapshot, so don't read it when we have one.
const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined
const existing =
readWorktreeMetaForHost(store, worktreeId, executionHostId) ??
readWorktreeMetaForRepo(store, worktreeId, repo) ??
getRepoOwnedWorktreeMeta(
repo,
worktreeId,
src/main/persistence/host-qualified-worktree-meta.ts
+22 -1
View File
@@ -1,4 +1,5 @@
import type { ExecutionHostId } from '../../shared/execution-host'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import type { Repo } from '../../shared/repo-types'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
/**
@@ -52,6 +53,26 @@ export function readWorktreeMetaForHost(
return store.getWorktreeMetaForHost?.(worktreeId, executionHostId)
}
/**
* The same two reads keyed off a repo row, so the resolve-then-read pair lives in one place. Four
* call sites had open-coded it identically, which is the shape that lets one copy drift from the
* rest (F7/F8).
*/
export function readAllWorktreeMetaForRepo(
store: Pick<HostQualifiedWorktreeMetaStore, 'getAllWorktreeMeta' | 'getAllWorktreeMetaForHost'>,
repo: Pick<Repo, 'connectionId' | 'executionHostId'>
): Record<string, WorktreeMeta> {
return readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo))
}
export function readWorktreeMetaForRepo(
store: Pick<HostQualifiedWorktreeMetaStore, 'getWorktreeMetaForHost'>,
worktreeId: string,
repo: Pick<Repo, 'connectionId' | 'executionHostId'>
): WorktreeMeta | undefined {
return readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo))
}
export function writeWorktreeMetaForHost(
store: Pick<HostQualifiedWorktreeMetaStore, 'setWorktreeMeta' | 'setWorktreeMetaForHost'>,
worktreeId: string,
src/main/providers/pty-process-inspection.ts
+13 -3
View File
@@ -11,14 +11,24 @@ export type PtyProcessInspection = TerminalProcessInspection
type CompletionSensitivePtyProvider = IPtyProvider & {
inspectProcess?: (
id: string,
options?: { expectedIncarnationId?: PtyIncarnationId }
options?: PtyProcessInspectionOptions
) => Promise<PtyProcessInspection>
}
/**
* `scanChildProcesses` marks a read whose answer decides something once, rather than a poll that
* self-corrects on its next tick. Only hosts where the child answer costs a process-table read
* act on it; everywhere else the answer was already captured.
*/
export type PtyProcessInspectionOptions = {
expectedIncarnationId?: PtyIncarnationId
scanChildProcesses?: boolean
}
export async function inspectPtyProviderProcess(
provider: IPtyProvider,
ptyId: string,
options?: { expectedIncarnationId?: PtyIncarnationId }
options?: PtyProcessInspectionOptions
): Promise<PtyProcessInspection> {
if (provider.hasPty?.(ptyId) === false) {
throw new Error('terminal_gone')
@@ -37,7 +47,7 @@ export async function inspectPtyProviderProcess(
export async function inspectPtyProviderProcessForRenderer(
provider: IPtyProvider,
ptyId: string,
options?: { expectedIncarnationId?: PtyIncarnationId }
options?: PtyProcessInspectionOptions
): Promise<PtyProcessInspection> {
try {
return await inspectPtyProviderProcess(provider, ptyId, options)
src/main/providers/ssh-pty-provider-rpc-operations.ts
+4 -2
View File
@@ -56,13 +56,15 @@ export function createSshPtyProviderRpcOperations({ mux, toRelayPtyId }: SshPtyP
// Guarded by ssh-pty-inspect-observation-identity.test.ts; #17525 removes the poll.
inspectProcess: async (
id: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
): Promise<PtyProcessInspection> => {
return (await mux.request('pty.inspectProcess', {
id: toRelayPtyId(id),
...(options?.expectedIncarnationId
? { expectedIncarnationId: options.expectedIncarnationId }
: {})
: {}),
// Additive request member: an older relay ignores it and answers as it always did.
...(options?.scanChildProcesses === true ? { scanChildProcesses: true } : {})
})) as PtyProcessInspection
},
serialize: async (ids: string[]): Promise<string> => {
src/main/providers/ssh-pty-provider.ts
+1 -1
View File
@@ -63,7 +63,7 @@ export class SshPtyProvider implements IPtyProvider {
this.rpcOperations.getForegroundProcess(id)
inspectProcess = (
id: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
): Promise<PtyProcessInspection> => this.rpcOperations.inspectProcess(id, options)
serialize = (ids: string[]): Promise<string> => this.rpcOperations.serialize(ids)
revive = (state: string): Promise<void> => this.rpcOperations.revive(state)
src/main/pty/node-pty-self-exit-pseudoconsole-close.test.ts
+180
View File
@@ -0,0 +1,180 @@
import { readFileSync } from 'node:fs'
import { join } from 'node:path'
import { describe, expect, it } from 'vitest'
/**
* A shell that exits by itself must still close its pseudoconsole.
*
* `ClosePseudoConsole` is the only thing that reaps a ConPTY's console host —
* Orca's own job-ownership patch says so, because `CreatePseudoConsole` spawns
* that host before the per-pty job exists and it is therefore not a job member.
* Upstream node-pty calls it from exactly one place, `PtyKill`, which begins by
* looking the baton up by id — and the exit watcher in `SetupExitCallback`
* erased the baton the moment the shell died. So on the self-exit path (typing
* `exit`, which is how panes usually close) that lookup missed, `PtyKill` did
* nothing at all, and the pseudoconsole was never closed.
*
* There is a SECOND, independent defect on the same path: the `useConptyDll`
* branch of `WindowsPtyAgent.kill()` disposed the conout worker only from an
* `_outSocket.on('data')` handler, and no more data arrives once the shell has
* gone — so that worker leaked too. The non-DLL branch beside it already
* disposed unconditionally. The desktop always sets `useConptyDll`, so it hit
* both; the relay sets neither and hit only the first.
*
* Measured on Windows 11 / awin, 20 cycles, handles bucketed by NT object type,
* totals before -> after:
*
* self-exit, relay spawn 225 -> 285 becomes 219 -> 219 FLAT
* self-exit, desktop spawn 239 -> 439 becomes 222 -> 222 FLAT
* explicit kill, relay spawn 225 -> 285 becomes 219 -> 219 FLAT
* explicit kill, desktop spawn 235 -> 395 becomes 219 -> 219 FLAT
*
* Neither fix alone is enough on the desktop: the pseudoconsole close is worth
* +1 Process +1 File per terminal, the dispose +2 Thread +4 File.
*
* WHY THIS IS A PATCH-CONTENT PIN AND NOT A BEHAVIOURAL TEST: the defect is
* only observable as a per-NT-type handle count, which needs
* `NtQuerySystemInformation(SystemExtendedHandleInformation)`. Nothing in the
* repo can read that, and the cheaper Windows-observable proxies do not
* discriminate — the console host process is reaped either way (the leak is a
* handle to an already-exited object, not an orphaned process), and the
* `\\.\pipe\conpty-*` entries disappear either way. Both were measured and
* rejected as assertions rather than assumed. So this pins the mechanism
* instead, which is the real risk: a future resync of the vendored patch
* silently dropping the hunk.
*/
const PATCH = readFileSync(join(__dirname, '../../../config/patches/node-pty@1.1.0.patch'), 'utf8')
/**
* Just the `PtyKill` hunk. Several markers below also occur in the `PtyConnect`
* hunk above it, and a bare `indexOf` on the whole patch silently matched the
* wrong one — an assertion that then held regardless of what `PtyKill` did.
*/
const ptyKillHunk = (() => {
// Anchored on the hunk header's function context rather than its line
// numbers, which shift whenever anything above it in the patch changes.
const header = /^@@ .* @@ static Napi::Value PtyKill\(.*$/m.exec(PATCH)
if (!header) {
throw new Error('no PtyKill hunk in config/patches/node-pty@1.1.0.patch')
}
const from = header.index
const next = PATCH.indexOf('\n@@ ', from + 1)
return PATCH.slice(from, next === -1 ? undefined : next)
})()
/**
* `indexOf` that throws instead of returning -1. A missing marker must fail the
* assertion that depends on it, not quietly make a slice or comparison vacuous.
*/
function indexIn(haystack: string, marker: string): number {
const at = haystack.indexOf(marker)
if (at === -1) {
throw new Error(`marker not found in the PtyKill hunk: ${marker}`)
}
return at
}
describe('node-pty patch: pseudoconsole close on the self-exit path', () => {
it('does not let the exit watcher free the baton while the close is still owed', () => {
// Pinned as one block: the erase must stay INSIDE the consoleClosed guard.
// Upstream ran it unconditionally, which is the line that caused the leak,
// and a resync that re-flattens this is the failure mode worth catching.
expect(PATCH).toContain(
[
'+ baton->shellExited = true;',
'+ if (baton->consoleClosed) {',
'+ const bool removed = remove_pty_baton(baton->id);',
'+ assert(removed);',
'+ (void)removed;',
'+ }'
].join('\n')
)
})
it('closes the pseudoconsole from PtyKill even after the shell has exited', () => {
// hpc is copied out under the lock, so the close survives the baton's removal.
expect(PATCH).toContain('+ hpc = handle->hpc;')
expect(PATCH).toContain('+ pfnClosePseudoConsole(hpc);')
})
it('resolves the ConPTY DLL before it claims the close', () => {
// LoadConptyDll throws when conpty.dll is missing. Throwing after
// consoleClosed was set would strand the pseudoconsole for good: the retry
// finds the work claimed and does nothing.
//
// Anchored inside PtyKill, not by a bare indexOf: the identical line also
// appears in the PtyConnect hunk, earlier in the file, and matching that one
// made this assertion pass no matter where PtyKill resolved the DLL.
const dllResolve = indexIn(
ptyKillHunk,
'+ HANDLE hLibrary = LoadConptyDll(info, useConptyDll);'
)
const claim = indexIn(ptyKillHunk, '+ handle->consoleClosed = true;')
expect(dllResolve).toBeLessThan(claim)
})
it('reaches hShell only under the null check the watcher can trip', () => {
// Pinned as one block. The watcher nulls hShell on exit, and upstream
// dereferenced it unconditionally; every remaining use — the duplication and
// the failure fallback below it — must stay inside this guard.
const start = indexIn(ptyKillHunk, '+ if (useConptyDll && handle->hShell != nullptr) {')
const end = indexIn(ptyKillHunk, '+ if (handle->shellExited) {')
const guarded = ptyKillHunk.slice(start, end)
expect(guarded).toContain('DuplicateHandle(GetCurrentProcess(), handle->hShell')
expect(guarded).toContain('TerminateProcess(handle->hShell, 1);')
// No ADDED line outside that guard may terminate through hShell. Removed
// (`-`) lines still carry upstream's unguarded call, which is the point.
const strayAdds = PATCH.replace(guarded, '')
.split('\n')
.filter((line) => line.startsWith('+') && line.includes('TerminateProcess(handle->hShell'))
expect(strayAdds).toEqual([])
})
it('frees the baton from PtyKill when the shell has already exited', () => {
// The other half of the two-sided handshake. Without it a self-exit followed
// by kill() — the ordinary pane close — leaks one baton and one entry in the
// vector get_pty_baton scans linearly, forever.
expect(ptyKillHunk).toContain(
[
'+ if (handle->shellExited) {',
'+ const bool removed = remove_pty_baton(id);',
'+ assert(removed);',
'+ (void)removed;'
].join('\n')
)
})
it('still kills the shell when DuplicateHandle fails', () => {
// A null hShellDup is indistinguishable from the self-exit case, so a
// swallowed failure would leave the shell running after its pane closed —
// a worse outcome than the leak this patch exists to fix.
expect(PATCH).toContain(
[
'+ hShellDup = nullptr;',
'+ TerminateProcess(handle->hShell, 1);',
'+ }'
].join('\n')
)
})
it('keeps the close idempotent so a second kill cannot double-close', () => {
expect(PATCH).toContain('+ if (handle != nullptr && !handle->consoleClosed) {')
expect(PATCH).toContain('+ handle->consoleClosed = true;')
})
})
describe('node-pty patch: conout worker disposal on the self-exit path', () => {
// The desktop's larger half: 8 of its 10 leaked handles per terminal.
it('disposes the conout worker unconditionally in the useConptyDll branch', () => {
expect(PATCH).toContain('+ this._conoutSocketWorker.dispose();')
// The data handler is what never fired once the shell had gone.
expect(PATCH).toContain("- this._outSocket.on('data', function () {")
expect(PATCH).toContain('- _this._conoutSocketWorker.dispose();')
})
it('applies the same change to the TypeScript source the patch also carries', () => {
expect(PATCH).toContain('+ this._conoutSocketWorker.dispose();')
expect(PATCH).toContain("- this._outSocket.on('data', () => {")
})
})
src/main/runtime/orca-runtime-restore-structured-agent-session-tabs-once.ts
+1 -1
View File
@@ -153,7 +153,7 @@ export class OrcaRuntimeWithRestoreStructuredAgentSessionTabsOnce extends OrcaRu
async inspectTerminalProcess(
terminalSelector: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
): Promise<PtyProcessInspection> {
const leaf = this.resolveLiveLeafForHandle(terminalSelector)
if (!leaf?.ptyId || !this.ptyController) {
src/main/runtime/rpc/methods/terminal/terminal-inspect-process-params.test.ts
+92
View File
@@ -0,0 +1,92 @@
// The host half of the same contract: an RPC schema silently strips keys it does not declare, which
// is exactly what forward compatibility needs and exactly how a caller's option can vanish inside
// one version. `scanChildProcesses` has to be declared here, and only here -- the sibling handle
// methods have no use for it and must keep refusing it.
import { describe, expect, it, vi } from 'vitest'
import type { ZodType } from 'zod'
import { TERMINAL_QUERY_METHODS } from './terminal-query-methods'
import { TerminalHandle, TerminalInspectProcess } from './unary-schemas'
/** The method as registered, so a schema swap on the definition cannot pass unseen. */
function inspectProcessMethod() {
const method = TERMINAL_QUERY_METHODS.find((entry) => entry.name === 'terminal.inspectProcess')
if (!method) {
throw new Error('terminal.inspectProcess is not registered')
}
return method
}
async function callRegisteredHandler(
params: Record<string, unknown>
): Promise<{ terminal: string; options: unknown }> {
const method = inspectProcessMethod()
const parsed = (method.params as ZodType).parse(params)
const inspectTerminalProcess = vi.fn(async () => ({
foregroundProcess: null,
hasChildProcesses: false
}))
await method.handler(parsed, { runtime: { inspectTerminalProcess } } as never, undefined as never)
const [terminal, options] = inspectTerminalProcess.mock.calls[0] as unknown as [string, unknown]
return { terminal, options }
}
describe('terminal.inspectProcess registration', () => {
// The half the schema test alone cannot see: pointing the method back at the shared handle schema
// compiles, parses, and silently drops the option. This exercises the registered definition.
it('carries scanChildProcesses from the wire into the runtime call', async () => {
await expect(
callRegisteredHandler({ terminal: 'term_1', scanChildProcesses: true })
).resolves.toEqual({ terminal: 'term_1', options: { scanChildProcesses: true } })
})
it('carries it alongside the incarnation fence', async () => {
await expect(
callRegisteredHandler({
terminal: 'term_1',
expectedIncarnationId: 'inc-1',
scanChildProcesses: true
})
).resolves.toEqual({
terminal: 'term_1',
options: { expectedIncarnationId: 'inc-1', scanChildProcesses: true }
})
})
it('keeps the legacy one-argument shape for a bare poll', async () => {
await expect(callRegisteredHandler({ terminal: 'term_1' })).resolves.toEqual({
terminal: 'term_1',
options: undefined
})
})
})
describe('terminal.inspectProcess params', () => {
it('preserves scanChildProcesses', () => {
expect(TerminalInspectProcess.parse({ terminal: 'term_1', scanChildProcesses: true })).toEqual({
terminal: 'term_1',
scanChildProcesses: true
})
})
it('preserves it alongside the incarnation fence', () => {
expect(
TerminalInspectProcess.parse({
terminal: 'term_1',
expectedIncarnationId: 'inc-1',
scanChildProcesses: true
})
).toEqual({ terminal: 'term_1', expectedIncarnationId: 'inc-1', scanChildProcesses: true })
})
it('leaves it absent for a polling caller', () => {
expect(TerminalInspectProcess.parse({ terminal: 'term_1' })).toEqual({ terminal: 'term_1' })
})
// The shape that produced the bug, pinned so nobody "simplifies" the method back onto the shared
// handle schema: TerminalHandle drops the option on the floor without complaining.
it('shows why the shared handle schema could not carry it', () => {
expect(TerminalHandle.parse({ terminal: 'term_1', scanChildProcesses: true })).toEqual({
terminal: 'term_1'
})
})
})
src/main/runtime/rpc/methods/terminal/terminal-query-methods.ts
+15 -8
View File
@@ -1,6 +1,7 @@
import { defineMethod, type RpcAnyMethod } from '../../core'
import {
TerminalHandle,
TerminalInspectProcess,
TerminalListParams,
TerminalRead,
TerminalRecoverPane,
@@ -65,15 +66,21 @@ export const TERMINAL_QUERY_METHODS: RpcAnyMethod[] = [
}),
defineMethod({
name: 'terminal.inspectProcess',
params: TerminalHandle,
handler: async (params, { runtime }) => ({
process: await runtime.inspectTerminalProcess(
params.terminal,
params.expectedIncarnationId
params: TerminalInspectProcess,
handler: async (params, { runtime }) => {
const options = {
...(params.expectedIncarnationId
? { expectedIncarnationId: params.expectedIncarnationId }
: undefined
)
})
: {}),
...(params.scanChildProcesses === true ? { scanChildProcesses: true } : {})
}
return {
process: await runtime.inspectTerminalProcess(
params.terminal,
Object.keys(options).length > 0 ? options : undefined
)
}
}
}),
defineMethod({
name: 'terminal.isRunningAgent',
src/main/runtime/rpc/methods/terminal/unary-schemas.ts
+11
View File
@@ -13,6 +13,17 @@ export const TerminalFocus = TerminalHandle.extend({
navigation: z.enum(['caller', 'host']).optional()
})
/**
* `terminal.inspectProcess` carries one member the sibling handle methods must not: whether the
* caller's answer decides something once, which is what licenses the host to pay for a process-table
* read. Extended rather than added to `TerminalHandle` so `clearBuffer`/`agentStatus`/`isRunningAgent`
* keep refusing an option they have no use for.
*/
export const TerminalInspectProcess = TerminalHandle.extend({
// Additive request member understood by newer hosts; legacy hosts safely ignore it.
scanChildProcesses: z.boolean().optional()
})
export const TerminalListParams = z.object({
worktree: OptionalString,
limit: OptionalFiniteNumber,
src/main/runtime/runtime-managed-worktree-queries.test.ts
+118 -2
View File
@@ -40,14 +40,18 @@ function metadata(overrides: Partial<WorktreeMeta> = {}): WorktreeMeta {
}
}
function queries(store: RuntimeStore): RuntimeManagedWorktreeQueries {
function queries(
store: RuntimeStore,
overrides: Partial<ConstructorParameters<typeof RuntimeManagedWorktreeQueries>[0]> = {}
): RuntimeManagedWorktreeQueries {
return new RuntimeManagedWorktreeQueries({
getStore: () => store,
listResolved: async () => [],
resolveRepo: async () => store.getRepos()[0]!,
selectRepos: () => store.getRepos(),
scanRepo: async () => ({ ok: true, worktrees: [] }),
listKnownHostIds: () => []
listKnownHostIds: () => [],
...overrides
})
}
@@ -105,3 +109,115 @@ describe('RuntimeManagedWorktreeQueries.listDetected', () => {
expect(legacy.worktrees[0]).not.toHaveProperty('visibilitySource')
})
})
describe('RuntimeManagedWorktreeQueries.list host scope', () => {
// Measured on hardware before this fix, same runtime and same refusing SSH host in the same
// second: the UNSCOPED listing reported `omittedHostIds: ["local","ssh:ssh-scope-refused"]` with
// `--host` selectors, while the SCOPED listing reported `{"hostIds":[],"omittedHostIds":[]}`.
// A listing that covered nothing, reporting no gaps, is indistinguishable from a repo that
// genuinely has no worktrees -- the thing docs/reference/ssh-execution-boundary.md forbids.
function sshStore(): RuntimeStore {
const repo = folderRepo({
id: 'repo-ssh',
kind: 'git',
connectionId: 'conn-1',
path: '/home/dev/app'
})
return {
getRepos: () => [repo],
getRepo: () => repo,
getAllWorktreeMeta: () => ({}),
getWorktreeMeta: () => undefined,
setWorktreeMeta: vi.fn(),
getAllWorktreeLineage: () => ({}),
getSettings: () => settings
} as unknown as RuntimeStore
}
it('names the scoped repo host as omitted when the listing covered nothing', async () => {
const result = await queries(sshStore()).list('repo-ssh', 50)
expect(result.totalCount).toBe(0)
expect(result.hostScope).toEqual({
hostIds: [],
omittedHostIds: ['ssh:conn-1']
})
})
it('does not report the scoped host as omitted once it contributes rows', async () => {
const store = sshStore()
const result = await queries(store, {
listResolved: async () =>
[
{
id: 'repo-ssh::/home/dev/app',
repoId: 'repo-ssh',
path: '/home/dev/app',
hostId: 'ssh:conn-1'
}
] as never
}).list('repo-ssh', 50)
expect(result.hostScope?.hostIds).toEqual(['ssh:conn-1'])
expect(result.hostScope?.omittedHostIds).toEqual([])
})
// The caller scoped the listing, so the hosts they excluded must not come back as gaps.
it('never names a host the caller scoped out', async () => {
const scoped = await queries(sshStore(), {
listKnownHostIds: () => ['local', 'ssh:other', 'runtime:elsewhere'] as never
}).list('repo-ssh', 50)
expect(scoped.hostScope?.omittedHostIds).toEqual(['ssh:conn-1'])
})
// `getRepoExecutionHostId` derives the host from two spellings, and a scoped listing that named
// the wrong one would be worse than naming none. These pin both.
it('names the local host for a scoped local repo', async () => {
const repo = folderRepo({ id: 'repo-local', kind: 'git', path: '/workspace/local' })
const store = {
getRepos: () => [repo],
getRepo: () => repo,
getAllWorktreeMeta: () => ({}),
getWorktreeMeta: () => undefined,
setWorktreeMeta: vi.fn(),
getAllWorktreeLineage: () => ({}),
getSettings: () => settings
} as unknown as RuntimeStore
const result = await queries(store).list('repo-local', 50)
expect(result.hostScope?.omittedHostIds).toEqual(['local'])
})
it('prefers executionHostId over connectionId for the scoped host', async () => {
const repo = folderRepo({
id: 'repo-runtime',
kind: 'git',
connectionId: 'conn-legacy',
executionHostId: 'runtime:env-1',
path: '/workspace/runtime'
})
const store = {
getRepos: () => [repo],
getRepo: () => repo,
getAllWorktreeMeta: () => ({}),
getWorktreeMeta: () => undefined,
setWorktreeMeta: vi.fn(),
getAllWorktreeLineage: () => ({}),
getSettings: () => settings
} as unknown as RuntimeStore
const result = await queries(store).list('repo-runtime', 50)
expect(result.hostScope?.omittedHostIds).toEqual(['runtime:env-1'])
})
it('still reports every configured host when the listing is unscoped', async () => {
const unscoped = await queries(sshStore(), {
listKnownHostIds: () => ['local', 'ssh:conn-1'] as never
}).list(undefined, 50)
expect(unscoped.hostScope?.omittedHostIds).toEqual(['local', 'ssh:conn-1'])
})
})
src/main/runtime/runtime-managed-worktree-queries.ts
+6 -6
View File
@@ -2,7 +2,7 @@ import type { DetectedWorktreeListResult, Worktree } from '../../shared/worktree
import type { Repo } from '../../shared/repo-types'
import type { RuntimeWorktreeListResult } from '../../shared/runtime-types'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import { buildWorktreeListingPage } from './worktree-listing-host-scope'
import { buildWorktreeListingPage, listingKnownHostIds } from './worktree-listing-host-scope'
import { readWorktreeMetaForHost } from '../persistence/host-qualified-worktree-meta'
import { getRepoOwnedWorktreeMeta } from '../worktree-metadata-ownership'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
@@ -80,7 +80,7 @@ export class RuntimeManagedWorktreeQueries {
throw new Error('invalid_limit')
}
const resolved = await this.deps.listResolved()
const repoId = repoSelector ? (await this.deps.resolveRepo(repoSelector)).id : null
const scopedRepo = repoSelector ? await this.deps.resolveRepo(repoSelector) : null
const pathsByRepo = new Map<string, string[]>()
for (const worktree of resolved) {
const paths = pathsByRepo.get(worktree.repoId) ?? []
@@ -100,12 +100,12 @@ export class RuntimeManagedWorktreeQueries {
)
const worktrees = resolved.filter(
(worktree) =>
(!repoId || worktree.repoId === repoId) &&
(!scopedRepo || worktree.repoId === scopedRepo.id) &&
this.isVisible(worktree, matchers.get(worktree.repoId), sourceDefaultsSupported)
)
// Why: a `--repo` listing was scoped by the caller, so naming every configured host as
// omitted would report a gap the caller deliberately excluded.
return buildWorktreeListingPage(worktrees, limit, repoId ? [] : this.deps.listKnownHostIds())
// See `listingKnownHostIds`: a scoped listing must still name the host it was asked about.
const knownHostIds = listingKnownHostIds(scopedRepo, () => this.deps.listKnownHostIds())
return buildWorktreeListingPage(worktrees, limit, knownHostIds)
}
resolveRepoForConnection(selector: string, connectionId?: string | null): Promise<Repo> {
src/main/runtime/runtime-pty-controller-contract.ts
+1 -1
View File
@@ -109,7 +109,7 @@ export type RuntimePtyController = {
getForegroundProcess(ptyId: string): Promise<string | null>
inspectProcess?(
ptyId: string,
options?: { expectedIncarnationId?: PtyIncarnationId }
options?: { expectedIncarnationId?: PtyIncarnationId; scanChildProcesses?: boolean }
): Promise<PtyProcessInspection>
confirmForegroundProcess?(ptyId: string): Promise<string | null>
confirmShellForeground?(ptyId: string): Promise<boolean>
src/main/runtime/worktree-launch-host-repo.ts
+8 -2
View File
@@ -17,7 +17,10 @@ export type WorktreeHostRouting<T extends LaunchHostRepo> =
| { kind: 'resolved'; hostId: ExecutionHostId; repo: T | null }
/** No row carries this repo id and the worktree names no host — nothing ever named a host. */
| { kind: 'unowned' }
/** Rival rows disagree about the host; guessing one is the cross-host leak. */
/**
* No single trustworthy host: rival rows disagree, or the resolved row named one that cannot be
* parsed. Guessing is the cross-host leak in both cases.
*/
| { kind: 'ambiguous' }
/**
@@ -33,7 +36,10 @@ export function resolveWorktreeHostRouting<T extends LaunchHostRepo & ExecutionH
): WorktreeHostRouting<T> {
const resolution = resolveWorktreeExecutionHost(createRepoRowExecutionHostLookup(repos), worktree)
if (resolution.kind === 'unresolved') {
return resolution.reason === 'ambiguous' ? { kind: 'ambiguous' } : { kind: 'unowned' }
// Only `unknown` — nothing anywhere carries the id — becomes `unowned`, which callers dispose of
// as a plain local folder. `malformed` is a row that declared a host and named an unparseable
// one, so it joins `ambiguous`: guessing is the cross-host leak either way.
return resolution.reason === 'unknown' ? { kind: 'unowned' } : { kind: 'ambiguous' }
}
return { kind: 'resolved', hostId: resolution.hostId, repo: resolution.owner }
}
src/main/runtime/worktree-listing-host-scope.ts
+27 -1
View File
@@ -1,4 +1,5 @@
import type { ExecutionHostId } from '../../shared/execution-host'
import type { Repo } from '../../shared/repo-types'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import { selectHostBalancedPage } from '../../shared/host-balanced-listing-page'
import type { RuntimeListingHostScope } from '../../shared/runtime-listing-host-scope'
@@ -62,3 +63,28 @@ export function buildWorktreeListingHostScope(args: {
}
return { hostIds: [...covered].sort(), omittedHostIds: [...omitted].sort() }
}
/**
* Which hosts a listing claims to have been looking at.
*
* A `--repo` listing was scoped by the caller, so naming every configured host would report gaps
* the caller deliberately excluded. Naming NONE — which is what a scoped listing did before — means
* the scope can never report a gap at all, for any host kind, because `covered` and `omitted` are
* both derived from the returned rows plus this list. A scoped listing whose scan did not succeed
* then answers `{hostIds: [], omittedHostIds: []}`: byte-identical to a repo that genuinely has no
* worktrees, which is the one thing docs/reference/ssh-execution-boundary.md forbids a listing from
* implying.
*
* Measured before the fix, on one runtime with one refusing SSH host, in the same second: the
* unscoped listing reported `omittedHostIds: ["local", "ssh:<target>"]` while the scoped listing
* reported `[]`.
*
* Naming the single host the caller asked about costs nothing when rows come back — it lands in
* `covered`, so it is never reported omitted — and is the whole answer when they do not.
*/
export function listingKnownHostIds(
scopedRepo: Repo | null,
listKnownHostIds: () => Iterable<ExecutionHostId>
): Iterable<ExecutionHostId> {
return scopedRepo ? [getRepoExecutionHostId(scopedRepo)] : listKnownHostIds()
}
src/main/ssh/ssh-relay-deploy.ts
+18 -7
View File
@@ -743,6 +743,7 @@ function uploadStageNamespaceIfSupported(
const NODE_PTY_VERSION = '1.1.0'
const NODE_PTY_CONSOLE_LIST_PATCH_FILENAME = 'node-pty-1.1.0-console-list-agent-patch.cjs'
const NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME = 'node-pty-1.1.0-windows-pty-teardown-patch.cjs'
const NODE_PTY_MASTER_CLOEXEC_PATCH_FILENAME = 'node-pty-1.1.0-master-cloexec-patch.cjs'
const NODE_PTY_CLOEXEC_STATUS_PREFIX = 'ORCA-NPTY-CLOEXEC:'
/**
@@ -791,7 +792,8 @@ function nativeDepsProbeJs(successToken: string): string {
// Why: node-pty's Windows wrapper defers conpty.node until first spawn, so require("node-pty") alone can't prove the binding is healthy.
const loadNodePty =
'require("node-pty"); require("node-pty/lib/utils").loadNativeModule(process.platform==="win32"&&Number(require("os").release().split(".")[2])>=18309?"conpty":"pty");' +
`if(process.platform==="win32"){require("./${NODE_PTY_CONSOLE_LIST_PATCH_FILENAME}").assertPatchedNodePtyConsoleListAgent(process.cwd())}`
`if(process.platform==="win32"){require("./${NODE_PTY_CONSOLE_LIST_PATCH_FILENAME}").assertPatchedNodePtyConsoleListAgent(process.cwd());` +
`require("./${NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME}").assertPatchedNodePtyWindowsTeardown(process.cwd())}`
return `(()=>{const missing=[];try{${loadNodePty}}catch{missing.push("node-pty")}try{require("@parcel/watcher")}catch{missing.push("@parcel/watcher")}if(missing.length){console.log("${NATIVE_DEPS_MISSING_PREFIX}"+missing.join(","));process.exitCode=1}else{console.log(${JSON.stringify(successToken)})}})()`
}
@@ -1327,10 +1329,16 @@ async function applyNodePtyMasterCloexecPatch(
nodePath: string,
signal?: AbortSignal
): Promise<NodePtyMasterCloexecOutcome> {
// Both Unix relay platforms leak, by different bugs: Linux inherits the master through forkpty()'s
// no-O_CLOEXEC path, macOS orphans one throwaway /dev/ptmx fd per spawn in pty_posix_spawn. Only
// Windows, which has no fds, is short-circuited -- and answering 'fixed' from a gate that ran
// nothing is exactly how a leaking darwin tree got published to the shared cache.
// Both Unix relay platforms leak the pty master, by different bugs: Linux inherits it through
// forkpty()'s no-O_CLOEXEC path, macOS orphans one throwaway /dev/ptmx fd per spawn in
// pty_posix_spawn. Windows is short-circuited because it has no fds for a master to leak into --
// and answering 'fixed' from a gate that ran nothing is exactly how a leaking darwin tree got
// published to the shared cache.
//
// What 'fixed' means here is exactly "this tree does not leak the pty MASTER", which is the only
// thing the shared native-deps cache keys on. It is NOT a statement that a Windows relay leaks
// nothing: it leaked one Windows File handle per terminal until the ConPTY teardown patch above,
// by a mechanism that has nothing to do with fds. Read this gate as scoped to its own question.
if (isWindowsRemoteHost(hostPlatform) || isWindowsRelayPlatform(platform)) {
return 'fixed'
}
@@ -1530,8 +1538,11 @@ async function rebuildNativeDeps(
}
function windowsNodePtyPatchCommand(nodePath: string): string {
// Why: pnpm patches do not cross the SSH boundary; apply the version-checked fallback to the remote npm package.
return `& ${powerShellLiteral(nodePath)} ${powerShellLiteral(NODE_PTY_CONSOLE_LIST_PATCH_FILENAME)}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }`
// Why: pnpm patches do not cross the SSH boundary; apply the version-checked fallbacks to the remote npm package.
return [
`& ${powerShellLiteral(nodePath)} ${powerShellLiteral(NODE_PTY_CONSOLE_LIST_PATCH_FILENAME)}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }`,
`& ${powerShellLiteral(nodePath)} ${powerShellLiteral(NODE_PTY_WINDOWS_TEARDOWN_PATCH_FILENAME)}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }`
].join('; ')
}
async function makeNodePtySpawnHelperExecutable(
src/main/windows/windows-pty-job.ts
+4 -2
View File
@@ -118,8 +118,10 @@ export function terminatePtyJob(proc: IPty): JobTerminationOutcome {
/**
* Pids still alive in a PTY's tree, or null when there is no answer.
*
* Measured on Windows 11: once the shell exits, node-pty drops its handle
* record and closes the job, so a terminated tree reports **null**, not `[]`.
* Measured on Windows 11: once the shell exits, node-pty closes the job, so a
* terminated tree reports **null**, not `[]`. (Its handle record now outlives
* the shell until `kill()` runs — see config/patches/node-pty@1.1.0.patch — but
* the nulled job handle is what makes the answer null either way.)
* Null therefore means "unverifiable" in the sense of
* docs/reference/ssh-execution-boundary.md — this build has no job support,
* the terminal is not a ConPTY, or it is no longer tracked. It is never
src/preload/api/pty-api.ts
+1 -1
View File
@@ -112,7 +112,7 @@ export type PtyApi = {
getForegroundProcess: (id: string) => Promise<string | null>
inspectProcess: (
id: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
) => Promise<TerminalProcessInspection>
confirmForegroundProcess: (id: string) => Promise<string | null>
getCwd: (id: string) => Promise<string>
src/preload/api/pty-bridge-stream-and-serialization.ts
+1 -1
View File
@@ -7,7 +7,7 @@ import type { TerminalProcessInspection } from '../../shared/terminal-process-in
export const ptyStreamAndSerializationApi = {
inspectProcess: (
id: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
): Promise<TerminalProcessInspection> =>
ipcRenderer.invoke('pty:inspectProcess', { id, ...options }),
confirmForegroundProcess: (id: string): Promise<string | null> =>
src/relay/pty-child-process-inspection.ts
+81
View File
@@ -0,0 +1,81 @@
/**
* Whether anything is running under a pane's shell.
*
* Split out of `pty-shell-utils` because it is a distinct question from "what is in front" and
* carries its own platform reasoning, its own cost budget, and the verdict vocabulary from
* docs/reference/ssh-execution-boundary.md.
*/
import { queryWindowsPaneProcessInventory } from '../main/providers/windows-foreground-process-rows'
import { getProcessTableIndex } from '../shared/process-table-index'
import {
getFreshProcessTableSnapshot,
getProcessTableSnapshot
} from '../shared/process-table-snapshot-reader'
import type { PtyChildProcessVerdict } from '../shared/terminal-process-inspection'
import { isProcessAlive } from './pty-shell-utils'
/**
* Check whether a process has child processes.
*
* Why the shared snapshot and not `pgrep -P`: this answers one field of
* `pty.inspectProcess`, which every tracked pane polls on a 750ms/2000ms
* cadence, and the fork was neither cached nor coalesced. procps-ng opens six
* procfs files per process to resolve a ppid — including a `/proc/<pid>/ctty`
* that never exists on Linux — so one call cost O(host process count) syscalls,
* ~4k opens per pgrep on a 690-process host, at up to 8 forks/sec (#13537).
* `getForegroundProcessName` in the same RPC already captured the TTL-cached
* `ps` table, whose index carries the parent/child map, so the answer is free.
*
* `fresh` opts out of that TTL. A poll can read a 500ms-old table because its
* next tick corrects it, but a close or cleanup decision acts on the answer
* once and destructively — a child that started inside the TTL would be killed
* with no confirmation. `pgrep` scanned per call, so anything that decides
* has to keep scanning per call.
*/
export async function inspectPtyChildProcesses(
pid: number,
options?: { fresh?: boolean }
): Promise<PtyChildProcessVerdict> {
if (process.platform === 'win32') {
// Windows has no `ps`, but it does have a process table, and the pane walk over it already
// exists for the foreground reader. Answering `false` from nothing was the older shape: a
// hardcoded negative is indistinguishable from a measurement, and every close guard reads it
// as "nothing is running here".
//
// Deliberately the TTL-cached table even when `fresh` is asked for: on a relay without the
// native binding this falls back to the CIM scan, whose own 1.36s runtime is longer than the
// 500ms TTL a fresh read would be refreshing, so a "fresh" answer is not meaningfully fresher
// while N sequential ones are an N x 1.36s stall.
const inventory = await queryWindowsPaneProcessInventory(pid)
if (inventory) {
return inventory.candidates.length > 0 ? 'children' : 'no-children'
}
// A null inventory is an unreadable table OR a snapshot that never showed the root, and
// neither of those looked at the pane. The one answer available without the table is a root
// the kernel says is gone: nothing runs under a shell that does not exist.
return isProcessAlive(pid) ? 'unverifiable' : 'no-children'
}
try {
const rows = options?.fresh
? await getFreshProcessTableSnapshot()
: await getProcessTableSnapshot()
return (getProcessTableIndex(rows).childrenByPpid.get(pid)?.length ?? 0) > 0
? 'children'
: 'no-children'
} catch {
return 'unverifiable'
}
}
/**
* The boolean the wire has always carried. `unverifiable` keeps spelling itself `false` here on
* purpose: this value reaches clients too old to know the third answer, and it is read both as
* "busy, do not close" and as "the agent has taken over, safe to type into", so no single mapping
* of `unverifiable` is safe for both. Callers that can act on the distinction read the verdict.
*/
export async function processHasChildren(
pid: number,
options?: { fresh?: boolean }
): Promise<boolean> {
return (await inspectPtyChildProcesses(pid, options)) === 'children'
}
src/relay/pty-handler-spawn-admission.test.ts
+2 -1
View File
@@ -2,6 +2,7 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import * as ptyChildProcessInspection from './pty-child-process-inspection'
import * as ptyShellUtils from './pty-shell-utils'
import * as processTableSnapshotReader from '../shared/process-table-snapshot-reader'
@@ -92,7 +93,7 @@ describe('PtyHandler', () => {
})
it('rescans the process table for a close decision but not for a poll', async () => {
const hasChildren = vi.mocked(ptyShellUtils.processHasChildren)
const hasChildren = vi.mocked(ptyChildProcessInspection.processHasChildren)
const snapshot = vi
.spyOn(processTableSnapshotReader, 'getStrictProcessTableSnapshotWithAge')
.mockResolvedValue({
src/relay/pty-handler-test-harness.ts
+2 -2
View File
@@ -1,6 +1,6 @@
import { vi } from 'vitest'
import type { Mock } from 'vitest'
import * as ptyShellUtils from './pty-shell-utils'
import * as ptyChildProcessInspection from './pty-child-process-inspection'
import { PtyHandler } from './pty-handler'
import type { RelayDispatcher } from './dispatcher'
@@ -115,7 +115,7 @@ export function beginPtyHandlerTest(mocks: PtyHandlerTestMocks): {
notifyOutput: vi.fn(),
dispose: vi.fn()
})
vi.spyOn(ptyShellUtils, 'processHasChildren').mockResolvedValue(false)
vi.spyOn(ptyChildProcessInspection, 'processHasChildren').mockResolvedValue(false)
mockPtySpawn.mockReturnValue({ ...mockPtyInstance })
src/relay/pty-handler-windows-child-process-evidence.test.ts
+132
View File
@@ -0,0 +1,132 @@
// Regression guard for the Windows SSH child-process answer. The relay used to return a hardcoded
// `false` here, which every close guard reads as "nothing is running in this pane" -- so a Windows
// SSH pane running a build closed with no prompt. The answer now comes from the process table, and
// the one thing it may never do again is fabricate a negative.
//
// The second contract is cost. `pty.inspectProcess` is the polled path (750ms/2000ms per tracked
// pane) and a relay host has no `@vscode/windows-process-tree`, so its table read falls back to a
// 1.36s CIM scan. Polling that would reinstate the fork storm the shared table exists to prevent,
// so only a caller whose answer decides something asks for the scan.
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
const { mockPtySpawn, mockPtyInstance, mockCreateShellPromptReadinessProbe } = vi.hoisted(() => ({
mockPtySpawn: vi.fn(),
mockCreateShellPromptReadinessProbe: vi.fn(),
mockPtyInstance: {
pid: process.pid,
process: 'xterm-256color',
onData: vi.fn(),
onExit: vi.fn(),
write: vi.fn(),
resize: vi.fn(),
kill: vi.fn(),
clear: vi.fn(),
pause: vi.fn(),
resume: vi.fn()
}
}))
vi.mock('node-pty', () => ({ spawn: mockPtySpawn }))
vi.mock('../main/pty/posix-pty-process-groups', () => ({
forceKillPosixPtyProcessGroups: vi.fn((_pid: number, fallback: () => void) => fallback())
}))
vi.mock('../main/shell-prompt-readiness-probe', () => ({
createShellPromptReadinessProbe: mockCreateShellPromptReadinessProbe
}))
import * as ptyChildProcessInspection from './pty-child-process-inspection'
import type { PtyHandler } from './pty-handler'
import {
beginPtyHandlerTest,
createPtyRequestHelpers,
endPtyHandlerTest
} from './pty-handler-test-harness'
import type { MockDispatcher } from './pty-handler-test-harness'
type Inspection = {
foregroundProcess: string | null
hasChildProcesses: boolean
childProcessEvidence?: string
}
describe('PtyHandler Windows child-process evidence', () => {
let dispatcher: MockDispatcher
let handler: PtyHandler
let originalPlatform: PropertyDescriptor | undefined
let inspectChildren: ReturnType<typeof vi.spyOn>
const { spawnPty } = createPtyRequestHelpers(() => dispatcher)
/** Spawn under the harness's POSIX platform, then answer as the Windows relay would. */
async function spawnThenBecomeWindows(): Promise<string> {
const { id } = await spawnPty()
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
return id
}
async function inspect(params: Record<string, unknown>): Promise<Inspection> {
return (await dispatcher.callRequest('pty.inspectProcess', params)) as Inspection
}
beforeEach(() => {
;({ dispatcher, handler, originalPlatform } = beginPtyHandlerTest({
mockPtySpawn,
mockPtyInstance,
mockCreateShellPromptReadinessProbe
}))
inspectChildren = vi
.spyOn(ptyChildProcessInspection, 'inspectPtyChildProcesses')
.mockResolvedValue('no-children')
})
afterEach(async () => {
await endPtyHandlerTest(handler, originalPlatform)
})
it('publishes what the host observed when the caller pays for the scan', async () => {
const id = await spawnThenBecomeWindows()
inspectChildren.mockResolvedValue('children')
const result = await inspect({ id, scanChildProcesses: true })
expect(inspectChildren).toHaveBeenCalledWith(mockPtyInstance.pid)
expect(result.childProcessEvidence).toBe('children')
expect(result.hasChildProcesses).toBe(true)
})
it('reports an observed-empty pane as no-children, not merely false', async () => {
const id = await spawnThenBecomeWindows()
inspectChildren.mockResolvedValue('no-children')
const result = await inspect({ id, scanChildProcesses: true })
// Asserted alongside the value so the case fails if the answer stops coming from a real read.
expect(inspectChildren).toHaveBeenCalledWith(mockPtyInstance.pid)
expect(result.childProcessEvidence).toBe('no-children')
expect(result.hasChildProcesses).toBe(false)
})
it('keeps the compatibility boolean false when the host could not observe the pane', async () => {
const id = await spawnThenBecomeWindows()
inspectChildren.mockResolvedValue('unverifiable')
const result = await inspect({ id, scanChildProcesses: true })
expect(result.childProcessEvidence).toBe('unverifiable')
// Clients too old to read the verdict also read `true` as "an agent took the PTY, safe to
// type into it", so `unverifiable` must not be promoted to `true` on the shared boolean.
expect(result.hasChildProcesses).toBe(false)
})
it('never reads the process table for a poll, and says so instead of guessing', async () => {
const id = await spawnThenBecomeWindows()
const result = await inspect({ id })
expect(inspectChildren).not.toHaveBeenCalled()
expect(result.childProcessEvidence).toBe('unverifiable')
expect(result.hasChildProcesses).toBe(false)
})
})
src/relay/pty-handler.ts
+23 -7
View File
@@ -10,11 +10,11 @@ import type { RelayDispatcher, RequestContext } from './dispatcher'
import {
resolveDefaultShell,
resolveProcessCwd,
processHasChildren,
getForegroundProcessName,
isProcessAlive,
listShellProfiles
} from './pty-shell-utils'
import { inspectPtyChildProcesses, processHasChildren } from './pty-child-process-inspection'
import { getRelayShellLaunchConfig, isRelayWslShell } from './pty-shell-launch'
import { RetiredPaneSurfaceRegistry } from './retired-pane-surfaces'
import { addWslEnvKeys } from '../shared/wsl-env'
@@ -55,6 +55,7 @@ import {
import { isTuiAgent } from '../shared/tui-agent-config'
import type { TuiAgent } from '../shared/tui-agent'
import { forceKillPosixPtyProcessGroups } from '../main/pty/posix-pty-process-groups'
import type { PtyChildProcessVerdict } from '../shared/terminal-process-inspection'
import { terminatePtyJob } from '../main/windows/windows-pty-job'
import { stripInheritedBuildModeEnv } from '../main/pty/build-mode-env'
import { stripLegacyTerminalShimEnv } from '../main/pty/legacy-terminal-shim-dir'
@@ -2610,6 +2611,7 @@ export class PtyHandler {
private async inspectProcess(params: Record<string, unknown>): Promise<{
foregroundProcess: string | null
hasChildProcesses: boolean
childProcessEvidence?: PtyChildProcessVerdict
foregroundProcessEvidence?: RemoteForegroundEvidence
}> {
pruneRetiredPtyIncarnations(this.retiredIncarnations)
@@ -2716,14 +2718,28 @@ export class PtyHandler {
evidence?.verdict === 'live'
? (evidence.processName ?? managed.pty.process) || null
: managed.pty.process || null
// Derive child liveness from the same capture; do not fork a second process-table probe for
// each field/pane in an event burst.
//
// Why Windows is gated on the caller asking: this is the one field whose Windows answer costs
// a process-table read, and `inspectProcess` is the polled path (750ms/2000ms per tracked
// pane). A relay host has no `@vscode/windows-process-tree`, so the read falls back to the
// 1.36s CIM scan, and polling that would reinstate exactly the fork storm the shared table
// exists to prevent (#15209, #15036). Close and cleanup decisions ask for the scan by name;
// a poll gets the honest `unverifiable` instead of a fabricated negative.
const childProcessEvidence: PtyChildProcessVerdict = rows
? rows.some((row) => row.ppid === managed.pty.pid)
? 'children'
: 'no-children'
: process.platform === 'win32' && params.scanChildProcesses !== true
? 'unverifiable'
: await inspectPtyChildProcesses(managed.pty.pid)
return {
foregroundProcess,
// Derive child liveness from the same capture; do not fork a second
// process-table probe for each field/pane in an event burst. Windows
// has no evidence capture, so preserve the compatibility child probe.
hasChildProcesses: rows
? rows.some((row) => row.ppid === managed.pty.pid)
: await processHasChildren(managed.pty.pid),
// `unverifiable` keeps spelling itself `false` on the compatibility field, which is what
// every client too old to read the verdict receives.
hasChildProcesses: childProcessEvidence === 'children',
childProcessEvidence,
...(evidence ? { foregroundProcessEvidence: evidence } : {})
}
}
src/relay/pty-shell-utils.test.ts
+73 -5
View File
@@ -14,10 +14,10 @@ vi.mock('child_process', () => ({
import { resetWindowsProcessRowsSnapshotForTests } from '../main/providers/windows-foreground-process-rows'
import { __setWindowsProcessTreeLoaderForTests } from '../main/windows/windows-process-table'
import { resetProcessTableSnapshotForTests } from '../shared/process-table-snapshot-reader'
import { inspectPtyChildProcesses, processHasChildren } from './pty-child-process-inspection'
import {
getForegroundProcessName,
isProcessAlive,
processHasChildren,
resolveDefaultCwd,
resolveWindowsDefaultShell
} from './pty-shell-utils'
@@ -42,6 +42,14 @@ function mockExecFile(
* Feed the native Windows snapshot. A real snapshot always contains the
* querying process, and the reader rejects a table without it.
*/
/** A native reader that answers, but with no snapshot -- an unreadable table, not an empty one. */
function mockUnreadableWindowsProcessTable(): void {
__setWindowsProcessTreeLoaderForTests(() => ({
ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 },
getAllProcesses: (cb: (value: undefined) => void) => cb(undefined)
}))
}
function mockWindowsProcessTable(
rows: { pid: number; ppid: number; name: string; commandLine?: string }[]
): void {
@@ -594,19 +602,79 @@ describe('processHasChildren', () => {
})
})
it('reports no children when the process table is unreadable', async () => {
it('reports an unreadable POSIX table as unverifiable, and still spells it false on the wire', async () => {
await withProcessPlatform('linux', async () => {
mockExecFile(() => new Error('ps table unavailable'))
await expect(inspectPtyChildProcesses(100)).resolves.toBe('unverifiable')
await expect(processHasChildren(100)).resolves.toBe(false)
})
})
})
it('spawns nothing on Windows, where the answer was always false', async () => {
describe('inspectPtyChildProcesses on Windows', () => {
// Why this describe exists: the relay used to `return false` here unconditionally, and a
// hardcoded negative is indistinguishable from a measurement. Every close guard reads it as
// "nothing is running here", so a Windows SSH pane running a build closed with no prompt.
it('walks the process table rather than answering from nothing', async () => {
await withProcessPlatform('win32', async () => {
await expect(processHasChildren(100)).resolves.toBe(false)
mockWindowsProcessTable([
{ pid: 100, ppid: 99, name: 'cmd.exe', commandLine: 'cmd.exe' },
{ pid: 101, ppid: 100, name: 'PING.EXE', commandLine: 'ping -n 40 127.0.0.1' }
])
expect(execFileMock).not.toHaveBeenCalled()
await expect(inspectPtyChildProcesses(100)).resolves.toBe('children')
await expect(processHasChildren(100)).resolves.toBe(true)
})
})
it('finds a grandchild the shell backgrounded, not just direct children', async () => {
await withProcessPlatform('win32', async () => {
mockWindowsProcessTable([
{ pid: 100, ppid: 99, name: 'cmd.exe', commandLine: 'cmd.exe' },
{ pid: 101, ppid: 100, name: 'node.exe', commandLine: 'node build.js' },
{ pid: 102, ppid: 101, name: 'tsc.exe', commandLine: 'tsc --watch' }
])
await expect(inspectPtyChildProcesses(102)).resolves.toBe('no-children')
await expect(inspectPtyChildProcesses(101)).resolves.toBe('children')
})
})
it('separates an observed-empty shell from a table it could not read', async () => {
await withProcessPlatform('win32', async () => {
mockWindowsProcessTable([{ pid: 100, ppid: 99, name: 'cmd.exe', commandLine: 'cmd.exe' }])
await expect(inspectPtyChildProcesses(100)).resolves.toBe('no-children')
resetWindowsProcessRowsSnapshotForTests()
mockUnreadableWindowsProcessTable()
const alive = vi.spyOn(process, 'kill').mockReturnValue(true as never)
try {
await expect(inspectPtyChildProcesses(100)).resolves.toBe('unverifiable')
// The compatibility boolean keeps spelling unverifiable `false`: it reaches clients that
// cannot read the verdict, and they read `true` as "an agent took the PTY, safe to type".
await expect(processHasChildren(100)).resolves.toBe(false)
} finally {
alive.mockRestore()
}
})
})
it('does not read a missing shell as unverifiable when the kernel says it is gone', async () => {
await withProcessPlatform('win32', async () => {
// The root is absent from the snapshot, which on its own cannot distinguish a filtered
// table from an exited shell. Only ESRCH settles it.
mockWindowsProcessTable([{ pid: 900, ppid: 1, name: 'explorer.exe' }])
const gone = vi.spyOn(process, 'kill').mockImplementation(() => {
const error = new Error('no such process') as NodeJS.ErrnoException
error.code = 'ESRCH'
throw error
})
try {
await expect(inspectPtyChildProcesses(100)).resolves.toBe('no-children')
} finally {
gone.mockRestore()
}
})
})
})
src/relay/pty-shell-utils.ts
+1 -41
View File
@@ -11,10 +11,7 @@ import {
import { getFirstCommandToken } from '../shared/command-token-scanner'
import { getProcessTableIndex, type ProcessTableIndex } from '../shared/process-table-index'
import { PS_MAX_BUFFER_BYTES, type ProcessTableRow } from '../shared/process-table-snapshot'
import {
getFreshProcessTableSnapshot,
getProcessTableSnapshot
} from '../shared/process-table-snapshot-reader'
import { getProcessTableSnapshot } from '../shared/process-table-snapshot-reader'
import { selectForegroundProcessCandidate } from '../shared/foreground-process-selection'
import {
resolveOuterWrapperForegroundProcess,
@@ -169,43 +166,6 @@ export async function resolveProcessCwd(pid: number, fallbackCwd: string): Promi
return fallbackCwd
}
/**
* Check whether a process has child processes.
*
* Why the shared snapshot and not `pgrep -P`: this answers one field of
* `pty.inspectProcess`, which every tracked pane polls on a 750ms/2000ms
* cadence, and the fork was neither cached nor coalesced. procps-ng opens six
* procfs files per process to resolve a ppid — including a `/proc/<pid>/ctty`
* that never exists on Linux — so one call cost O(host process count) syscalls,
* ~4k opens per pgrep on a 690-process host, at up to 8 forks/sec (#13537).
* `getForegroundProcessName` in the same RPC already captured the TTL-cached
* `ps` table, whose index carries the parent/child map, so the answer is free.
*
* `fresh` opts out of that TTL. A poll can read a 500ms-old table because its
* next tick corrects it, but a close or cleanup decision acts on the answer
* once and destructively — a child that started inside the TTL would be killed
* with no confirmation. `pgrep` scanned per call, so anything that decides
* has to keep scanning per call.
*/
export async function processHasChildren(
pid: number,
options?: { fresh?: boolean }
): Promise<boolean> {
// Windows has no `ps`; the previous `pgrep` fork always failed here too, so
// this keeps the same answer without spawning anything to reach it.
if (process.platform === 'win32') {
return false
}
try {
const rows = options?.fresh
? await getFreshProcessTableSnapshot()
: await getProcessTableSnapshot()
return (getProcessTableIndex(rows).childrenByPpid.get(pid)?.length ?? 0) > 0
} catch {
return false
}
}
// Why: signal 0 probes existence without delivering a signal. Only ESRCH ("no
// such process") proves the pid is gone; EPERM means it exists but is
// unsignalable, so treat every non-ESRCH outcome as alive. Kept conservative so
src/renderer/src/components/cmd-j/palette-live-status.test.tsx
+4 -11
View File
@@ -451,7 +451,7 @@ describe('palette live status', () => {
expect(dotLabels()).toEqual(['Needs permission'])
})
it('cuts the pip out of the dialog surface, and out of accent when selected', async () => {
it('keeps the attention glyph knockout popover-colored when its row is selected', async () => {
setAgentState('working')
await act(async () => {
testRoot.render(
@@ -469,18 +469,11 @@ describe('palette live status', () => {
</PaletteLiveStatusProvider>
)
})
const pip = testContainer.querySelector<HTMLElement>('[aria-hidden="true"].rounded-full')
const pip = testContainer.querySelector<HTMLElement>('[aria-hidden="true"]')
expect(pip).not.toBeNull()
// Why popover and not background: the CommandDialog surface is --popover (#171717 dark), while
// --background is the app canvas (#0a0a0a) — the mismatch punched a dark halo through each row.
expect(pip?.className).toContain('bg-popover')
expect(pip?.className).toContain('ring-popover')
expect(pip?.className).not.toContain('bg-background')
expect(pip?.className).toContain(
'group-data-[selected=true]:bg-[var(--jump-palette-selection-surface)]'
)
expect(pip?.className).toContain(
'group-data-[selected=true]:ring-[var(--jump-palette-selection-surface)]'
)
expect(pip?.className).toContain('rounded-full')
expect(pip?.className).not.toContain('group-data-[selected=true]')
})
})
src/renderer/src/components/cmd-j/palette-live-status.tsx
+2 -10
View File
@@ -9,7 +9,6 @@ import {
buildExplicitEntriesByTabId,
type TabPaneInputSources
} from '@/components/sidebar/smart-attention'
import { cn } from '@/lib/utils'
import { isExplicitAgentStatusFresh } from '@/lib/agent-status'
import { getLiveAgentStatusByWorktreeId } from '@/lib/worktree-activity-state'
import {
@@ -255,15 +254,8 @@ export function PaletteRecentTabStatusDot({
<span className="relative inline-flex size-3.5 shrink-0 items-center justify-center">
{fallback}
<span
className={cn(
// Why popover, not background: the dialog surface is --popover (#171717 in dark), while
// --background is the app canvas (#0a0a0a) — using it punched a dark halo through every
// dark-mode row. Selected rows use --jump-palette-selection-surface so the cutout tracks
// the stronger keyboard highlight from main.css.
'pointer-events-none absolute -right-0.5 -bottom-0.5 flex items-center justify-center rounded-full',
'bg-popover ring-2 ring-popover',
'group-data-[selected=true]:bg-[var(--jump-palette-selection-surface)] group-data-[selected=true]:ring-[var(--jump-palette-selection-surface)]'
)}
// The popover-colored knockout separates the glyph from its icon without inheriting row selection.
className="pointer-events-none absolute -right-0.5 -bottom-0.5 flex items-center justify-center rounded-full bg-popover ring-2 ring-popover"
aria-hidden="true"
>
<RecentTabAttentionBadgeGlyph badge={badge} />
src/renderer/src/components/tab-bar/TabStripScrollIndicator.test.tsx
+223
View File
@@ -0,0 +1,223 @@
// @vitest-environment happy-dom
import React, { createRef } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { cleanup, fireEvent, render } from '@testing-library/react'
import { TabStripScrollIndicator } from './TabStripScrollIndicator'
import type { TabStripScrollMetrics } from './tab-strip-scroll-metrics'
afterEach(() => {
cleanup()
vi.restoreAllMocks()
})
const OVERFLOW_METRICS: TabStripScrollMetrics = {
hasOverflow: true,
canScrollStart: false,
canScrollEnd: true,
thumbSizeFraction: 0.4,
thumbOffsetFraction: 0
}
const NO_OVERFLOW_METRICS: TabStripScrollMetrics = {
hasOverflow: false,
canScrollStart: false,
canScrollEnd: false,
thumbSizeFraction: 1,
thumbOffsetFraction: 0
}
describe('TabStripScrollIndicator', () => {
it('renders null when there is no overflow', () => {
const { container } = render(<TabStripScrollIndicator metrics={NO_OVERFLOW_METRICS} />)
expect(container.firstChild).toBeNull()
})
it('renders under the tabs with bottom-0 and idle 2px height', () => {
const { getByTestId } = render(<TabStripScrollIndicator metrics={OVERFLOW_METRICS} />)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator).toBeTruthy()
expect(indicator.className).toContain('bottom-0')
expect(indicator.className).toContain('h-[2px]')
expect(indicator.className).toContain('z-[12]')
expect(indicator.className).toContain('opacity-0')
expect(indicator.className).toContain('group-hover/tab-strip:opacity-100')
const thumb = getByTestId('tab-strip-scroll-thumb')
expect(thumb).toBeTruthy()
})
it('expands to 3px and becomes opaque on pointer hover, restores on leave', () => {
const { getByTestId } = render(<TabStripScrollIndicator metrics={OVERFLOW_METRICS} />)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator.className).toContain('h-[2px]')
expect(indicator.className).toContain('opacity-0')
fireEvent.pointerEnter(indicator)
expect(indicator.className).toContain('h-[3px]')
expect(indicator.className).toContain('opacity-100')
fireEvent.pointerLeave(indicator)
expect(indicator.className).toContain('h-[2px]')
expect(indicator.className).toContain('opacity-0')
})
it('applies pointer-events-none when disabled', () => {
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} disabled={true} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
expect(indicator.className).toContain('pointer-events-none')
expect(indicator.className).not.toContain('group-hover/tab-strip:pointer-events-auto')
})
it('stays hidden and unexpanded on hover when disabled', () => {
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} disabled={true} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
fireEvent.pointerEnter(indicator)
expect(indicator.className).toContain('opacity-0')
expect(indicator.className).not.toContain('opacity-100')
expect(indicator.className).toContain('h-[2px]')
})
it('does not forward wheel events when disabled', () => {
const scrollContainer = document.createElement('div')
scrollContainer.scrollLeft = 0
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator
metrics={OVERFLOW_METRICS}
scrollContainerRef={scrollContainerRef}
disabled={true}
/>
)
fireEvent.wheel(getByTestId('tab-strip-scroll-indicator'), { deltaX: 40, deltaY: 0 })
expect(scrollContainer.scrollLeft).toBe(0)
})
it('forwards wheel events to scrollContainer', () => {
const scrollContainer = document.createElement('div')
scrollContainer.scrollLeft = 0
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
fireEvent.wheel(indicator, { deltaX: 40, deltaY: 0 })
expect(scrollContainer.scrollLeft).toBe(40)
})
it('handles track click and smooth scrolls container', () => {
const scrollContainer = document.createElement('div')
Object.defineProperty(scrollContainer, 'scrollWidth', { value: 1000, configurable: true })
Object.defineProperty(scrollContainer, 'clientWidth', { value: 400, configurable: true })
scrollContainer.scrollLeft = 0
const scrollToMock = vi.fn()
scrollContainer.scrollTo = scrollToMock
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
vi.spyOn(indicator, 'getBoundingClientRect').mockReturnValue({
left: 100,
right: 500,
top: 30,
bottom: 33,
width: 400,
height: 3,
x: 100,
y: 30,
toJSON: () => {}
} as DOMRect)
// Click track at clientX = 300 (offset 200 within 400px track)
fireEvent.pointerDown(indicator, { button: 0, clientX: 300 })
expect(scrollToMock).toHaveBeenCalledTimes(1)
expect(scrollToMock).toHaveBeenCalledWith(
expect.objectContaining({
behavior: 'smooth'
})
)
})
it('scrolls container when dragging thumb', () => {
const scrollContainer = document.createElement('div')
Object.defineProperty(scrollContainer, 'scrollWidth', { value: 1000, configurable: true })
Object.defineProperty(scrollContainer, 'clientWidth', { value: 400, configurable: true })
scrollContainer.scrollLeft = 0
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId } = render(
<TabStripScrollIndicator
metrics={{
...OVERFLOW_METRICS,
thumbSizeFraction: 0.4
}}
scrollContainerRef={scrollContainerRef}
/>
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
const thumb = getByTestId('tab-strip-scroll-thumb')
// Start drag on thumb
fireEvent.pointerDown(thumb, { button: 0, clientX: 50 })
expect(indicator.className).toContain('h-[3px]')
// Move pointer by 60px
fireEvent(window, new MouseEvent('pointermove', { clientX: 110 }))
expect(scrollContainer.scrollLeft).toBeGreaterThan(0)
// Release drag
fireEvent(window, new MouseEvent('pointerup'))
expect(indicator.className).toContain('h-[2px]')
})
it('cancels an active thumb drag when it becomes disabled', () => {
const scrollContainer = document.createElement('div')
Object.defineProperty(scrollContainer, 'scrollWidth', { value: 1000, configurable: true })
Object.defineProperty(scrollContainer, 'clientWidth', { value: 400, configurable: true })
scrollContainer.scrollLeft = 0
const scrollContainerRef = createRef<HTMLElement>()
;(scrollContainerRef as React.MutableRefObject<HTMLElement>).current = scrollContainer
const { getByTestId, rerender } = render(
<TabStripScrollIndicator metrics={OVERFLOW_METRICS} scrollContainerRef={scrollContainerRef} />
)
const indicator = getByTestId('tab-strip-scroll-indicator')
Object.defineProperty(indicator, 'clientWidth', { value: 400, configurable: true })
fireEvent.pointerDown(getByTestId('tab-strip-scroll-thumb'), { button: 0, clientX: 50 })
expect(document.body.style.userSelect).toBe('none')
rerender(
<TabStripScrollIndicator
metrics={OVERFLOW_METRICS}
scrollContainerRef={scrollContainerRef}
disabled={true}
/>
)
expect(document.body.style.userSelect).toBe('')
expect(document.body.style.cursor).toBe('')
fireEvent(window, new MouseEvent('pointermove', { clientX: 300 }))
expect(scrollContainer.scrollLeft).toBe(0)
})
})
src/renderer/src/components/tab-bar/TabStripScrollIndicator.tsx
+181 -8
View File
@@ -1,4 +1,4 @@
import React, { useCallback, useLayoutEffect, useRef, useState } from 'react'
import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
import {
computeTabStripThumbLayout,
type TabStripScrollMetrics,
@@ -7,13 +7,24 @@ import {
const EMPTY_THUMB_LAYOUT: TabStripThumbLayout = { widthPx: 0, leftPx: 0 }
export function TabStripScrollIndicator({
metrics
}: {
export type TabStripScrollIndicatorProps = {
metrics: TabStripScrollMetrics
}): React.JSX.Element | null {
scrollContainerRef?: React.RefObject<HTMLElement | null>
disabled?: boolean
}
export function TabStripScrollIndicator({
metrics,
scrollContainerRef,
disabled = false
}: TabStripScrollIndicatorProps): React.JSX.Element | null {
const trackRef = useRef<HTMLDivElement>(null)
const cleanupDragRef = useRef<(() => void) | null>(null)
const [thumbLayout, setThumbLayout] = useState<TabStripThumbLayout>(EMPTY_THUMB_LAYOUT)
const [isHovered, setIsHovered] = useState(false)
const [isDragging, setIsDragging] = useState(false)
const [isScrolling, setIsScrolling] = useState(false)
const scrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
const remeasureThumb = useCallback((): void => {
const track = trackRef.current
@@ -37,23 +48,185 @@ export function TabStripScrollIndicator({
return () => resizeObserver.disconnect()
}, [remeasureThumb])
useEffect(() => {
const scrollContainer = scrollContainerRef?.current
if (!scrollContainer) {
return
}
const handleScroll = (): void => {
setIsScrolling(true)
if (scrollTimeoutRef.current) {
clearTimeout(scrollTimeoutRef.current)
}
scrollTimeoutRef.current = setTimeout(() => {
setIsScrolling(false)
}, 800)
}
scrollContainer.addEventListener('scroll', handleScroll, { passive: true })
return () => {
scrollContainer.removeEventListener('scroll', handleScroll)
if (scrollTimeoutRef.current) {
clearTimeout(scrollTimeoutRef.current)
}
}
}, [scrollContainerRef])
useEffect(() => {
return () => {
cleanupDragRef.current?.()
}
}, [])
// Why: hiding the indicator mid-drag would otherwise leave window listeners and body cursor/user-select stuck.
useEffect(() => {
if (disabled || !metrics.hasOverflow) {
cleanupDragRef.current?.()
}
}, [disabled, metrics.hasOverflow])
const handleThumbPointerDown = (e: React.PointerEvent<HTMLDivElement>): void => {
if (e.button !== 0 || disabled) {
return
}
e.preventDefault()
e.stopPropagation()
const scrollContainer = scrollContainerRef?.current
const track = trackRef.current
if (!scrollContainer || !track) {
return
}
const startX = e.clientX
const startScrollLeft = scrollContainer.scrollLeft
const trackWidth = track.clientWidth
const maxLeft = Math.max(1, trackWidth - thumbLayout.widthPx)
const maxScrollLeft = Math.max(0, scrollContainer.scrollWidth - scrollContainer.clientWidth)
if (maxScrollLeft <= 0 || maxLeft <= 0) {
return
}
setIsDragging(true)
const prevUserSelect = document.body.style.userSelect
const prevCursor = document.body.style.cursor
document.body.style.userSelect = 'none'
document.body.style.cursor = 'grabbing'
const onPointerMove = (moveEvent: PointerEvent): void => {
const deltaX = moveEvent.clientX - startX
const scrollDelta = (deltaX / maxLeft) * maxScrollLeft
scrollContainer.scrollLeft = Math.max(
0,
Math.min(maxScrollLeft, startScrollLeft + scrollDelta)
)
}
const cleanup = (): void => {
setIsDragging(false)
document.body.style.userSelect = prevUserSelect
document.body.style.cursor = prevCursor
window.removeEventListener('pointermove', onPointerMove)
window.removeEventListener('pointerup', cleanup)
window.removeEventListener('pointercancel', cleanup)
cleanupDragRef.current = null
}
cleanupDragRef.current = cleanup
window.addEventListener('pointermove', onPointerMove)
window.addEventListener('pointerup', cleanup)
window.addEventListener('pointercancel', cleanup)
}
const handleTrackPointerDown = (e: React.PointerEvent<HTMLDivElement>): void => {
if (e.button !== 0 || disabled) {
return
}
if (e.target !== trackRef.current) {
return
}
e.preventDefault()
e.stopPropagation()
const scrollContainer = scrollContainerRef?.current
const track = trackRef.current
if (!scrollContainer || !track) {
return
}
const trackRect = track.getBoundingClientRect()
const clickX = e.clientX - trackRect.left
const trackWidth = track.clientWidth
const thumbWidth = thumbLayout.widthPx
const maxLeft = Math.max(1, trackWidth - thumbWidth)
const maxScrollLeft = Math.max(0, scrollContainer.scrollWidth - scrollContainer.clientWidth)
if (maxScrollLeft <= 0 || maxLeft <= 0) {
return
}
const targetThumbLeft = Math.max(0, Math.min(maxLeft, clickX - thumbWidth / 2))
const targetScrollLeft = (targetThumbLeft / maxLeft) * maxScrollLeft
scrollContainer.scrollTo({
left: targetScrollLeft,
behavior: 'smooth'
})
}
const handleWheel = (e: React.WheelEvent<HTMLDivElement>): void => {
const scrollContainer = scrollContainerRef?.current
if (!scrollContainer || disabled) {
return
}
// Why: forward wheel events to tab container so scrolling over the indicator scrolls the strip.
const delta = Math.abs(e.deltaX) > Math.abs(e.deltaY) ? e.deltaX : e.deltaY
scrollContainer.scrollLeft += delta
}
if (!metrics.hasOverflow) {
return null
}
// Why: during a tab drag the indicator fully yields — no reveal, no pointer/wheel interaction.
const isExpanded = !disabled && (isHovered || isDragging)
const isVisible = !disabled && (isHovered || isDragging || isScrolling)
return (
// Why: top rail keeps the active tab's bottom underline unobstructed.
// Why: under-tab position matches editor tab scrollbar conventions, auto-hiding when idle so it never mimics an underline.
<div
ref={trackRef}
className="pointer-events-none absolute inset-x-0 top-0 z-[1] h-px bg-muted-foreground/10"
data-testid="tab-strip-scroll-indicator"
className={`absolute inset-x-0 bottom-0 z-[12] select-none transition-[height,background-color,opacity] duration-150 ease-out ${
isExpanded ? 'h-[3px] bg-muted-foreground/10 cursor-pointer' : 'h-[2px] bg-transparent'
} ${
disabled
? 'opacity-0 pointer-events-none'
: isVisible
? 'opacity-100 pointer-events-auto'
: 'opacity-0 group-hover/tab-strip:opacity-100 pointer-events-none group-hover/tab-strip:pointer-events-auto'
}`}
onPointerEnter={() => setIsHovered(true)}
onPointerLeave={() => setIsHovered(false)}
onPointerDown={handleTrackPointerDown}
onWheel={handleWheel}
// Why: decorative rail — keyboard/AT users scroll the strip with the adjacent labelled scroll buttons.
aria-hidden
>
<div
className="absolute top-0 h-full rounded-full bg-muted-foreground/40 transition-[left,width] duration-75 ease-out"
data-testid="tab-strip-scroll-thumb"
className={`absolute bottom-0 h-full rounded-full transition-colors duration-150 ease-out ${
isDragging
? 'bg-foreground/40 cursor-grabbing'
: isHovered
? 'bg-muted-foreground/50 cursor-grab'
: 'bg-muted-foreground/25 cursor-default'
}`}
style={{
width: `${thumbLayout.widthPx}px`,
left: `${thumbLayout.leftPx}px`
}}
onPointerDown={handleThumbPointerDown}
/>
</div>
)
src/renderer/src/components/tab-bar/drop-indicator.ts
+1 -1
View File
@@ -26,7 +26,7 @@ export function getDropIndicatorClasses(dropIndicator: DropIndicator): string {
// flips the strip between "fits exactly" and "overflows by 1px", which jitters
// every tab by 1px because the browser preserves scrollLeft near the end.
export const ACTIVE_TAB_INDICATOR_CLASSES =
'pointer-events-none absolute inset-x-0 bottom-0 h-[2px] bg-[color-mix(in_srgb,var(--foreground)_60%,var(--card))] z-10'
'pointer-events-none absolute inset-x-0 bottom-0 h-[2px] bg-[color-mix(in_srgb,var(--foreground)_60%,var(--card))] z-20'
export function getTabRootStateClasses(isActive: boolean): string {
return isActive
src/renderer/src/components/tab-bar/tab-bar-surface.tsx
+6 -2
View File
@@ -157,7 +157,7 @@ export function renderTabBarSurface({
<SortableContext items={sortableIds}>
{/* Why: no-drag lets tab interactions work inside the titlebar's drag region (outer container stays window-draggable). */}
<div
className="relative flex min-h-0 min-w-0 max-w-full flex-[0_1_auto]"
className="group/tab-strip relative flex min-h-0 min-w-0 max-w-full flex-[0_1_auto]"
style={{ WebkitAppRegion: 'no-drag' } as React.CSSProperties}
>
<div
@@ -181,7 +181,11 @@ export function renderTabBarSurface({
/>
) : null}
</div>
<TabStripScrollIndicator metrics={tabStripOverflowState} />
<TabStripScrollIndicator
metrics={tabStripOverflowState}
scrollContainerRef={tabStripRef}
disabled={tabStripDragScroll.isTabDragActive}
/>
</div>
</SortableContext>
{tabStripOverflowState.hasOverflow ? (
src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts
+6 -4
View File
@@ -5,7 +5,7 @@ import { makePaneKey } from '../../../../shared/stable-pane-id'
import { closeWebRuntimeTerminal } from '@/runtime/web-runtime-session'
import { resolveLeafCloseCopyKind } from '../terminal/terminal-close-copy-kind'
import { RUNNING_CLOSE_PROBE_TIMEOUT_MS } from '../terminal/running-terminal-close-guard'
import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection'
import { probePtyRunningWork } from '../terminal/pty-running-work-probe'
import {
detachTerminalPaneToTab,
isTerminalTabStripDropTarget,
@@ -99,12 +99,14 @@ export function useTerminalPaneCloseActions(controller: TerminalPaneBindingContr
copyKind: getCloseDialogCopyKind(paneId)
})
const probeTimeout = setTimeout(() => decide(confirmClose), RUNNING_CLOSE_PROBE_TIMEOUT_MS)
void inspectRuntimeTerminalProcess(settings, ptyId)
.then((process) => {
// Why the shared probe rather than a direct inspect: this is the same question the tab-close
// guard asks, and the two must not drift on what an unanswered host means.
void probePtyRunningWork(settings, [ptyId], { timeoutMs: RUNNING_CLOSE_PROBE_TIMEOUT_MS })
.then((probes) => {
clearTimeout(probeTimeout)
decide(() => {
if (
!process.hasChildProcesses ||
probes[0]?.verdict !== 'live' ||
settings?.skipCloseTerminalWithRunningProcessConfirm
) {
executeClosePane(paneId)
src/renderer/src/components/terminal/pty-running-work-probe-child-evidence.test.ts
+94
View File
@@ -0,0 +1,94 @@
// The probe is the one place an inspection becomes a verdict, so it is the one place that has to
// know `hasChildProcesses` cannot hold the third answer. A host that could not read its own process
// table spells that the same way as one that read it and found nothing; Windows relays spelled it
// `false` unconditionally, which read here as `exited` -- an idle verdict for a pane nobody looked at.
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { inspectRuntimeTerminalProcessMock } = vi.hoisted(() => ({
inspectRuntimeTerminalProcessMock: vi.fn()
}))
vi.mock('@/runtime/runtime-terminal-inspection', () => ({
inspectRuntimeTerminalProcess: inspectRuntimeTerminalProcessMock
}))
import { probePtyRunningWork } from './pty-running-work-probe'
const SETTINGS = { activeRuntimeEnvironmentId: null }
describe('probePtyRunningWork child-process evidence', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it('asks the host to pay for a scan, because these probes back destructive decisions', async () => {
inspectRuntimeTerminalProcessMock.mockResolvedValue({
foregroundProcess: 'cmd.exe',
hasChildProcesses: false,
childProcessEvidence: 'no-children'
})
await probePtyRunningWork(SETTINGS, ['pty-a'], { timeoutMs: 1000 })
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledWith(SETTINGS, 'pty-a', {
scanChildProcesses: true
})
})
it('reports a host that could not observe the pane as unverifiable, not exited', async () => {
inspectRuntimeTerminalProcessMock.mockResolvedValue({
foregroundProcess: 'cmd.exe',
hasChildProcesses: false,
childProcessEvidence: 'unverifiable'
})
const [probe] = await probePtyRunningWork(SETTINGS, ['pty-a'], { timeoutMs: 1000 })
expect(probe).toMatchObject({
verdict: 'unverifiable',
reason: 'host_child_processes_unobserved',
timedOut: false
})
})
it('reports an observed-empty pane as exited', async () => {
inspectRuntimeTerminalProcessMock.mockResolvedValue({
foregroundProcess: 'cmd.exe',
hasChildProcesses: false,
childProcessEvidence: 'no-children'
})
const [probe] = await probePtyRunningWork(SETTINGS, ['pty-a'], { timeoutMs: 1000 })
expect(probe?.verdict).toBe('exited')
})
it('reports an observed child as live', async () => {
inspectRuntimeTerminalProcessMock.mockResolvedValue({
foregroundProcess: 'PING.EXE',
hasChildProcesses: true,
childProcessEvidence: 'children'
})
const [probe] = await probePtyRunningWork(SETTINGS, ['pty-a'], { timeoutMs: 1000 })
expect(probe?.verdict).toBe('live')
})
it('keeps the boolean meaning for a host that never published the verdict', async () => {
inspectRuntimeTerminalProcessMock.mockResolvedValueOnce({
foregroundProcess: 'node',
hasChildProcesses: true
})
inspectRuntimeTerminalProcessMock.mockResolvedValueOnce({
foregroundProcess: 'bash',
hasChildProcesses: false
})
const [live] = await probePtyRunningWork(SETTINGS, ['pty-a'], { timeoutMs: 1000 })
const [idle] = await probePtyRunningWork(SETTINGS, ['pty-b'], { timeoutMs: 1000 })
expect(live?.verdict).toBe('live')
expect(idle?.verdict).toBe('exited')
})
})
src/renderer/src/components/terminal/pty-running-work-probe.ts
+19 -2
View File
@@ -54,14 +54,31 @@ export async function probePtyRunningWork(
return
}
try {
const inspection = await inspectRuntimeTerminalProcess(settings, ptyId)
// Why the flag: this probe backs decisions that act once and destructively, so it is worth
// a host process-table read on platforms where the child question costs one. The polled
// inspections deliberately do not ask for it.
const inspection = await inspectRuntimeTerminalProcess(settings, ptyId, {
scanChildProcesses: true
})
probe.timedOut = false
if (isClientOnlyUnverifiableInspection(inspection)) {
probe.verdict = 'unverifiable'
probe.reason = inspection.reason
return
}
probe.verdict = inspection.hasChildProcesses ? 'live' : 'exited'
// `hasChildProcesses` cannot hold the third answer: a host that could not read its own
// process table spells that the same way as one that read it and found nothing. Windows
// relays spelled it `false` unconditionally, which read here as `exited`.
if (inspection.childProcessEvidence === 'unverifiable') {
probe.verdict = 'unverifiable'
probe.reason = 'host_child_processes_unobserved'
return
}
probe.verdict =
(inspection.childProcessEvidence ??
(inspection.hasChildProcesses ? 'children' : 'no-children')) === 'children'
? 'live'
: 'exited'
delete probe.reason
} catch {
// Why: `inspectRuntimeTerminalProcess` already maps every failure it can classify onto a
src/renderer/src/components/terminal/running-terminal-close-guard.test.ts
+3 -1
View File
@@ -111,7 +111,9 @@ describe('guardRunningTerminalClose', () => {
guard(onClose)
await settleProbe()
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledWith(expect.anything(), 'pty-a')
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledWith(expect.anything(), 'pty-a', {
scanChildProcesses: true
})
expect(onClose).not.toHaveBeenCalled()
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1' })
})
src/renderer/src/components/terminal/terminal-close-confirm-keyboard-vs-mouse.test.ts
+5 -2
View File
@@ -5,7 +5,7 @@
*
* Mouse close path: SortableTab (X onClick / onAuxClick button===1) -> onClose
* -> Terminal.tsx handleCloseTab -> closeTerminalTab() -> running-process guard.
* Keyboard path: Cmd+W -> TerminalPane.handleRequestClosePane -> inspectRuntimeTerminalProcess
* Keyboard path: Cmd+W -> TerminalPane.handleRequestClosePane -> probePtyRunningWork
* (split panes) or closeTerminalTab's guard (last pane) -> CloseTerminalDialog.
*/
import { readFileSync } from 'node:fs'
@@ -99,8 +99,11 @@ describe('#10142 close confirmation policy is the same for keyboard and mouse',
'utf8'
)
const handler = source.slice(source.indexOf('const handleRequestClosePane'))
// The shared probe, not a direct inspect: the pane path asks the same question as the tab
// guard, and routing both through one measurement is what stops them drifting on what an
// unanswered host means.
expect(handler.slice(0, handler.indexOf('useImperativeHandle'))).toContain(
'inspectRuntimeTerminalProcess'
'probePtyRunningWork'
)
})
src/renderer/src/runtime/__fixtures__/web-session-terminal-orphan-recovery-regression-fixtures.ts
+3 -1
View File
@@ -96,7 +96,9 @@ export function listResult(
totalCount: terminals.length,
truncated: options.truncated ?? false,
...(options.hostScope === undefined
? { hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] } }
? // A host names the execution hosts it covered, not its own environment id: answering
// `terminal.list` on a paired runtime reports `local` (verified against a live runtime).
{ hostScope: { hostIds: ['local'], omittedHostIds: [] } }
: { hostScope: options.hostScope })
}
}
src/renderer/src/runtime/host-live-terminal-probe.ts
+6 -3
View File
@@ -1,5 +1,6 @@
import type { RuntimeTerminalListResult } from '../../../shared/runtime-types'
import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope'
import { hostScopeCensusIsComplete } from '../../../shared/runtime-listing-host-scope'
/**
* Asks the host whether ANY terminal is live in an environment, for the one
@@ -74,10 +75,12 @@ async function probeHost(
if (response.ok === false || !isTerminalListResult(response.result)) {
return 'unverifiable'
}
// An omitted execution host is an incomplete census. In particular, a relay
// can list its local PTYs while an SSH child host is still starting up.
// A host this listing owed coverage for and did not deliver leaves an incomplete census — a
// relay can list its local PTYs while an SSH child host is still starting up. A peer runtime
// is not such a host: it answers `--environment` for itself, and reading its disclosure entry
// as a gap latched this probe forever (#18595).
const hostScope = response.result.hostScope
if (hostScope && hostScope.omittedHostIds.length > 0) {
if (hostScope && !hostScopeCensusIsComplete(hostScope)) {
return 'unverifiable'
}
const { terminals, totalCount } = response.result
src/renderer/src/runtime/host-session-mirror-empty-inventory-settle.test.ts
+83
View File
@@ -335,6 +335,89 @@ describe('empty host inventory settling the session mirror', () => {
expect(hasHostSessionMirrorHydrated(environmentId, WORKTREE)).toBe(true)
})
// #18595, the reporter's shape: the answering host holds a mirrored row for a peer runtime, so
// that peer is named in `omittedHostIds` for every listing it will ever give. Reading the
// disclosure list as the gate made this probe permanently `unverifiable`, the mirror never
// hydrated, and every remote pane parked forever.
it('settles on a peer runtime it disclosed but never owed coverage for', async () => {
const result = await probeHostLiveTerminals(
'env-peer-disclosed',
vi.fn(async () => ({
id: 'peer-disclosed',
ok: true as const,
result: {
terminals: [],
totalCount: 0,
truncated: false,
hostScope: { hostIds: ['local'], omittedHostIds: ['runtime:env-peer'] }
},
_meta: { runtimeId: 'runtime' }
}))
)
expect(result).toBe('none')
})
it('reports a live terminal through a peer-runtime disclosure', async () => {
const result = await probeHostLiveTerminals(
'env-peer-live',
vi.fn(async () => ({
id: 'peer-live',
ok: true as const,
result: {
terminals: [{ handle: 'terminal-1' }],
totalCount: 1,
truncated: false,
hostScope: { hostIds: ['local'], omittedHostIds: ['runtime:env-peer'] }
},
_meta: { runtimeId: 'runtime' }
}))
)
expect(result).toBe('live')
})
it('stays unverifiable for an SSH host the answering runtime did owe coverage for', async () => {
const result = await probeHostLiveTerminals(
'env-ssh-gap',
vi.fn(async () => ({
id: 'ssh-gap',
ok: true as const,
result: {
terminals: [],
totalCount: 0,
truncated: false,
hostScope: { hostIds: ['local'], omittedHostIds: ['ssh:box-1'] }
},
_meta: { runtimeId: 'runtime' }
}))
)
expect(result).toBe('unverifiable')
})
// A worktree-scoped listing whose target is a runtime host covers nothing and names `local`
// among its omissions. `local` is what keeps this unverifiable — the point is that the
// runtime-only rule does not rescue a listing that answered for nothing.
it('stays unverifiable when the listing covered no host at all', async () => {
const result = await probeHostLiveTerminals(
'env-covered-nothing',
vi.fn(async () => ({
id: 'covered-nothing',
ok: true as const,
result: {
terminals: [],
totalCount: 0,
truncated: false,
hostScope: { hostIds: [], omittedHostIds: ['local', 'runtime:env-peer'] }
},
_meta: { runtimeId: 'runtime' }
}))
)
expect(result).toBe('unverifiable')
})
it('rejects a present malformed host scope as unverifiable', async () => {
const result = await probeHostLiveTerminals(
'env-malformed-scope',
src/renderer/src/runtime/runtime-terminal-inspection.test.ts
+52
View File
@@ -146,6 +146,58 @@ describe('runtime terminal owner routing', () => {
expect(localHasChildren).not.toHaveBeenCalled()
})
// Why these exist: the close guards ask the host to pay for a real child-process read, and the
// environment path used to drop the option before it reached the wire. The host then declined to
// scan and answered `unverifiable`, which the guard reads as running work -- a confirmation
// dialog on every idle close of a remote Windows pane. Found by review on #18591.
it('forwards scanChildProcesses to the PTY owning environment', async () => {
await inspectRuntimeTerminalProcess(
{ activeRuntimeEnvironmentId: 'env-2' },
'remote:env-1@@terminal-1',
{ scanChildProcesses: true }
)
expect(runtimeCall).toHaveBeenCalledWith({
selector: 'env-1',
method: 'terminal.inspectProcess',
params: { terminal: 'terminal-1', scanChildProcesses: true },
timeoutMs: 15_000
})
})
it('forwards scanChildProcesses alongside the incarnation fence', async () => {
await inspectRuntimeTerminalProcess(
{ activeRuntimeEnvironmentId: 'env-2' },
'remote:env-1@@terminal-1',
{ expectedIncarnationId: 'incarnation-1', scanChildProcesses: true }
)
expect(runtimeCall).toHaveBeenCalledWith({
selector: 'env-1',
method: 'terminal.inspectProcess',
params: {
terminal: 'terminal-1',
expectedIncarnationId: 'incarnation-1',
scanChildProcesses: true
},
timeoutMs: 15_000
})
})
it('omits scanChildProcesses when the caller is only polling', async () => {
await inspectRuntimeTerminalProcess(
{ activeRuntimeEnvironmentId: 'env-2' },
'remote:env-1@@terminal-1'
)
expect(runtimeCall).toHaveBeenCalledWith({
selector: 'env-1',
method: 'terminal.inspectProcess',
params: { terminal: 'terminal-1' },
timeoutMs: 15_000
})
})
it('maps an old host inspection to client-only unverifiable', async () => {
runtimeCall.mockResolvedValue({
ok: true,
src/renderer/src/runtime/runtime-terminal-inspection.ts
+7 -3
View File
@@ -138,7 +138,7 @@ export function recordRuntimeTerminalInputForPtyId(ptyId: string, timestamp = Da
export async function inspectRuntimeTerminalProcess(
settings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null | undefined,
ptyId: string,
options?: { expectedIncarnationId?: string }
options?: { expectedIncarnationId?: string; scanChildProcesses?: boolean }
): Promise<RuntimeTerminalProcessInspection> {
const ownerEnvironmentId = getRemoteRuntimePtyEnvironmentId(ptyId)
const target = ownerEnvironmentId
@@ -148,7 +148,7 @@ export async function inspectRuntimeTerminalProcess(
const remote = isRemoteInspectionPtyId(ptyId)
if (target.kind !== 'environment' || !terminal) {
try {
const result = await (options?.expectedIncarnationId
const result = await (options
? window.api.pty.inspectProcess(ptyId, options)
: window.api.pty.inspectProcess(ptyId))
return normalizeInspectionResult(result, remote)
@@ -169,7 +169,11 @@ export async function inspectRuntimeTerminalProcess(
terminal,
...(options?.expectedIncarnationId
? { expectedIncarnationId: options.expectedIncarnationId }
: {})
: {}),
// Why forwarded: the close guards pass this so the host pays for a real child-process read.
// Dropped here, the host declines to scan and answers `unverifiable`, which the guard reads
// as running work -- a confirmation dialog on every idle close of a remote Windows pane.
...(options?.scanChildProcesses === true ? { scanChildProcesses: true } : {})
},
{ timeoutMs: 15_000 }
)
src/renderer/src/runtime/web-session-terminal-orphan-recovery-host-scope-gate.test.ts
+98
View File
@@ -0,0 +1,98 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import {
ENVIRONMENT_ID,
listResult,
makeSnapshot,
makeState
} from './__fixtures__/web-session-terminal-orphan-recovery-regression-fixtures'
import {
clearWebSessionTerminalOrphanRecoveryForTests,
recoverWebSessionTerminalOrphansBeforeApply
} from './web-session-terminal-orphan-recovery'
/**
* The second consumer of `hostScopeCensusIsComplete`. Pruning here needs two consecutive
* authoritative inventories that omit the surface, so an incomplete census must hold the binding
* open indefinitely — and a peer runtime named in `omittedHostIds` must not be read as one, or
* the ghost binding is retained forever (#18595).
*/
const LEAVES = [{ leafId: 'leaf-1', handle: 'term-ghost' }]
/**
* `listResult` substitutes a default scope when handed `undefined`, so the absent-scope case has
* to drop the key itself — passing `undefined` through the fixture silently tests the default.
*/
async function recoverTwice(worktree: string, hostScope: Record<string, unknown> | null) {
const state = makeState(worktree, LEAVES)
const call = vi.fn(async ({ method }: { method: string }) => {
if (method === 'terminal.list') {
const listed = listResult(worktree, [])
if (hostScope === null) {
delete (listed as { hostScope?: unknown }).hostScope
} else {
listed.hostScope = hostScope as never
}
return { ok: true as const, result: listed }
}
return { ok: false as const, error: { code: 'conflict', message: 'unexpected' } }
})
await recoverWebSessionTerminalOrphansBeforeApply(
state,
makeSnapshot(worktree, 'epoch-1', LEAVES),
ENVIRONMENT_ID,
{ call: call as never }
)
return recoverWebSessionTerminalOrphansBeforeApply(
state,
makeSnapshot(worktree, 'epoch-2', LEAVES),
ENVIRONMENT_ID,
{ call: call as never }
)
}
describe('orphan recovery reads the host scope as coverage owed, not as disclosure', () => {
beforeEach(() => {
clearWebSessionTerminalOrphanRecoveryForTests()
})
it('prunes a twice-absent binding when the only omission is a peer runtime', async () => {
const settled = await recoverTwice('repo::peer-disclosed', {
hostIds: ['local'],
omittedHostIds: ['runtime:env-peer']
})
expect(settled?.tabs).toEqual([])
})
it('retains the binding when an SSH host this runtime does query went unanswered', async () => {
const settled = await recoverTwice('repo::ssh-gap', {
hostIds: ['local'],
omittedHostIds: ['ssh:box-1']
})
expect(settled?.tabs).toEqual([
expect.objectContaining({ leafId: 'leaf-1', terminal: 'term-ghost' })
])
})
it('retains the binding when the listing covered no host at all', async () => {
const settled = await recoverTwice('repo::covered-nothing', {
hostIds: [],
omittedHostIds: ['local', 'runtime:env-peer']
})
expect(settled?.tabs).toEqual([
expect.objectContaining({ leafId: 'leaf-1', terminal: 'term-ghost' })
])
})
// Why: a host predating `hostScope` (v1.4.187) cannot say what it covered, and absence of the
// claim is never the claim.
it('retains the binding when the host is too old to publish a scope', async () => {
const settled = await recoverTwice('repo::no-scope', null)
expect(settled?.tabs).toEqual([
expect.objectContaining({ leafId: 'leaf-1', terminal: 'term-ghost' })
])
})
})
src/renderer/src/runtime/web-session-terminal-orphan-recovery-inventory.ts
+4 -3
View File
@@ -18,6 +18,7 @@ import {
type RecoverySurface
} from './web-session-terminal-orphan-recovery-surface'
import { runInTerminalRecoveryRpcLane } from './web-session-terminal-orphan-recovery-rpc-lane'
import { hostScopeCensusIsComplete } from '../../../shared/runtime-listing-host-scope'
type RuntimeCall = (args: {
selector: string
@@ -163,9 +164,9 @@ export async function resolveTerminalOrphanInventory(args: {
listedByHandle.set(terminal.handle, terminal)
}
}
// Older hosts omit hostScope entirely; an unscoped absence cannot prove a PTY exited.
const hostScopeUnverifiable =
listed.hostScope === undefined || listed.hostScope.omittedHostIds.length > 0
// Older hosts omit hostScope entirely; an unscoped absence cannot prove a PTY exited. A peer
// runtime named in `omittedHostIds` is disclosure rather than a gap this host owed (#18595).
const hostScopeUnverifiable = !hostScopeCensusIsComplete(listed.hostScope)
const dispositions = new Map<string, RecoveryDisposition>()
const claims: RuntimeTerminalOrphanAdoptionClaim[] = []
for (const surface of inventorySurfaces) {
src/renderer/src/runtime/web-session-terminal-pending-handle-recovery.test.ts
+5 -5
View File
@@ -145,7 +145,7 @@ describe('web session pending terminal handle recovery', () => {
topologyRevisions: { [WORKTREE_ID]: 4 },
totalCount: 1,
truncated: false,
hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] }
hostScope: { hostIds: ['local'], omittedHostIds: [] }
}
}
}
@@ -316,7 +316,7 @@ describe('web session pending terminal handle recovery', () => {
topologyRevisions: { [WORKTREE_ID]: 4 },
totalCount: 1,
truncated: false,
hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] }
hostScope: { hostIds: ['local'], omittedHostIds: [] }
}
}
}
@@ -394,7 +394,7 @@ describe('web session pending terminal handle recovery', () => {
topologyRevisions: { [WORKTREE_ID]: 4 },
totalCount: 1,
truncated: false,
hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] }
hostScope: { hostIds: ['local'], omittedHostIds: [] }
}
}
}
@@ -459,7 +459,7 @@ describe('web session pending terminal handle recovery', () => {
terminals: [],
totalCount: 0,
truncated: false,
hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] }
hostScope: { hostIds: ['local'], omittedHostIds: [] }
}
}))
@@ -655,7 +655,7 @@ describe('web session pending terminal handle recovery', () => {
terminals: [],
totalCount: 0,
truncated: false,
hostScope: { hostIds: [ENVIRONMENT_ID], omittedHostIds: [] }
hostScope: { hostIds: ['local'], omittedHostIds: [] }
}
}))
src/shared/execution-host.test.ts
+13
View File
@@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
import {
ALL_EXECUTION_HOSTS_SCOPE,
LOCAL_EXECUTION_HOST_ID,
getExecutionHostLabel,
getLocalExecutionHostLabel,
getRepoExecutionHostId,
getRepoSshConnectionId,
@@ -169,4 +170,16 @@ describe('execution host id delimiter invariant', () => {
targetId: 'a|b'
})
})
// "All hosts" is the everything-scope. Answering with it for an id that names no host shows one
// unroutable row as though it were on every host, which is the opposite of what it is.
it('labels an id that names no host as one unknown host, not as every host', () => {
for (const id of ['ssh:', 'ssh:a|b', 'ssh:%zz', 'runtime:', 'quantum:box'] as const) {
expect(getExecutionHostLabel(id as never)).toBe('Unknown host')
}
expect(getExecutionHostLabel(null)).toBe('Unknown host')
expect(getExecutionHostLabel(ALL_EXECUTION_HOSTS_SCOPE)).toBe('All hosts')
expect(getExecutionHostLabel('ssh:box')).toBe('box')
expect(getExecutionHostLabel('runtime:env-1')).toBe('env-1')
})
})
src/shared/execution-host.ts
+7 -2
View File
@@ -226,13 +226,18 @@ export function getSettingsFocusedExecutionHostId(
: LOCAL_EXECUTION_HOST_ID
}
export function getExecutionHostLabel(id: ExecutionHostScope): string {
export function getExecutionHostLabel(id: ExecutionHostScope | null | undefined): string {
if (id === ALL_EXECUTION_HOSTS_SCOPE) {
return 'All hosts'
}
const parsed = parseExecutionHostId(id)
if (!parsed) {
return 'All hosts'
// Not "All hosts": an id that names no host is one *unknown* host, and answering with the
// everything-scope label shows an unroutable row as though it were on every host.
// Plain English like every other label in this module (`Local Mac`, `This computer`,
// `All hosts`) — none of them resolve through the renderer's i18n catalog, so a lone
// translated string here would read inconsistently.
return 'Unknown host'
}
switch (parsed.kind) {
case 'local':
src/shared/folder-workspace-execution-host.test.ts
+188
View File
@@ -174,6 +174,194 @@ describe('folder workspace execution host', () => {
expect(resolveFolderWorkspaceHost(state({ repos: [] }), 'fw-1')).toEqual({ kind: 'local' })
})
// SSH ownership has two spellings on a repo row. A row carrying only `executionHostId: 'ssh:*'`
// has no `connectionId`, and reading the raw field counted it as a local repo — so a workspace
// whose files live on an SSH host resolved `local`, which is an execute-here answer for a remote
// path. These fire on well-formed rows; nothing malformed is involved.
it('resolves a repo that names its SSH host only through executionHostId', () => {
const resolved = resolveFolderWorkspaceHost(
state({
repos: [
repo({
id: 'repo-1',
path: '/work/app/a',
projectGroupId: 'group-1',
executionHostId: 'ssh:box'
})
]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'ssh', targetId: 'box' })
})
it('mixes such a repo with a local one as ambiguous rather than local', () => {
const resolved = resolveFolderWorkspaceHost(
state({
repos: [
repo({ id: 'repo-1', path: '/work/app/a', projectGroupId: 'group-1' }),
repo({
id: 'repo-2',
path: '/work/app/b',
projectGroupId: 'group-1',
executionHostId: 'ssh:box'
})
]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'ambiguous' })
})
it('matches a scope connection against such a repo instead of calling it ambiguous', () => {
const resolved = resolveFolderWorkspaceHost(
state({
folderWorkspaces: [workspace({ connectionId: 'box' })],
repos: [
repo({
id: 'repo-1',
path: '/work/app/a',
projectGroupId: 'group-1',
executionHostId: 'ssh:box'
})
]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'ssh', targetId: 'box' })
})
it('reads the target off the host, so a percent-encoded id decodes', () => {
const resolved = resolveFolderWorkspaceHost(
state({
repos: [
repo({
id: 'repo-1',
path: '/work/app/a',
projectGroupId: 'group-1',
executionHostId: `ssh:${encodeURIComponent('box 1')}`
})
]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'ssh', targetId: 'box 1' })
})
// Deliberately unchanged: a `runtime:` row's nested SSH target is not this client's to dial, but
// narrowing that here would be a second behaviour change riding on the SSH fix.
it('leaves a runtime row contributing its nested connection exactly as before', () => {
const resolved = resolveFolderWorkspaceHost(
state({
repos: [
repo({
id: 'repo-1',
path: '/work/app/a',
projectGroupId: 'group-1',
executionHostId: 'runtime:env-1',
connectionId: 'nested-box'
})
]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'ssh', targetId: 'nested-box' })
})
it('still answers local for a runtime pin, which the type cannot express otherwise', () => {
const resolved = resolveFolderWorkspaceHost(
state({
folderWorkspaces: [workspace({ executionHostId: 'runtime:env-1' })]
}),
'fw-1'
)
expect(resolved).toEqual({ kind: 'local' })
})
// The candidate FILTER decides which rows reach the resolver, and it read `repo.connectionId` raw
// too — so an SSH-only repo outside the project-group subtree was dropped before any of the above
// could classify it. Every test before this one uses a repo inside the subtree, which is never
// filtered, so none of them could have caught it (found in review by CodeRabbit).
describe('a repo matched only by path, outside the project-group subtree', () => {
const sshOnlyPathRepo = repo({
id: 'repo-path',
path: '/work/app/nested',
executionHostId: 'ssh:box'
})
it('survives the scope-connection filter instead of being dropped as connectionless', () => {
const scoped = state({
folderWorkspaces: [workspace({ connectionId: 'box' })],
repos: [sshOnlyPathRepo]
})
expect(findFolderWorkspaceCandidateRepos(scoped, 'fw-1')).toEqual([sshOnlyPathRepo])
expect(resolveFolderWorkspaceHost(scoped, 'fw-1')).toEqual({ kind: 'ssh', targetId: 'box' })
})
// Pins the resolver, not the filter: under the old raw read BOTH rows came back connectionless,
// so they matched each other by accident and this case survived the filter either way. The
// legacy-vs-unified pairing below is the one that discriminates.
it('survives the group-connection filter when the group is on that same SSH host', () => {
const scoped = state({
repos: [
repo({
id: 'repo-group',
path: '/work/app/group',
projectGroupId: 'group-1',
executionHostId: 'ssh:box'
}),
sshOnlyPathRepo
]
})
expect(findFolderWorkspaceCandidateRepos(scoped, 'fw-1')).toHaveLength(2)
expect(resolveFolderWorkspaceHost(scoped, 'fw-1')).toEqual({ kind: 'ssh', targetId: 'box' })
})
// Both sides of the group comparison are resolved, so the legacy spelling on one side and the
// unified spelling on the other still match.
it('matches a legacy-spelled group repo against a unified-spelled path repo', () => {
const scoped = state({
repos: [
repo({
id: 'repo-group',
path: '/work/app/group',
projectGroupId: 'group-1',
connectionId: 'box'
}),
sshOnlyPathRepo
]
})
expect(findFolderWorkspaceCandidateRepos(scoped, 'fw-1')).toHaveLength(2)
expect(resolveFolderWorkspaceHost(scoped, 'fw-1')).toEqual({ kind: 'ssh', targetId: 'box' })
})
// A `runtime:` row's nested target is still read from the raw field, so it matches a scope
// connection exactly as it does today. Pinned so the carve-out stays a decision.
it('leaves a runtime row matching the scope connection through its nested target', () => {
const runtimePathRepo = repo({
id: 'repo-path',
path: '/work/app/nested',
executionHostId: 'runtime:env-1',
connectionId: 'box'
})
const scoped = state({
folderWorkspaces: [workspace({ connectionId: 'box' })],
repos: [runtimePathRepo]
})
expect(findFolderWorkspaceCandidateRepos(scoped, 'fw-1')).toEqual([runtimePathRepo])
})
})
it('reads each repository membership once while collecting candidates', () => {
let membershipReads = 0
const repos = Array.from({ length: 32 }, (_, index) => {
src/shared/folder-workspace-execution-host.ts
+31 -7
View File
@@ -17,7 +17,7 @@ import type { ProjectGroup } from './project-group-types'
import type { Repo } from './repo-types'
import { isPathInsideOrEqual } from './cross-platform-path'
import { getProjectGroupSubtreeIds } from './project-groups'
import { parseExecutionHostId } from './execution-host'
import { getRepoExecutionHostId, parseExecutionHostId } from './execution-host'
export type FolderWorkspaceHostState = {
folderWorkspaces: readonly FolderWorkspace[]
@@ -36,6 +36,24 @@ export function normalizeConnectionId(value: string | null | undefined): string
return value?.trim() || null
}
/**
* The SSH target whose filesystem holds this repo's files, or `null` for anything else.
*
* SSH ownership has two spellings on a repo row — the legacy `connectionId` field and the unified
* `executionHostId` — so reading the raw field sees only one of them and a row carrying only
* `executionHostId: 'ssh:<target>'` reads as if it had no connection at all. Every comparison in
* this file goes through here: the candidate filters decide which rows reach the resolver, so
* reading raw in either place drops the row before the resolver can classify it.
*
* A non-SSH host falls back to the raw field so a `runtime:` row keeps contributing its nested
* target exactly as it does today. That target is not this client's to dial, but changing it is a
* separate defect with its own reasoning — see the note in `resolveFolderWorkspaceHost`.
*/
function getRepoScopeConnectionId(repo: Repo): string | null {
const host = parseExecutionHostId(getRepoExecutionHostId(repo))
return host?.kind === 'ssh' ? host.targetId : normalizeConnectionId(repo.connectionId)
}
function getFolderScopeCandidateRepos(args: {
folderPath: string
projectGroupId: string
@@ -59,18 +77,18 @@ function getFolderScopeCandidateRepos(args: {
if (args.connectionId) {
return [
...groupRepos,
...pathRepos.filter((repo) => normalizeConnectionId(repo.connectionId) === args.connectionId)
...pathRepos.filter((repo) => getRepoScopeConnectionId(repo) === args.connectionId)
]
}
if (groupRepos.length === 0) {
return pathRepos
}
const groupConnectionIds = new Set(
groupRepos.map((repo) => normalizeConnectionId(repo.connectionId))
)
// Both sides resolved: comparing a resolved path repo against a raw group read would reintroduce
// the same mismatch from the other direction.
const groupConnectionIds = new Set(groupRepos.map(getRepoScopeConnectionId))
return [
...groupRepos,
...pathRepos.filter((repo) => groupConnectionIds.has(normalizeConnectionId(repo.connectionId)))
...pathRepos.filter((repo) => groupConnectionIds.has(getRepoScopeConnectionId(repo)))
]
}
@@ -102,6 +120,12 @@ export function resolveFolderWorkspaceHost(
}
const explicitHost = parseExecutionHostId(workspace.executionHostId)
if (explicitHost) {
// A `runtime:` workspace deliberately answers `local`, and `FolderWorkspaceHost` has no runtime
// variant to answer with instead. That omission is known: a runtime environment's own server
// normalizes its work to `local`, and the nested SSH target on such a row is addressable only as
// the pair (environmentId, targetId) — handing it to this client's SSH table would dial a
// same-named box in the wrong namespace. Widening the type is its own change, not an oversight
// here.
return explicitHost.kind === 'ssh'
? { kind: 'ssh', targetId: explicitHost.targetId }
: { kind: 'local' }
@@ -114,7 +138,7 @@ export function resolveFolderWorkspaceHost(
let hasLocalRepo = false
const connectionIds = new Set<string>()
for (const repo of candidateRepos) {
const connectionId = normalizeConnectionId(repo.connectionId)
const connectionId = getRepoScopeConnectionId(repo)
if (connectionId) {
connectionIds.add(connectionId)
} else {
src/shared/relay-artifacts.ts
+4
View File
@@ -57,6 +57,10 @@ export const RELAY_ARTIFACTS: readonly RelayArtifact[] = [
// title request with no title and no error.
{ filename: 'wsl-transcript-fs-process-entry.js' },
{ filename: 'node-pty-1.1.0-console-list-agent-patch.cjs', windowsOnly: true },
// The ConPTY teardown release the desktop's own node-pty patch already carries; pnpm patches do
// not cross the SSH boundary, so a relay ran the unpatched npm tree and leaked one Windows File
// handle per terminal for the life of the relay process.
{ filename: 'node-pty-1.1.0-windows-pty-teardown-patch.cjs', windowsOnly: true },
// Only Linux relays run it, but it ships everywhere: the manifest's only
// platform axis is Windows, and a second one would buy nothing but a fork in
// the hash. Its presence is what moves a host to a fresh relay directory, and
src/shared/runtime-listing-host-scope.test.ts
+81
View File
@@ -0,0 +1,81 @@
import { describe, expect, it } from 'vitest'
import { hostScopeCensusIsComplete } from './runtime-listing-host-scope'
/**
* The gate and the disclosure list answer different questions off the same field. These pin the
* cases where they disagree — which is every case that mattered in #18595.
*/
describe('hostScopeCensusIsComplete', () => {
it('reads a peer runtime as disclosure, not as coverage this host owed', () => {
expect(
hostScopeCensusIsComplete({ hostIds: ['local'], omittedHostIds: ['runtime:env-7'] })
).toBe(true)
})
it('still reads an SSH host as a gap, because this runtime does query those', () => {
expect(hostScopeCensusIsComplete({ hostIds: ['local'], omittedHostIds: ['ssh:box-1'] })).toBe(
false
)
})
it('reads a mixed omission as incomplete on the strength of the SSH host alone', () => {
expect(
hostScopeCensusIsComplete({
hostIds: ['local'],
omittedHostIds: ['runtime:env-7', 'ssh:box-1']
})
).toBe(false)
})
it('calls a clean census complete', () => {
expect(hostScopeCensusIsComplete({ hostIds: ['local', 'ssh:box-1'], omittedHostIds: [] })).toBe(
true
)
})
// Defence in depth: `listKnownExecutionHostIds` always seeds `local`, so a real scope that
// covered nothing also omits `local` and is refused by the runtime-only rule anyway. This
// branch is what stops a scope that answered for no host from ever reading complete if that
// ever stops holding.
it('refuses a listing that covered no host at all', () => {
expect(hostScopeCensusIsComplete({ hostIds: [], omittedHostIds: ['runtime:env-7'] })).toBe(
false
)
expect(hostScopeCensusIsComplete({ hostIds: [], omittedHostIds: [] })).toBe(false)
})
// Why: `hostScope` shipped in v1.4.187. An older host cannot say what it covered, and absence
// of the claim is never the claim — this must stay the first branch.
it('refuses a host too old to publish a scope', () => {
expect(hostScopeCensusIsComplete(undefined)).toBe(false)
})
// Why: without this the runtime-only rule trusts a coverage claim it cannot read — the shape
// `{hostIds: ['runtime:'], omittedHostIds: ['runtime:env-7']}` was `unverifiable` before the
// gate changed and must not become `complete` on the strength of an unparseable id.
it('refuses a coverage claim with no legible host in it', () => {
expect(
hostScopeCensusIsComplete({
hostIds: ['runtime:' as never],
omittedHostIds: ['runtime:env-7']
})
).toBe(false)
})
// Why "at least one legible" and not "all legible": a host that later gains a kind this client
// cannot parse must not report an incomplete census forever — that is this bug in a new coat.
it('accepts a coverage claim carrying one legible host beside an unreadable one', () => {
expect(
hostScopeCensusIsComplete({ hostIds: ['local', 'newkind:x' as never], omittedHostIds: [] })
).toBe(true)
})
it('refuses an unparseable host id rather than discounting it', () => {
expect(
hostScopeCensusIsComplete({
hostIds: ['local'],
omittedHostIds: ['runtime:' as never]
})
).toBe(false)
})
})
src/shared/runtime-listing-host-scope.ts
+30 -1
View File
@@ -1,4 +1,4 @@
import type { ExecutionHostId } from './execution-host'
import { parseExecutionHostId, type ExecutionHostId } from './execution-host'
/**
* What a bounded listing did and did not cover, by execution host. An absent scope means the
@@ -10,3 +10,32 @@ export type RuntimeListingHostScope = {
hostIds: ExecutionHostId[]
omittedHostIds: ExecutionHostId[]
}
/**
* Whether the answering runtime enumerated every host its listing owed coverage for.
*
* `omittedHostIds` is a disclosure list and deliberately over-names — `omitted-host-scope-selectors.ts`
* keeps ids for servers that are no longer paired so a caller can still see the gap. That makes it the
* wrong input for a completeness gate, which needs "coverage owed and not delivered". The two jobs pull
* in opposite directions, and reading the disclosure list as the gate latched every remote pane on any
* client that had ever paired outward (#18595).
*
* A `runtime:` host is never owed coverage by the runtime answering: a paired runtime is a peer with its
* own control plane, reached with `--environment`, and this runtime has no paired-runtime PTY provider to
* have queried. Its terminals are its own answer to give, so its presence here is disclosure, not a gap.
*/
export function hostScopeCensusIsComplete(scope: RuntimeListingHostScope | undefined): boolean {
// A host too old to publish a scope cannot claim one; absence is never completeness.
if (scope === undefined) {
return false
}
// A listing that covered no host proves nothing, and an unreadable coverage claim is not a
// claim: `isTerminalListResult` checks only that `hostIds` is an array, so at least one covered
// id has to be legible before the claim can be believed. Deliberately "at least one" rather than
// "all": a host that later gains a kind this client cannot parse would otherwise report an
// incomplete census forever, which is the bug this predicate exists to stop.
if (!scope.hostIds.some((hostId) => parseExecutionHostId(hostId))) {
return false
}
return scope.omittedHostIds.every((hostId) => parseExecutionHostId(hostId)?.kind === 'runtime')
}
src/shared/terminal-process-inspection.ts
+13
View File
@@ -1,5 +1,15 @@
import type { RemoteForegroundEvidence } from './foreground-process-evidence'
/**
* What the execution host observed about processes running under a PTY's shell.
*
* Separate from `hasChildProcesses` because a boolean cannot hold the third answer. The host that
* could not read its own process table and the host that read it and found nothing both had to
* spell themselves `false`, and every close guard reads `false` as "nothing is running here".
* Windows relays spelled it `false` unconditionally.
*/
export type PtyChildProcessVerdict = 'children' | 'no-children' | 'unverifiable'
/** Reasons the renderer could not observe the execution host. */
export type ClientOnlyUnverifiableReason =
| 'transport_loss'
@@ -17,6 +27,7 @@ export type ClientOnlyUnverifiableInspection = {
verdict: 'unverifiable'
reason: string
foregroundProcessEvidence?: never
childProcessEvidence?: never
authorityGeneration?: never
observationEpoch?: never
capturedAgeMs?: never
@@ -30,6 +41,8 @@ export type HostProcessInspection = {
hasChildProcesses: boolean
/** Optional on old hosts; the renderer treats an omitted field as old-host unverifiable. */
foregroundProcessEvidence?: RemoteForegroundEvidence
/** Absent on hosts that predate the member, and on answers the host did not pay to observe. */
childProcessEvidence?: PtyChildProcessVerdict
verdict?: never
reason?: never
}
src/shared/worktree-execution-host-resolution.test.ts
+34
View File
@@ -156,6 +156,40 @@ describe('resolveWorktreeExecutionHost', () => {
it('reports an unknown owner distinctly from a conflicting one', () => {
expect(resolve([], { repoId: 'r' })).toEqual({ kind: 'unresolved', reason: 'unknown' })
})
// `unknown` is a verdict the launch path disposes of as a plain local folder, so a row that
// declared a host and named an unparseable one must not share the word — it has to fail closed.
it('reports a row naming an unparseable host distinctly from an unknown one', () => {
for (const executionHostId of ['ssh:', 'ssh:a|b', 'ssh:%zz', 'runtime:', 'quantum:box']) {
expect(resolve([{ id: 'r', executionHostId }], { repoId: 'r' })).toEqual({
kind: 'unresolved',
reason: 'malformed'
})
}
})
it('does not recover a host from the connectionId such a row overrode', () => {
expect(
resolve([{ id: 'r', executionHostId: 'ssh:a|b', connectionId: 'openclaw' }], {
repoId: 'r'
})
).toEqual({ kind: 'unresolved', reason: 'malformed' })
})
it('still resolves every row that names a parseable host', () => {
expect(resolve([{ id: 'r', executionHostId: 'ssh:box' }], { repoId: 'r' })).toMatchObject({
kind: 'resolved',
hostId: 'ssh:box'
})
expect(resolve([{ id: 'r', connectionId: 'box' }], { repoId: 'r' })).toMatchObject({
kind: 'resolved',
hostId: 'ssh:box'
})
expect(resolve([{ id: 'r' }], { repoId: 'r' })).toMatchObject({
kind: 'resolved',
hostId: 'local'
})
})
})
it('ignores an unparseable host id rather than treating it as a host', () => {
src/shared/worktree-execution-host-resolution.ts
+33 -5
View File
@@ -54,7 +54,29 @@ export type WorktreeExecutionHostResolution<T extends ExecutionHostOwnerRow> =
/** Display metadata only. The decisions are `hostId` / `connectionId`. */
owner: T | null
}
| { kind: 'unresolved'; reason: 'ambiguous' | 'unknown' }
/**
* Three reasons, not two, and deliberately not collapsed. `unknown` (nothing carries the id) is a
* verdict the launch path may legitimately dispose of as a plain local folder; `malformed` (the
* row named a host that cannot be parsed) must fail closed. A vocabulary that cannot express the
* difference guarantees it is lost at the first caller that switches on it — the same shape as
* #18006, where one word had to stand for two liveness situations.
*/
| { kind: 'unresolved'; reason: 'ambiguous' | 'unknown' | 'malformed' }
/**
* The owner row's host, or `null` when the row names one that cannot be parsed.
*
* Module-private and deliberately not a second exported reading of a repo row: only this resolution
* needs the distinction, because only this resolution is routing. `getRepoExecutionHostId` stays the
* answer everywhere else — its fall-through to `local` is harmless for the grouping, label and index
* callers that make up nearly all of its ~340 call sites, and is wrong only when the value decides
* where work runs.
*/
function resolveOwnerRowHostId(row: ExecutionHostOwnerRow): ExecutionHostId | null {
return row.executionHostId?.trim()
? normalizeExecutionHostId(row.executionHostId)
: getRepoExecutionHostId(row)
}
export function resolveWorktreeExecutionHost<T extends ExecutionHostOwnerRow>(
lookup: ExecutionHostOwnerLookup<T>,
@@ -80,9 +102,13 @@ export function resolveWorktreeExecutionHost<T extends ExecutionHostOwnerRow>(
if (match.kind !== 'resolved') {
return { kind: 'unresolved', reason: match.kind === 'ambiguous' ? 'ambiguous' : 'unknown' }
}
const hostId = resolveOwnerRowHostId(match.owner)
if (!hostId) {
return { kind: 'unresolved', reason: 'malformed' }
}
return {
kind: 'resolved',
hostId: getRepoExecutionHostId(match.owner),
hostId,
connectionId: getRepoSshConnectionId(match.owner),
owner: match.owner
}
@@ -115,12 +141,14 @@ export function createRepoRowExecutionHostLookup<T extends ExecutionHostOwnerRow
if (!owner) {
return { kind: 'missing' }
}
const ownerHostId = getRepoExecutionHostId(owner)
return rows.some((repo) => getRepoExecutionHostId(repo) !== ownerHostId)
const ownerHostId = resolveOwnerRowHostId(owner)
return rows.some((repo) => resolveOwnerRowHostId(repo) !== ownerHostId)
? { kind: 'ambiguous' }
: { kind: 'resolved', owner }
},
// A row naming an unparseable host matches no host, which is what stops a worktree on a real
// host from adopting it.
byHost: (repoId, hostId) =>
rowsFor(repoId).find((repo) => getRepoExecutionHostId(repo) === hostId) ?? null
rowsFor(repoId).find((repo) => resolveOwnerRowHostId(repo) === hostId) ?? null
}
}