Skip to content

fix(app): stabilize session view state#329

Merged
Astro-Han merged 8 commits into
devfrom
codex/fix-session-state-controller
Apr 29, 2026
Merged

fix(app): stabilize session view state#329
Astro-Han merged 8 commits into
devfrom
codex/fix-session-state-controller

Conversation

@Astro-Han

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

Copy link
Copy Markdown
Owner

Summary

  • Add a session view controller that separates route session state from visible session state.
  • Route timeline, composer state, prompt submit, abort, handoff, permission, question, and todo surfaces through the visible session while route data is still loading.
  • Strengthen session scroll and switch E2E coverage so regressions catch top jumps, partial render windows, mixed-session frames, composer unmounts, and page errors.

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.ts

Screenshots or Recordings

No screenshot attached. This is a visible session-transition behavior fix covered by the targeted Playwright E2E checks listed above.

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

    • Composer and prompt input can be driven by an externally provided session identity; timeline view separates route vs visible session state for smoother session rendering.
  • Tests

    • New and expanded e2e and unit tests covering session view transitions, scroll-position preservation, submit-via-modifier behavior, and page-error capture.
  • Bug Fixes

    • Improved session consistency during submissions, session switches, restores, and related UI transitions.

@coderabbitai

coderabbitai Bot commented Apr 29, 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: 6c4f3c53-b9e1-4971-bf2f-e76ddddfacc0

📥 Commits

Reviewing files that changed from the base of the PR and between 78d0c4c and 8d25fc0.

📒 Files selected for processing (6)
  • packages/app/e2e/session/session-scroll-position.spec.ts
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
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/timeline-session-state.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
packages/app/e2e/**/*.spec.ts

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

packages/app/e2e/**/*.spec.ts: Import test utilities from ../fixtures instead of @playwright/test
Test files should be named with the pattern feature-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 with withSession(sdk, title, callback) for temporary sessions instead of calling sdk.session.delete(...) directly
Prefer the project fixture for tests that need a dedicated project with LLM mocking
Use data-component, data-action, or semantic roles for selectors instead of CSS class names or IDs
Use modKey from utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
In terminal tests, type through the browser using runTerminal() and waitTerminalReady() instead of writing to the PTY through the SDK
Never use wall-clock waits like page.waitForTimeout(...) to make a test pass
Wait on observable state with expect(...), expect.poll(...), or existing helpers instead of assuming work is finished after an action
Use locator assertions like toBeVisible(), toHaveCount(0), and toHaveAttribute(...) for normal UI state verification
Use expect.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 ../actions to account for Windows canonicalization
Test one feature per test file
Call project.trackSession(sessionID, directory?) and project.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 (45)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:28.494Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.
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: 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: 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: 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: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:05.946Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
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.
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-29T13:27:28.494Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:28.494Z
Learning: In pawwork’s Bun+Solid test environment (under packages/app/src/pages/session/**.test.ts), do not expect `createMemo((current) => ...)` to be recomputed/invalidated the same way it would in a real browser runtime. Avoid adding signal-driven transition tests that rely on Solid reactive invalidation paths in this environment (e.g., flipping inputs via `createSignal`/`createRoot` just to observe `createMemo` behavior), since it can be misleading. Instead, review/author tests as follows: (1) cover pure transition/state-update logic with plain unit tests (e.g., `nextSessionViewState`), and (2) cover the actual browser reactive behavior with E2E tests (e.g., the session-switch E2E spec). When reviewing this Bun+Solid test suite, do not treat the absence of signal-driven `createSessionViewController` tests as a missing-coverage gap.

Applied to files:

  • packages/app/src/pages/session/timeline-session-state.test.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/timeline-session-state.test.ts
  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 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/timeline-session-state.test.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • 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 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/timeline-session-state.test.ts
  • packages/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
  • packages/app/e2e/session/session-scroll-position.spec.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/timeline-session-state.test.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.

Applied to files:

  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-29T13:27:25.687Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/e2e/session/session-scroll-position.spec.ts:171-230
Timestamp: 2026-04-29T13:27:25.687Z
Learning: In `packages/app/e2e/session/session-scroll-position.spec.ts`, the `describe()` formatter is intentionally duplicated in `installPageErrorProbe` (inside `page.addInitScript`, runs in browser context) and `collectPageErrors` (runs in Node.js/Playwright context). These are separate execution contexts — `addInitScript` serializes its closure to a string for injection — so extracting a shared helper would add cross-context indirection. Do NOT flag this duplication as a maintenance issue.

Applied to files:

  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/e2e/session/session-scroll-position.spec.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.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.

Applied to files:

  • packages/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 : Test one feature per test file

Applied to files:

  • packages/app/src/pages/session/timeline-session-state.test.ts
📚 Learning: 2026-04-25T12:52:49.735Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/opencode/src/share/session.ts:27-27
Timestamp: 2026-04-25T12:52:49.735Z
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/timeline-session-state.test.ts
  • packages/app/e2e/session/session-scroll-position.spec.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.ts
📚 Learning: 2026-04-27T11:18:47.332Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/mcp-exa.test.ts:1-186
Timestamp: 2026-04-27T11:18:47.332Z
Learning: In `packages/opencode/test/tool/mcp-exa.test.ts` (Astro-Han/pawwork), the tests intentionally use raw `bun:test` async cases with `Effect.runPromise(...)` and per-case `HttpClient.make(...)` fakes rather than the `testEffect(...)` harness. The maintainer has explicitly decided not to migrate, because the HttpClient fake wiring is itself the behavior under test and switching to `testEffect` would be style 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.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/timeline-session-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/timeline-session-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/timeline-session-state.test.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/timeline-session-state.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
  • packages/app/e2e/session/session-scroll-position.spec.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/timeline-session-state.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
📚 Learning: 2026-04-29T13:27:28.494Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:28.494Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.

Applied to files:

  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 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-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/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-28T12:01:13.981Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:104-108
Timestamp: 2026-04-28T12:01:13.981Z
Learning: In `packages/opencode/src/session/subagent-run.ts` (Astro-Han/pawwork, PR `#287`), `rowLocks` and `partsByToolCall` are intentionally keyed by `tool_call_id` alone (not a composite `(parent_session_id, tool_call_id)` key). `tool_call_id` is AI-SDK-generated as `call_<nanoid>`, making it globally unique across sessions in practice. `readByToolCallID(parentID, toolCallID)` takes `parentID` solely as an access-control/ownership guard, not because IDs repeat across parents. Composite-keying would add unnecessary API surface per YAGNI for v1; revisit only if AI SDK moves off nanoid-based call IDs. Do NOT re-flag the single-key maps in this file.

Applied to files:

  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
📚 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/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session.tsx
📚 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/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-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 : 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
  • packages/app/src/pages/session.tsx
  • 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 : 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
  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 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-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-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/e2e/session/session-scroll-position.spec.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/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-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.

Applied to files:

  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-27T10:08:57.448Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/diagnostics.test.ts:313-318
Timestamp: 2026-04-27T10:08:57.448Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/diagnostics.ts`, PR `#264`), `SessionDiagnostics.LoopAction` is a TypeScript string literal union type (`"observe" | "block" | "stop"`), NOT a runtime enum. There is no runtime form to introspect via `Object.values()`. The test pattern `const all: SessionDiagnostics.LoopAction[] = ["observe", "block", "stop"]; expect(all).toHaveLength(3)` is intentional and correct — it provides compile-time exhaustiveness via the type annotation. Do NOT suggest replacing this with `Object.values()` introspection or flag the runtime length check as insufficient.

Applied to files:

  • packages/app/e2e/session/session-scroll-position.spec.ts
📚 Learning: 2026-04-21T13:45:45.149Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 99
File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35
Timestamp: 2026-04-21T13:45:45.149Z
Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.

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
📚 Learning: 2026-04-27T10:33:12.228Z
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.

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 : Import test utilities from `../fixtures` instead of `playwright/test`

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-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/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-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/e2e/session/session-scroll-position.spec.ts
🔇 Additional comments (14)
packages/app/src/pages/session/session-view-controller.ts (1)

1-89: Well-structured session view controller implementation.

The separation of pure transition logic (nextVisibleSessionID, nextSessionViewState) from the reactive wrapper (createSessionViewController) follows good design principles. This approach aligns well with the testing strategy mentioned in the learnings—pure functions can be unit tested directly while reactive behavior is covered by E2E tests.

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

1-7: Clean re-export structure preserving backward compatibility.

The legacy nextTimelineSessionID alias maintains backward compatibility while centralizing implementation in session-view-controller.ts. This aligns with the commit message "clarify visible session helpers."

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

1-12: Focused test validating re-export wiring.

The test correctly uses toBe() for reference equality, confirming that the re-exports point to the same function instances. This avoids duplicate behavioral testing while ensuring the module contract is maintained.

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

514-527: Correct integration of SessionViewController as the source of truth.

The sessionView controller properly separates route session (params.id) from visible session (timelineSessionID). The composer state is correctly initialized with the visible session identity, ensuring that permission/question requests and blocked state track the displayed session rather than the route.


657-666: deferRender keyed on timelineSessionKey controls composer flash timing.

This change shifts the composer flash suppression from route session key to visible session key, aligning with the PR objective to prevent composer actions targeting the wrong session during transitions.


1920-1925: Scoped restoring state to timeline session prevents cross-session UI contamination.

The restoring() memo now filters by variables?.sessionID !== timelineSessionID(), ensuring the restoring indicator only displays when the mutation targets the currently visible session. This prevents incorrect UI state during session transitions.


2040-2045: Explicit displaySessionID/displaySessionKey props enable session-scoped composer behavior.

Passing these props for the "session" variant allows SessionComposerRegion to correctly identify the visible session, consistent with the wiring in session-composer-region.tsx (lines 59-68) which uses these props to drive sync.session.set().

packages/app/src/pages/session/composer/session-composer-state.ts (2)

20-26: Clean externalization of session identity.

The refactored createSessionComposerState now accepts the session identity as a dependency injection, allowing callers (session.tsx) to provide the visible session identity. This enables the composer to correctly track permission/question requests and blocked state for the displayed session during transitions.


135-161: Robust todo dock visibility with session identity tracking.

The effect correctly captures expectedSessionID at the start and re-validates with activeSessionID() === expectedSessionID before hiding the dock. This prevents race conditions where navigating away and back could cause the dock to hide unexpectedly.

packages/app/e2e/session/session-scroll-position.spec.ts (5)

162-169: Good extensibility for submit key testing.

The optional submitKey parameter enables testing both Enter and Mod+Enter submission paths without code duplication.


171-230: Comprehensive page error instrumentation.

The dual-context error capture (browser via addInitScript + Node.js via page.on('pageerror')) provides thorough coverage. The describe() duplication is intentional per the retrieved learning—these run in separate execution contexts.


247-369: Rich transition probing with frame-level ownership tracking.

The enhanced probe captures:

  • Per-message session ownership via messageOwners mapping
  • DOM removal detection via MutationObserver
  • Composer/message-list validity at each sample

This enables the comprehensive assertions in the session-switching test that detect mixed-session frames and component unmounts during transitions.


472-522: Valuable coverage for Mod+Enter submission path.

This test ensures the scroll-to-latest behavior works correctly for keyboard shortcuts, not just plain Enter. The test structure mirrors the existing Enter test, maintaining consistency.


570-598: Thorough frame-level assertions catch transition regressions.

The assertions comprehensively validate:

  • No mixed ownership frames (line 570-572)
  • No removed component mounts (line 573)
  • Valid composer dock state (line 574)
  • Valid message list state (line 575)
  • No page errors (line 576, 588, 597-598)

This directly addresses the PR objective of detecting "mixed-session frames, composer unmounts, page errors."


📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
E2E Session Testing
packages/app/e2e/session/session-scroll-position.spec.ts
Replaces simple message-count probes with a session-transition probe capturing route/visible session id, per-message ownership, composer dock/message-list presence/count/height, DOM removal detection, scroll timeline geometry, and page-error instrumentation; adds submit-key test and stronger transition assertions.
Prompt Input Component
packages/app/src/components/prompt-input.tsx
Adds sessionID? and sessionIDControlled? props and derives a unified activeSessionID() used for all session-scoped reads/guards; submission wiring now passes resolved session id and isNewSession predicate.
Prompt Submission Logic & Tests
packages/app/src/components/prompt-input/submit.ts, packages/app/src/components/prompt-input/submit.test.ts
Generalizes createPromptSubmit / PromptSubmitInput to accept optional sessionID and isNewSession accessors; submission, abort, worktree, auto-create, agent/validation, and queuing logic now use unified accessors; tests updated/added to verify route vs visible session identity behavior.
Session View Controller & Tests
packages/app/src/pages/session/session-view-controller.ts, packages/app/src/pages/session/session-view-controller.test.ts
New SolidJS view controller providing route vs visible session accessors, sessionKey, nextSessionViewState/nextVisibleSessionID, and transitioning() semantics; tests validate readiness, transition behavior, and directory-scoped clearing.
Timeline State Re-exports & Tests
packages/app/src/pages/session/timeline-session-state.ts, packages/app/src/pages/session/timeline-session-state.test.ts
Re-exports timeline/session helpers (nextVisibleSessionID alias, nextSessionViewState, sessionKey, types) from the new controller; tests adjusted to assert export wiring/identity.
Session Page & Composer
packages/app/src/pages/session.tsx, packages/app/src/pages/session/composer/session-composer-region.tsx, packages/app/src/pages/session/composer/session-composer-state.ts
Page uses createSessionViewController for timeline identity/readiness; composer region accepts displaySessionID/displaySessionKey overrides and passes sessionIDControlled to PromptInput; composer state accepts a sessionID() provider and migrates all session reads to the active session accessor.
Tests / Refactors
packages/app/src/pages/session/*.test.ts
Adds/updates tests around view controller and timeline session wiring; refactors various evaluate/conditional utilities for E2E assertions; no public API removals.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • #312 — Modifies the same E2E session scroll spec to add in-page timeline scroll probing and assertions about scroll geometry/anchoring.
  • #313 — Refactors session-timeline visibility logic and nextTimelineSessionID usage; closely related to the new session view controller and re-exports.
  • #208 — Related edits to prompt input and submit logic introducing externally-controlled session identity patterns.

Poem

🐇 I hopped through routes and visible keys,
I nudged prompts gently on breezy leaves,
Timelines steady, composers keep their place,
Tests watch scrolls and mounts with careful grace,
A rabbit cheers for smoother session space.

✨ 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-session-state-controller

@Astro-Han Astro-Han added bug Something isn't working app Application behavior and product flows ui Design system and user interface P1 High priority labels Apr 29, 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 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.

@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.

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 | 🟠 Major

Move followup/revert gating off params.id before rendering the visible-session composer.

This composer is now keyed by displaySessionID={timelineSessionID()}, but followup, revert, and the isChildSession() 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 | 🟠 Major

Drive todo refresh from the visible session too.

createSessionComposerState now reads todo state from timelineSessionID(), but the page-level refresh effect later still polls sync.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

📥 Commits

Reviewing files that changed from the base of the PR and between 7b3617a and a563556.

📒 Files selected for processing (11)
  • packages/app/e2e/session/session-scroll-position.spec.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/pages/session/session-view-controller.test.ts
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/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 createStore over multiple createSignal calls in SolidJS

Files:

  • packages/app/src/pages/session/session-view-controller.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
  • packages/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 ../fixtures instead of @playwright/test
Test files should be named with the pattern feature-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 with withSession(sdk, title, callback) for temporary sessions instead of calling sdk.session.delete(...) directly
Prefer the project fixture for tests that need a dedicated project with LLM mocking
Use data-component, data-action, or semantic roles for selectors instead of CSS class names or IDs
Use modKey from utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
In terminal tests, type through the browser using runTerminal() and waitTerminalReady() instead of writing to the PTY through the SDK
Never use wall-clock waits like page.waitForTimeout(...) to make a test pass
Wait on observable state with expect(...), expect.poll(...), or existing helpers instead of assuming work is finished after an action
Use locator assertions like toBeVisible(), toHaveCount(0), and toHaveAttribute(...) for normal UI state verification
Use expect.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 ../actions to account for Windows canonicalization
Test one feature per test file
Call project.trackSession(sessionID, directory?) and project.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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • 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 : 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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • 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 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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • 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 : 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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/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.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/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.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
  • 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/session-view-controller.test.ts
  • packages/app/src/pages/session/timeline-session-state.ts
  • packages/app/src/pages/session/timeline-session-state.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/session-view-controller.ts
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/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.ts
  • packages/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.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/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.ts
  • packages/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.ts
  • packages/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.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/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.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/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.ts
  • packages/app/src/pages/session/composer/session-composer-region.tsx
  • packages/app/src/pages/session/composer/session-composer-state.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/components/prompt-input.tsx
  • packages/app/e2e/session/session-scroll-position.spec.ts
  • packages/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.ts
  • packages/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.tsx
  • packages/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.ts
  • packages/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.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/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.ts
  • packages/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 displaySessionID and displaySessionKey provide external control over session identity when variant === "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 in handoffPrompt and info lookups 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 PromptInput now receives the visible session identity via sessionID and sessionIDControlled={!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 SessionTransitionSample type comprehensively captures transition state including message ownership, DOM presence, and geometry. The CapturedPageError type provides structured error capture for the page error probes.


162-169: LGTM!

The submitKey parameter 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 (via removedComposerDock), and partial render windows (via message count tracking). The MutationObserver for removal detection is a solid approach for catching transient DOM states.


470-520: LGTM!

Good addition to cover the Mod+Enter submission path, which may have different code paths than plain Enter. The test follows established patterns and uses the cross-platform modKey helper correctly.


567-586: LGTM!

Excellent assertion coverage addressing the specific regressions mentioned in PR objectives:

  • mixedOwnerFrames: Catches mixed-session frames
  • removedMountFrames: Catches composer/message list unmounts during transitions
  • invalidComposerFrames/invalidMessageListFrames: Catches partial render states
  • pageErrors: 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 pageErrorEvents assertion and disposal properly validate the entire test flow and clean up the listener.

Comment thread packages/app/e2e/session/session-scroll-position.spec.ts
Comment thread packages/app/src/pages/session/session-view-controller.test.ts
@Astro-Han

Copy link
Copy Markdown
Owner Author

Design Review: Session View Controller

Overall design is solid — decoupling route session from visible session via SessionViewController cleanly fixes the mixed-source transition bugs.

No blocking (P0/P1) issues found.

Severity Count Highlights
P0 0
P1 0
P2 2 Dead code, naming inconsistency
P3 2 Duplicate tests, fragile comparator

Edge cases verified ✅

  • Navigate to new session, messages not ready → visible stays on old session ✅
  • Messages load → swap to new session ✅
  • Navigate away from session entirely → visible cleared ✅
  • Directory changes mid-transition → stale state cleared ✅
  • Composer/submit/abort target visible session during transition ✅
  • deferRender keyed on timelineSessionKey (correct) ✅

Regression risk: Low

The PR is additive. Callers that don't pass sessionID/isNewSession fall back to params.id (old behavior). Home page composer is unaffected.

One area to watch: the deferRender key change from sessionKey()timelineSessionKey() shifts when the composer flash happens. The E2E session-switch test covers this.

Inline comments posted below on specific files/lines.

Comment thread packages/app/src/pages/session/session-view-controller.ts Outdated
Comment thread packages/app/src/pages/session/composer/session-composer-state.ts Outdated
Comment thread packages/app/src/pages/session/timeline-session-state.test.ts Outdated
Comment thread packages/app/e2e/session/session-scroll-position.spec.ts
@Astro-Han

Copy link
Copy Markdown
Owner Author

Handled the two non-inline CodeRabbit findings in 78d0c4c.

  • P1: followup and revert composer gating now use the visible session identity instead of the route session.
  • P2: todo refresh now tracks the visible session identity and drops stale refreshes when visible state changes.

Verification after review fixes:

  • bun run --cwd packages/app typecheck
  • bun --cwd packages/app test -- src/pages/session/session-view-controller.test.ts src/pages/session/timeline-session-state.test.ts src/pages/session/composer/session-composer-state.test.ts
  • bun run --cwd packages/app test:e2e -- session/session-scroll-position.spec.ts

@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.

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 | 🟠 Major

Fetch 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 before sessionView has switched the visible session. That recreates the same mixed-session frame this controller is meant to eliminate. Key the resource/refetch path off timelineSessionID() (and compare against that same ID when reading artifactHistory.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 | 🟠 Major

Keep last-turn state on timelineSessionID().

userMessages / visibleUserMessages still follow params.id, while the timeline and composer now follow timelineSessionID(). 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 makes turnDiffs(), turn-mode review, and syncSessionModel(...) 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

📥 Commits

Reviewing files that changed from the base of the PR and between a563556 and 78d0c4c.

📒 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 createStore over multiple createSignal calls 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

@Astro-Han

Copy link
Copy Markdown
Owner Author

Handled the two latest outside-diff comments in 8d25fc0.

  • P2: artifact history and Files-derived artifacts now key off the visible session via timelineSessionID(), and stale artifact results are compared against that same visible id.
  • P2: last-turn state now derives from timelineVisibleUserMessages(), so turn diffs, turn-mode review, and syncSessionModel follow the same visible session as timeline and composer during route transitions.

Verification before pushing:

  • bun run --cwd packages/app typecheck
  • bun --cwd packages/app test -- src/pages/session/session-view-controller.test.ts src/pages/session/timeline-session-state.test.ts src/pages/session/session-model-helpers.test.ts
  • bun run --cwd packages/app test:e2e -- session/session-scroll-position.spec.ts

@Astro-Han Astro-Han merged commit 239177b into dev Apr 29, 2026
25 checks passed
@Astro-Han Astro-Han deleted the codex/fix-session-state-controller branch April 29, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows bug Something isn't working P1 High priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant