Skip to content

fix: expire stale session running state#262

Merged
Astro-Han merged 1 commit into
devfrom
codex/fix-renderer-cpu
Apr 27, 2026
Merged

fix: expire stale session running state#262
Astro-Han merged 1 commit into
devfrom
codex/fix-renderer-cpu

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Apr 27, 2026

Copy link
Copy Markdown
Owner

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, while session.status was 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

cd packages/app
bun test --preload ./happydom.ts ./src/pages/session/session-running-state.test.ts
bun run typecheck

cd ../desktop-electron
bun run build

Manual check performed:

  • Built the desktop production bundle.
  • Started Electron preview with the copied dev data from the affected installed app.
  • Opened the target session through opencode://open-project?directory=/Users/yuhan/workspace/openclaw-setup.
  • Confirmed the target session rendered with no .session-progress-whip and document.getAnimations({ subtree: true }).filter(a => a.playState === "running").length === 0.
  • Confirmed the Electron main, GPU, and renderer processes sampled at 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

  • I linked the related issue, or stated why there is no issue
  • This PR has type, scope, and priority labels, or I requested maintainer labeling
  • I listed the relevant verification steps, including tests when behavior changed
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, or generated/local file changes when relevant
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

  • New Features

    • Sessions now stay marked active for 30 seconds when awaiting assistant replies; session activity is tracked reactively with timed updates.
  • Bug Fixes

    • Prevents selecting a parent message when a session is not active; session-busy checks are now used consistently.
  • Tests

    • Enhanced tests for session running detection, fallback timeout behavior, and malformed message handling.

@coderabbitai

coderabbitai Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f402f76c-370c-4916-922f-a149dc68c3e9

📥 Commits

Reviewing files that changed from the base of the PR and between d975474 and 8ce1dc1.

📒 Files selected for processing (5)
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/session-running-state.test.ts
  • packages/app/src/pages/session/session-running-state.ts
📜 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)
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-app
  • GitHub Check: typecheck
  • GitHub Check: unit-opencode
  • GitHub Check: unit-desktop
  • GitHub Check: unit-app
  • GitHub Check: analyze-js-ts
  • GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (1)
packages/app/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/app/AGENTS.md)

Always prefer createStore over multiple createSignal calls in SolidJS

Files:

  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/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-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/layout/sidebar-items.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session.tsx
📚 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/layout/sidebar-items.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/app/src/pages/session/session-running-state.test.ts
📚 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/layout/sidebar-items.tsx
📚 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/layout/sidebar-items.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/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/layout/sidebar-items.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/app/src/pages/session/session-running-state.test.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.tsx
  • 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 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.tsx
  • packages/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.ts
  • packages/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-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/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-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-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.

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/packages/app/src/testing/**/*.ts : Test-only hooks must be inert unless explicitly enabled and should not add normal-runtime listeners, reactive subscriptions, or per-update allocations

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
📚 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
🔇 Additional comments (3)
packages/app/src/pages/session.tsx (1)

1675-1679: busy() is now scoped to the right reactive source.

This removes the old “arbitrary session ID” footgun and keeps abort/queue gating aligned with the self-expiring running accessor for the current page session.

packages/app/src/pages/session/message-timeline.tsx (1)

261-280: Stale parent-message highlighting is correctly gated now.

Using createSessionRunning(...) here and clearing the pending()?.parentID path when working() is false keeps the active-message semantics consistent with the new fallback expiry.

packages/app/src/pages/session/session-running-state.ts (1)

38-72: Reactive expiry helper looks solid.

The timer is rescheduled from the current fallback expiry and cleared both on dependency changes and owner cleanup, which is the right shape for keeping stale-running UI self-healing without leaking timers.


📝 Walkthrough

Walkthrough

Introduces a reactive createSessionRunning accessor and a 30s fallback window for pending assistant messages; refactors consumers to use the accessor and adds helpers/tests for deterministic time-based fallback expiry.

Changes

Cohort / File(s) Summary
Session Running Core
packages/app/src/pages/session/session-running-state.ts
Adds PENDING_MESSAGE_FALLBACK_MS, extends isSessionRunning with optional now, adds runningFallbackExpiresAt, and implements createSessionRunning which schedules timeouts to auto-invalidate fallback.
Tests for Running State
packages/app/src/pages/session/session-running-state.test.ts
Makes time deterministic by passing { now }, adds assertions for fallback expiry and malformed messages, verifies runningFallbackExpiresAt behavior.
Component Integrations
packages/app/src/pages/layout/sidebar-items.tsx, packages/app/src/pages/session.tsx, packages/app/src/pages/session/message-timeline.tsx
Replace direct isSessionRunning(...) calls with createSessionRunning(...) accessor usage; update conditionals and active message selection to use the memoized working() accessor and clear parentID when not working.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

