fix: expire stale session running state#262
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🧰 Additional context used📓 Path-based instructions (1)packages/app/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Files:
🧠 Learnings (21)📓 Common learnings📚 Learning: 2026-04-23T15:26:07.250ZApplied to files:
📚 Learning: 2026-04-26T16:34:54.895ZApplied to files:
📚 Learning: 2026-04-25T12:52:47.074ZApplied to files:
📚 Learning: 2026-04-23T07:23:23.849ZApplied to files:
📚 Learning: 2026-04-23T15:10:21.635ZApplied to files:
📚 Learning: 2026-04-24T03:51:54.050ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-23T15:25:31.118ZApplied to files:
📚 Learning: 2026-04-20T14:21:56.373ZApplied to files:
📚 Learning: 2026-04-20T14:36:21.288ZApplied to files:
📚 Learning: 2026-04-20T14:36:21.288ZApplied to files:
📚 Learning: 2026-04-25T12:52:32.462ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-22T09:32:54.556ZApplied to files:
📚 Learning: 2026-04-22T08:49:47.800ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughIntroduces a reactive Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant createSessionRunning
participant Memo as Memoization
participant Effect as createEffect
participant Timer as setTimeout
participant Fallback as runningFallbackExpiresAt
Component->>createSessionRunning: request working() accessor
createSessionRunning->>Memo: create memoized boolean accessor
createSessionRunning->>Effect: start effect watching status/messages
Effect->>Fallback: compute expiry from status/messages
Fallback-->>Effect: return expiry timestamp or undefined
alt expiry present and in future
Effect->>Timer: schedule timeout at expiry
Timer-->>Effect: tick -> recompute
Effect-->>Memo: update memo (false)
Memo-->>Component: working() updated
else no expiry or already expired
Effect-->>Memo: update memo immediately
Memo-->>Component: working() updated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a more robust mechanism for determining if a session is running by implementing a time-based fallback for incomplete assistant messages. It adds a createSessionRunning primitive for reactive state management and updates various components to use this new logic. Feedback includes a suggestion to increase the timer buffer in setTimeout to prevent race conditions and a recommendation to use more defensive optional chaining when accessing message timestamps.
Astro-Han
left a comment
There was a problem hiding this comment.
鸡蛋里挑骨头的 review。整体方向是对的,30s 时间窗 fallback 解决 #252 的根因(stale 永远 truthy)。下面按 P-level 列了 1 个语义不一致 + 几个测试与可维护性的小坑。
- P1: 1 处(
pendingmemo 没跟着升级,stale 仍会被高亮) - P2: 2 处(
createSessionRunning的 reactive timer 没单测;busy()非当前 session 不会 reactive 过期) - P3: 4 处 (constants/naming/test magic/permissions gating 重复)
d975474 to
bc759bd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/app/src/pages/session/session-running-state.test.ts (1)
87-91: 🧹 Nitpick | 🔵 TrivialMissing reactive timer test for
createSessionRunning.The pure function tests are thorough, but
createSessionRunning's timer-driven reactivity—the core behavioral change in this PR—lacks test coverage. If someone accidentally removes thesetTickcall or timer logic, these tests will still pass.Consider adding a test using
createRoot+vi.useFakeTimers()that verifies:
- Fresh assistant message →
running()returnstrue- Advancing time past 30s →
running()flips tofalse- Cleanup disposes timer without dangling callbacks
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session/session-running-state.test.ts` around lines 87 - 91, Add a reactive unit test for createSessionRunning using createRoot and vi.useFakeTimers(): within a createRoot scope call createSessionRunning(...) to obtain running() and ensure initially running() === true after feeding a fresh assistant message, then vi.advanceTimersByTime(30_001) and assert running() becomes false, and finally dispose the root and verify no pending timers/callbacks remain (e.g., advance timers further and ensure no errors or state changes). Make sure the test specifically exercises the setTick/timer logic inside createSessionRunning, uses fake timers (vi.useFakeTimers()) and cleans up the createRoot to confirm timer disposal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/app/src/pages/session/session-running-state.test.ts`:
- Around line 87-91: Add a reactive unit test for createSessionRunning using
createRoot and vi.useFakeTimers(): within a createRoot scope call
createSessionRunning(...) to obtain running() and ensure initially running() ===
true after feeding a fresh assistant message, then
vi.advanceTimersByTime(30_001) and assert running() becomes false, and finally
dispose the root and verify no pending timers/callbacks remain (e.g., advance
timers further and ensure no errors or state changes). Make sure the test
specifically exercises the setTick/timer logic inside createSessionRunning, uses
fake timers (vi.useFakeTimers()) and cleans up the createRoot to confirm timer
disposal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a6499a88-f3d6-4a15-a06b-b1a9999b8de1
📒 Files selected for processing (5)
packages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/session/session-running-state.test.tspackages/app/src/pages/session/session-running-state.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-desktop
- GitHub Check: typecheck
- GitHub Check: unit-opencode
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (1)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.tspackages/app/src/pages/session/session-running-state.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/e2e/sidebar/sidebar-leading-slot.spec.ts:5-55
Timestamp: 2026-04-23T15:25:31.118Z
Learning: In Astro-Han/pawwork E2E tests, reaching a real "running" session state requires the `project` fixture (for model bootstrap) plus `llm.wait(1)` orchestration. The bare `sdk` fixture used by `withSession` does not trigger an LLM call even with `agent: "build"` + `system` prompt set via `sdk.session.promptAsync`, so simulating running state is not lightweight in the current test infrastructure.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 247
File: packages/ui/src/components/message-part.tsx:1322-1324
Timestamp: 2026-04-26T16:34:54.895Z
Learning: In Astro-Han/pawwork (`packages/ui/src/components/message-part.tsx`), the `taskId` createMemo and `childSessionId` createMemo both intentionally read only from `partMetadata().sessionId` (populated post-execution), not from `input.task_id` / `input.subagent_session_id`. This has always been the case — the original code never read the input field either. Adding an `input.subagent_session_id` fallback would be a new capability, not a bug fix. Do NOT flag the absence of this fallback as a regression in PR `#247` or future PRs unless there is a concrete case where metadata is not populated.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:32.462Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:54.050Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
📚 Learning: 2026-04-25T12:52:47.074Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/share/session.ts:27-27
Timestamp: 2026-04-25T12:52:47.074Z
Learning: In `packages/opencode/src/share/session.ts`, the local `ensureEnabled` closure is intentionally duplicated from `ShareRuntime.ensureEnabled` in `runtime.ts`. Using the shared `ShareRuntime.ensureEnabled` effect directly would leak `CloudShareGate` back into the `R` (requirement) type of `share`, `unshare`, and `create`, because the shared effect re-yields the service rather than capturing an already-resolved gate reference. The current pattern resolves `gate` once at the top of the outer `Effect.gen` and then closes over it in a local `ensureEnabled`, keeping the inner effects' requirement types clean. A comment in the file points to `runtime.ts` to make the intentional duplication discoverable. The real fix would be refactoring `ensureEnabled` to accept a gate parameter, but that change is larger than the DRY benefit warrants. Do not flag this duplication as a maintenance issue.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-26T16:34:54.895Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 247
File: packages/ui/src/components/message-part.tsx:1322-1324
Timestamp: 2026-04-26T16:34:54.895Z
Learning: In Astro-Han/pawwork (`packages/ui/src/components/message-part.tsx`), the `taskId` createMemo and `childSessionId` createMemo both intentionally read only from `partMetadata().sessionId` (populated post-execution), not from `input.task_id` / `input.subagent_session_id`. This has always been the case — the original code never read the input field either. Adding an `input.subagent_session_id` fallback would be a new capability, not a bug fix. Do NOT flag the absence of this fallback as a regression in PR `#247` or future PRs unless there is a concrete case where metadata is not populated.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-24T03:51:54.050Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:54.050Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use fixture-managed cleanup with `withSession(sdk, title, callback)` for temporary sessions instead of calling `sdk.session.delete(...)` directly
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-23T15:26:07.250Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-25T12:52:32.462Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:32.462Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.tspackages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/pages/session.tsxpackages/app/src/pages/session/message-timeline.tsxpackages/app/src/pages/layout/sidebar-items.tsxpackages/app/src/pages/session/session-running-state.tspackages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-23T15:25:31.118Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/e2e/sidebar/sidebar-leading-slot.spec.ts:5-55
Timestamp: 2026-04-23T15:25:31.118Z
Learning: In Astro-Han/pawwork E2E tests, reaching a real "running" session state requires the `project` fixture (for model bootstrap) plus `llm.wait(1)` orchestration. The bare `sdk` fixture used by `withSession` does not trigger an LLM call even with `agent: "build"` + `system` prompt set via `sdk.session.promptAsync`, so simulating running state is not lightweight in the current test infrastructure.
Applied to files:
packages/app/src/pages/session/session-running-state.tspackages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:21:56.373Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:56.373Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.
Applied to files:
packages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : Prefer `DateTime.nowAsDate` over `new Date(yield* Clock.currentTimeMillis)` when you need a `Date` in Effect code
Applied to files:
packages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-20T14:36:21.288Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : For background loops or scheduled tasks, use `Effect.repeat` or `Effect.schedule` with `Effect.forkScoped` in the layer definition
Applied to files:
packages/app/src/pages/session/session-running-state.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Never use wall-clock waits like `page.waitForTimeout(...)` to make a test pass
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `it.effect(...)` when the test should run with `TestClock` and `TestConsole`. Use `it.live(...)` when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `expect.poll(...)` for probing mock or backend state rather than transient DOM visibility
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-24T00:02:53.315Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:53.315Z
Learning: In Astro-Han/pawwork E2E tests (`packages/app/e2e/**/*.spec.ts`), `project.trackDirectory()` and `project.trackSession()` cannot be called before `project.open()` — the `project` fixture throws until `open()` initializes its internal state. The correct pattern is: call `project.trackSession(sessionID)` from inside the `beforeGoto` callback (where state already exists), call `project.trackDirectory(directory)` and cross-workspace `project.trackSession(id, directory)` immediately after `project.open()` returns, and rely on explicit `finally` cleanup (e.g. `cleanupSession` / `cleanupTestProject`) for any resources created before `open()` that cannot yet be tracked via the fixture.
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-22T09:32:54.556Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:54.556Z
Learning: In `packages/opencode/test/provider/provider.test.ts`, the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) rather than `testEffect(...)` for all tests. Converting individual tests to `testEffect` while leaving the rest on the async pattern would create internal inconsistency. A full harness migration of this file is the right approach if the pattern needs to change, but that should be a separate PR.
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Wait on observable state with `expect(...)`, `expect.poll(...)`, or existing helpers instead of assuming work is finished after an action
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/app/src/pages/session/session-running-state.test.ts
🔇 Additional comments (5)
packages/app/src/pages/session/session-running-state.ts (1)
1-73: LGTM!The implementation correctly addresses the stale running state issue:
PENDING_MESSAGE_FALLBACK_MSprovides a bounded 30s window with clear documentationrunningFallbackExpiresAtcleanly separates the expiry calculation logiccreateSessionRunningproperly manages the reactive timeout with cleanup- The single
createSignalfor tick is appropriate here since it's independent state (not coupled with other signals)- The
+10ms buffer on line 64 ensures the timer fires after the expiry thresholdpackages/app/src/pages/layout/sidebar-items.tsx (1)
151-158: LGTM!The integration correctly uses
createSessionRunningwith accessor functions wrapping the store lookups. TheisWorkingmemo appropriately gates onhasPermissions()first before delegating to the reactivesessionRunning()accessor.packages/app/src/pages/session.tsx (1)
1675-1679: LGTM!The refactor simplifies
busy()to a parameter-less function that delegates tocurrentRunning(). This eliminates the previous concern about non-current sessions not receiving reactive timer updates—the function now only checks the current session, which is the only use case at all callsites.packages/app/src/pages/session/message-timeline.tsx (2)
261-261: LGTM!Correctly switches to the reactive
createSessionRunninghelper, which will automatically expire stale running states via the internal timer.
279-286: LGTM!The change to
const parentID = working() ? pending()?.parentID : undefinedproperly addresses the stale active-highlight issue. Whenworking()returnsfalse(including after the 30s fallback expires),parentIDbecomesundefined, preventing stale user messages from being marked as active. This ensures thecontent-visibilityoptimization on lines 936-938 is correctly applied.
bc759bd to
8ce1dc1
Compare
Summary
Fix stale session running state so interrupted or incomplete historical assistant messages cannot keep PawWork visually active forever.
This PR adds a bounded fallback for fresh incomplete assistant messages and a Solid reactive timeout so the UI recomputes when that fallback expires. It applies the shared running-state helper to the session timeline, sidebar session item, and current-session busy checks.
Why
#252 reports sustained CPU usage in the installed desktop app after normal session usage. During diagnosis, a copied installed-app session showed the latest assistant message lacked
time.completed, whilesession.statuswas idle. The UI treated that stale message as still running, which kept progress/spinner animations active and caused renderer/GPU work even though the session was effectively idle.The previous helper only looked at the latest incomplete assistant message. That made stale data indistinguishable from a genuinely fresh in-flight turn.
This addresses the confirmed stale-running UI cause found during the #252 investigation. #252 can remain open as the broader tracking issue for any future high CPU or memory causes that are not covered by this path.
Related Issue
Addresses #252
How To Verify
Manual check performed:
opencode://open-project?directory=/Users/yuhan/workspace/openclaw-setup..session-progress-whipanddocument.getAnimations({ subtree: true }).filter(a => a.playState === "running").length === 0.0.0%CPU after the session was idle.Screenshots or Recordings
Not included. This is a running-state/performance fix; the manual check used process sampling and DevTools runtime inspection rather than a visible UI change.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Bug Fixes
Tests