Skip to content

feat(prompt): isolate drafts and history across workspaces#765

Merged
Astro-Han merged 24 commits into
devfrom
claude/prompt-draft-isolation
May 19, 2026
Merged

feat(prompt): isolate drafts and history across workspaces#765
Astro-Han merged 24 commits into
devfrom
claude/prompt-draft-isolation

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

Implements v7 of the prompt-draft + history isolation design for #750. Adds a route-aware portable homepage runtime owner that follows the user across workspace homepages without ever entering concrete sessions, a deep-link pinned draft scope that coexists with portable while staying bound to its target directory, directory-scoped prompt history with a stale-rAF guard, resolved comment @mention metadata captured at create-time so moved comments don't re-resolve into a different workspace, a revision-guarded two-phase submit lifecycle (synchronous UI reset; owner teardown only on async success; restore reads from the still-intact owner on failure), a fire-and-forget homepage-draft migration sentinel that adopts the current opened directory's pre-v7 draft into the portable owner, external-absolute-chip click no-op, and a shared path-canonicalization helper.

Why

#750 reported that homepage drafts, file chips, comments, and ArrowUp history were silently crossing workspace and session boundaries. Concrete examples: a draft typed on workspace A's homepage appeared inside workspace B's session, an @src/a.ts mention sent from workspace B's homepage resolved to B's same-named file instead of A's, and pressing ArrowUp in workspace B surfaced entries typed in workspace A. The root causes were a broad cross-directory carry-over module, global prompt-history persistence, send-time path resolution against the submit sessionDirectory, and unguarded async clear/restore paths that overwrote newer user input on delayed callbacks.

Related Issue

Closes #750.

PR2 follow-up #749 (typed @src/app.ts unique-match auto-conversion into file chips) is explicitly out of scope; this PR preserves the current behavior that only suggestion-list selections become file pills.

Human Review Status

Pending

Review Focus

