Skip to content

Clarify composer workspace and harness context - #7886

Open
zhengwin wants to merge 16 commits into
mainfrom
composer-context-metadata
Open

zhengwin wants to merge 16 commits into
mainfrom
composer-context-metadata

Conversation

@zhengwin

@zhengwin zhengwin commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Related issue

N/A — this refines the existing composer-context stack.

Summary

  • Persist normalized working-directory/worktree context from New Session and hydrate it in Chat, distinguishing a linked worktree and its branch from an ordinary folder.
  • Keep advanced harness/model/effort configuration behind the existing settings picker, and clarify the workspace action as Use this folder.
  • Prevent unavailable harness selection, expose the actionable readiness warning from the entire disabled row, and keep the selected harness's Edit action visible (plain at rest, underlined on hover).

ELI5: the composer now remembers where a session should run, clearly says when that location is a real Git worktree, and stops users from choosing a harness that cannot start.

New Session workspace + worktree
              |
              v
     normalized composer context
              |
              v
       create / hydrate Chat

Harness readiness -> ready: selectable + configurable
                  -> broken: disabled + row-wide repair tooltip

Test Plan

  • cd web && npm test -- --run src/components/composer/ComposerSettingsButton.test.tsx src/components/composer/ComposerWorkspaceStatus.test.tsx src/components/composer/HarnessPicker.test.tsx src/hooks/useComposerContext.test.tsx src/lib/composerContext.test.ts src/pages/ChatPage.composer.test.tsx src/shell/NewChatDialog.test.tsx src/shell/WorkspacePicker.test.tsx (929 tests)
  • Pinned UI Snapshot Update workflow passed all 180 visual cases and regenerated 13 reviewed baselines limited to the approved composer/harness/workspace changes.
  • cd web && npm run type-check
  • cd web && npm run lint
  • cd web && npm run format:check
  • pre-commit run --files <all changed files>
  • Confirmed every rebased commit has a matching DCO Signed-off-by trailer.

Demo

  • Visual demo attached below
  • Non-visual evidence provided below or in Test Plan
  • Not applicable — no behavioral change

GitHub attachment upload is unavailable from this integration environment, so no screenshot or recording was committed as a workaround. The requester reviewed and accepted the live UI behavior before final integration.

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Manual verification covered New Session workspace/worktree status, Chat hydration, Advanced settings routing, unavailable-harness selection lock and warning tooltip, and the persistent selected-harness Edit affordance in light/dark and narrow layouts. The requester explicitly accepted the final UI and requested no additional visual/manual pass after the main rebase.

Changelog

The composer now preserves workspace context, clearly identifies linked worktrees, and explains unavailable harnesses before you try to select them.

@github-actions github-actions Bot added the size/XL Pull request size: XL label Sep 21, 2026
@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

UI Snapshot doesn't match the committed baseline.

If this UI change is intentional, update the baseline — each path renders in the same pinned image, so the result matches this gate:

  • Label the PR (recommended): add the update-ui-snapshot label — the bot regenerates the baseline in the pinned image, pushes it back here, and re-runs the checks.
  • Locally with Docker: run tests/e2e_ui/visual/regen_baseline_docker.sh, review the PNG, then commit + push.

Diff PNGs (expected_=baseline, actual_=your render, diff_) are in the run artifact. Full guide: tests/e2e_ui/visual/README.md.

@zhengwin
zhengwin force-pushed the composer-context-metadata branch from 3fddbdb to 3080d0d Compare September 21, 2026 08:38
@zhengwin
zhengwin changed the base branch from main to queue-composer-ux September 21, 2026 09:36
@zhengwin
zhengwin force-pushed the queue-composer-ux branch 4 times, most recently from 7be5c5c to 43bd8a4 Compare September 21, 2026 13:12
Base automatically changed from queue-composer-ux to main September 21, 2026 13:29
@zhengwin
zhengwin force-pushed the composer-context-metadata branch from 3080d0d to 10727c4 Compare September 21, 2026 22:27
@zhengwin zhengwin changed the title Composer context metadata Clarify composer workspace and harness context Sep 21, 2026
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
@zhengwin
zhengwin force-pushed the composer-context-metadata branch from 10727c4 to c9a57fb Compare September 21, 2026 22:37
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
@zhengwin zhengwin added update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated and removed size/XL Pull request size: XL labels Sep 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Regenerated the visual baseline(s) in the pinned Playwright image and pushed to this PR. CI will re-run on the new commit.

@github-actions github-actions Bot removed the update-ui-snapshot Used to tell CI that the baseline UI snapshot should be regenerated label Sep 21, 2026
@github-actions github-actions Bot added the size/XL Pull request size: XL label Sep 21, 2026
Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
@zhengwin
zhengwin force-pushed the composer-context-metadata branch from 8035155 to 182874e Compare September 21, 2026 23:03
@zhengwin
zhengwin marked this pull request as ready for review September 21, 2026 23:06
@omnigent-ci

omnigent-ci Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

  1. Chat shows the source repository instead of the session’s new worktree.
    web/src/pages/ChatPage.tsx:2598 prefers the persisted working-directory label over session.workspace. When creating a worktree, that label contains the source repository path; the server updates session.workspace to the actual created worktree. Chat therefore probes the source repository, displays the wrong directory, and hides the linked-worktree status.

    • Fix: Prefer the canonical session.workspace, using the label only when that field is absent.
    • Verify: Create a session in a new worktree, then reopen Chat. Its directory and branch must match the created worktree, not the source checkout. Add coverage for this hydration path.
  2. Unavailable harness rows lose their existing in-app setup path.
    web/src/shell/NewChatDialog.tsx:1665 removes Edit and disables selection for all unavailable harnesses, including those needing installation or authentication. However, the only action opening HarnessSetupDialog is the setup notice for the currently selected harness. An initially unavailable, unselected harness can no longer reach that flow. The updated tests select the harness before making it unavailable, which misses this regression.

    • Fix: Keep ordinary selection disabled, but provide a separate Set up action for setup-required harnesses.
    • Verify: With installation enabled, Claude ready, and Codex reporting binary-missing or needs-auth, open New Session. Codex’s installation or authentication dialog must remain reachable without first selecting it while healthy.

Security vulnerabilities

No demonstrable security vulnerabilities found. No dependency, lockfile, or optional-extra changes are present.

Non-blocking notes

The binary visual baselines were not visually inspected, and tests were not rerun. The reported test and snapshot results remain unverified.

Approach

Use the server’s existing session.workspace as the authoritative execution directory and the existing Git-status hook to identify linked worktrees. Reserve persisted composer labels for intent the server does not otherwise retain. Separating harness setup from harness selection preserves both the new availability restriction and the existing repair workflow.

Summary

Changes requested for two confirmed correctness regressions: new-worktree sessions display the source checkout, and initially unavailable harnesses lose access to in-app setup. Both affect the workflows this PR aims to clarify. The remaining changes fit the stated outcome; no clearly unrelated work or demonstrated security issue was found.


Automated review by Polly · workflow run

Signed-off-by: Jackson Zheng <36802691+zhengwin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Pull request size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant