Skip to content

fix: keep session chat width when panel opens#289

Merged
Astro-Han merged 3 commits into
devfrom
codex/fix-session-chat-width
Apr 28, 2026
Merged

fix: keep session chat width when panel opens#289
Astro-Han merged 3 commits into
devfrom
codex/fix-session-chat-width

Conversation

@Astro-Han

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

Copy link
Copy Markdown
Owner

Summary

Keep the session chat column centered and capped on desktop when the right utility panel opens, while preserving the existing right-panel review visibility logic.

Why

Opening the right utility panel currently disables the centered message layout, so the chat timeline expands to the available width and all messages reflow even when there is still enough space for the capped column.

Related Issue

Closes #277

How To Verify

List the commands you ran and any manual checks you performed.

bun install --frozen-lockfile
bun --cwd packages/app test:e2e session/right-panel-width.spec.ts --grep "session chat column stays capped"
bun --cwd packages/app test:e2e session/right-panel-width.spec.ts
bun --cwd packages/app typecheck

Screenshots or Recordings

Not attached. This is a layout-state regression covered by the E2E width assertion, with no new visual treatment.

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

  • Bug Fixes

    • Improved desktop layout centering and side panel open-state handling so the main view stays centered on desktop and panel interactions behave consistently.
  • Tests

    • Strengthened end-to-end tests for right-panel persistence, resize behavior, and chat turn-list width to prevent regressions across reloads.

@Astro-Han Astro-Han added bug Something isn't working P2 Medium priority ui Design system and user interface labels Apr 28, 2026
@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 47 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fdd99dde-2f22-4277-9356-717fdf392ed9

📥 Commits

Reviewing files that changed from the base of the PR and between 94ab0d7 and 63fca64.

📒 Files selected for processing (9)
  • packages/app/e2e/session/right-panel-width.spec.ts
  • packages/app/src/pages/session.tsx
  • packages/opencode/script/seed-e2e.ts
  • packages/opencode/test/config/config.test.ts
  • packages/opencode/test/config/seed-e2e.test.ts
  • packages/opencode/test/preload.ts
  • packages/opencode/test/shared/config-deps-lock.ts
  • packages/opencode/test/shared/mock-npm-install.ts
  • packages/opencode/test/tool/registry.test.ts
📝 Walkthrough

Walkthrough

E2E test updates to locate and conditionally toggle the right utility panel; a new test verifies the session turn list width persists after opening the panel. Session page logic changes: the desktop "centered" layout is now true for desktop regardless of side-panel open state and related memos/conditions were renamed/updated.

Changes

Cohort / File(s) Summary
E2E Tests
packages/app/e2e/session/right-panel-width.spec.ts
Replace #right-panel id locator with accessible role/name, read aria-hidden and conditionally toggle the panel; add an isolated session test that measures and polls the turn list width before/after opening the right panel.
Session Layout Logic
packages/app/src/pages/session.tsx
Rename memo to desktopSidePanelOpen, set centered to always true when isDesktop() is true (remove dependency on side-panel open state), and update the wantsReview expression to use the renamed memo.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped in to check each panel and seam,
Measured the widths and kept them serene.
No sudden reflow when the sidebar slides,
Messages stay steady — no jagged rides.
A tiny rabbit cheers this butter-smooth scene.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: keeping the session chat width centered when the right panel opens, matching the core fix in session.tsx.
Description check ✅ Passed The description follows the template with all required sections completed: Summary, Why, Related Issue, How To Verify, Screenshots/Recordings note, and a checked Checklist.
Linked Issues check ✅ Passed The PR directly addresses issue #277 by changing the centered layout logic from depending on panel open state to being always true on desktop, fixing the unnecessary chat reflow when the right panel opens.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the chat width regression: E2E test updates to verify the fix and session.tsx layout state changes to base centered layout on available width rather than panel state.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-session-chat-width

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

@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