app

Poem

🐰 A little hop, a ticking ear,
I guard the messages we hold dear.
Thirty seconds, then I see—
If silence stays, the state turns free.
Hooray for timers, soft and sly!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: expiring stale session running state to fix sustained UI activity from incomplete historical messages.
Description check ✅ Passed The PR description fully addresses the template with comprehensive Summary, Why, Related Issue, How To Verify, and completed Checklist sections; all required information is present.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-renderer-cpu

Comment @coderabbitai help to get the list of available commands and usage tips.

@Astro-Han Astro-Han added bug Something isn't working P1 High priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels Apr 27, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/app/src/pages/session/session-running-state.ts
Comment thread packages/app/src/pages/session/session-running-state.ts Outdated

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

鸡蛋里挑骨头的 review。整体方向是对的,30s 时间窗 fallback 解决 #252 的根因(stale 永远 truthy)。下面按 P-level 列了 1 个语义不一致 + 几个测试与可维护性的小坑。

  • P1: 1 处(pending memo 没跟着升级,stale 仍会被高亮)
  • P2: 2 处(createSessionRunning 的 reactive timer 没单测;busy() 非当前 session 不会 reactive 过期)
  • P3: 4 处 (constants/naming/test magic/permissions gating 重复)

Comment thread packages/app/src/pages/session/message-timeline.tsx
Comment thread packages/app/src/pages/session/session-running-state.ts Outdated
Comment thread packages/app/src/pages/session/session-running-state.ts Outdated
Comment thread packages/app/src/pages/session/session-running-state.ts Outdated
Comment thread packages/app/src/pages/session/session-running-state.ts Outdated
Comment thread packages/app/src/pages/session/session-running-state.test.ts Outdated
Comment thread packages/app/src/pages/session/session-running-state.test.ts
Comment thread packages/app/src/pages/session.tsx Outdated
Comment thread packages/app/src/pages/layout/sidebar-items.tsx Outdated
@Astro-Han Astro-Han force-pushed the codex/fix-renderer-cpu branch 2 times, most recently from d975474 to bc759bd Compare April 27, 2026 05:09

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
packages/app/src/pages/session/session-running-state.test.ts (1)

87-91: 🧹 Nitpick | 🔵 Trivial

Missing 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 the setTick call or timer logic, these tests will still pass.

Consider adding a test using createRoot + vi.useFakeTimers() that verifies:

  1. Fresh assistant message → running() returns true
  2. Advancing time past 30s → running() flips to false
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between ae800e6 and d975474.

📒 Files selected for processing (5)
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/session-running-state.test.ts
  • packages/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 createStore over multiple createSignal calls in SolidJS

Files:

  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/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.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/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.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/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.tsx
  • 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 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.tsx
  • packages/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.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/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.tsx
  • packages/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.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/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.tsx
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/layout/sidebar-items.tsx
  • packages/app/src/pages/session/session-running-state.ts
  • packages/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.ts
  • packages/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_MS provides a bounded 30s window with clear documentation
  • runningFallbackExpiresAt cleanly separates the expiry calculation logic
  • createSessionRunning properly manages the reactive timeout with cleanup
  • The single createSignal for tick is appropriate here since it's independent state (not coupled with other signals)
  • The +10 ms buffer on line 64 ensures the timer fires after the expiry threshold
packages/app/src/pages/layout/sidebar-items.tsx (1)

151-158: LGTM!

The integration correctly uses createSessionRunning with accessor functions wrapping the store lookups. The isWorking memo appropriately gates on hasPermissions() first before delegating to the reactive sessionRunning() accessor.

packages/app/src/pages/session.tsx (1)

1675-1679: LGTM!

The refactor simplifies busy() to a parameter-less function that delegates to currentRunning(). 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 createSessionRunning helper, which will automatically expire stale running states via the internal timer.


279-286: LGTM!

The change to const parentID = working() ? pending()?.parentID : undefined properly addresses the stale active-highlight issue. When working() returns false (including after the 30s fallback expires), parentID becomes undefined, preventing stale user messages from being marked as active. This ensures the content-visibility optimization on lines 936-938 is correctly applied.

@Astro-Han Astro-Han force-pushed the codex/fix-renderer-cpu branch from bc759bd to 8ce1dc1 Compare April 27, 2026 05:19
@Astro-Han Astro-Han merged commit 99f98ff into dev Apr 27, 2026
24 of 25 checks passed
@Astro-Han Astro-Han deleted the codex/fix-renderer-cpu branch April 27, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P1 High priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant