fix(app): stabilize session view state#329
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 (6)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)packages/app/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Files:
packages/app/e2e/**/*.spec.ts📄 CodeRabbit inference engine (packages/app/e2e/AGENTS.md)
Files:
🧠 Learnings (45)📓 Common learnings📚 Learning: 2026-04-29T13:27:28.494ZApplied to files:
📚 Learning: 2026-04-28T11:24:35.312ZApplied to files:
📚 Learning: 2026-04-28T18:53:46.234ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-28T05:36:25.456ZApplied to files:
📚 Learning: 2026-04-24T03:51:56.211ZApplied to files:
📚 Learning: 2026-04-28T04:56:21.338ZApplied to files:
📚 Learning: 2026-04-29T13:27:25.687ZApplied to files:
📚 Learning: 2026-04-27T11:19:24.963ZApplied to files:
📚 Learning: 2026-04-28T05:36:24.561ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-25T12:52:49.735ZApplied to files:
📚 Learning: 2026-04-23T15:25:31.118ZApplied to files:
📚 Learning: 2026-04-27T11:18:47.332ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-24T00:02:53.315ZApplied to files:
📚 Learning: 2026-04-23T07:23:23.849ZApplied to files:
📚 Learning: 2026-04-23T15:10:21.635ZApplied to files:
📚 Learning: 2026-04-29T13:27:28.494ZApplied to files:
📚 Learning: 2026-04-28T12:49:09.792ZApplied to files:
📚 Learning: 2026-04-26T16:34:57.130ZApplied to files:
📚 Learning: 2026-04-28T12:01:13.981ZApplied to files:
📚 Learning: 2026-04-28T12:01:16.559ZApplied to files:
📚 Learning: 2026-04-28T04:38:11.771ZApplied to files:
📚 Learning: 2026-04-20T14:36:21.288ZApplied to files:
📚 Learning: 2026-04-28T05:36:22.128ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-28T13:10:01.345ZApplied to files:
📚 Learning: 2026-04-24T03:51:54.050ZApplied to files:
📚 Learning: 2026-04-24T05:39:58.329ZApplied to files:
📚 Learning: 2026-04-27T12:59:49.844ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-28T04:38:21.935ZApplied to files:
📚 Learning: 2026-04-27T10:08:57.448ZApplied to files:
📚 Learning: 2026-04-21T13:45:45.149ZApplied to files:
📚 Learning: 2026-04-24T05:48:36.205ZApplied to files:
📚 Learning: 2026-04-27T10:33:12.228ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-23T15:25:27.182ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.113ZApplied to files:
📚 Learning: 2026-04-24T00:02:50.599ZApplied to files:
📚 Learning: 2026-04-25T12:52:35.631ZApplied to files:
🔇 Additional comments (14)
📝 WalkthroughWalkthroughAdds external session identity control to prompt input/submission, introduces a Session View Controller to manage route vs visible timeline sessions, re-keys composer/page/state to the controller, and extends E2E tests with richer session-transition, scroll-geometry, DOM-removal, and page-error probes. Changes
Sequence Diagram(s)sequenceDiagram
participant Router
participant SessionViewController
participant Timeline
participant Composer
participant PromptInput
participant E2E_Test
Router->>SessionViewController: provide routeSessionID, routeReady, directory
SessionViewController->>Timeline: expose timelineSessionID / timelineSessionKey
Timeline->>Composer: messagesReady / timelineMessages
Composer->>PromptInput: render with sessionID (controlled or params)
E2E_Test->>PromptInput: sendVisiblePrompt(submitKey?)
PromptInput->>Composer: createPromptSubmit(sessionID, isNewSession)
Composer->>Timeline: append message / update scroll geometry
E2E_Test->>SessionViewController: installSessionTransitionProbe(), collect samples (owners, mounts, scroll)
E2E_Test->>E2E_Test: readPageErrorProbe() -> assert no page errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Code Review
This pull request introduces a SessionViewController to manage session transitions and visibility, decoupling the PromptInput and SessionComposer components from global route parameters. It also enhances E2E testing by adding probes for session transitions and page errors, and includes a new test case for 'mod-enter' submissions. I have no feedback to provide.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app/src/pages/session.tsx (2)
2033-2086:⚠️ Potential issue | 🟠 MajorMove followup/revert gating off
params.idbefore rendering the visible-session composer.This composer is now keyed by
displaySessionID={timelineSessionID()}, butfollowup,revert, and theisChildSession()guard in the same JSX still come from route-scoped state. While the visible session is still A and the route has already advanced to B, the footer can briefly hide, disable, or show actions for B against A’s composer, which reintroduces mixed-session frames.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session.tsx` around lines 2033 - 2086, The followup/revert gating must be evaluated against the same session key used to render the composer to avoid mixed-session frames: compute and guard followup and revert props (and the isChildSession() check) using the resolved timelineSessionID() (or a local const derived from timelineSessionID()) rather than route-scoped params.id before passing props into SessionComposerRegion; update references to params.id in the followup/revert/onSend/onAbort logic so they only run when the computed displaySessionID matches the current route/session key, and pass undefined for followup/revert when they don't match so the composer mounts with props consistent with displaySessionID.
520-537:⚠️ Potential issue | 🟠 MajorDrive todo refresh from the visible session too.
createSessionComposerStatenow reads todo state fromtimelineSessionID(), but the page-level refresh effect later still pollssync.session.todo(params.id, ...). During an A → B switch, the dock keeps rendering A while polling immediately moves to B, so A’s todo state can freeze for the whole transition window.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session.tsx` around lines 520 - 537, The page-level todo refresh is still polling using the route param (sync.session.todo(params.id, ...)) which can diverge from the currently rendered session (sessionView.visible / timelineSessionID) during A→B switches; change the refresh effect to drive polling from the visible session ID (use timelineSessionID() or sessionView.visible.id) so the poll target follows the composer/timeline (created by createSessionComposerState({ sessionID: timelineSessionID })). Locate the effect that calls sync.session.todo(params.id, ...) and replace params.id with timelineSessionID() (or read sessionView.visible.id) and ensure you guard for empty/undefined IDs the same way the timelineMessages memo does.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/e2e/session/session-scroll-position.spec.ts`:
- Around line 171-230: Extract the duplicated describe() logic into a single
shared helper (e.g., formatError or describeValue) and replace the inline
describe definitions in installPageErrorProbe and collectPageErrors to call that
helper; update the page.addInitScript closure to reference the helper-exported
function (or define a tiny wrapper that delegates to the shared helper) so both
installPageErrorProbe and collectPageErrors use the same implementation and
remove duplication.
In `@packages/app/src/pages/session/session-view-controller.test.ts`:
- Around line 5-45: Add a new test that uses reactive signals (e.g.,
createSignal) inside a single createRoot so you exercise the
createMemo((current) => ...) path in createSessionViewController: initialize
signals for routeSessionID and routeMessagesReady, instantiate
createSessionViewController with those signal accessors, assert initial
controller.route and controller.visible states, then update the signals to
simulate an in-place route change (flip routeSessionID and/or
routeMessagesReady) and assert that the controller preserves the previous
visible session while updating route and that controller.transitioning() changes
appropriately; reference createSessionViewController, routeSessionID,
routeMessagesReady, createRoot, controller.visible, controller.route, and
controller.transitioning when locating where to add this test.
---
Outside diff comments:
In `@packages/app/src/pages/session.tsx`:
- Around line 2033-2086: The followup/revert gating must be evaluated against
the same session key used to render the composer to avoid mixed-session frames:
compute and guard followup and revert props (and the isChildSession() check)
using the resolved timelineSessionID() (or a local const derived from
timelineSessionID()) rather than route-scoped params.id before passing props
into SessionComposerRegion; update references to params.id in the
followup/revert/onSend/onAbort logic so they only run when the computed
displaySessionID matches the current route/session key, and pass undefined for
followup/revert when they don't match so the composer mounts with props
consistent with displaySessionID.
- Around line 520-537: The page-level todo refresh is still polling using the
route param (sync.session.todo(params.id, ...)) which can diverge from the
currently rendered session (sessionView.visible / timelineSessionID) during A→B
switches; change the refresh effect to drive polling from the visible session ID
(use timelineSessionID() or sessionView.visible.id) so the poll target follows
the composer/timeline (created by createSessionComposerState({ sessionID:
timelineSessionID })). Locate the effect that calls sync.session.todo(params.id,
...) and replace params.id with timelineSessionID() (or read
sessionView.visible.id) and ensure you guard for empty/undefined IDs the same
way the timelineMessages memo does.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd06f362-a9d4-435c-98d7-42893e3e0faa
📒 Files selected for processing (11)
packages/app/e2e/session/session-scroll-position.spec.tspackages/app/src/components/prompt-input.tsxpackages/app/src/components/prompt-input/submit.test.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/pages/session.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/session-view-controller.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/pages/session/timeline-session-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). (12)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-opencode
- GitHub Check: unit-desktop
- GitHub Check: typecheck
- GitHub Check: unit-app
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (2)
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/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/session-view-controller.tspackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsxpackages/app/e2e/session/session-scroll-position.spec.tspackages/app/src/pages/session.tsx
packages/app/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (packages/app/e2e/AGENTS.md)
packages/app/e2e/**/*.spec.ts: Import test utilities from../fixturesinstead of@playwright/test
Test files should be named with the patternfeature-name.spec.ts
Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Use camelCase for variable names in tests
Use SCREAMING_SNAKE_CASE for constants in tests
Use fixture-managed cleanup withwithSession(sdk, title, callback)for temporary sessions instead of callingsdk.session.delete(...)directly
Prefer theprojectfixture for tests that need a dedicated project with LLM mocking
Usedata-component,data-action, or semantic roles for selectors instead of CSS class names or IDs
UsemodKeyfrom utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
In terminal tests, type through the browser usingrunTerminal()andwaitTerminalReady()instead of writing to the PTY through the SDK
Never use wall-clock waits likepage.waitForTimeout(...)to make a test pass
Wait on observable state withexpect(...),expect.poll(...), or existing helpers instead of assuming work is finished after an action
Use locator assertions liketoBeVisible(),toHaveCount(0), andtoHaveAttribute(...)for normal UI state verification
Useexpect.poll(...)for probing mock or backend state rather than transient DOM visibility
Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Use direct locators when the interaction is simple and a helper would not add clarity
When validating routing, assert against canonical or resolved workspace slugs using shared helpers from../actionsto account for Windows canonicalization
Test one feature per test file
Callproject.trackSession(sessionID, directory?)andproject.trackDirectory(directory)for any resources created outside the fixture so teardown can clean them up
Files:
packages/app/e2e/session/session-scroll-position.spec.ts
🧠 Learnings (35)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
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: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
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: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
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.
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:57.130Z
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.
📚 Learning: 2026-04-28T18:53:46.234Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 308
File: packages/opencode/test/server/global-session-list.test.ts:224-259
Timestamp: 2026-04-28T18:53:46.234Z
Learning: In `packages/opencode/test/server/global-session-list.test.ts` (Astro-Han/pawwork), all `it.live(...)` test bodies intentionally use `Effect.promise(async () => { ... })` rather than `Effect.gen(function* () { ... })`. This is the established local convention for the route/pagination tests in this file (e.g., "supports cursor pagination", "session route orders by creation time", "session routes omit undefined optional fields"). Do NOT flag individual `it.live` cases in this file as needing conversion to `Effect.gen`; any harness-style migration would need to cover the entire file.
Applied to files:
packages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-state.tspackages/app/e2e/session/session-scroll-position.spec.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 : Call `project.trackSession(sessionID, directory?)` and `project.trackDirectory(directory)` for any resources created outside the fixture so teardown can clean them up
Applied to files:
packages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/e2e/session/session-scroll-position.spec.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/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/e2e/session/session-scroll-position.spec.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 : Test one feature per test file
Applied to files:
packages/app/src/pages/session/session-view-controller.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-view-controller.test.tspackages/app/src/pages/session/timeline-session-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 : When validating routing, assert against canonical or resolved workspace slugs using shared helpers from `../actions` to account for Windows canonicalization
Applied to files:
packages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.ts
📚 Learning: 2026-04-24T03:51:56.211Z
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:56.211Z
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/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsx
📚 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 locator assertions like `toBeVisible()`, `toHaveCount(0)`, and `toHaveAttribute(...)` for normal UI state verification
Applied to files:
packages/app/src/pages/session/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/e2e/session/session-scroll-position.spec.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/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/session-view-controller.tspackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsxpackages/app/e2e/session/session-scroll-position.spec.tspackages/app/src/pages/session.tsx
📚 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/session-view-controller.test.tspackages/app/src/pages/session/timeline-session-state.tspackages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/session-view-controller.tspackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsxpackages/app/src/pages/session.tsx
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In Astro-Han/pawwork (`packages/opencode`), the canonical module-as-namespace pattern is: each module file ends with `export * as <ModuleName> from "./<module-file>"` (e.g., `export * as SubagentRun from "./subagent-run"` at the bottom of `subagent-run.ts`). Consumers then import the namespace as a named binding (`import { SubagentRun } from "../session/subagent-run"`) and access members via `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. This self-reexport is load-bearing — without it the named binding resolves to nothing. This pattern is used uniformly across session/, config/, tool/, provider/, etc. Do NOT flag these self-reexports as dead code or redundant in any file.
Applied to files:
packages/app/src/pages/session/timeline-session-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 : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/src/pages/session/timeline-session-state.test.tspackages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/app/src/pages/session/timeline-session-state.test.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/e2e/session/session-scroll-position.spec.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/timeline-session-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/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.ts
📚 Learning: 2026-04-29T10:34:22.399Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Applied to files:
packages/app/src/pages/session/timeline-session-state.test.tspackages/app/src/components/prompt-input/submit.test.ts
📚 Learning: 2026-04-27T12:59:49.844Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.
Applied to files:
packages/app/src/components/prompt-input/submit.test.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsxpackages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-24T05:39:58.329Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Applied to files:
packages/app/src/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-26T16:34:57.130Z
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:57.130Z
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/components/prompt-input/submit.test.tspackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsxpackages/app/e2e/session/session-scroll-position.spec.tspackages/app/src/pages/session.tsx
📚 Learning: 2026-04-24T05:48:39.493Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:39.493Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.
Applied to files:
packages/app/src/components/prompt-input/submit.test.tspackages/app/src/components/prompt-input.tsx
📚 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/composer/session-composer-region.tsxpackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-28T12:01:16.559Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/session.ts:518-542
Timestamp: 2026-04-28T12:01:16.559Z
Learning: In `packages/opencode/src/session/session.ts` (Astro-Han/pawwork, PR `#287`), the `updatePart` subtask guard intentionally allows first writes (where `existing === undefined`) to pass through without calling `lifecycleFieldsChanged`. This is required for `Session.fork()`, migration, and import paths that replay historical `SubtaskPart` rows via `updatePart()` outside any `SubagentRunWriterContext`. Only mutations of an already-persisted row (`existing` is defined) are policed. Do NOT suggest adding a lifecycle check on first-write in this guard.
Applied to files:
packages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.ts
📚 Learning: 2026-04-28T04:38:11.771Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Applied to files:
packages/app/src/pages/session/composer/session-composer-state.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/components/prompt-input.tsx
📚 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 : Use `Effect.gen(function* () { ... })` for Effect composition
Applied to files:
packages/app/src/pages/session/composer/session-composer-state.ts
📚 Learning: 2026-04-28T05:36:22.128Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/session/compaction.ts:169-191
Timestamp: 2026-04-28T05:36:22.128Z
Learning: In `packages/opencode/src/session/compaction.ts` (Astro-Han/pawwork), the internal helper `splitTurn` intentionally returns a raw `Effect.gen(...)` rather than being wrapped with `Effect.fnUntraced`. Converting `splitTurn` (and similar plain-Effect.gen internal helpers in this file, e.g. `turns`, `completedCompactions`, `buildPrompt`) to `Effect.fnUntraced` is a repo-wide convention sweep deferred to a dedicated follow-up PR. The same sweep covers `tool/tool.ts` and other helpers flagged in the same sync. Do NOT re-flag the absence of `Effect.fnUntraced` on `splitTurn` or other internal helpers in `compaction.ts` during upstream-sync PRs; flag it only in the dedicated convention-sweep PR.
Applied to files:
packages/app/src/pages/session/composer/session-composer-state.ts
📚 Learning: 2026-04-25T12:52:35.631Z
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:35.631Z
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/components/prompt-input/submit.tspackages/app/e2e/session/session-scroll-position.spec.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 under packages/app/e2e, do not manually call `project.trackSession(sessionID)` when you obtain a `sessionID` via `project.prompt(text)`. The `project.prompt()` implementation already registers `trackSession(next.sessionID, active.directory)` automatically after the prompt submission is observed and the active session is resolved, so calling `project.trackSession(sessionID)` again will create duplicate session ownership/teardown handling.
Applied to files:
packages/app/e2e/session/session-scroll-position.spec.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/e2e/session/session-scroll-position.spec.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 : In terminal tests, type through the browser using `runTerminal()` and `waitTerminalReady()` instead of writing to the PTY through the SDK
Applied to files:
packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-23T15:25:27.182Z
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:27.182Z
Learning: In Astro-Han/pawwork E2E tests (e.g., *.spec.ts under packages/app/e2e), reaching a real "running" session state is not achievable with the bare `sdk` fixture. Use the `project` fixture (to bootstrap the model) and orchestrate the transition with `llm.wait(1)`; even if you set `agent: "build"` and a `system` prompt via `sdk.session.promptAsync`, the current test infrastructure does not trigger an actual LLM call, so it won’t simulate "running" cheaply. Review any attempt to mock/force "running" using only `sdk` as likely ineffective unless it also uses `project` + `llm.wait(1)`.
Applied to files:
packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-24T00:02:50.599Z
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:50.599Z
Learning: For Astro-Han/pawwork E2E tests under packages/app/e2e/**/*.spec.ts, do not call project.trackDirectory() or project.trackSession() before project.open() has run. The project fixture throws until open() initializes internal state. Use this ordering pattern: (1) call project.trackSession(sessionID) inside the beforeGoto callback (where state is already available), (2) call project.trackDirectory(directory) and any cross-workspace tracking like project.trackSession(id, directory) immediately after project.open() returns, and (3) if you create any resources before open() that cannot yet be tracked via the fixture, ensure you clean them up explicitly in finally blocks (e.g., cleanupSession / cleanupTestProject).
Applied to files:
packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-24T05:48:36.205Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In E2E parity tests, prefer using the existing `[data-action]` coverage when asserting UI parity. For elements whose trigger props set `data-action` (e.g., `data-action="prompt-model"` and `data-action="prompt-model-variant"` on prompt input chip triggers), you generally do not need to add separate assertions driven by `[data-component]` for parity. Avoid duplicating component-specific queries when the `[data-action]` selector sweep already includes the elements; any extra unioning of selectors should be treated as optional belt-and-suspenders rather than required.
Applied to files:
packages/app/e2e/session/session-scroll-position.spec.ts
🔇 Additional comments (12)
packages/app/src/pages/session/composer/session-composer-region.tsx (5)
33-34: LGTM!The new optional props
displaySessionIDanddisplaySessionKeyprovide external control over session identity whenvariant === "session", enabling the session view controller pattern described in the PR objectives.
59-68: LGTM!The memoized display identifiers correctly delegate to props when
variant === "session"and fall back to route-derived values otherwise. The null-safety guards inhandoffPromptandinfolookups are appropriate.
86-91: LGTM!The handoff effect correctly targets the visible session key, ensuring prompt state persists against the session the user is actually viewing rather than the route session during transitions.
112-128: LGTM!The readiness effect now depends on
displaySessionKey(), correctly resetting the delay when the visible session changes rather than the route session.
276-291: LGTM!The
PromptInputnow receives the visible session identity viasessionIDandsessionIDControlled={!home()}, ensuring composer actions target the correct session during transitions. This directly addresses the PR objective of preventing composer actions from targeting the wrong session.packages/app/src/pages/session/timeline-session-state.ts (1)
1-2: LGTM!Clean delegation to the new session view controller. This preserves the existing public API for consumers while consolidating session visibility logic in one place.
packages/app/e2e/session/session-scroll-position.spec.ts (6)
30-47: LGTM!The new
SessionTransitionSampletype comprehensively captures transition state including message ownership, DOM presence, and geometry. TheCapturedPageErrortype provides structured error capture for the page error probes.
162-169: LGTM!The
submitKeyparameter cleanly extends the helper to support testing different submission key combinations.
247-354: LGTM!The session transition probe is well-designed to capture the exact regressions mentioned in PR objectives: mixed-session frames (via
messageOwners), composer unmounts (viaremovedComposerDock), and partial render windows (via message count tracking). TheMutationObserverfor removal detection is a solid approach for catching transient DOM states.
470-520: LGTM!Good addition to cover the
Mod+Entersubmission path, which may have different code paths than plainEnter. The test follows established patterns and uses the cross-platformmodKeyhelper correctly.
567-586: LGTM!Excellent assertion coverage addressing the specific regressions mentioned in PR objectives:
mixedOwnerFrames: Catches mixed-session framesremovedMountFrames: Catches composer/message list unmounts during transitionsinvalidComposerFrames/invalidMessageListFrames: Catches partial render statespageErrors: Catches JS errors during session switching
588-597: LGTM!The navigation cleanup (lines 588-592) ensures the page isn't viewing a session during teardown, preventing potential errors. The final
pageErrorEventsassertion and disposal properly validate the entire test flow and clean up the listener.
Design Review: Session View ControllerOverall design is solid — decoupling route session from visible session via No blocking (P0/P1) issues found.
Edge cases verified ✅
Regression risk: LowThe PR is additive. Callers that don't pass One area to watch: the
|
|
Handled the two non-inline CodeRabbit findings in 78d0c4c.
Verification after review fixes:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app/src/pages/session.tsx (2)
751-775:⚠️ Potential issue | 🟠 MajorFetch artifact history from the visible session, not the route session.
This resource still keys and validates against
params.id, so the files side panel can advance to the next route session beforesessionViewhas switched the visible session. That recreates the same mixed-session frame this controller is meant to eliminate. Key the resource/refetch path offtimelineSessionID()(and compare against that same ID when readingartifactHistory.latest).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session.tsx` around lines 751 - 775, The artifact history resource is keyed to params.id which can mismatch the currently visible session and cause mixed-session frames; change the createResource key and its refetch trigger to use timelineSessionID() instead of params.id, and when reading artifactHistory.latest compare history.sessionID against timelineSessionID() (not params.id) inside the artifactFiles memo (referencing artifactHistory, artifactHistory.latest, createResource, timelineSessionID(), artifactFiles, and deriveArtifactFiles) so the files side panel always reflects the visible session.
582-594:⚠️ Potential issue | 🟠 MajorKeep last-turn state on
timelineSessionID().
userMessages/visibleUserMessagesstill followparams.id, while the timeline and composer now followtimelineSessionID(). During a route change where the next session is not ready yet,lastUserMessage()can point at the route session (or go empty) even though the old session is still the visible one. That makesturnDiffs(), turn-mode review, andsyncSessionModel(...)vulnerable to mixed-session state during the transition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/pages/session.tsx` around lines 582 - 594, userMessages / visibleUserMessages currently track params-based messages(), causing lastUserMessage() to point at the route session during transitions; change these memos to derive from timelineSessionID() so the "last-turn" state follows the timeline/composer. Specifically, update the createMemo for userMessages and visibleUserMessages to use timelineSessionID() (or include it in their dependency access) instead of relying solely on messages()/params, ensure revertMessageID() usage stays the same, and verify lastUserMessage (and consumers like turnDiffs, turn-mode review, and syncSessionModel) now read the timeline-bound visibleUserMessages to avoid mixed-session state during route changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/app/src/pages/session.tsx`:
- Around line 751-775: The artifact history resource is keyed to params.id which
can mismatch the currently visible session and cause mixed-session frames;
change the createResource key and its refetch trigger to use timelineSessionID()
instead of params.id, and when reading artifactHistory.latest compare
history.sessionID against timelineSessionID() (not params.id) inside the
artifactFiles memo (referencing artifactHistory, artifactHistory.latest,
createResource, timelineSessionID(), artifactFiles, and deriveArtifactFiles) so
the files side panel always reflects the visible session.
- Around line 582-594: userMessages / visibleUserMessages currently track
params-based messages(), causing lastUserMessage() to point at the route session
during transitions; change these memos to derive from timelineSessionID() so the
"last-turn" state follows the timeline/composer. Specifically, update the
createMemo for userMessages and visibleUserMessages to use timelineSessionID()
(or include it in their dependency access) instead of relying solely on
messages()/params, ensure revertMessageID() usage stays the same, and verify
lastUserMessage (and consumers like turnDiffs, turn-mode review, and
syncSessionModel) now read the timeline-bound visibleUserMessages to avoid
mixed-session state during route changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6db9ce46-551a-4fd5-a713-3ff29e5ec7aa
📒 Files selected for processing (1)
packages/app/src/pages/session.tsx
📜 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). (11)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-opencode
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: typecheck
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-desktop
- 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
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/pages/session.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
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: 270
File: packages/opencode/src/tool/agent.ts:23-27
Timestamp: 2026-04-28T04:38:11.771Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/tool/agent.ts`), the `subagent_session_id` field in the `Parameters` schema accepts any `Schema.String` rather than a branded `SessionID`. This is inherited upstream behavior (adopted in PR `#270`, an upstream-sync graft of upstream PR `#23244`). The fix — validating `subagent_session_id` as a `SessionID` brand up front so malformed/typo'd IDs fail explicitly rather than silently forking a new subagent session — is intentionally deferred to a follow-up PR or upstream report. Do NOT re-flag this as a blocking issue in PR `#270` or in future upstream-sync PRs that carry the same schema; flag it only in a PawWork-authored PR that touches `agent.ts` parameter validation.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
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:57.130Z
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: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
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.
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.
📚 Learning: 2026-04-26T16:34:57.130Z
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:57.130Z
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
📚 Learning: 2026-04-28T12:01:16.559Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/session.ts:518-542
Timestamp: 2026-04-28T12:01:16.559Z
Learning: In `packages/opencode/src/session/session.ts` (Astro-Han/pawwork, PR `#287`), the `updatePart` subtask guard intentionally allows first writes (where `existing === undefined`) to pass through without calling `lifecycleFieldsChanged`. This is required for `Session.fork()`, migration, and import paths that replay historical `SubtaskPart` rows via `updatePart()` outside any `SubagentRunWriterContext`. Only mutations of an already-persisted row (`existing` is defined) are policed. Do NOT suggest adding a lifecycle check on first-write in this guard.
Applied to files:
packages/app/src/pages/session.tsx
📚 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 : Call `project.trackSession(sessionID, directory?)` and `project.trackDirectory(directory)` for any resources created outside the fixture so teardown can clean them up
Applied to files:
packages/app/src/pages/session.tsx
📚 Learning: 2026-04-28T13:10:01.345Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/tool/agent.ts:89-102
Timestamp: 2026-04-28T13:10:01.345Z
Learning: In `packages/opencode/src/tool/agent.ts` (Astro-Han/pawwork, PR `#287`), `makeReadLastCompletedAssistantText` intentionally scans the full child session message history (no `limit`) to find the latest completed assistant text part for `partial_result` on cancellation. Cancellation is a rare path so the extra read cost is acceptable; tool-heavy children can push the last stable assistant text far back. Do NOT suggest re-adding a fixed small limit (e.g., `limit: 5`) to this function.
Applied to files:
packages/app/src/pages/session.tsx
📚 Learning: 2026-04-28T05:36:22.128Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/session/compaction.ts:169-191
Timestamp: 2026-04-28T05:36:22.128Z
Learning: In `packages/opencode/src/session/compaction.ts` (Astro-Han/pawwork), the internal helper `splitTurn` intentionally returns a raw `Effect.gen(...)` rather than being wrapped with `Effect.fnUntraced`. Converting `splitTurn` (and similar plain-Effect.gen internal helpers in this file, e.g. `turns`, `completedCompactions`, `buildPrompt`) to `Effect.fnUntraced` is a repo-wide convention sweep deferred to a dedicated follow-up PR. The same sweep covers `tool/tool.ts` and other helpers flagged in the same sync. Do NOT re-flag the absence of `Effect.fnUntraced` on `splitTurn` or other internal helpers in `compaction.ts` during upstream-sync PRs; flag it only in the dedicated convention-sweep PR.
Applied to files:
packages/app/src/pages/session.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/session.tsx
📚 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
|
Handled the two latest outside-diff comments in 8d25fc0.
Verification before pushing:
|
Summary
Why
Fixes the recurring session UI class behind #286, #312, and #313: URL route state could move ahead of the fully rendered session, while timeline and composer code read mixed sources. That could cause old-hash submit scroll jumps, session switch flicker, or composer actions targeting the wrong session during transition.
Related Issue
Related to #286, #312, and #313.
How To Verify
bun test --preload ./happydom.ts src/components/prompt-input/submit.test.ts src/pages/session/session-view-controller.test.ts src/pages/session/timeline-session-state.test.ts bun run --cwd packages/app typecheck git diff --check bun run --cwd packages/app test:e2e -- session/session-scroll-position.spec.tsScreenshots or Recordings
No screenshot attached. This is a visible session-transition behavior fix covered by the targeted Playwright E2E checks listed above.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Tests
Bug Fixes