Skip to content

Commit d30449b

Browse files
committed
fix(restore): address recursive gatekeeper findings
Preserve inactive workspace identities, selected drafts, attachments, and late prompt hydration so Electron and Tauri restore every tab instead of only the globally active one. Fence renderer and message hydration races, harden cross-host stale-owner recovery, serialize shutdown after state capture, and make Windows process cleanup use consistent identities and refreshed descendant trees. Distinguish explicit workspace deletion from transient stops, prevent request ownership reactivation, and retire migrated legacy state without shadowing future envelopes. Add PR behavioral checks and regression coverage across UI persistence, Electron lifecycle/process cleanup, server workspace shutdown, and Tauri client state. Validated with 93 Electron tests, 53 focused UI tests, 23 server tests, 65 Rust tests, all TypeScript typechecks, cargo check, targeted rustfmt, and the production UI build.
1 parent 6396958 commit d30449b

36 files changed

Lines changed: 575 additions & 208 deletions

.github/workflows/pr-build.yml

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,49 @@ jobs:
5656
actions_artifacts_retention_days: 7
5757
actions_artifacts_name_prefix: pr-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }}-
5858
set_versions: false
59+
60+
tests:
61+
needs: authorize
62+
if: ${{ needs.authorize.outputs.allowed == 'true' && !github.event.pull_request.draft }}
63+
runs-on: ubuntu-latest
64+
steps:
65+
- name: Checkout
66+
uses: actions/checkout@v4
67+
with:
68+
ref: ${{ github.event.pull_request.head.sha }}
69+
70+
- name: Setup Node
71+
uses: actions/setup-node@v4
72+
with:
73+
node-version: 22
74+
cache: npm
75+
76+
- name: Setup Rust
77+
uses: dtolnay/rust-toolchain@stable
78+
79+
- name: Install dependencies
80+
run: npm ci
81+
82+
- name: Typecheck desktop clients
83+
run: npm run typecheck
84+
85+
- name: Test Electron client state
86+
run: npm run test:native --workspace @neuralnomads/codenomad-electron-app
87+
88+
- name: Test UI restoration authority
89+
run: >-
90+
node --import tsx --test
91+
packages/ui/src/stores/client-state-codec.test.ts
92+
packages/ui/src/stores/app-session-snapshot-merge.test.ts
93+
packages/ui/src/stores/message-v2/message-hydration-authority.test.ts
94+
95+
- name: Test server lifecycle
96+
run: >-
97+
node --import tsx --test
98+
packages/server/src/index.test.ts
99+
packages/server/src/shutdown.test.ts
100+
packages/server/src/workspaces/manager.test.ts
101+
102+
- name: Test Tauri client state
103+
working-directory: packages/tauri-app/src-tauri
104+
run: cargo test --locked client_state

packages/electron-app/electron/main/client-state-cross-host.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ test("stale retirement crash cannot retire a successor", async (t) => {
9898
assert.equal(JSON.parse(readFileSync(ownerFile(directory), "utf8")).runToken, "successor")
9999
})
100100

101-
test("stale recovery is identity guarded and blocked by live or uncertain participants", (t) => {
101+
test("stale recovery is identity guarded and blocked by non-claiming live participants", (t) => {
102102
for (const value of [
103103
{ alive: false, identity: undefined, recover: true },
104104
{ alive: true, identity: "reused", recover: true },
@@ -149,7 +149,7 @@ test("ordinary release removes only its participant", (t) => {
149149
assert.deepEqual(readdirSync(directory).filter((name) => name.startsWith("participant.") || name.startsWith("retired.")), [])
150150
})
151151

152-
test("primary crash remains fenced by its live secondary cohort", async (t) => {
152+
test("primary crash remains fenced by its non-claiming secondary cohort", async (t) => {
153153
const directory = temp(t), firstStart = join(directory, "first-start")
154154
const primary = child(directory, firstStart), secondary = child(directory, firstStart)
155155
writeFileSync(firstStart, "")

packages/electron-app/electron/main/client-state-cross-host.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,13 @@ function recoveryClaimants(
181181
try { unlinkSync(recoveryPath(directory, participant)) } catch {}
182182
continue
183183
}
184-
if (readIfExists(recoveryPath(directory, participant)) !== observedOwner) return undefined
184+
const claimPath = recoveryPath(directory, participant)
185+
let claim = readIfExists(claimPath)
186+
for (let attempt = 0; claim !== observedOwner && attempt < 20; attempt += 1) {
187+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5)
188+
claim = readIfExists(claimPath)
189+
}
190+
if (claim !== observedOwner) return undefined
185191
claimants.push(participant)
186192
}
187193
return claimants

packages/electron-app/electron/main/client-state-ipc.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,24 @@ export function setupClientStateIPC(
3939
}
4040
const handle = (
4141
channel: string,
42-
operation: (argument: unknown) => unknown,
42+
operation: (argument: unknown, token: unknown) => unknown,
4343
) => ipcMain.handle(channel, async (event, token: unknown, argument: unknown) => {
4444
validate(event)
4545
clientState.assertRendererAccessToken(token)
46-
return operation(argument)
46+
return operation(argument, token)
4747
})
4848

4949
ipcMain.handle("client-state:claimAccess", async (event, token: unknown) => {
5050
validate(event)
5151
return clientState.claimClientStateAccess(token)
5252
})
5353
handle("client-state:load", () => clientState.loadClientState())
54-
handle("client-state:save", (snapshot) => clientState.saveClientState(snapshot))
55-
handle("client-state:setRestoreEnabled", (enabled) => {
54+
handle("client-state:save", (snapshot, token) => clientState.saveClientState(snapshot, token))
55+
handle("client-state:setRestoreEnabled", (enabled, token) => {
5656
if (typeof enabled !== "boolean") throw new Error("Restore enabled must be a boolean")
57-
return clientState.setRestoreEnabled(enabled)
57+
return clientState.setRestoreEnabled(enabled, token)
5858
})
59-
handle("client-state:clear", () => clientState.clearClientState())
59+
handle("client-state:clear", (_argument, token) => clientState.clearClientState(token))
6060

6161
return (window: BrowserWindow): void => {
6262
window.webContents.on("did-navigate", (_event, url) => {

packages/electron-app/electron/main/client-state-lifecycle.test.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,18 @@ test("late old-window detach preserves replacement tracker during shutdown", asy
6464
h.lifecycle.detachMainWindow(h.window)
6565
h.appEvents.get("before-quit")?.({ preventDefault: () => {} })
6666
await (h.lifecycle as any).shutdown
67-
assert.deepEqual(h.calls, ["hide", "renderer", "stop", "replacement-native", "release"])
67+
assert.deepEqual(h.calls, ["hide", "renderer", "replacement-native", "stop", "release"])
6868
})
6969

70-
test("Windows session end starts cleanup without vetoing or explicitly exiting", async () => {
70+
test("Windows session end vetoes termination until cleanup exits explicitly", async () => {
7171
const h = harness()
7272
let prevented = false
7373
h.windows.get("query-session-end")?.({ preventDefault: () => { prevented = true } })
7474
h.windows.get("session-end")?.()
7575
await (h.lifecycle as any).sessionEnd; await tick()
76-
assert.equal(prevented, false)
77-
assert.deepEqual(h.calls, ["renderer", "stop", "native", "release"])
78-
assert.equal(h.exits(), 0)
76+
assert.equal(prevented, true)
77+
assert.deepEqual(h.calls, ["renderer", "native", "stop", "release"])
78+
assert.equal(h.exits(), 1)
7979
})
8080

8181
test("session end does not force-exit an already-hung ordinary shutdown", async () => {
@@ -84,8 +84,8 @@ test("session end does not force-exit an already-hung ordinary shutdown", async
8484
h.appEvents.get("before-quit")?.({ preventDefault: () => {} })
8585
h.windows.get("query-session-end")?.({ preventDefault: () => { prevented = true } })
8686
await tick()
87-
assert.equal(prevented, false)
88-
assert.deepEqual(h.calls, ["hide", "renderer", "stop"])
87+
assert.equal(prevented, true)
88+
assert.deepEqual(h.calls, ["hide", "renderer"])
8989
assert.equal(h.exits(), 0)
9090
})
9191

@@ -94,11 +94,11 @@ test("ordinary quit hides promptly and waits for CLI stop confirmation", async (
9494
const h = harness({ stop: () => new Promise<void>((resolve) => { confirmStop = resolve }) })
9595
h.appEvents.get("before-quit")?.({ preventDefault: () => {} })
9696
await tick()
97-
assert.deepEqual(h.calls, ["hide", "renderer", "stop", "native"])
97+
assert.deepEqual(h.calls, ["hide", "renderer", "native", "stop"])
9898
assert.equal(h.exits(), 0)
9999
confirmStop()
100100
await (h.lifecycle as any).shutdown; await tick()
101-
assert.deepEqual(h.calls, ["hide", "renderer", "stop", "native", "release"])
101+
assert.deepEqual(h.calls, ["hide", "renderer", "native", "stop", "release"])
102102
assert.equal(h.exits(), 1)
103103
})
104104

@@ -108,15 +108,18 @@ test("ordinary quit does not exit when CLI cleanup is unconfirmed", async () =>
108108
await assert.rejects((h.lifecycle as any).shutdown, /unconfirmed/)
109109
await tick()
110110
assert.equal(h.exits(), 0)
111-
assert.deepEqual(h.calls, ["hide", "renderer", "stop", "native"])
111+
assert.deepEqual(h.calls, ["hide", "renderer", "native", "stop"])
112112
})
113113

114-
test("CLI termination starts before a hung native flush", async () => {
115-
const h = harness({ nativeFlush: () => new Promise(() => {}) })
114+
test("CLI termination waits for the native snapshot flush", async () => {
115+
let release!: () => void
116+
const h = harness({ nativeFlush: () => new Promise<void>((resolve) => { release = resolve }) })
116117
h.appEvents.get("before-quit")?.({ preventDefault: () => {} })
117118
await tick()
118-
assert.deepEqual(h.calls, ["hide", "renderer", "stop", "native"])
119+
assert.deepEqual(h.calls, ["hide", "renderer", "native"])
119120
assert.equal(h.exits(), 0)
121+
release(); await (h.lifecycle as any).shutdown
122+
assert.deepEqual(h.calls, ["hide", "renderer", "native", "stop", "release"])
120123
})
121124

122125
test("closing the final window hides it before requesting quit", () => {

packages/electron-app/electron/main/client-state-lifecycle.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ export class ClientStateLifecycle {
5858
})
5959

6060
if (this.dependencies.isWindows ?? process.platform === "win32") {
61-
window.on("query-session-end", () => {
61+
window.on("query-session-end", (event) => {
6262
if (this.exitAllowed) return
63+
event.preventDefault()
6364
this.promoteToSessionEnd(window)
6465
})
6566
window.on("session-end", () => this.promoteToSessionEnd(window))
@@ -93,15 +94,15 @@ export class ClientStateLifecycle {
9394
private startShutdown(window: BrowserWindow | null): Promise<void> {
9495
if (this.shutdown) return this.shutdown
9596
const stages = (async () => {
96-
const rendererFlush = this.runStage("renderer shutdown flush", () => this.flushRenderer(window))
97-
const cliStop = this.dependencies.cliManager.shutdown().then(() => null, (error: unknown) => error)
98-
await rendererFlush
97+
await this.runStage("renderer shutdown flush", () => this.flushRenderer(window))
9998
await this.runStage("native shutdown flush", () => this.flushNative())
100-
const cliError = await cliStop
101-
if (cliError) throw cliError
99+
await this.dependencies.cliManager.shutdown()
102100
await this.runStage("primary release", () => this.dependencies.clientStateManager.drainAndReleasePrimary())
103101
})()
104-
this.shutdown = stages
102+
this.shutdown = stages.catch((error) => {
103+
this.shutdown = null
104+
throw error
105+
})
105106
return this.shutdown
106107
}
107108

@@ -114,7 +115,8 @@ export class ClientStateLifecycle {
114115
private promoteToSessionEnd(window: BrowserWindow): void {
115116
if (this.exitAllowed || this.sessionEnd) return
116117
this.sessionEnd = this.startShutdown(window)
117-
void this.sessionEnd.catch((error) => {
118+
void this.sessionEnd.then(() => this.exit()).catch((error) => {
119+
this.sessionEnd = null
118120
console.warn("[client-state] OS session-end cleanup was not contained before termination", error)
119121
})
120122
}

packages/electron-app/electron/main/client-state-process-identity.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function getProcessStartIdentity(pid: number): string | undefined {
5151
"-NoProfile",
5252
"-NonInteractive",
5353
"-Command",
54-
`(Get-Process -Id ${pid} -ErrorAction Stop).StartTime.ToUniversalTime().Ticks`,
54+
`(Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}" -ErrorAction Stop).CreationDate.ToUniversalTime().Ticks`,
5555
],
5656
"win32",
5757
)

packages/electron-app/electron/main/client-state.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ test("first shared primary deterministically migrates legacy host envelopes", as
9494
const manager = new ClientStateManager(electron, undefined, { crossHostElectionDirectory: election, legacyTauriDataPath: tauri })
9595
assert.deepEqual(manager.loadClientState().snapshot, { savedAt: 20, host: "tauri" })
9696
assert.equal(manager.getWindowState(), undefined)
97-
assert.equal(existsSync(join(electron, "client-state.json")), true)
98-
assert.equal(existsSync(join(tauri, "client-state.json")), true)
97+
assert.equal(existsSync(join(electron, "client-state.json")), false)
98+
assert.equal(existsSync(join(tauri, "client-state.json")), false)
9999
await manager.drainAndReleasePrimary()
100100
})
101101

0 commit comments

Comments
 (0)