The state-machine pieces are the highest-risk surface and benefit most from careful review: submit.ts two-phase clear (clearInput resets UI synchronously; confirmOwnerCleared tears down the owner snapshot only on async success under the captured revision; restoreInput reads from the still-intact owner on failure), the editor-input.ts carry-hydration effect (pinned beats portable; isHomepageDraftEmpty considers prompt + context + images so chips and image attachments are never overwritten; shouldReset also clears the matching owner so user-initiated clears don't leave a stale snapshot), the portable-draft.ts + pinned-draft.ts owner singletons, and the homepage-migration.ts sentinel's failure semantics (fire-and-forget, sentinel only marked complete after the full adopt + remove cycle succeeds).

Design version implemented: v7. History capture happens via addToHistory(currentPrompt, mode) at submit start (before any clear), so portable owner clears never race history capture. Queue path is unreachable for portable/pinned (the existing shouldQueue && !creatingNewSession gate is route-only); ownership.kind is always route there. Multi-window migration uses a single-renderer assumption with soft idempotence — every step (portable.record, removePersisted, sentinel write) is independently idempotent. Resolved-mention schema: displayText + resolvedPath + fingerprint (checksum of 64-char window around the mention midpoint) + start/end; matching prefers range-equality and falls back to occurrence order; free-text mentions without metadata are dropped rather than re-resolved against the submit directory.

Risk Notes

PR1 deliberately does NOT enumerate other workspaces' pre-v7 homepage drafts for migration; only the currently opened directory's draft is adopted into the portable owner. Other workspaces' drafts stay in their per-route stores and self-heal through portable mirroring on next edit. Documented as a scope simplification — full enumeration is a follow-up.

Portable owner is runtime-only and does not survive app restart by design. Concrete session drafts (route-scoped Persist) are unchanged and continue to persist.

Visible UI change is small: the external-absolute file chip now renders with aria-disabled, dimmed opacity, cursor-not-allowed, an "External" tooltip prefix, and a click that no-ops; the trash button still works. No snap target was added (no nearby snap fixture for the chip surface) and no manual screenshot was captured. The conditional UI checklist item is left unticked; reviewer should walk a homepage with a portable-carried chip in bun run dev:desktop to confirm the dimmed-disabled state.

Owner-level integration is heavily unit-tested but the full route-walk (homepage A → homepage B → session → back to homepage A; deep-link arrival with existing portable hidden; submit failure on portable preserves the draft) is left to CI E2E and manual verification before merge.

How To Verify

typecheck (turbo, whole repo):      8/8 tasks ok
packages/app bun test:              1299 pass / 0 fail / 3088 expects across 183 files
packages/opencode bun test:         2767 pass / 9 skip / 1 todo / 0 fail / 11994 expects across 217 files
shell-frame-contract.test.ts:       8 pass / 0 fail (migration storage extracted to non-shell module)
new unit suites:                    path-canonical (28), mention-metadata (15), portable-draft (17),
                                    pinned-draft (16), homepage-migration (12), submit-ownership (11),
                                    draft-isolation.integration (7), context-items (10), draft-carryover (4 v7)
i18n parity test:                   1 pass / 0 fail (en + zh-Hans key parity preserved)
E2E (playwright):                   not run locally; left to CI
dev:desktop walk:                   not run (autonomous session) — recommended before merge

Screenshots or Recordings

Not captured in this autonomous run. Reviewer should verify the external-absolute chip's disabled-dimmed appearance in bun run dev:desktop: drag-create a chip whose path points outside sdk.directory, confirm hover cursor is not-allowed, click is a no-op, tooltip says "External · ", and the trash button still removes it.

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for file mentions in comments via @filename syntax for context attachment
    • Introduced portable draft system for prompt persistence across workspace changes
    • Added pinned draft support for deep-linked sessions with embedded prompts
    • Implemented automatic migration of legacy homepage drafts
  • Improvements

    • External files now display as disabled in context items with visual distinction
    • Prompt history is now scoped per workspace instead of global
    • Enhanced draft ownership detection and lifecycle management

Astro-Han added 12 commits May 19, 2026 16:32
Move the inline `absolute()` from build-request-parts.ts into a shared
path-canonical.ts module with typed exports: isAbsoluteLike,
toAbsoluteFilePath, isUnderDirectory, and compactFilePath. All three
call sites in build-request-parts are updated. Behavior is identical
to the original inline helper for all existing test cases.
Record absolute-path metadata (displayText, resolvedPath, fingerprint,
start/end range) when a comment @-mention is captured, and validate it
at submit time with captureCommentMentions / resolveCommentMentions.

Free-text @mentions without metadata return [] from resolveCommentMentions
(no fallback re-resolve), preventing silent same-name cross-workspace
file attachment when a portable draft moves between workspaces.

- mention-metadata.ts: new pure module implementing capture + resolve
- prompt.tsx: FileContextItem gains optional resolvedMentions field
- build-request-parts.ts: remove parseCommentMentions, switch to
  resolveCommentMentions; ContextFile gains resolvedMentions field
- build-request-parts.test.ts: update existing @mention test to supply
  resolvedMentions; add cross-workspace and no-metadata drop tests
Replace the single global Persist.global history store with a per-directory
LRU cache (max 12 entries). Each directory gets its own Persist.workspace
storage for normal and shell history. No legacy keys are passed, so old
global history is ignored per v7 spec.

- history-store-factory.ts: extracted factory (Persist.workspace, no legacy keys)
- history-navigation.ts: LRU cache + ensureStores(); history/shellHistory become
  () => HistoryStore accessors; directoryToken signal + createEffect resets
  historyIndex/-1 and savedPrompt/null on directory change; rAF stale guard
  compares captured token before writing to DOM
- history-navigation.test.ts: 6 unit tests verifying Persist.workspace target
  shape, storage isolation per directory, key isolation per mode, no legacy keys
Replace the broad cross-directory draft-carryover snapshot with a
route-aware portable homepage runtime owner (design v7).

Key changes:
- portable-draft.ts: new PortableDraftOwner factory (runtime-only Solid
  signal, not persisted). Holds at most one snapshot per app run, keyed
  by sourceFilesystemDirectory. Implements record / consumeForHomepage /
  hide / restore / clear with revision-guarded stale-clear protection.
- portable-draft.test.ts: 17 unit tests covering record semantics
  (deep-equal no-bump, empty clears, per-field revision bumps),
  consumeForHomepage (self-move guard, non-empty target guard, move+bump),
  and clear (stale-revision guard, already-cleared returns true).
- draft-carryover.ts: replaced with a thin backwards-compat shim that
  re-exports usePortableDraft() from portable-draft.ts.
- draft-carryover.test.ts: rewritten as integration scenarios exercising
  v7 semantics (A→B move, self-move guard, non-empty guard, empty clears).
- editor-input.ts: carry-over createEffect now tracks params.id so it
  runs only on homepage routes (sessionID undefined). Session routes call
  portable.hide(). recordDraftEdit replaced with portable.record() on
  homepage routes only; session routes do not mirror. Text hydration on
  consume extracts text parts only — context+image hydration deferred to T5.

No stray consumers of recordDraftEdit/consumeCarryOver/clearCarryOver remain.
Introduce PinnedDraftOwner (pinned-draft.ts) that binds a deep-link
prefill prompt exclusively to its source directory. Unlike the portable
owner which floats between homepages, the pinned slot stays directory-
bound until release() or clearAll(expectedRevision) is called.

- adopt() creates/replaces the slot (revision 1); only called from
  the deep-link handler in layout.tsx.
- recordEdit() updates slot content without releasing it; empty content
  keeps the slot alive so typing fresh text remains pinned-scope.
- clearAll(rev?) / release() destroy the slot; clearAll returns false
  on stale revision for submit-lifecycle guards (T7).
- editor-input.ts checks pinned BEFORE portable in the homepage-carry
  effect: if pinnedSlot.directory === dir, project its content and skip
  portable consumption. handleInput routes edits to pinned (when slot
  matches) or portable (otherwise).
- layout.tsx handleDeepLinks calls pinned.adopt() for link.prompt so
  the prefill is directory-scoped from the moment the deep-link lands.

16 tests in pinned-draft.test.ts; all 84 prompt-input tests pass; typecheck clean.
On first boot after upgrading to v7, migrates the current workspace's
legacy route-scoped homepage draft (workspace:prompt) into the portable
runtime owner, then quarantines the source slot so future portable carries
are not clobbered. Sets a global sentinel (prompt-portable-migration) to
complete so the migration runs only once. On any failure the sentinel stays
non-complete and the migration retries on next boot.

PR1 scope: only the currently-opened directory is handled. Other workspaces'
drafts remain in their route-scoped stores and self-heal through subsequent
record() calls when the user visits those homepages.
@coderabbitai

coderabbitai Bot commented May 19, 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 42 minutes and 14 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 575ab216-23a1-406b-9bb9-456ecc5f4234

📥 Commits

Reviewing files that changed from the base of the PR and between d8a8d8a and b0c21bf.

📒 Files selected for processing (8)
  • packages/app/src/components/prompt-input/context-items.test.ts
  • packages/app/src/components/prompt-input/context-items.tsx
  • packages/app/src/components/prompt-input/editor-input.ts
  • packages/app/src/components/prompt-input/homepage-migration-storage.test.ts
  • packages/app/src/components/prompt-input/homepage-migration-storage.ts
  • packages/app/src/components/prompt-input/mention-metadata.test.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/pages/layout.tsx
📝 Walkthrough

Walkthrough

This PR implements comprehensive cross-workspace draft isolation to prevent prompts and file references from leaking across directories. It replaces global draft carry-over and history storage with directory-scoped owners (portable and pinned), structured mention metadata with fingerprinting, and ownership-aware submission with revision guards. A v7→v8 migration adopts legacy homepage prompts into the new system.

Changes

Cross-Workspace Draft and History Isolation with Ownership-Aware Submission

Layer / File(s) Summary
Path canonicalization and external chip detection
packages/app/src/components/prompt-input/path-canonical.ts, path-canonical.test.ts
Core utilities detect POSIX/Windows/UNC absolute paths, convert relative to absolute, check directory containment with platform-specific case rules, classify context items as external (outside workspace), and generate compact labels with extension preservation.
Mention metadata capture and resolution
packages/app/src/components/prompt-input/mention-metadata.ts, mention-metadata.test.ts
Replaces free-text @mention parsing with structured metadata: captureCommentMentions scans at edit time, computes fingerprints (normalized window checksums), and records resolved paths; resolveCommentMentions strictly validates by position and fingerprint at submit time, rejecting mentions without metadata (no fallback).
Portable and pinned draft owners
packages/app/src/components/prompt-input/portable-draft.ts, portable-draft.test.ts, pinned-draft.ts, pinned-draft.test.ts
Two-owner system: PortableDraftOwner mirrors homepage edits and moves to new directories (bumping revision and updating source directory), while PinnedDraftOwner holds directory-bound deep-link slots (revision 1 on adopt). Both canonicalize file paths against source directory so references remain anchored even if homepage moves.
Submit ownership detection and revision guards
packages/app/src/components/prompt-input/submit-ownership.ts, submit-ownership.test.ts, submit.ts (ownership-aware sections)
Determines ownership (route/portable/pinned) for submission: on homepage, selects portable or pinned only when directory matches (pinned takes precedence). Captures revisions before awaiting, enforces revision matches during clear/clearAll to prevent losing newly typed content if ownership diverges during async gaps.
Editor-input homepage vs session branching
packages/app/src/components/prompt-input/editor-input.ts
Route-aware editor reconciliation: on homepage, replays pinned draft if bound to directory, else consumes portable snapshot. During edits, mirrors prompt/context/images to active owner (pinned if bound, else portable), tracking resolvedMentionsMap from context items. Session routes never mirror to owners.
Directory-scoped prompt history with LRU cache
packages/app/src/components/prompt-input/history-store-factory.ts, history-navigation.ts, history-navigation.test.ts, history-comment-map.ts, history.ts
Replaces global history stores with per-directory persistent stores (max 12 directories LRU, evicts by last-used). createDirectoryHistoryStore isolates normal/shell history per directory. buildCommentByIDMap gates comment metadata when portable draft is active (prevents cross-workspace history bleed). History comments now persist resolvedMentions for replay.
V7→V8 homepage draft migration
packages/app/src/components/prompt-input/homepage-migration.ts, homepage-migration.test.ts, homepage-migration-storage.ts, homepage-migration-storage.test.ts
One-shot migration: runHomepageMigration reads idempotent global sentinel, loads legacy per-directory homepage prompts, adopts meaningful content into portable owner, clears legacy store, writes completion/failure sentinel with adoptedDirectory and failure reason. Platform-aware storage I/O delegates to desktop or web localStorage.
Comment context mention capture in sessions
packages/app/src/pages/session/use-session-comment-context.ts, use-session-comment-context.test.ts, packages/app/src/pages/session/file-tabs.tsx
createSessionCommentContext requires sourceFilesystemDirectory() input and calls captureCommentMentions when adding/updating comments. FileTabContent uses sdk.directory to capture resolvedMentions. Updated/added comments flow resolvedMentions through context API so mentions are persisted and replayed.
Request parts building with mention resolution
packages/app/src/components/prompt-input/build-request-parts.ts, build-request-parts.test.ts
Refactored request construction: replaces inline absolute() helper with toAbsoluteFilePath for canonical paths. Comment mention extraction switches from parseCommentMentions to resolveCommentMentions with metadata, dropping mentions when resolvedMentions is absent. Mention-derived file parts use match.resolvedPath for filename/URL.
Comment routing with workspace boundary checks
packages/app/src/components/prompt-input/comment-routing.ts
Adds safety guard to openComment: blocks absolute-like paths not under current sdk.directory, preventing out-of-workspace file routing. Uses useSDK and isAbsoluteLike for validation.
Context item UI with external chip rendering
packages/app/src/components/prompt-input/context-items.tsx, context-items.test.ts
PromptContextItems accepts optional sourceFilesystemDirectory prop, uses isExternalChip to classify paths. External chips show "External" tooltip (not directory/filename), gain data-external and aria-disabled attributes, disable click navigation, apply visual styling. Tests verify classification across all path types.
Prompt context API with replaceAll
packages/app/src/context/prompt.tsx, prompt.test.ts
Extended FileContextItem with optional resolvedMentions field. Added replaceAll(items: ContextItem[]) method to context API for atomic full replacement (regenerates keys). Wired through createPromptBinding and implemented in createPromptSession.
Layout and page integration
packages/app/src/pages/layout.tsx, session.tsx, packages/app/src/components/prompt-input.tsx
layout.tsx imports pinned draft owner and runs homepage migration on directory change. Deep-link handling now adopts prompts into pinned draft (avoiding carry-over) instead of URL embedding. session.tsx passes sourceFilesystemDirectory to comment context. prompt-input.tsx passes it to PromptContextItems.
Internationalization
packages/app/src/i18n/en.ts, zh.ts
English: prompt.context.externalFile: "External". Chinese: prompt.context.externalFile: "外部".
Draft carryover backward compatibility
packages/app/src/components/prompt-input/draft-carryover.ts, draft-carryover.test.ts
Reduces module to re-export shim of usePortableDraft from portable-draft.ts. Removes old carry-over functions and DraftSnapshot interface. Rewrites tests to validate new portable owner semantics.
Cross-workspace isolation regression tests
packages/app/src/components/prompt-input/draft-isolation.integration.test.ts
Comprehensive integration suite: verifies portable draft move from /A to /B with ownership re-anchoring and /A non-detection; portable ownership on destination homepage but not session routes; pinned precedence; stale-revision guards; path/mention anchoring to original source even after consumption.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Astro-Han/pawwork#329: Concurrent ownership/clear-restore logic in submit flow vs sessionID/isNewSession passing in the same module.
  • Astro-Han/pawwork#107: Parallel @mention handling refactoring in build-request-parts.ts (URL generation helper vs metadata-driven parsing and path derivation).
  • Astro-Han/pawwork#515: Overlapping submit readiness and hydration gating changes in same submission pipeline.

Suggested labels

enhancement, harness

Poem

🐇 Hops across workspaces with drafts now tied to home,
Mentions fingerprinted, paths can safely roam.
No more leaks between directories bold,
Each reference anchored to the workspace it holds.
Thumps paw on the solid foundation we've built! 🌳

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/prompt-draft-isolation

@github-actions github-actions Bot added app Application behavior and product flows ui Design system and user interface P2 Medium priority labels May 19, 2026

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

Suggested priority: P2 (includes user-path files (packages/app/src/components/prompt-input.tsx, packages/app/src/components/prompt-input/build-request-parts.test.ts, packages/app/src/components/prompt-input/build-request-parts.ts, packages/app/src/components/prompt-input/comment-routing.ts, packages/app/src/components/prompt-input/context-items.test.ts, packages/app/src/components/prompt-input/context-items.tsx, packages/app/src/components/prompt-input/draft-carryover.test.ts, packages/app/src/components/prompt-input/draft-carryover.ts, packages/app/src/components/prompt-input/draft-isolation.integration.test.ts, packages/app/src/components/prompt-input/editor-input.ts, packages/app/src/components/prompt-input/history-comment-map.ts, packages/app/src/components/prompt-input/history-navigation.test.ts, packages/app/src/components/prompt-input/history-navigation.ts, packages/app/src/components/prompt-input/history-store-factory.ts, packages/app/src/components/prompt-input/homepage-migration-storage.ts, packages/app/src/components/prompt-input/homepage-migration.test.ts, packages/app/src/components/prompt-input/homepage-migration.ts, packages/app/src/components/prompt-input/mention-metadata.test.ts, packages/app/src/components/prompt-input/mention-metadata.ts, packages/app/src/components/prompt-input/path-canonical.test.ts, packages/app/src/components/prompt-input/path-canonical.ts, packages/app/src/components/prompt-input/pinned-draft.test.ts, packages/app/src/components/prompt-input/pinned-draft.ts, packages/app/src/components/prompt-input/portable-draft.test.ts, packages/app/src/components/prompt-input/portable-draft.ts, packages/app/src/components/prompt-input/submit-ownership.test.ts, packages/app/src/components/prompt-input/submit.ts, packages/app/src/context/prompt.test.ts, packages/app/src/context/prompt.tsx, packages/app/src/i18n/en.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/layout.tsx)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added the bug Something isn't working label May 19, 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 refactors prompt draft management by introducing directory-scoped "portable" and "pinned" owners, along with a migration path for legacy homepage drafts. Feedback highlights a critical race condition in the submission logic that could lead to data loss if users type during asynchronous operations, and suggests moving ownership detection to the start of the process. Additionally, the migration trigger in the layout is noted as unreliable when the directory is not immediately available, and logic errors were identified in the new path canonicalization helpers regarding trailing slashes and empty labels for exact directory matches.

Comment thread packages/app/src/components/prompt-input/submit.ts Outdated
Comment thread packages/app/src/pages/layout.tsx Outdated
Comment thread packages/app/src/components/prompt-input/path-canonical.ts
Comment thread packages/app/src/components/prompt-input/path-canonical.ts
Astro-Han added 4 commits May 19, 2026 19:19
persisted() reaches usePlatform() and must run inside the reactive
root. The lazy ensureStores() in createHistoryNavigation triggered from
addToHistory() — which fires from submit's async tail — was landing
outside the root and throwing 'Platform context must be used within a
context provider', breaking every e2e spec that submits from the home
hero.

Pre-warm the cache during setup and on every sdk.directory change so
out-of-root cache reads are always hits.
detectSubmitOwnership() reads pinned.current() and portable.snapshot()
to freeze the revision used by confirmOwnerCleared. Running it after
worktree/session create awaits captured the revision AFTER a user's
mid-await typing, then cleared the owner under that bumped revision —
wiping the new typing.

Move the detection next to the other pre-await captures so the revision
matches submit-time state.
onMount fires once at boot, before currentDir() is populated by the
autoselect pass, so the migration silently skipped for the whole
session. Convert to a createEffect with a one-shot started flag so the
migration runs as soon as a directory becomes available; runHomepage
Migration is already idempotent via the sentinel.
isUnderDirectory previously returned false for directories with a
trailing separator and for the POSIX root '/'. compactFilePath returned
an empty label when the path exactly matched the source directory,
rendering an unlabelled chip. Trim trailing separators before comparing,
treat empty as root, recognise forward-slash UNC, and fall back to the
directory basename when the strip leaves no remainder.
@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 40 -> 48 (+8) 56 -> 48 (-8) 77 -> 64 (-13) 27 -> 14 (-13) 33.4 -> 33.3 (-0.1) 183.3 -> 116.7 (-66.6) 4 -> 3 (-1) 0 -> 0 (0) pass
default / long-session-input-lag 40 -> 40 (0) 64 -> 48 (-16) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 48 -> 48 (0) 72 -> 72 (0) 0 -> 0 (0) 0 -> 0 (0) 33.3 -> 16.8 (-16.5) 33.3 -> 33.4 (+0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 24 -> 16 (-8) 24 -> 16 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 32 (+8) 32 -> 48 (+16) 64 -> 65 (+1) 14 -> 15 (+1) 50 -> 50 (0) 133.3 -> 116.6 (-16.7) 2 -> 3 (+1) 0 -> 0 (0) pass
default / terminal-side-panel-open 56 -> 56 (0) 56 -> 64 (+8) 0 -> 0 (0) 0 -> 0 (0) 33.4 -> 33.4 (0) 33.4 -> 50.1 (+16.7) 0 -> 1 (+1) 0 -> 0 (0) pass
default / session-scroll-reading 32 -> 32 (0) 40 -> 32 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls
low-end / session-scroll-reading-long 64 -> 72 (+8) 88 -> 72 (-16) 80 -> 75 (-5) 56 -> 41 (-15) 16.8 -> 16.8 (0) 166.7 -> 233.3 (+66.6) 4 -> 4 (0) 0.011 -> 0.011 (0) pass
low-end / session-timeline-recompute 128 -> 128 (0) 136 -> 136 (0) 117 -> 115 (-2) 241 -> 212 (-29) 116.6 -> 100 (-16.6) 200.1 -> 183.3 (-16.8) 4 -> 4 (0) 0.081 -> 0.081 (0) pass
low-end / concurrent-shimmer-extreme 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass

Astro-Han added 3 commits May 19, 2026 20:16
…stays anchored to A

A homepage draft picked with @src/app.ts stored a relative path. After the
draft moved to workspace B, build-request-parts re-anchored it against B's
sessionDirectory and submitted /repo-B/src/app.ts — operating on a same-
name file in the wrong workspace.

Rewrite relative file-bearing paths in both prompt parts and context items
to absolute paths at record() time using the source filesystem directory.
After the move, those paths stay /repo-A/... and submit on B leaves them
unchanged (isAbsoluteLike). The fix also flows through legacy migration,
since migration goes through portable.record.

Adds canonicalization unit tests and a cross-workspace integration test
asserting the request URL is file:///repo-A/src/app.ts after A→B carry.
build-request-parts now drives comment file attachments off
item.resolvedMentions, but the real comment producers — file-tabs.tsx
addCommentToContext/updateCommentInContext and the createSessionCommentContext
controller used by review comments — never invoked captureCommentMentions.
Production submits silently dropped every @src/foo.ts in a comment body.

Run captureCommentMentions against sdk.directory on add and recapture on
every update so deleted mentions clear stale metadata, and plumb the source
filesystem directory accessor into createSessionCommentContext.

Adds tests that the real add path writes resolvedMentions and that an
update with no mentions overwrites stale ones with an empty array.
…eplay

ArrowUp recall calls replaceComments with PromptHistoryComment entries that
previously dropped resolvedMentions, so a history-replayed comment with
@src/foo.ts no longer attached the mentioned file.

Carry resolvedMentions through PromptHistoryComment, clonePromptHistoryComments,
historyComments(), and applyHistoryComments so recall produces the same
file parts the original submit would have.

@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 (3)
packages/app/src/components/prompt-input/history.ts (1)

110-120: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Include resolvedMentions in history entry equality checks.

isCommentEqual ignores the newly persisted resolvedMentions, so dedupe on Line 106 can treat semantically different history entries as identical and keep stale mention mappings.

Proposed fix
 function isCommentEqual(commentA: PromptHistoryComment, commentB: PromptHistoryComment) {
+  const mentionsA = commentA.resolvedMentions ?? []
+  const mentionsB = commentB.resolvedMentions ?? []
+  const sameMentions =
+    mentionsA.length === mentionsB.length &&
+    mentionsA.every((m, i) => {
+      const n = mentionsB[i]
+      return (
+        !!n &&
+        m.displayText === n.displayText &&
+        m.resolvedPath === n.resolvedPath &&
+        m.fingerprint === n.fingerprint &&
+        m.start === n.start &&
+        m.end === n.end
+      )
+    })
   return (
     commentA.path === commentB.path &&
     commentA.comment === commentB.comment &&
     commentA.origin === commentB.origin &&
     commentA.preview === commentB.preview &&
     commentA.selection.start === commentB.selection.start &&
     commentA.selection.end === commentB.selection.end &&
     commentA.selection.side === commentB.selection.side &&
-    commentA.selection.endSide === commentB.selection.endSide
+    commentA.selection.endSide === commentB.selection.endSide &&
+    sameMentions
   )
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/components/prompt-input/history.ts` around lines 110 - 120,
The equality function isCommentEqual currently omits the resolvedMentions field
so different history entries with different resolvedMentions can be treated as
identical; update isCommentEqual to include a comparison of
commentA.resolvedMentions and commentB.resolvedMentions (deep/equivalence check
appropriate for the type) so the dedupe logic that calls isCommentEqual
preserves distinct entries and their mention mappings; ensure the comparison
handles undefined/null and uses a stable deep-equality routine (or JSON/string
compare if ordering is stable) consistent with how resolvedMentions is
persisted.
packages/app/src/components/prompt-input/editor-input.ts (1)

209-247: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard owner-clear path when context chips still exist.

shouldReset ignores prompt.context.items(), so on homepage an empty editor with existing context chips can hit the reset branch and record an empty payload into portable/pinned owners (Line 224 onward). That clears ownership state while chips still exist in composer state.

Suggested fix
-    const shouldReset = !NON_EMPTY_TEXT.test(rawText) && !hasNonText && images.length === 0
+    const hasContextItems = prompt.context.items().length > 0
+    const shouldReset = !NON_EMPTY_TEXT.test(rawText) && !hasNonText && images.length === 0 && !hasContextItems
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/components/prompt-input/editor-input.ts` around lines 209 -
247, The reset branch is firing even when context chips remain because
shouldReset doesn't consider prompt.context.items(); update the guard so the
owner-clear logic only runs when context is empty. Specifically, either include
a check for prompt.context.items().length === 0 in the shouldReset expression
(alongside NON_EMPTY_TEXT, hasNonText, images) or add an early guard before the
owner-clear block (the if (!params.id) section) that returns/aborts if
prompt.context.items().length > 0; ensure this prevents calling
portable.record(...) or pinned.recordEdit(...) when context chips exist while
keeping the existing behavior that still resets mirror,
prompt.set(DEFAULT_PROMPT, 0), closePopover(), resetHistoryNavigation(), and
imperatives.queueScroll().
packages/app/src/components/prompt-input/submit.ts (1)

254-261: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve resolvedMentions when restoring failed route submits.

The route fallback rebuilds comment context from CommentItem, but this type/helper drops resolvedMentions. After any failed submit, a retry from the restored draft will silently lose metadata-backed @mentions, so buildRequestParts() no longer emits the mentioned file parts.

💡 Suggested fix
 type CommentItem = {
   path: string
   selection?: FileSelection
   comment?: string
   commentID?: string
   commentOrigin?: "review" | "file"
   preview?: string
+  resolvedMentions?: ResolvedMention[]
 }

 const restoreCommentItems = (items: CommentItem[]) => {
   for (const item of items) {
     prompt.context.add({
       type: "file",
       path: item.path,
       selection: item.selection,
       comment: item.comment,
       commentID: item.commentID,
       commentOrigin: item.commentOrigin,
       preview: item.preview,
+      resolvedMentions: item.resolvedMentions,
     })
   }
 }

You’ll also need to add the missing ResolvedMention type import at the top of the file.

Also applies to: 342-352

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/components/prompt-input/submit.ts` around lines 254 - 261,
The CommentItem type used to rebuild comment context during route fallback is
missing resolvedMentions which causes metadata-backed `@mentions` to be lost on
retry; update the CommentItem definition (and add the missing ResolvedMention
import) to include resolvedMentions?: ResolvedMention[] and then ensure the
restore helper that reconstructs comments for failed route submits
preserves/resets the resolvedMentions field so buildRequestParts() still emits
mentioned file parts — search for CommentItem and the restore/rebuild logic
(also present around the other occurrence referenced) and wire the
resolvedMentions through.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/app/src/components/prompt-input/context-items.tsx`:
- Around line 65-66: Remove the aria-disabled attribute from the chip wrapper
that conditionally sets aria-disabled={external ? "true" : undefined}; this
wrapper (the chip container tied to the external variable) should not advertise
itself as disabled because the remove button (the interactive remove button
rendered around lines 103-113) remains operable; simply delete the aria-disabled
prop and keep the existing visual disabled styling (cursor-not-allowed,
opacity-70) so assistive tech doesn't get a conflicting state while the remove
button stays interactive.

In `@packages/app/src/components/prompt-input/mention-metadata.test.ts`:
- Around line 135-149: In the "matches second of two same-name mentions when
first is gone" test, explicitly assert the number of resolved matches so it
cannot return two; after calling resolveCommentMentions({ comment: modified,
metadata }) add an assertion like expect(result.length).toBeLessThanOrEqual(1)
(or expect(result.length).toBe(1) if you expect exactly one) before iterating
over result; keep the existing checks that for any match returned
match.resolvedPath equals "/repo/src/a.ts". This touches the local variable
result produced by resolveCommentMentions and the existing loop over result.

---

Outside diff comments:
In `@packages/app/src/components/prompt-input/editor-input.ts`:
- Around line 209-247: The reset branch is firing even when context chips remain
because shouldReset doesn't consider prompt.context.items(); update the guard so
the owner-clear logic only runs when context is empty. Specifically, either
include a check for prompt.context.items().length === 0 in the shouldReset
expression (alongside NON_EMPTY_TEXT, hasNonText, images) or add an early guard
before the owner-clear block (the if (!params.id) section) that returns/aborts
if prompt.context.items().length > 0; ensure this prevents calling
portable.record(...) or pinned.recordEdit(...) when context chips exist while
keeping the existing behavior that still resets mirror,
prompt.set(DEFAULT_PROMPT, 0), closePopover(), resetHistoryNavigation(), and
imperatives.queueScroll().

In `@packages/app/src/components/prompt-input/history.ts`:
- Around line 110-120: The equality function isCommentEqual currently omits the
resolvedMentions field so different history entries with different
resolvedMentions can be treated as identical; update isCommentEqual to include a
comparison of commentA.resolvedMentions and commentB.resolvedMentions
(deep/equivalence check appropriate for the type) so the dedupe logic that calls
isCommentEqual preserves distinct entries and their mention mappings; ensure the
comparison handles undefined/null and uses a stable deep-equality routine (or
JSON/string compare if ordering is stable) consistent with how resolvedMentions
is persisted.

In `@packages/app/src/components/prompt-input/submit.ts`:
- Around line 254-261: The CommentItem type used to rebuild comment context
during route fallback is missing resolvedMentions which causes metadata-backed
`@mentions` to be lost on retry; update the CommentItem definition (and add the
missing ResolvedMention import) to include resolvedMentions?: ResolvedMention[]
and then ensure the restore helper that reconstructs comments for failed route
submits preserves/resets the resolvedMentions field so buildRequestParts() still
emits mentioned file parts — search for CommentItem and the restore/rebuild
logic (also present around the other occurrence referenced) and wire the
resolvedMentions through.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5422e03f-8afa-49d7-91f5-9f60cef1b74d

📥 Commits

Reviewing files that changed from the base of the PR and between 31236e7 and d8a8d8a.

📒 Files selected for processing (38)
  • packages/app/src/components/prompt-input.tsx
  • packages/app/src/components/prompt-input/build-request-parts.test.ts
  • packages/app/src/components/prompt-input/build-request-parts.ts
  • packages/app/src/components/prompt-input/comment-routing.ts
  • packages/app/src/components/prompt-input/context-items.test.ts
  • packages/app/src/components/prompt-input/context-items.tsx
  • packages/app/src/components/prompt-input/draft-carryover.test.ts
  • packages/app/src/components/prompt-input/draft-carryover.ts
  • packages/app/src/components/prompt-input/draft-isolation.integration.test.ts
  • packages/app/src/components/prompt-input/editor-input.ts
  • packages/app/src/components/prompt-input/history-comment-map.ts
  • packages/app/src/components/prompt-input/history-navigation.test.ts
  • packages/app/src/components/prompt-input/history-navigation.ts
  • packages/app/src/components/prompt-input/history-store-factory.ts
  • packages/app/src/components/prompt-input/history.ts
  • packages/app/src/components/prompt-input/homepage-migration-storage.test.ts
  • packages/app/src/components/prompt-input/homepage-migration-storage.ts
  • packages/app/src/components/prompt-input/homepage-migration.test.ts
  • packages/app/src/components/prompt-input/homepage-migration.ts
  • packages/app/src/components/prompt-input/mention-metadata.test.ts
  • packages/app/src/components/prompt-input/mention-metadata.ts
  • packages/app/src/components/prompt-input/path-canonical.test.ts
  • packages/app/src/components/prompt-input/path-canonical.ts
  • packages/app/src/components/prompt-input/pinned-draft.test.ts
  • packages/app/src/components/prompt-input/pinned-draft.ts
  • packages/app/src/components/prompt-input/portable-draft.test.ts
  • packages/app/src/components/prompt-input/portable-draft.ts
  • packages/app/src/components/prompt-input/submit-ownership.test.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/context/prompt.test.ts
  • packages/app/src/context/prompt.tsx
  • packages/app/src/i18n/en.ts
  • packages/app/src/i18n/zh.ts
  • packages/app/src/pages/layout.tsx
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/file-tabs.tsx
  • packages/app/src/pages/session/use-session-comment-context.test.ts
  • packages/app/src/pages/session/use-session-comment-context.ts

Comment thread packages/app/src/components/prompt-input/context-items.tsx Outdated
Comment thread packages/app/src/components/prompt-input/mention-metadata.test.ts
@Astro-Han

Copy link
Copy Markdown
Owner Author

Re: CodeRabbit's three outside-diff-range comments

  • editor-input.ts:209-247 (shouldReset ignores context chips): fixed in 44704c8.
  • submit.ts:254-261 (CommentItem drops resolvedMentions on route restore): fixed in 622e86f.
  • history.ts:110-120 (isCommentEqual should include resolvedMentions): skipping. captureCommentMentions is a pure function of (comment text, sourceFilesystemDirectory), and prompt history is per-directory after PR [Bug] Prompt drafts and history can leak across sessions or directories #750. Two history entries that produce the same text+selection+path+origin+preview tuple — but different resolvedMentions — would require the same workspace producing non-deterministic capture output for identical inputs, which can't happen. Dedupe stays correct without the field. Happy to revisit if a concrete reproduction shows up.

removePersisted returns a Promise on desktop (window.api.storeDelete) but
the layout-side clearLegacyHomepage adapter dropped it on the floor: the
async wrapper resolved immediately, a desktop reject became an unhandled
rejection, and runHomepageMigration wrote status: "complete" anyway. Next
boot would never retry the legacy cleanup.

Move the platform-aware delete into createMigrationStorageIO alongside read
and write, await it in clearLegacyHomepage, and add a unit test mocking
desktop removeItem to reject so we'd catch a regression of this shape.
Astro-Han added 4 commits May 19, 2026 21:03
shouldReset previously only looked at text/non-text/image parts in the
editor, so an empty textarea with surviving chips on the homepage would
fall into the reset branch and record {empty} into the portable/pinned
owner — clearing ownership while the chips were still rendered. Include
prompt.context.items().length in the guard.
…ubmit

CommentItem and restoreCommentItems lost resolvedMentions on the route
fallback path, so a failed submit followed by a retry would silently drop
metadata-backed @mentions from the rebuilt comments. Pass the field
through so buildRequestParts still emits the mentioned file parts.
The chip wrapper had aria-disabled="true" for external chips, but the
inner remove button stays fully operable. Advertising the container as
disabled gave assistive tech a state that conflicted with the button's
behaviour. The visual styling (cursor-not-allowed, dimmed opacity, the
External tooltip prefix) already communicates the disabled chip-action
state without misleading screen readers.
The test description claimed 'at most one resolves' but never enforced
the bound. A sloppy implementation that returned both same-name matches
would still pass. Add an explicit length cap.
@Astro-Han Astro-Han force-pushed the claude/prompt-draft-isolation branch from 422ac14 to b0c21bf Compare May 19, 2026 13:03
@Astro-Han Astro-Han merged commit c76939e into dev May 19, 2026
26 checks passed
@Astro-Han Astro-Han deleted the claude/prompt-draft-isolation branch May 19, 2026 13:26
Astro-Han added a commit that referenced this pull request May 20, 2026
Upstream history (anomalyco/opencode) is no longer a shared ancestor of
PawWork's dev branch, so `git merge` from upstream is physically blocked
by no-common-ancestor regardless of attributes. The driver only fires for
intra-PawWork 3-way merges, where it silently drops dev's PawWork-internal
changes when feature branches pull dev forward (most recently lost #765's
detectSubmitOwnership additions from PR B's dev merge, leaving #765's new
tests in the worktree without the symbols they import).

The carve-out goal — preserve PawWork UI when re-anchoring on upstream
opencode — now lives wherever the next anchor sync ends up: a one-off
checkout of the listed paths back to HEAD, reviewed as part of that
intentional event. No silent path-based override remains.

- .gitattributes: drop the pawwork-keep-ours block (LF pinning kept)
- packages/ui/script/verify-merge-driver.sh: delete (validates a driver
  that no longer exists)
Astro-Han added a commit that referenced this pull request May 20, 2026
Resolved SDK gen conflicts by regenerating from the merged OpenAPI source.
SDK now exposes:
- externalResult.list (from PR B)
- RateLimit, command-inline, draft-isolation surfaces from dev

Legacy question/blocker routes stay deleted (PR B's intent preserved).
typecheck + bun test pass on app/opencode/core/ui/sdk.
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 P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Prompt drafts and history can leak across sessions or directories

1 participant