Skip to content

fix(dashboard): inherit spawning session folder for Start in new worktree - #9046

Merged
bolichen97 merged 1 commit into
mainfrom
fix/worktree-inherit-folder-6347
Sep 6, 2026
Merged

fix(dashboard): inherit spawning session folder for Start in new worktree#9046
bolichen97 merged 1 commit into
mainfrom
fix/worktree-inherit-folder-6347

Conversation

@chenmingwei23

@chenmingwei23 chenmingwei23 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What is the problem?

Clicking "Start in new worktree" on a suggest_followup card opens a new session scoped to the worktree, but that session lands at the top level of the sidebar - even when the session it was spawned from is filed under a project folder. The reporter (@dominik-richter) runs multiple worktrees per project, one session per worktree, so every worktree session had to be dragged into the right folder by hand.

In one sentence he would recognise: a worktree session now opens in the same sidebar folder as the session you started it from, instead of at the top level.

Why it matters to the user

For a builder who keeps one folder per project and one session per worktree, the top-level default breaks the grouping on every single worktree they open. The project directory was already resolved correctly - only the sidebar folder was unset - so the sessions looked misfiled while actually being scoped right, and the only remedy was a manual drag each time.

How the fix solves it - symptom to root cause

  • Symptom: the new session appears at the sidebar top level.
  • The session is not opened by the backend worktree endpoint at all. src/kiro_crew/dashboard/handlers/worktree.py only creates the directory and returns {ok, path, branch, ...}; it carries no folder concept (confirmed: zero folder references on main).
  • The session is opened on the frontend, in followupStartInWorktree (website/src/pages/ChatPage.tsx), which called createSlot({ mode, project: path, activate: false }) - with no folder_id. That omission is the whole bug: a session created without a folder is filed nowhere.
  • The primitive to fix it shipped in feat: file new sessions into a sidebar folder at creation (#6118) #6146 (merged 2026-09-01): createSlot / session_create accept a folder_id, and createSlot already forwards it to api.createChatSlot.
  • The spawning session's folder is already in hand: currentSlot (the object that supplies .project two lines up) carries folder_id. The fix reads currentSlot?.folder_id and passes it through the existing argument. When the spawning session is itself unfiled, folder_id is undefined, so the new session lands top-level - exactly today's behaviour. No default folder is invented; unfiled stays unfiled, and no new API surface is added.

The blocked label on #6347 is stale: it was applied 2026-08-27 with the sole unblock condition "PR #6146 merges. No other action taken." #6146 merged 2026-09-01, so the blocker has expired - noting it here so nobody re-applies the label. Credit to @iamwhatever's triage comment, which prescribed this exact fix ("read the spawning session's folder in the worktree handler and pass it through the new argument, with the fallback staying top-level when the spawning session is unfiled") and confirmed the premise by measuring that the handler had no folder reference.

What tests we did

  • website/src/test/ChatPageFollowup.test.tsx - added two pins, run with vitest run (single file, no watch):
    • inheritance: spawning session filed under folder-proj; asserts api.createChatSlot is called with that folder in its folder_id positional arg. Mutation-verified: removing folder_id: originFolderId from the createSlot call reddens exactly this test with AssertionError: expected undefined to be 'folder-proj', the fallback case still passes, and every unrelated case still passes; reverting restores green.
    • fallback: unfiled spawning session; asserts no folder is passed (top-level behaviour preserved).
  • Consumer check: ChatPageW3Coverage.test.tsx (48 tests) also exercises the worktree flow - still green (56 tests across both files).
  • tsc --noEmit clean.
  • Screenshot evidence below rendered from the real built SPA via the repo's Playwright-fixture harness (no gateway, no git), showing the KiroCrew folder go from 1 session to 2 as the worktree session is filed under it.

Screenshots

Before - the spawning session alone under the KiroCrew folder (count 1):

before: spawning session filed under KiroCrew

After - "Start in new worktree" opens feat/ws-rate-limit nested under the same KiroCrew folder (count 2), not at the top level:

after: worktree session inherits the folder

Light-theme parity is in the same directory (01-origin-filed-light.png, 02-worktree-session-inherits-folder-light.png). Reviewers can also see the change in the Files changed tab.

Other suggestions

@iamwhatever's triage raised whether folder inheritance should apply to every agent-initiated session or only the worktree button. This PR takes the narrower, cheaper default the issue offered as its baseline (inherit on the worktree path, no new API surface, no target-folder override). Broadening it to other session-creation paths, or adding an explicit override, would be a separate change and a separate decision.

Pattern harvest

Defect class: a capability's plumbing existed end to end (createSlot accepted folder_id, the server honoured it) but one call site omitted the optional argument, so the feature silently defaulted to "unfiled". The bug was invisible to a symbol search - folder_id is present all around the call - and only visible by reading what the one caller actually passed.

Rule candidate: when a fix is "thread an existing optional argument through one more caller", pin it with a test that (a) populates the argument on the INPUT and (b) asserts the value reaches the boundary call by its stable identity, then mutation-verify by deleting the argument - an assertion that would pass with the argument absent is vacuous. A positive-only assertion ("a folder was passed") is not enough; pair it with the unfiled complement so the fallback is pinned too.

Closes #6347

@chenmingwei23
chenmingwei23 requested a review from a team September 6, 2026 14:55
@chenmingwei23
chenmingwei23 requested a review from a team as a code owner September 6, 2026 14:55
@chenmingwei23
chenmingwei23 requested a review from cixuuz September 6, 2026 14:55
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of e6d0ae141a33b9b1fdea92f06f7ad07846218392 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A one-call-site omission fixed at the call site, with the fallback pinned and no new surface — the proportionate shape for this bug.

[DESIGN-REVIEWED] e6d0ae1

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of e6d0ae141a33b9b1fdea92f06f7ad07846218392 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e6d0ae1

False positive or not applicable? A repository writer can comment:
/ai-review override gpt e6d0ae141a33b9b1fdea92f06f7ad07846218392: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed e6d0ae141a33b9b1fdea92f06f7ad07846218392 — this comment is updated in place on each push.

Review details

The change threads folder_id correctly: createSlot accepts folder_id in its options type (line 2760), extracts it as folderId (2770), and passes it at positional arg index 8 of api.createChatSlot (2795) — exactly what the new tests assert. The currentSlot?.folder_id dependency is added to the callback's dep array, and the unfiled fallback (undefined) is preserved. No candidates existed, and nothing grounded surfaces under falsification.

No findings.

[OPUS-REVIEWED] e6d0ae1

Verdict parsed from the review's SHA-scoped output markers for commit e6d0ae141a33b9b1fdea92f06f7ad07846218392.

False positive or not applicable? A repository writer can comment:
/ai-review override fable e6d0ae141a33b9b1fdea92f06f7ad07846218392: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of e6d0ae141a33b9b1fdea92f06f7ad07846218392 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence checked: the folder_id primitive exists (src/api/client.ts:3001, positional arg 8 confirmed), sibling call sites at ChatPage.tsx:9328/9344 already carry the origin's folder, the committed-screenshot and per-PR capture-harness conventions are recorded in temp-screenshots/README.md and .jscpd.json's own comment, and I counted the createSlot call sites for unfixed siblings.

First-Principles-Verdict: PASS

One omitted optional argument was the whole defect; the fix threads it through the existing parameter, adds no surface, and pins both directions.

What this change ships

Intent: a worktree session opened from a filed session lands in that same sidebar folder — a FIX.

  1. Worktree sessions now inherit the spawning session's sidebar folder — justified (reported defect Feature request: inherit the sidebar folder when "Start in new worktree" opens a new session #6347, fixed at the omitting call site).
  2. Unfiled spawning sessions still open the worktree session top-level — justified (explicit no-behavior-change fallback, pinned by test).
  3. Two vitest pins for inherit + fallback — justified.
  4. New one-shot capture harness capture-worktree-inherit-folder.mjs — rides along; matches the recorded per-card harness convention in .jscpd.json.
  5. .jscpd.json exemption entry for that harness — rides along; mechanical support for item 4, per that file's own recorded rule.
  6. Four PNGs committed under temp-screenshots/ — rides along; convention documented in temp-screenshots/README.md (ux-review gates on the path).

Watch

  • One counted sibling of the root cause "session spawned from a filed session, created without folder_id": the error-handoff create at website/src/pages/ChatPage.tsx:2019 (grepped all 30 createSlot( call sites; the mode-switch/clean-toggle recreates at 9328/9344 already inherit). The description declares broadening deferred ("would be a separate change and a separate decision"), so this is accepted-and-deferred, not a demand — but that one path is the concrete remainder.

[FIRST-PRINCIPLES-REVIEWED] e6d0ae1

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of e6d0ae141a33b9b1fdea92f06f7ad07846218392 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

The only visible change — the worktree session filed under its parent's folder — is exactly what the blind reader saw and correctly understood as a second conversation in that folder.

The diff adds no controls or strings; every confusion the blind reader logged (icon cluster, "auto", the pre-filled draft, carried-over messages) is pre-existing surface untouched by this change. Before/after states are screenshotted in both themes, and nothing persistent transforms in place, so no recording is owed.

[UX-REVIEWED] e6d0ae1

@chenmingwei23
chenmingwei23 force-pushed the fix/worktree-inherit-folder-6347 branch from 704e76c to 579135a Compare September 6, 2026 15:08
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@chenmingwei23
chenmingwei23 force-pushed the fix/worktree-inherit-folder-6347 branch from 579135a to e6d0ae1 Compare September 6, 2026 15:19
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) September 6, 2026 20:26
@bolichen97
bolichen97 merged commit f71ed38 into main Sep 6, 2026
92 of 100 checks passed
@bolichen97
bolichen97 deleted the fix/worktree-inherit-folder-6347 branch September 6, 2026 20:26
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: inherit the sidebar folder when "Start in new worktree" opens a new session

2 participants