🤖 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/right-panel-width.spec.ts`:
- Around line 49-50: The test currently uses a fragile ID selector via
page.locator("#right-panel"); replace this with a semantic role or data-*
selector per the e2e policy (e.g., use page.getByRole(...) for the appropriate
ARIA region like "complementary" or
page.locator('[data-component="right-panel"]') if a data attribute exists), and
update the variable aside to use that new locator; ensure the subsequent check
using aside.getAttribute("aria-hidden") remains unchanged so only the locator
source is swapped (update any fixtures or markup to include the chosen
data-component/data-action attribute if needed).
- Line 43: The test currently reads the panel width immediately after toggling
and can race with the open/close CSS transition; replace the direct immediate
width read with a deterministic poll/assertion that waits for a settled width
using Playwright's expect.poll or expect on a locator. Concretely, after the
click on the toggle (the code that clicks the panel toggle in
right-panel-width.spec.ts), use expect.poll(() => page.evaluate(/* read panel
width via getBoundingClientRect or boundingBox */)).toSatisfy(...) or
expect(locator).toHaveJSProperty/ toHaveCSS with a stable value so the assertion
retries until layout finishes; apply the same change to the second occurrence
around the lines currently called out (the block at 62-67) so both assertions
wait for an observable settled state instead of reading immediately.
🪄 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: d546d773-312c-4ec6-953a-ef88b3a8cdc5

📥 Commits

Reviewing files that changed from the base of the PR and between 74656aa and aca40d5.

📒 Files selected for processing (2)
  • packages/app/e2e/session/right-panel-width.spec.ts
  • 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). (12)
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-desktop
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-app
  • GitHub Check: typecheck
  • GitHub Check: unit-opencode
  • GitHub Check: smoke-macos-arm64
  • 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.tsx
  • packages/app/e2e/session/right-panel-width.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/right-panel-width.spec.ts
🧠 Learnings (18)
📓 Common learnings
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/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.
📚 Learning: 2026-04-23T15:26:07.250Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.

Applied to files:

  • packages/app/src/pages/session.tsx
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.

Applied to files:

  • packages/app/src/pages/session.tsx
  • packages/app/e2e/session/right-panel-width.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.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 lowercase, descriptive test names (e.g., 'sidebar can be toggled')

Applied to files:

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

Applied to files:

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

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.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/right-panel-width.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 files should be named with the pattern `feature-name.spec.ts`

Applied to files:

  • packages/app/e2e/session/right-panel-width.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 camelCase for variable names in tests

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.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/right-panel-width.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/right-panel-width.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/right-panel-width.spec.ts
🔇 Additional comments (2)
packages/app/src/pages/session.tsx (1)

464-466: Good decoupling of layout centering from review-panel visibility state.

This keeps desktop chat/composer width behavior stable while preserving the existing review gating logic through wantsReview.

Also applies to: 1127-1130

packages/app/e2e/session/right-panel-width.spec.ts (1)

2-3: Nice use of fixture-managed session lifecycle in the new width test.

withSession(...) keeps cleanup consistent and avoids direct delete calls in test flow.

As per coding guidelines "Use fixture-managed cleanup with withSession(sdk, title, callback) for temporary sessions instead of calling sdk.session.delete(...) directly".

Also applies to: 42-47

Comment thread packages/app/e2e/session/right-panel-width.spec.ts
Comment thread packages/app/e2e/session/right-panel-width.spec.ts Outdated

@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 updates the session page layout logic and adds end-to-end tests for the right panel. Specifically, it renames desktopReviewOpen to desktopSidePanelOpen and ensures the centered state is active on desktop regardless of the side panel's visibility. Additionally, a new E2E test verifies that the session chat column width remains capped when the right panel is opened. I have no feedback to provide.

@Astro-Han Astro-Han force-pushed the codex/fix-session-chat-width branch from 4b99d1e to 94ab0d7 Compare April 28, 2026 11:25

@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: 1

🤖 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/right-panel-width.spec.ts`:
- Around line 42-68: Add a second smaller-desktop viewport run in the "session
chat column stays capped when right panel opens" test to cover the
below-threshold case: after the initial 1600px assertions (using
page.setViewportSize and measuring turnList via sessionTurnListSelector into
widthBefore), set a narrower viewport (e.g., 1200px or another width that makes
the 1000px cap no longer fit with the right panel open), navigate the same flow
(ensure aside is hidden, open the right panel via rightToggle), then assert the
chat column reflows by checking turnList.evaluate(...) returns a width smaller
than widthBefore (use expect or expect.poll). Reference the existing variables
turnList, widthBefore, rightToggle, and aside to implement this second scenario.
🪄 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: fa5fae59-b6d4-4ba4-bd49-a5eba466ed72

📥 Commits

Reviewing files that changed from the base of the PR and between aca40d5 and 94ab0d7.

📒 Files selected for processing (2)
  • packages/app/e2e/session/right-panel-width.spec.ts
  • 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). (12)
  • GitHub Check: smoke-macos-arm64
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-app
  • GitHub Check: typecheck
  • GitHub Check: unit-windows-desktop
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-desktop
  • GitHub Check: unit-opencode
  • 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/e2e/session/right-panel-width.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/right-panel-width.spec.ts
🧠 Learnings (31)
📓 Common learnings
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/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/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: 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 lowercase, descriptive test names (e.g., 'sidebar can be toggled')
📚 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 lowercase, descriptive test names (e.g., 'sidebar can be toggled')

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/e2e/session/right-panel-width.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/e2e/session/right-panel-width.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/right-panel-width.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/right-panel-width.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/e2e/session/right-panel-width.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 `data-component`, `data-action`, or semantic roles for selectors instead of CSS class names or IDs

Applied to files:

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

Applied to files:

  • packages/app/e2e/session/right-panel-width.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 : Wait on observable state with `expect(...)`, `expect.poll(...)`, or existing helpers instead of assuming work is finished after an action

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.spec.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/e2e/session/right-panel-width.spec.ts
📚 Learning: 2026-04-23T15:26:07.250Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.

Applied to files:

  • packages/app/e2e/session/right-panel-width.spec.ts
  • 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 : Never use wall-clock waits like `page.waitForTimeout(...)` to make a test pass

Applied to files:

  • packages/app/e2e/session/right-panel-width.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 direct locators when the interaction is simple and a helper would not add clarity

Applied to files:

  • packages/app/e2e/session/right-panel-width.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 : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.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/right-panel-width.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/e2e/session/right-panel-width.spec.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/e2e/session/right-panel-width.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/right-panel-width.spec.ts
📚 Learning: 2026-04-28T06:47:20.342Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/tool/question.ts:6-16
Timestamp: 2026-04-28T06:47:20.342Z
Learning: In Astro-Han/pawwork, `packages/opencode/src/tool/question.ts` is a permanent PawWork carve-out: `Question.Prompt` (defined in `packages/opencode/src/question/index.ts`) remains a Zod schema, NOT an Effect Schema. The `ts-expect-error` boundary in `tool/registry.ts` marks the Zod ↔ Effect Schema seam. The dropped `packages/opencode/test/tool/question.test.ts` covered the old Zod-mixed surface. Schema-bound regression tests for this tool are intentionally deferred until `question/index.ts` migrates to Effect Schema in its own dedicated PR. Do NOT request restoring question tool tests in upstream-sync PRs; flag it only in the PR that migrates `question/index.ts` to Effect Schema.

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.spec.ts
📚 Learning: 2026-04-28T04:56:18.533Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:18.533Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.

Applied to files:

  • packages/app/e2e/session/right-panel-width.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/right-panel-width.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/right-panel-width.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/e2e/session/right-panel-width.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.tsx

Comment thread packages/app/e2e/session/right-panel-width.spec.ts
@Astro-Han Astro-Han force-pushed the codex/fix-session-chat-width branch from 94ab0d7 to 7948139 Compare April 28, 2026 11:37
@Astro-Han Astro-Han merged commit 6461f6c into dev Apr 28, 2026
22 of 23 checks passed
@Astro-Han Astro-Han deleted the codex/fix-session-chat-width branch May 2, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Session chat flow reflows when right panel is opened

1 participant