diff --git a/docs/feature-map/README.md b/docs/feature-map/README.md index 5cee41a5b35..c2e5bd87c27 100644 --- a/docs/feature-map/README.md +++ b/docs/feature-map/README.md @@ -144,7 +144,7 @@ this area is reached from inside it unless stated otherwise. | Tool approvals | Approve or deny a tool call the agent proposes | Inline card in the transcript | `components/ApprovalCard.tsx` | `handlers/sessions_parts/live_turn.py` | `GET /api/approvals`, `POST /api/approvals/{id}/{action}` | | MCP elicitation | Answer a question an MCP server itself asks mid tool call: a small form built from the server's schema, or a link to visit and confirm. No trust setting answers it; the person does, or the wait runs out and the ask is cancelled | Inline card in the transcript | `pages/chat/ElicitationCard.tsx`, `pages/chat/page/transcriptRendering.tsx` | `dashboard/handlers/chat_elicitation.py`, `dashboard/chat_turn/elicitation_prompt.py`, `acp/elicitation.py` | `POST /api/chat/slots/{slot}/elicitation` | | Question cards | Agent asks a multiple-choice question in chat and waits; the answers return as the tool result and show as a collapsible "N questions answered" card under the tool row | Pending: card above the composer. Answered: \"N questions answered\" card under the `ask_question` tool row in the transcript | `components/PendingQuestionCard.tsx`, `pages/chat/AskAnswersCard.tsx` | `handlers/ask_question.py` | `POST /api/ask-question`, `GET /api/ask-question/pending`, `POST /api/ask-question/dismiss`, `POST /api/ask-question/{ask_id}/answer`, `POST /api/agent-ask/open`, `POST /api/agent-ask/{ask_id}/wait`, `POST /api/agent-ask/{ask_id}/withdraw` | -| Files in chat | Browse, attach, and upload workspace files, including signature-verified audio containers; search the rail by file NAME or by file CONTENTS, including the text inside Word/PowerPoint/Excel documents; preview one in the side panel — a .docx renders its headings, lists, tables and bold/italic runs, falling back to extracted plaintext; right-click a file row → Download saves it to the machine running the browser through the credential-gated download endpoint; drag a file or folder row onto the composer to mention it where it is dropped, the same `@` mention the row's **Add to chat** inserts at the caret (a folder whose path holds a space or `@` has no folder reference, so the drop is refused with a note and the row menu shows **Add to chat** disabled with the same reason); complete a relative path in the composer by typing `./` or `../`, which lists one directory level of the project (never above its root); the header carries the per-project actions — reveal the project directory, and open a terminal already in it; the side-panel preview's ⋯ menu offers **Open in Files**, which opens the same file in the Files app's own tabbed tree through its `?path=` deep link (`/file-explorer?path=`): the file lands in the tab whose root already contains it with the folders between expanded, or in a new tab rooted at its folder, and the Files backend — not the link — decides whether the path may be shown; the row is offered only while the Files app is enabled at its native route and the path is absolute; a path named in the transcript becomes a clickable chip (and a diff block or tool line offers **Open**) only once the gateway confirms it is a file or folder on disk, every path a view shows being asked about in one request | Chat right panel → Files tab, Name/Content toggle below the search field; right-click a file row → **Download**; drag a tree row onto the composer; composer attach; composer `./` / `../` path token; Files tab header → reveal button and **Project actions** → **Open terminal in the project directory**, whose row carries a one-line note saying what a terminal is; side-panel file preview → ⋯ → **Open in Files**; a path chip, diff-block header or tool-call line in the transcript | `pages/chat/FileBrowserRail.tsx`, `pages/chat/FilesHomePanel.tsx`, `api/fileGrep.ts`, `components/FileRenderers.tsx`, `components/MarkdownPanelHeader.tsx` (`OverflowMenu` — the **Open in Files** row), `apps/file-explorer/deepLink.ts` + `apps/file-explorer/FileExplorerPage.tsx` (the `?path=` deep link's URL shape and its consumption), `pierre/PierreWorkspaceTreeImpl.tsx`, `lib/treeEntryDrag.ts`, `components/composerTreeDrop.ts`, `components/ChatInput.tsx`, `components/FilePickerMenu.tsx`, `components/composerTokens.ts`, `hooks/usePathKind.ts` (the shared existence cache and batch behind transcript path chips, `components/DiffBlock.tsx` and `pages/chat/ToolCallLine.tsx`) | `handlers/files.py`, `dashboard/file_api/pinned_io.py` (`api_file_kinds`), `src/kiro_crew/apps/routes_proxy.py` (`handle_app_api_proxy`), `src/kiro_crew/apps/builtins/file_explorer/server.py` (`_h_resolve`) | `GET /api/file-read`, `POST /api/file-kinds` (the batched path-kind probe), `GET /api/file-office-preview`, `GET /api/file-download`, `POST /api/file-grep`, `GET /api/file-search`, `GET /api/path-complete`, `POST /api/upload`, `POST /api/upload/file`, `GET /apps/file-explorer/api/resolve` (the deep link's one probe) | +| Files in chat | Browse, attach, and upload workspace files, including signature-verified audio containers; search the rail by file NAME or by file CONTENTS, including the text inside Word/PowerPoint/Excel documents; preview one in the side panel — a .docx renders its headings, lists, tables and bold/italic runs, falling back to extracted plaintext; right-click a file row → Download saves it to the machine running the browser through the credential-gated download endpoint; drag a file or folder row onto the composer to mention it where it is dropped, the same `@` mention the row's **Add to chat** inserts at the caret (a folder whose path holds a space or `@` has no folder reference, so the drop is refused with a note and the row menu shows **Add to chat** disabled with the same reason); complete a relative path in the composer by typing `./` or `../`, which lists one directory level of the project (never above its root); the header carries the per-project actions — reveal the project directory, and open a terminal already in it; the side-panel preview's ⋯ menu offers **Open in Files**, which opens the same file in the Files app's own tabbed tree through its `?path=` deep link (`/file-explorer?path=`): the file lands in the tab whose root already contains it with the folders between expanded, or in a new tab rooted at its folder, and the Files backend — not the link — decides whether the path may be shown; the row is offered only while the Files app is enabled at its native route and the path is absolute; an OS file dropped anywhere on the Files rail uploads into the project root, which the drop overlay names, and a directory row's menu → **Upload files…** picks files to upload into that directory; either refuses a name that already exists there with a notice instead of replacing the file; a path named in the transcript becomes a clickable chip (and a diff block or tool line offers **Open**) only once the gateway confirms it is a file or folder on disk, every path a view shows being asked about in one request | Chat right panel → Files tab, Name/Content toggle below the search field; right-click a file row → **Download**; drag a tree row onto the composer; composer attach; composer `./` / `../` path token; Files tab header → reveal button and **Project actions** → **Open terminal in the project directory**, whose row carries a one-line note saying what a terminal is; side-panel file preview → ⋯ → **Open in Files**; a path chip, diff-block header or tool-call line in the transcript; drag an OS file onto the Files rail (the drop overlay), or a directory row's menu → **Upload files…** | `pages/chat/FileBrowserRail.tsx`, `pages/chat/FilesHomePanel.tsx`, `api/fileGrep.ts`, `components/FileRenderers.tsx`, `components/MarkdownPanelHeader.tsx` (`OverflowMenu` — the **Open in Files** row), `apps/file-explorer/deepLink.ts` + `apps/file-explorer/FileExplorerPage.tsx` (the `?path=` deep link's URL shape and its consumption), `pierre/PierreWorkspaceTreeImpl.tsx`, `lib/treeEntryDrag.ts`, `components/composerTreeDrop.ts`, `components/ChatInput.tsx`, `components/FilePickerMenu.tsx`, `components/composerTokens.ts`, `pages/chat/useDirectoryUpload.tsx` (the upload and error-notice hook shared by the drop overlay and the picker; no new user-facing entry point), `hooks/usePathKind.ts` (the shared existence cache and batch behind transcript path chips, `components/DiffBlock.tsx` and `pages/chat/ToolCallLine.tsx`) | `handlers/files.py`, `dashboard/file_api/pinned_io.py` (`api_file_kinds`), `src/kiro_crew/apps/routes_proxy.py` (`handle_app_api_proxy`), `src/kiro_crew/apps/builtins/file_explorer/server.py` (`_h_resolve`) | `GET /api/file-read`, `POST /api/file-kinds` (the batched path-kind probe), `GET /api/file-office-preview`, `GET /api/file-download`, `POST /api/file-grep`, `GET /api/file-search`, `GET /api/path-complete`, `POST /api/upload`, `POST /api/upload/file`, `POST /api/directory-upload`, `GET /apps/file-explorer/api/resolve` (the deep link's one probe) | | Git tab | Side-panel tab listing every Git repository the chat works in, one collapsible section per repository headed by its path: the chat's project folder first (tagged Project) when it is a repository, then each repository the agent's tool calls edited or ran commands in, most recent first; each section shows branch, uncommitted changes and recent commits | Chat right panel → **Git** tab | `pages/chat/ActivityViewer.tsx`, `components/GitReposPanel.tsx`, `components/GitPanel.tsx` | `dashboard/git_repo_touches.py`, `file_api/project_dirs.py`, `file_api/git_panel.py` | `GET /api/project/git/repos`, `GET /api/project/git/status`, `GET /api/project/git/log` | | Slide preview for decks | Shows a `.pptx` / `.ppt` opened in the side panel as rendered slides (pager + thumbstrip) when LibreOffice is on the gateway host; without it, a row naming the install command (with an agent hand-off) over the `.pptx` text outline, or over the download card for `.ppt`, which has no text to extract; a deck whose visible text the credential screen would redact is not rendered and shows the redacted outline under a row saying so; on Windows (no descriptor-relative file operations to pin the render's staging) slides are not rendered and the outline is shown with the reason | Side panel → Files tab → click a deck; any deck chip in chat | `components/FileRenderers.tsx` (`OfficeSlidesRenderer`) | `handlers/office_slides.py` | `GET /api/file-office-slides`, `GET /api/file-office-slide`, `GET /api/file-office-preview` | | Terminal panel | Two shells on the gateway host: an app-wide docked PTY, and a per-chat terminal whose tab lives in that chat's panel state (opens on the chat's working dir; switches with the session). A shell that exits on its own closes its tab in the window attached to it, and the docked panel with its last tab (another window still holding that tab is not told, and its next dial opens a fresh shell); a known non-zero exit leaves a Terminal note in the bell feed, Windows included. Both tab strips, the docked terminal's and the chat right panel's, carry a close menu — **Close**, **Close other tabs**, **Close tabs to the right** and **Close all tabs**, which the terminal strip names **Close other terminals**, **Close terminals to the right** and **Close all terminals** and the workspace strip follows with a note that the Changes, Artifacts and Files views stay open; closing a terminal tab stops its shell, a batch that stops more than one shell asks first, counting and naming the shells, and one that would discard unsaved file edits names the files before closing. With `dashboard.terminal.reuse_current` on, Run in terminal always COPIES the command to the clipboard for a manual paste and never runs it — so the confirm dialog, which promises a copy, cannot mislead. When a settled terminal tab exists it is also revealed (the docked panel opens and that tab is made active, and a popped-out terminal window is raised) so the shell to paste into is in front; when none is (no tab, or a restored tab still unconfirmed during hydration) the raw command is copied verbatim with no tab minted and nothing executed | Header terminal toggle (docked); chat right panel → **+** menu → **Terminal**, or Files tab header → **Project actions** → **Open terminal in the project directory** — both go through one `openProjectTerminal` callback, so they cannot disagree about the cwd (per-chat); workspace chat composer (main chat or shared pane) → `! ` → **Review terminal command** → review → **Run** (fresh docked terminal); right-click a tab, or on touch hold it and lift without moving, for the close menu | `components/BottomTerminalPanel.tsx`, `components/TabCloseMenu.tsx`, `pages/chat/SidePanel.tsx`, `pages/chat/SidePanelTabChip.tsx` (the right panel's tab chips, their close menu and a terminal tab's live title), `pages/chat/SidePanelTabBodies.tsx` (the per-chat terminal's tab body), `pages/chat/FilesHomePanel.tsx`, `pages/ChatPage.tsx` (run-in-terminal reuse), `components/ChatPane.tsx`, `components/ChatInput.tsx`, `components/RunInTerminalConfirm.tsx`, `hooks/useTerminalCommand.ts`, `hooks/useBottomTerminal.ts` (`reuseCurrentTab`), `utils/terminalExitClose.ts` | `handlers/terminal.py` | `POST /api/terminal/sessions`, `GET /api/ws/terminal/{session_id}` | diff --git a/docs/system-specs/modules/learn-cron-dashboard.md b/docs/system-specs/modules/learn-cron-dashboard.md index 38268c89e5f..9cfd8edc26a 100644 --- a/docs/system-specs/modules/learn-cron-dashboard.md +++ b/docs/system-specs/modules/learn-cron-dashboard.md @@ -3737,6 +3737,9 @@ transaction lease remains unchanged. **Project git**: GET `/api/project/git?path=` (checked-out branch label for the activity panel). `path` is matched against the gateway's own known project directories (every live chat slot's `project` plus the recorded recent-projects list) using pure string normalisation (`expanduser` + `normpath`), with no filesystem access on the untrusted value; an unrecognised directory is refused with 403 plus an SEL audit record, and only the matched server-held string is stat'd. Response is `{"repo": false}`, `{"repo": true, "repoRoot", "isRepoRoot", "branch"}`, or the detached form `{"repo": true, "repoRoot", "isRepoRoot", "detached": true, "head"}`, plus `path` on the envelope. `isRepoRoot` is true only when the project directory IS the work-tree root, compared on the unredacted realpaths; a project nested inside a repository rooted higher up answers false, which is what the follow-up card reads to withhold its worktree action (`POST /api/worktree/create` refuses a toplevel above the session's project unless another session is scoped to that toplevel, and the card does not offer branching a whole enclosing repository from one folder of it). No `git` subprocess is spawned; repository discovery is the dashboard's one in-process walk, `git_repo_touches.find_git_root` (shared with the Git tab's repository list below: a linked worktree's `.git` file counts, `GIT_CEILING_DIRECTORIES` stops it as it stops git -- so a project under a ceiling reads as not a repository here exactly as the status and log routes' `git rev-parse --git-dir` reports it -- and the depth is bounded by `_GIT_ROOT_WALK_LIMIT`), and the branch comes from reading `.git/HEAD` directly (a `git` invocation parses repo-local config, which can carry `[include] path = ` and read an arbitrary file, so reading HEAD ourselves removes that class), and a linked worktree's `.git` pointer file (`gitdir: `) is followed to the real HEAD. Both metadata reads go through `hooks.safe_read_prefix` (realpath canonicalisation, sensitive-target rejection, `O_NOFOLLOW`, bounded read). `branch` and `head` pass through `redact` before being returned; `path` and `repoRoot` use the same redactor with one narrow path-specific exception, applied only on Darwin, for the fixed-shape `/private/var/folders/<2>/<30>/T` temp prefix, whose slash-inclusive entropy otherwise looks like a bare credential. The split sits at the END of the OS-generated `<2>/<30>` id: only that id is withheld from the scan, while the fixed literal `/T` and the complete user-controlled suffix still pass through `redact` together — handing `/T` to the redactor is what keeps the 40-byte credential window that would start on that `/` in scope. The SEL audit keeps the real path. Every failure mode degrades to "no branch" rather than erroring, since the only consumer is a decorative label. A detached HEAD gets a fixed 7-char prefix, not git's dynamic abbreviation. GET `/api/project/git/status?path=` and GET `/api/project/git/log?path=` use the same known-project boundary, widened by `_slot_git_repo_snapshot` to the git roots each live slot's tool calls touched (see **Project git repos** below). The status route first checks `os.path.isdir` and repeats that check after any subprocess failure; a missing or non-directory project returns `200 {"repo": false, "files": []}`. It performs no in-process inspection of `.git` or anything `.git` points to. Instead it runs sandboxed Git with `LC_ALL=C` and `LANGUAGE=C`: a bounded `git rev-parse --git-dir` probe owns repository discovery. A nonzero probe whose bounded stderr contains Git's `not a git repository` verdict is confirmed absence and returns `200 {"repo": false, "files": []}`. Symlink markers, gitdir files, parent-repository discovery, ceiling and cross-filesystem rules therefore follow Git. Any other probe failure, including sandbox refusal, dubious ownership, permission failure, timeout, kill, or missing executable, returns 503 with `code: "git_status_unavailable"` unless the project directory disappeared. After a successful probe, a nonzero `git branch --show-current` (which validates corrupt symbolic HEAD while accepting detached and unborn states), a failed or empty `git rev-parse --show-toplevel`, or a nonzero `git status` is also 503. **The filter-driver refusal is 503 too, not an empty listing, and it carries its OWN code.** A repository whose own config names a `filter.*.{clean,smudge,process}` driver -- anything `git lfs install --local` touched -- is refused because `git status` re-hashes modified files through a `clean` driver on every 5 s poll, and the probe refuses under its own separate cause when it cannot prove a driver absent. The match is deliberately WIDER than execution: `smudge` fires on checkout rather than on the status re-hash, a driver that no `.gitattributes` path maps to never runs at all, and the probe-failure branch refuses without knowing a driver exists. Refusing all three is right, since none of them can be proven safe -- but the user-facing message therefore says the config DECLARES a driver and that policy refuses the check, and never claims a program ran. Spelling that refusal `200 {"repo": true, "files": []}` makes it byte-identical to a genuinely clean repository, so the panel draws its green clean badge over a working tree whose status was never read; the panel's one job is surfacing uncommitted changes, and a confident "none" is worse there than no answer at all. The code is `git_status_filter_refused`, distinct from the `git_status_unavailable` its four siblings use, because this condition is not an outage: the cause is knowable, and for the `declared` cause it is permanent for that repository with no retry clearing it while the config stands (the `unreadable` cause is the one that can clear on its own, which is why only the declared copy promises permanence). One code for both would tell an LFS user their repository is broken on every poll forever. The panel renders it as a SINGLE `ErrorNotice` whose message names the cause, rather than the two generic notices the outage path shows: both routes refuse together because the cause is repo-level, so two boxes would report one condition twice. The refusal notice is TITLED and the outage notice is deliberately untitled -- titling both put two red boxes opening on the same word beside each other while they carry opposite retry semantics, so the difference between permanent and transient was a body-read away; titled against untitled separates them at a glance, and the outage title was restating its own first clause anyway. The refusal title names the OUTCOME ("Kiro can't check this repository for changes") rather than the mechanism: "status checks are off" collided with the outage string's "couldn't read the repository status", so one panel used the word status for two conditions of different severity. A coded status refusal reaching the non-coalesced branch renders that same title and localized copy, because the backend's English sentence under a localized heading is mixed-language copy in the other twelve catalogs. The header REFRESH control is also inert while BOTH routes refuse, and stays live for an outage: the refusal copy promises that retrying will not help, a re-fetch returns the same 503 every time, and a live control over that sentence contradicts it. It is disabled rather than hidden so the header does not reflow, and the glyph itself switches to the slashed variant, because dimming plus a hover tooltip did not tell a reader the control was dead. It is keyed on both routes rather than on the coalescing predicate deliberately: the refusal is permanent only while the repository config stands, and the notice's own agent hand-off invites changing it in the same window. The status query then recovers on its own 5 s interval while the log query -- no interval, 30 s `staleTime`, and a branch-change effect that cannot fire on the FIRST successful status because it needs a previous marker -- still holds its cached refusal. Keying the inert state on the coalescing predicate would therefore disable the one control able to clear that, leaving a notice claiming history cannot be shown while the changes list renders beneath it. The panel also refetches the log automatically as soon as the status route comes back CLEAN (not merely stops refusing: a status route failing for its own reason proves nothing about the config). The 503 body carries a `cause` discriminator alongside the code -- `declared` when repo config names a driver, `unreadable` when the probe could not prove one absent -- and the panel renders one localized sentence per cause. One code, because it is ONE condition (refused by policy) with two causes; a disjunction in the copy ("declares a driver, or could not be read") made the common case, an LFS repository on every visit, read a question the guard had already answered, and a reader could only partly follow it. Naming a driver as fact in the probe-failure branch would still be wrong, which is why that branch answers its own cause rather than borrowing this one. It stays an `ErrorNotice` with the `askAgent` hand-off because the string originates in a `{error, code}` body, which is what the `errors-use-error-notice` rule scopes on, and because a repo-supplied filter driver is precisely what the agent can explain or change -- routing it around that surface would strip the one affordance that helps here. HEAD is still validated BEFORE the filter guard, which `test_status_checks_corrupt_head_before_filter_refusal` asserts by spying that the guard is never reached, and the two codes now witness the ordering as well. A successful listing is capped at 500 entries and carries `truncated: true` when entries were dropped; the panel renders that as a capped count (`500+ uncommitted`) plus a `showing first 500` note on the Changes header, because without it the cap read as the total and an undercount was presented as a count. `/api/project/git/log` shares the status route's discovery boundary through the same `_probe_git_dir` probe (`LC_ALL=C`, `LANGUAGE=C`, bounded stderr) and the same verdict predicate, `_is_not_a_repo_verdict`, which is the one documented classification contract for both routes: a nonzero probe carrying Git's `not a git repository` verdict, or a project directory that disappeared, is `200 {"repo": false, "commits": []}`; every other probe failure (sandbox refusal, spawn failure, dubious ownership, permission failure, timeout, kill, corrupt repository metadata) is 503 with `code: "git_log_unavailable"`. The log route also applies the same filter-driver refusal and answers it with 503 `code: "git_log_filter_refused"`, since an empty commit list is what a repository with no commits legitimately returns. A nonzero `git log` AFTER a successful probe deliberately stays `200 {"repo": true, "commits": []}`: an unborn repository exits 128 there, so that path is genuinely ambiguous and separating it would need a stderr match of its own. The panel renders `git_log_unavailable` as the log-side outage notice -- untitled, with the localized `log_failed` copy rather than the backend's English sentence, mirroring `git_status_unavailable` -- and suppresses the clean-repository empty state while the log route has failed, because "no commits to display" beside a notice saying history could not be read presents an outage as an empty history. The Git panel treats only `repo: false` without a simultaneous status error as a terminal no-repository state: it replaces the loading label, hides the clean badge and clean-repository empty copy, and tells the user to click the folder name below the message box. The other two `['git-status', …]` consumers now receive 503s on operational failure where they got 200s, and each was checked rather than assumed. `FileBrowserRail` renders an inline `ErrorNotice` with the agent hand-off on an error in All mode, and deliberately not in Changed mode because the Pierre tree owns the same query failure there. `PierreWorkspaceTreeImpl` treats a status error as terminal for a Changed-mode load and renders that notice with a structured report. Both of those notices RECOGNISE the refusal codes and render the panel's own refusal copy rather than their generic failed copy, because the refusal is permanent for an LFS-configured repository: generic copy there would say "failed" on every 5 s poll, forever, with no cause and no statement that retrying cannot help — the exact outage spelling this refusal code exists to end. The recognition lives once in `website/src/utils/gitStatusError.ts` (`gitErrorCode`, `isGitFilterRefusal`, `gitFilterRefusalCause`, `gitFilterRefusalCopyKey`) and all three components read it, so one condition has one wording rather than three to keep in step. Neither of those two carries the panel's TITLE, deliberately: `ErrorNotice`'s `inline` variant lays title and message out as flex siblings, so at the rail's 300-520px and the tree's width the title wraps into a stack of two-word fragments beside a narrow column of message — visible in `capture-git-filter-refusal-surfaces.mjs`'s frames, which is what settled it. The title exists in the panel to separate a refusal from a coexisting outage notice, and neither of these surfaces renders a second notice beside this one. The status outage string also has a variant used when the log route failed too, which names the half it covers ("Couldn't read the changes list."): the default wording promises a commit list that is still on screen, so beside a notice saying history cannot be shown the two boxes first contradicted each other and then, once the clause was merely dropped, read as one failure reported twice. Naming each half is what separates them. Neither of the two reads non-200 as "not a repository"; both read `repo` only from a 200 body, so a refusal reaches them as absent data rather than as a negative answer. **Project git repos**: GET `/api/project/git/repos?slot=` lists the repositories the chat side panel's Git tab shows for one session, because most sessions start in the shared workspace (not a repository) and work in repositories elsewhere. `kiro_crew.dashboard.git_repo_touches` notices them from the session's own tool calls in `chat_runner` (both `tool_call` and its `tool_call_update` refinement): an edit call's target paths (`edit_target_strings`: the params walk the edit gates run, `target_paths`, plus patch headers, with no path expansion), a shell call's `working_dir`/`cwd`, and a leading `cd `/`pushd ` or `git -C ` in its command (absolute or `~` only; a relative edit path resolves against the session's project and is dropped without one). Each candidate walks up to the nearest `.git` entry through `find_git_root`, the same walk the branch label above uses (a linked worktree's `.git` file counts; `GIT_CEILING_DIRECTORIES` stops the walk as it stops git), and the real root is kept on the slot (`_ChatSlot._git_repos`, oldest to newest, capped at `MAX_SLOT_GIT_REPOS`) unless it is the home directory, the filesystem root or a sensitive path. The walk never runs on the runner's event path: `note_tool_call` snapshots bounded raw candidate strings and a per-call `truncated` bit on the loop, retaining them only until `finish_tool_call` confirms successful execution. Refused, failed, cancelled and unfinished calls discard their candidates and loss bit; turn cleanup also clears proposal-eviction uncertainty. Executed calls queue their candidates on the slot (`_git_repo_walk_pending`, distinct, a retouched path moved to the newest end, bounded by `_MAX_PENDING_CANDIDATES_PER_SLOT` with the oldest dropped first) and returns; one walker task per slot (`_git_repo_walk_task`, held in `state._background_tasks` like every other dashboard fire-and-forget task) resolves the whole pending list in a worker thread and loops until nothing is queued, so a `stat` that hangs on a dead mount holds that task and never the turn's stream, the runner's next event is handled at once, calls land in call order (two concurrent walks could list an older touch as the newest), and a burst of calls is one thread hop. A slot closed mid-walk receives its roots on an object nothing reads; a loop shut down mid-walk cancels the task and asyncio drops the thread's late result. No broadcast follows a walk: the Git tab polls the listing (`refetchInterval` 10 s), so a root recorded after the turn's event shows on the next poll. Each listed repository is a collapsible section whose status query polls every 5 s only while it is open, and only the `MAX_POLLING_SECTIONS` (3) most recently opened sections poll at once: repositories the agent edited open by themselves, so without the cap every dirty repository would run its git subprocesses on every interval. A section the user opens becomes the newest and always polls; an open section past the cap fetched once and refreshes on window focus and on its Refresh control. The list is in memory only; the first listing after a restore folds in the roots the transcript's `file_changes` rows resolve to (`_git_repos_seeded`), so edited repositories come back while a repository the agent only ran commands in reappears on its next touch. The response is `{repos: [{path, source}], omitted: , listed: , incomplete: }`: `incomplete` is always present and reflects the sticky, slot-local `_git_repos_incomplete` flag, or a row refused for redaction in this response. Candidate-count, edit-traversal, patch-parser, command-directory, command-text/argv, refinement-merge and path-length cuts carry their loss through the call's snapshot, including an empty snapshot, and publish it only on execution. Evicting executed paths from the walker backlog also sets the flag. Pending-proposal eviction retains only one turn-local uncertainty bit, not an unbounded tombstone list: a later successful result with no retained proposal conservatively marks the list incomplete; a refusal or turn cleanup does not. Evicting the oldest terminal receipt marks nothing incomplete: the refinement a receipt waits for is read from the same wire update as its result, so it has arrived long before 64 later results are recorded, and most receipts belong to calls that name no path. Worker-thread expansion/canonicalization path bounds and the ancestor-walk depth budget return loss alongside roots, as do transcript-seed count/path bounds, and the listing folds those flags back onto the slot even when no roots survived. Home, filesystem-root, sensitive, unanchored and not-a-repository exclusions do not themselves mean truncation. The flag is not a missing-repository count and never inflates `omitted`; the Git tab renders it once per snapshot as a localized muted informational line, even on an empty list: "Some repositories the agent worked in may not be listed. Each one appears here once the agent works in it again." No path expansion or filesystem access is added to loop-side bookkeeping. `omitted` is always present and counts distinct repositories this session worked in that the list leaves out because only the `MAX_SLOT_GIT_REPOS` most recent are kept, including transcript seeding: evicted roots are remembered by path (`_ChatSlot._git_repos_evicted`, capped at `MAX_SLOT_EVICTED_GIT_REPOS`) and a root touched again leaves that list, so revisits never inflate it; older evicted roots are kept only as a count (`_git_repos_evicted_overflow`), so past that bound the number is an upper estimate (a forgotten root that returns can be counted twice). Deleted repositories filtered from the response do not count. `listed` counts the agent-touched repositories the response actually lists (the project's own included only when the agent worked in it). The Git tab's capacity notice reads its numbers from `listed` and `omitted` ("The agent worked in repositories; the most recent are listed."), so an untouched project row or a deleted repository never skews them; the notice is a localized informational line below the list, shown when the count is positive, and names no action because the server keeps nothing it could reveal. When the list is non-empty the tab opens with a one-line caption saying the rows are the repositories the agent worked in during this chat, with a second sentence appended only when the chat's own project is among them to say the project folder is listed first, so the Project tag's meaning is visible without its tooltip; the empty state says how the list fills (the agent editing files or running commands in a repository); the listing-failed notice carries a Retry that refetches beside the agent hand-off. The session's project comes first when it sits in a repository (`source: "project"`, `path` the project directory itself), then every touched root that still has a `.git`, most recent first (`source: "agent"`), each `path` accepted verbatim by the status and log routes. A row whose path the credential redactor (`_redact_project_path`) would mask is left out of `repos` and of `listed` and sets `incomplete` for that response: the masked string is not a path those routes accept, so the row could only show an error, and the unmasked path never reaches the client. The touched roots join those routes' allow-list (and only theirs: the tree, path completion and branch label keep the project-only boundary) because they are server-derived from the session's own tool calls, never from a request. An app token reads only a slot it owns on its own session and transcript (`deny_app_slot_session_access`, audited when the slot exists); any other slot is the same 404 as a missing one, and for an app token the status and log routes admit only the touched roots of the slots it owns (`app_owns_slot_session`). **Project tree**: GET `/api/project/tree?path=` (workspace file listing for the Files tab's Pierre tree, and for a chat side-panel `folder` tab whose path IS the chat's project root — `FolderPanel` renders the same `PierreWorkspaceTree` there so descending expands in place instead of re-targeting the tab, and falls back to its one-level `browseFiles` listing for every other directory and whenever this endpoint does not answer; while the payload is `truncated` the tab keeps the tree but sends its search box to the recursive server search (`/api/file-search`), whose matches stand in for the tree until the query is cleared (the tree stays mounted, hidden, so the folders the user expanded are still open when it returns), because the tree's own filter can only find what the capped tree lists; the matches are the server's merged with the tree's own paths that contain the query (case-insensitive, de-duplicated, the server's first), because the server search runs under its own scan budget and can miss a file the tree is already showing; a line above them (`search_beyond_tree`) says results can include files past the 10,000-item limit -- not that it covers the whole workspace, which that budget cannot promise -- since on a whole tree the same box narrows the rows in place, and the tab's Refresh re-runs that search too, as the listing body's Refresh does, because it is the retry beside a failed search -- because a directory path chip can name any directory on the gateway while this allow-list deliberately does not). File tabs start with their optional project-tree rail hidden so the document receives the full panel width; the toolbar toggle and persisted `mc-files-rail-open` preference keep it available on demand. Same known-project allow-list, sensitive-path refusal, and SEL auditing as `/api/project/git`. Inside a git repository the listing is `git ls-files -z --cached --others --exclude-standard` with `cwd=` (tracked + untracked, `.gitignore` honored; a project dir that is a repo subdirectory lists only its own subtree); outside one -- or when that listing fails (a nonzero exit: its 8 MiB output cap, its 15 s timeout, or no sandbox backend) -- it is ONE bounded breadth-first pass (`_project_tree_walk`, `os.scandir` per folder, never following a link) that skips tool and heavy directories (`_PROJECT_TREE_SKIP_DIRS`), walks every other dot-directory (e.g. `.worktrees`), drops every entry, directory or file, that is dot-named or sits under a dot-path (judged on the read folder's real path, ancestors above the project root included) when `is_sensitive_resolved_path` fences its real path (`_project_tree_fence`), and stops reading after `_PROJECT_TREE_SCAN_LIMIT` (20 × the row cap, 200,000) directory entries, so its cost is a function of that number rather than of the size of the tree (about half a microsecond an entry). The budget is shared so one large folder cannot starve its siblings, or their subfolders, wherever it sorts, without cutting a tree that fits: each folder of a depth, and the depths below as one more claimant, is guaranteed a floor of half the even split the depth starts with, and the folders are read in order, each up to everything the claimants after it do not need for their floors -- capped at the row cap, more entries than one folder could show -- and never less than an even split of what is left. So a tree of at most about half the scan limit whose folders each hold no more than the row cap is read whole, wherever its large folder sorts. A folder holding more than its share is cut there and named in `truncatedDirectories`; the entries it did not read, subfolders included, are unknown. Telling a full folder from a cut one costs one look-ahead entry, drawn from the same budget, so no listing reads more than the limit. A folder whose row falls past the cap is never read, and a read cut by the budget is never judged hidden-only. A subfolder is queued when its parent is listed and read one level later, so each queued folder carries the `(st_dev, st_ino)` its parent's listing recorded, and is read only if it is still that folder (`_project_tree_scandir`): on POSIX the name is opened `O_DIRECTORY | O_NOFOLLOW`, the descriptor `fstat`ed and compared, and the descriptor itself listed; on Windows, which lists only by name, the name is `lstat`ed, compared and refused as a link (a name-surrogate reparse point) just before the `scandir`, leaving the window between those two calls; a reparse folder that holds its own contents, such as a cloud-files placeholder, is walked like any other folder. A folder swapped in between -- renamed away with another directory or a link put at its name -- is shown as a row, named in `unreadableDirectories` (a row that claims nothing about its contents, never `truncatedDirectories`, which every consumer renders as the item limit), and nothing behind it is listed; a root swapped for a link after the handler resolved it fails its `O_NOFOLLOW` open the same way and is named `.`. Response `{root, paths: [], directories: [], repo, truncated?, truncatedDirectories: [], hiddenOnlyDirectories: [], unreadableDirectories: [], linkedDirectories: []}`. `paths` and `directories` TOGETHER carry at most `_PROJECT_TREE_MAX_ENTRIES` (10,000) rows on both branches (`_project_tree_allot`): directory rows are spent first, shallowest first (the walk's breadth-first discovery order; the git branch orders its derived folders by depth, then segment by segment, which is the same order), because a folder row is what lets the user navigate to the rest -- but never past half the cap while there are files to show, so a tree with more folders than the cap still lists its root files (`README.md` included) -- and the files share their rows round-robin across their direct parent directories, the root first, so no one folder takes every row; half is a floor for each side, not a split, and rows the files cannot use go back to folder rows without those folders' files then sharing the file rows; a file whose folder lost its row is not listed either. Empty directories on the filesystem-walk path are rows like any other. Folder rows are capped together with files rather than kept as an uncapped skeleton, because uncapped folder rows make the walk, the per-row redaction and the serialization linear in the size of the tree, and the tree is refetched every 10 s while it is open (paused while the read is failing), so a large non-git project dir would stall the gateway. The client bounds each read: 15 s for the file search and the tree (`FILE_SEARCH_TIMEOUT_MS`), 10 s for the browse and recent-project listings (`BROWSE_FILES_TIMEOUT_MS`); a timed-out or failed read gets a cause-keyed notice, with Refresh/Retry offered only for those two causes. On the walk the file fairness is over what the walk READ: a folder it never reached contributes no file and, having a row, is named in `truncatedDirectories`. `hiddenOnlyDirectories` is walk-only: it names the directories the walk leaves childless although they are not empty on disk (no file or directory the listing keeps remains, and every entry is a `_PROJECT_TREE_SKIP_DIRS` directory or a fenced entry, so a `.docker/` holding only a fenced `config.json` is hidden-only), so the tree can tell them from genuinely empty ones (`_bg/` holding only `.kiro/` is the reported case); a directory with a listed file or a kept subfolder is never in it even when it also holds hidden entries; the root is judged by the same rule and, when its top level holds only such entries, is named as `.` (it is no directory row of its own), because the payload would otherwise be the empty-workspace shape and the dashboard would call the workspace empty — the frontend then shows a root-specific notice in the tree's place instead of the empty-workspace one (`This workspace contains only hidden items (skipped tool and cache folders, protected entries)`, `root_hidden_only` — not the per-folder row's wording, which with no folder row above it read as being inside a folder), with no action row since there is nothing to act on; and the git branch returns `[]` by construction, because a directory row exists there only as an ancestor of a listed file, so an ignored-only folder is absent rather than childless and the only childless directory that branch can produce is a truncated one. `linkedDirectories` is walk-only too: a symlink to a directory is a visible, navigable entry (`ls` shows it), so it does NOT make its folder hidden-only; the walk never follows a link (`_project_tree_is_link`: a symlink or a name-surrogate reparse point such as a Windows junction, `platform_compat.lstat_is_name_surrogate`; against link cycles) and so never reads beneath it, so the walk lists it as a directory row of its own and names it here — nothing beneath it is listed, and the frontend puts `Link to another folder: contents not listed` (`row_linked`) beneath its row instead of calling the link, or the folder holding only links, empty; links are told apart among the entries the hidden/skip filter KEPT, because what a folder shows must be predictable from the name alone — a `node_modules` or `.cache` that is a link to a directory elsewhere is filtered exactly like its real twin and counts as a hidden entry (a folder holding nothing else is hidden-only), so link-ness the user cannot see never decides visibility; a link the filter keeps is a row, and a folder holding only kept links is not hidden-only; inside a repository the list is empty by construction (git lists a symlink as a file). `unreadableDirectories` is walk-only too: a folder is a row as soon as its parent's read names it, so a kept, non-link child the process cannot read (permission denied is the usual cause) is still a row of its own, and when its own read fails -- `scandir` refusing it, its listing failing part-way, in which case what it yielded first is discarded rather than shown as if it were the folder, or the `lstat` that records its identity failing after its parent listed it (a parent readable but not searchable) -- it is named here rather than in `truncatedDirectories`, so the tree shows the folder while its parent is not childless at all; any read failure counts, not only `EACCES`, because the parent listed the entry and a row that makes no claim about its contents is the honest rendering whatever stopped the read. An unreadable folder whose row falls past the row cap is neither a row nor named here (its parent is in `truncatedDirectories` instead): this list names only folders the payload shows, so the notice below never names a folder the tree has no row for, and the same holds for `hiddenOnlyDirectories` and `linkedDirectories`. Because the value behind that entry is a failed read, it is an error and reaches the user only through `ErrorNotice`: the frontend renders one above the tree naming the folders (`Folders not readable: `, `unreadable_folders`, hand-off on since the listing holds no draft; always the alert in the danger tone — an error is never toned down to a status or a muted register, only shown or, at the user's word, not shown. Its ✕ DISMISSES it: the control is named `dismiss_unreadable` (`Dismiss: hide this notice until the unreadable folders change`), carried as its `aria-label` AND its `title`, so the promise a remembered dismissal makes is read before the click by hover as well as by a screen reader; the click leaves, where the notice stood, a one-line `role="status"` "Notice hidden — N folders still unreadable (shown again when they change) — Undo" (`unreadable_dismissed`, a plural key on the count of folders the payload still names, + `undo_dismiss_unreadable`; muted, saying what the ✕ did and did not do — hidden, not fixed, how many folders are still unreadable and that the notice returns when they change — but never which folders: a count is not a toned-down copy of the error's text) for the product's one undo window (`MOVE_UNDO_MS` on `useHeldWindow`: held while the pointer is over that spot or focus is inside it, retired for good when the notice returns on its own, never carried across a remount; the focus hold is cleared at every swap of the spot's content, because the control that was activated had focus and a removed element fires no `focusout` — left alone, the hold would pin the offer open for the life of the mount — and when that control had focus, focus is handed to its counterpart, Undo after the ✕ and the ✕ after Undo, so a keyboard dismissal lands on the way back rather than at `` with the window running) whose Undo restores the alert and CLEARS the remembered set, so a mis-click on a ✕ that sits beside "Ask the agent" at the same weight has a visible way back; the dismissal remembers the folders the notice named for that project — per project directory, in the expansion memory's own store under its own key (`treeUnreadableDismissals.ts`, one `createProjectPathMemory` instance: module map for the remount, `localStorage` mirror for the reload, the same caps) — and while every folder the payload names is remembered the notice is not rendered, so a folder that stays unreadable by design is not red alarm chrome on every visit; a folder outside the remembered set brings the alert back with the whole list, and a remembered folder that drops out of the payload is forgotten (the set is pruned to each landed payload, never to a mount that has not heard from the server yet, and in a `truncated` payload a remembered folder absent from it is kept while the cut could be what hid it -- its parent is absent too or is itself in `truncatedDirectories` -- and forgotten when its parent is listed whole), so the same folder failing anew alerts again rather than inheriting its old dismissal), plans no state row beneath such a folder (nothing beneath it is known, and a status line inside Pierre's shadow root can carry neither the hand-off nor an error), and gives the folder's own row a lock marker whose accessible label points at that notice (`Not readable — see the notice above`, `row_unreadable_marker`, Pierre's decoration `title`: the tooltip and the accessible name of the decoration span; once the notice is dismissed the label says so instead, `Not readable — notice dismissed`, `row_unreadable_marker_dismissed`, so it never sends the reader to a notice that is not there) — a signpost to where the failure is reported, not a report of it: it states no failure, carries no hand-off of its own, and is fed by the same `unreadableDirectories` list the notice renders, so the reader can tell that folder from the ones that show nothing for another reason and can never see a marker the notice does not name; the folder is never called empty, and the notice and the markers follow the payload and go when the folders read again. Its files are never listed (nothing read them); the root itself failing is recorded as `.` — the walk then yields nothing, so without it the payload would be indistinguishable from a workspace with no files in it, and the root is no directory row of its own, so it is named only here (the frontend shows, in the tree's place, an `ErrorNotice` whose lead is `No permission to read the workspace folder` (`root_unreadable` — it names its cause, so it cannot be read as the host's own `Couldn't load the file tree`, which is a failed request where this listing answered; permission is the failure a directory that passed the `isdir` check produces, and a root removed between that check and the walk shows the same notice for one refresh) and whose message is the redacted `root` path in a span of its own, with a zero-width break hint after each `/` so a narrow rail wraps it at a segment boundary rather than mid-word (a Windows-shaped root — drive letter or UNC — is shown in its forward-slash form first, `normalizeWindowsPath`, since its backslash segments would get no break point; any other path is untouched) — display only: the notice's `report` carries the sentence and the real path to the agent — followed by the hand-off and a Refresh that re-asks the listing, the same two actions as the host's failed-listing notice, instead of the empty-workspace notice; one level down the row sits under the folder it qualifies, here no folder row exists to anchor it); and the git branch returns `[]` by construction for the same reason as `hiddenOnlyDirectories`: `--others` cannot scan a directory git cannot read, so it contributes no untracked file and, with no indexed file beneath it, is absent rather than childless, while an indexed path beneath it still comes from the index (`--cached` reads no directory) and makes it an ordinary populated row. The frontend passes those directories to Pierre as explicit trailing-slash paths, keeps the workspace-level truncation notice, and appends one synthetic state row beneath every folder the payload leaves childless (`website/src/pierre/treeStateRows.ts`): `Some items not shown: limit of 10,000 items reached` for a `truncatedDirectories` entry (checked first, since a folder that lost its contents to the cap is not empty; the workspace-level notice, `workspace_truncated`, names the same `limit of 10,000 items reached`, so the two are read as one fact; neither promises that every folder is listed, because the cap counts folder rows too), `Link to another folder: contents not listed` for a `linkedDirectories` entry (checked before hidden-only: a link is never walked, so it is neither truncated nor hidden-only), `Contains only hidden items (skipped tool and cache folders, protected entries)` (`row_hidden_only`) for a `hiddenOnlyDirectories` entry (the cause, as `row_truncated` names its own — the parenthetical says what "hidden" means here, items the listing hides by nature, not items the app cut off), and `Empty folder` otherwise (an `unreadableDirectories` entry is skipped before the choice, whatever else names it, so a folder nothing read is never called empty; the order only fixes what a malformed payload shows). A truncated folder's row also carries the `some items not shown` badge (`workspace_directory_truncated`; the same "not shown" vocabulary as its state row, so "hidden" is reserved for what the listing hides), and the badge yields while the folder is expanded over its own state row, so the cut is stated once at a time (except under an `Open in folder tab` row, which does not state the cut, so the badge stays). `@pierre/trees` renders rows for paths and has no slot for a status line, so each state row is fed to the model as one synthetic child path whose basename is the localized label followed by a trailing zero-width space (U+200B, `STATE_ROW_MARKER`) — the row's identity in the plan, and only that: a real file can end in U+200B too, so the styling never keys on the path. The wrapper's `renderRowDecoration` gives a planned row (a path in the plan's set, the same set its selection and context-menu guards read) a decoration icon `STATE_ROW_ICON` — an `` with no symbol behind it — and `PIERRE_TREE_STATE_ROW_CSS` selects the row carrying that decoration (`:has()`), never the path suffix and never the label, so a real file named `x` plus U+200B, or one named "Empty folder", keeps its icon, its pointer and its open; the wrapper keeps the planned row inert (no open, no selection, no context menu) with one exception: where the host passes `onOpenFolder` (the Files rail, in the side panel and the pinned Files tab), a `truncatedDirectories` row reads `Open in folder tab` (`row_truncated_open`), carries its own decoration (`STATE_ROW_ACTION_ICON`, link-styled with the pointer) and, when selected, is deselected and hands its project-relative folder to the host, which opens that folder's tab (`FolderPanel`, the per-folder listing and name search with a growing page); that row still has no open, drag or context menu, and the folder's `some items not shown` badge stays on beside it because the row no longer names the cut, and a `/` in a translated label is rewritten to the division slash (U+2215) so a label can never mint a subfolder. While the panel's filter is active only the state rows whose FOLDER matches the query are fed to the model (`stateRowsMatchingFilter`: the folder's listed `dir/` path, lower-cased, contains the query as Pierre normalizes it — trimmed, backslashes read as slashes, lower-cased), because two things hold at once: Pierre matches every listed path, so a row fed on a match of its label alone ("hidden", "folder") would surface a status line as a search result; and Pierre force-expands every matching directory in its hide-non-matches mode, re-applying that on every store event, so a matched childless folder cannot be kept closed and, with its row withheld, would open over nothing — the down-chevron with no row beneath it that the state row exists to prevent. A fed row matches through the same folder prefix and sits under its open folder, the badge yielding to it as outside a filter; a blank query feeds the whole plan. Pierre focuses the first match, and the fed row sorts first under its folder, so when that focus lands on a state row the wrapper hands it to the folder the row qualifies (`focusPath`, which raises no store event) — a focus ring never sits on a status line. The two resets the filter brackets (filter on, filter off) carry the model's expansion in a host that does not persist it, because Pierre snapshots the expansion when a filter opens and restores it when it closes. No "loading" or "error" state row exists: whether a folder has children is decided by the payload the moment it lands and never by a fetch in flight; Changed mode plans no state rows at all, since every folder there is the parent of a changed file. `truncated` is true exactly when `truncatedDirectories` is non-empty -- files or folder rows were omitted. A truncated payload with no row at all (the walk stopped before it reached anything it lists, such as a root whose first entries are all hidden folders) shows its own `root_truncated` sentence, `Workspace too large to list: limit of 10,000 items reached`, in the empty-workspace notice's place (`workspace-tree-root-truncated`), since nothing there is known to be empty. Pierre's `flattenEmptyDirectories` folds a folder holding one listed subfolder and no file into one chain row whose path names its LAST folder, so the badge is looked up for every folder in the row's `flattenedSegments`, and yields to the state row only when the chain's last folder is the one truncated folder in it. Older clients may ignore the five additive arrays. `root`, every file path, every directory path, every truncation marker, every hidden-only marker, every linked marker, and every unreadable marker pass through the same segment-aware redaction before egress -- in the same worker thread as the listing and the JSON serialization (`_project_tree_body`), because each is linear in the size of the listing and none may run on the event loop; `root` is an absolute project path like `/api/project/git`'s `repoRoot`, so it takes the same Darwin temp-prefix exception described above, while the project-relative paths carry no OS-owned prefix and stay on the canonical redactor. **Folders past the cap**: `?dir=` lists one project-relative folder instead of the whole tree, under the same row cap, its paths still project-relative and the folder echoed as `dir`. Every check on `path` (the known-project allow-list, the sensitive-root refusal, the audit record) runs first and unchanged; `dir` must then be a plain relative path (no empty, `.` or `..` segment, no leading `/` or NUL, and on Windows no drive, `\` or `:`) whose real path is exactly its plain join under the root, so any link along it is refused, and strictly inside the root and not sensitive (`_project_tree_subdir`; `dir_refused` 403, `dir_not_found` 404, each audited as denied). The folder is then pinned: `_project_tree_pin` reaches it through `pinned_fs.open_pinned_descendant_dir` (a descriptor walk refusing a link at every component; on Windows that helper's `lstat` per prefix and its documented by-name residual) and records its `(st_dev, st_ino)`; the walk reads it only while it is still that folder (`_project_tree_walk_pinned`), and git lists it from the project root with the folder as a literal pathspec (`GIT_LITERAL_PATHSPECS`) rather than as its working directory, so an ancestor swapped for a link after the check lists nothing outside the project. On a platform that cannot pin by descriptor (Windows, where git walks into a junction) a folder listing uses the identity-checked walk instead of git, so it does not honour `.gitignore` there (the walk still skips `_PROJECT_TREE_SKIP_DIRS`); a git listing that names nothing is not shown as an empty folder row. The tree (`PierreWorkspaceTreeImpl`) asks for a folder's own listing when the user expands a folder named in `truncatedDirectories`, or when a host reveals one, under `['project-tree', projectDir, 'dir', folder]` so the host's Refresh re-reads it (not polled on their own: each whole-tree poll that lands re-reads at most three of the loaded folders that are open, in turn, since the whole listing cannot see a change inside a folder past the cap; a listing whose latest read fails is named in a notice above the tree, `folder_listing_failed`, whether or not earlier rows are still shown, with its own Retry button beside it, `folder_listing_retry`, that re-reads those folders); at most 20 folder listings, and at most 40,000 rows across them (`MAX_FOLDER_LISTING_ROWS`, the newest listing always kept), are kept per mount (a folder whose listing that bound drops while it is open is not reloaded until it is closed, so the bound never cycles), and `mergeFolderListings` (`pierre/treeFolderListings.ts`) folds them into the rendered payload, a loaded folder and every folder beneath it leaving `truncatedDirectories` unless its own listing cuts them again; a loaded folder whose listing answers 404 (deleted since) or 403 (refused, e.g. a link) is dropped and, like one the bounds drop, not reloaded until it is closed, while a notice of its own naming it, `folder_gone` for a 404 or `folder_refused` (a link or a protected folder) for a 403, with no retry since none can change it (a 404 also re-reads the whole listing once, which still named the folder), stays until the user dismisses it or a later load of that folder succeeds; a revealed folder is expanded, focused and selected. While the whole payload is truncated the Files rail (`FileBrowserRail`) sends its name filter to `/api/file-search` (`NameSearchResults`, files and folders, 50 rows) instead of the tree's local filter, under the folder tab's own note (`search_beyond_tree`; the same budgeted walk, the same caveat), its failure copy the folder tab's own (`SEARCH_FAILURE_KEYS`, `lib/searchErrorCause.ts`), its hits joined by the cut tree's own matching files through the folder tab's own `mergeTreeMatches` (`lib/treeSearchMatches.ts`), also when the search fails, the tree kept mounted behind it; a folder hit reveals that folder in the tree and a file hit opens it. +**Directory upload**: POST `/api/directory-upload?dir=` (multipart, one `file` part) — writes an OS file into a directory the Files panel already shows: a drop anywhere on the Files rail's drop overlay uploads into the project root (the tree's `root`, which the overlay names), and a directory row's menu → "Upload files…" uploads into that directory; both POST here. The handler opens with the dashboard owner gate (`require_owner_dashboard_request`, the same gate `/api/file-write` requires) before any query parsing, path probe or multipart read, so a non-owner holding a channel-created dashboard session cannot plant files into the workspace. `dir` goes through the same `_validate_dashboard_path`/`is_sensitive_path` gate `/api/file-write` trusts for writing INTO the workspace (no fixed allow-list root — any non-sensitive directory on disk), then must already exist as a directory; both that check and the destination re-check below run through `_run_path_probe` on the dedicated bounded path-probe pool, while the final write uses its path-transfer pool, never the event loop's shared `asyncio.to_thread` executor (AUTOSDE's `no-blocking-call-on-event-loop` rule is `blocking: true` — a stalled network-mounted `dir` resolving synchronously would otherwise freeze every other request and the heartbeat with it). **The destination, not just the directory, is gated**: the crew-home root is deliberately not itself sensitive (only its leaves — `security_policy.json`, `profiles/`, `admission_policy.json`, `computer_use.json` — are), so the full joined path (directory plus the uploaded filename) is re-validated through the identical gate before the extension check or the body is read, closing the case where a permitted directory plus a keystone-leaf filename would otherwise reach the keystone. Reuses `api_upload_file`'s admission gates rather than inventing new ones: the same extension allowlist (`_ALLOWED_IMAGE_EXT`/`_ALLOWED_TEXT_EXT`/`_ALLOWED_DOC_EXT`/`_ALLOWED_VIDEO_EXT`/`_ALLOWED_AUDIO_EXT`) and the same magic-byte content-signature check (`_content_matches_ext`, CWE-434). Its per-file cap is `dashboard.upload_max_mb` (read through `upload_max_bytes`) for every admitted type, video included, where `api_upload_file` lets a video run up to `_MAX_VIDEO_UPLOAD_BYTES`; a larger file answers 413 `file_too_large`. Deliberately does NOT run `api_upload_file`'s aggressive `[^\w.\-]` filename mangling (harmless there — every scratch upload is UUID-prefixed and never user-facing by name) or the outbound `file_send` gate's credential/secret content scan (`_gate_upload_file`'s `redact()` check exists for content LEAVING the workspace toward a third party; this endpoint writes a user's OWN file INTO their OWN workspace, the opposite trust direction, so porting that scan here would be inventing a gate rather than reusing one). Only the filename's directory components are stripped (`Path.name`) and the rest is kept as typed; an empty name, `.`, `..` or a name with a control character answers 400 `code: "invalid_filename"` on every platform, and on a Windows host so does a name Windows cannot store (`<>:"\|?*`, a trailing dot or space, a device name such as `CON`), before any write. One file per request by design (the frontend loops, one multipart POST per file, mirroring the Knowledge Base upload) — this is what makes per-file collision handling possible. Upload is create-only: an existing file at the destination name is never replaced, and the request answers 409 `code: "name_collision"` (a request that still sends `overwrite=1` gets the same 409), so the frontend tells the user the file already exists in that folder. A write the filesystem refuses (an unwritable destination, a full disk) answers a JSON 500 `{"error": "failed to write file", "code": "upload_failed"}`, never aiohttp's bare text 500. + +Ancestor and create safety, by platform. On POSIX, the write pins the destination directory's ancestor chain component-by-component (`pinned_fs.pin_parent`, walking the already-canonicalized directory rather than re-resolving it, so a directory swapped for a symlink after validation is refused). On Windows — which has no `O_NOFOLLOW` + `dir_fd` combination to pin a parent CHAIN with — the same window is closed a different way: `platform_compat.pin_directory(target_dir)` opens a handle WITHOUT `FILE_SHARE_DELETE`, and for as long as it stays open Windows refuses to rename or delete that directory OR any directory above it, so an ancestor cannot be swapped for a junction while the classify-then-create below runs; a link or junction already sitting at `target_dir`'s own name is refused at that same open (403 `symlink_refused`), a reparse folder that holds its own contents, such as a OneDrive cloud-files folder, is pinned like any other folder (`links_only=True`), and any other refusal of that open answers 500 `upload_failed`. Directory upload therefore stays AVAILABLE on Windows rather than being disabled there — the fail-closed `ancestor_pin_unsupported` (501) response only fires on a platform with neither primitive, which no host in the supported CI matrix (Linux, macOS, Windows) reaches. On POSIX the file is created by `pinned_fs.write_new_file_pinned`: an `O_EXCL` create under the pinned directory descriptor, so an existing name is never replaced and of two concurrent same-name uploads exactly one creates the file while the other gets 409 `name_collision`, with an identity-checked rollback when a write, flush or close fails. On Windows the same `O_EXCL` create, write and close run by name under the held `pin_directory` handle, and the rollback goes through `pinned_fs.unlink_verified_by_name`, which pins the folder by the same rule. The bytes are written under the final name, so a reader can see a partially written file while the upload runs, and a failed write removes it. A new file gets the mode `atomic_write` gives one by default: owner-only inside the data home, `0o666` under the umask elsewhere. A pre-existing symlink, junction, FIFO, socket or device at the destination name is refused rather than followed, checked through the pinned descriptor on POSIX and by name via `platform_compat.is_link_or_junction` on Windows. Response `{"ok": true, "path", "name"}` on success. **Theme / display**: GET `/api/theme/boot` (**unauthenticated** — same boundary as `/api/health`; returns `{mode, color, language, onboarded, import_onboarded, privacy_acked, crewmates_onboarded}` so the SPA can apply the workspace theme, pick its UI language, and resolve onboarding gates before the token flow completes; no secrets), GET/PUT `/api/config/theme` (read/persist workspace display prefs `{mode?, color?, language?, onboarded?, import_onboarded?, privacy_acked?, crewmates_onboarded?}`; `mode` restricted to `""`/`dark`/`light`/`system`; `language` is `""` (auto-detect from the browser) or a BCP-47 tag validated for **shape only** via `_LANGUAGE_TAG_RE`, so adding a shipped language stays a frontend-only data change; the PUT validates the whole body before writing and persists ONLY the named `dashboard.*` keys through `run_config_write(update_config_locked, ...)`, never a whole-document `save()`, and answers an unreadable `config.json` with `500 {"code": "config_unreadable"}` leaving the file untouched, a write the config guard refuses with `400 {"code": "config_write_refused"}`, and a failed write with `500 {"code": "config_write_failed"}` — see `config.md` → "`KiroCrewConfig.save() -> None`"). All four response sites are built by one helper (`handlers/core.py::_theme_payload`) so a newly-added display pref cannot be surfaced by some and omitted by others — see `config.md` → "Dashboard UI language". **Foreign-agent import**: authenticated GET `/api/onboarding/import/scan`, POST `/api/onboarding/import/apply`, PUT `/api/onboarding/import/state`; see "Foreign-Agent First-Run Import". **Memory**: GET/PUT `/api/memory/preferences`, GET/PUT `/api/memory/projects`, GET/PUT `/api/memory/history`, GET/PUT `/api/memory/settings` (consolidation config: `history_idle_hours`, `history_max_days`; writes to config.json, applies immediately to running consolidator). The three markdown pairs, the semantic and episodic routes, `stats`, `events` and `carve` also take an optional `?store=`: absent, the route answers from the global store regardless of `X-Session-Key`; present, reads of it are owner-gated (writes are owner-gated either way, see the memory-mutation gate in [memory-skills-hooks.md](memory-skills-hooks.md)), an undeclared name is 404 `unknown_memory_store` and a silo whose vector tier cannot be stood up is 503 `store_unavailable`. `settings` is install-wide and takes no store. @@ -4323,7 +4326,7 @@ GET `/api/file-office-slides?path=...` is the rendered-slides manifest for a `.p **Side-panel file tabs** (`MarkdownPanel.tsx`; the reads, the watch and the missing-file retry live in `useMarkdownPanelDisk.ts`): only the visible tab holds an `/api/file-watch` stream, and only while it is not being edited, not dirty and not missing; mounting, becoming visible and leaving edit mode each do a fresh read instead. A 404 keeps the last copy on screen under a missing-file banner and re-reads on a backoff from 1 s up to 30 s until the file is back or the tab is hidden. A close of a live-watched tab asks the disk first, dirty or not, and only a clean tab is re-targeted in place. A save leaves the tab dirty if it was edited again while the save was in flight. For a whole buffer, diff mode compares `/api/file-diff`'s `original` with the buffer read with universal newlines (CRLF and a lone CR as LF), the way `original` itself is read, so a CRLF file does not diff as every line changed; the editor's live diff, whose new side is the buffer, draws `original` in the line ending most of the editor's opening text uses (`PierreEditorImpl.tsx`), never the live buffer's, because a new baseline rebuilds the editor's document from that opening text. Neither changes the buffer or what a save writes. For a partial buffer, every successful read makes git's patch owed unless a diff fetch started after the read began; an owed patch is fetched only while diff mode is on and only once no fetch is out (one `/api/file-diff` in flight per tab, the next started when it settles), and every stream frame after the first re-reads even when its text equals the buffer: `/api/file-watch` caps its read too, and sends a frame on every write to a file that fills the cap, since a write past the cut leaves the capped text unchanged; such a frame carries an empty `content` when the capped text did not change, as a change signal only. Side-panel artifact tabs (`ArtifactPanel.tsx`) follow the same visible-tab rule through the shared `useVisibleTabCatchUp` hook: only the visible one holds a stream, and becoming visible refetches the artifact (see [artifacts.md](artifacts.md) → "Live refresh — file-backed artifacts"). **File diffs**: GET `/api/file-diff?path=...` returns the working-tree diff and HEAD content for the requested file, plus a `status` field — `not_git` (the repository preflight failed, so the file is outside any git work tree and there is no baseline; only the preflight may claim this), `untracked` (inside a repo, all-added `--no-index` diff — including every file of a freshly-initialized repo with no commits yet), `modified`, `clean`, or `error` (a git invocation past the preflight failed or timed out; distinct from `clean` so a git failure is never reported as "no changes"). Every reachable `status` value is served with HTTP 200; a sensitive path is 403 `{"error": "Access denied"}`, and the empty-path / file-not-found early returns reply `{"diff": "", "original": ""}` with no `status` key. Snapshot reads and every Git subprocess decode text explicitly as UTF-8 with replacement for malformed bytes, so Windows locale code pages cannot corrupt non-ASCII file content. Both text fields are credential-redacted through `redact_via_context`, the same pass `/api/file-read` applies to the panel's buffer, because the file panel diffs those two strings against each other for a whole buffer (and draws `diff` itself for a partial one) and one raw side renders an untouched credential line as a hunk. The pass sees whole field text and neither field is truncated before it, because slicing first can cut the tail a credential pattern needs and serve the surviving prefix as real bytes; that is also why the pass runs off the event loop. The served `diff` is never git's patch of the raw file: the HEAD body and the whole working file are each redacted first, and `diff` is `git diff --no-index` of those two masked bodies (in a private temp dir, header rewritten to the file's path). A patch shows lines out of their surrounding text, so the pass runs on whole bodies only. If that masked diff fails, `status` is `error` and `diff` is empty; the raw patch is never a fallback. `status` is computed from the unredacted `git diff`, so it still reports `modified` when the only change is inside a redacted span (`diff` is then empty). For a partial buffer the panel reads that pair as a change the patch cannot draw (a masked credential, the file mode or line endings), so its notice says git reports a change the diff view can't show (`components.markdownPanel.diff_change_not_shown`), never "no changes"; it names no cause and no line, since a line would point at the credential that changed. A whole buffer is compared line by line, so when its text matches HEAD it still reads as no changes. Binary changes without text hunks use a summary with a redacted filename without opening the working file; the working file read for a masked text diff has a 32 MiB limit (HEAD is redacted whole, as before), with overflow returning `status: "error"` and an empty diff rather than truncated text. -**Dashboard file API owners**: `dashboard/handlers/files.py` is the facade every file route, the handlers package and tests import and patch; `_facade_compose.compose` runs its owners on the facade's globals, so a patch of `files.` reaches them. New work goes to the owner of its family: `uploads.py` (composer uploads), `workspaces.py`, `pinned_io.py` (descriptor-checked open, read and pinned write, and the batched kind probe `POST /api/file-kinds`), `transfer.py` (file watch, download, stream), `office_preview.py`, `sheet.py`, `search.py`, `path_complete.py`, `grep.py` (the grep route and its content-search engines), `browse.py` (the directory-picker listings), `project_dirs.py` (known project dirs, branch label), `git_panel.py` (bounded git, status, log), `project_tree.py`, `dashboard_config.py`, `path_probe.py` (the `_run_path_probe` chokepoint), `outbox.py` (the outbox routes), `channel_upload.py` (the Slack and channel uploads and their admission gate), `desktop.py` (reveal, the native picker and screenshot) and `editor.py` (file read, raw, write and diff). The facade keeps the module state, the imports its owners read, and the seams they share (`_sel`, `_audit_file_send`, `_subagent_parent_session_key`, `_body_err_code`, `_walk_monotonic` and the viewer's `_owner_view_bypasses_credential_pass`). The two directory-picker routes (`api_browse_dirs`, `api_browse_files`) stay in the facade too: their skip sets hold a literal the internal content scan refuses on an added line. `_PathProbeBusy`, defined in `path_probe.py`, reports `kiro_crew.dashboard.handlers.files` as its module, the path callers catch it by, so a traceback names that path. Each construct a repository guard reads by path stays in its named homes, and in no other file of the family: the SVG policy literal, the diff route's git argv and the owner-view seam in `editor.py` (the facade also binds that seam's redactor), and the native-dialog spawns in `desktop.py`; `test/test_dashboard_files_composition_contract.py` pins the split and those homes. +**Dashboard file API owners**: `dashboard/handlers/files.py` is the facade every file route, the handlers package and tests import and patch; `_facade_compose.compose` runs its owners on the facade's globals, so a patch of `files.` reaches them. New work goes to the owner of its family: `uploads.py` (composer uploads and the Files panel directory upload), `workspaces.py`, `pinned_io.py` (descriptor-checked open, read and pinned write, and the batched kind probe `POST /api/file-kinds`), `transfer.py` (file watch, download, stream), `office_preview.py`, `sheet.py`, `search.py`, `path_complete.py`, `grep.py` (the grep route and its content-search engines), `browse.py` (the directory-picker listings), `project_dirs.py` (known project dirs, branch label), `git_panel.py` (bounded git, status, log), `project_tree.py`, `dashboard_config.py`, `path_probe.py` (the `_run_path_probe` chokepoint), `outbox.py` (the outbox routes), `channel_upload.py` (the Slack and channel uploads and their admission gate), `desktop.py` (reveal, the native picker and screenshot) and `editor.py` (file read, raw, write and diff). The facade keeps the module state, the imports its owners read, and the seams they share (`_sel`, `_audit_file_send`, `_subagent_parent_session_key`, `_body_err_code`, `_walk_monotonic` and the viewer's `_owner_view_bypasses_credential_pass`). The two directory-picker routes (`api_browse_dirs`, `api_browse_files`) stay in the facade too: their skip sets hold a literal the internal content scan refuses on an added line. `_PathProbeBusy`, defined in `path_probe.py`, reports `kiro_crew.dashboard.handlers.files` as its module, the path callers catch it by, so a traceback names that path. Each construct a repository guard reads by path stays in its named homes, and in no other file of the family: the SVG policy literal, the diff route's git argv and the owner-view seam in `editor.py` (the facade also binds that seam's redactor), and the native-dialog spawns in `desktop.py`; `test/test_dashboard_files_composition_contract.py` pins the split and those homes. **Terminal handler owners**: `dashboard/handlers/terminal.py` is a mirror facade (`kiro_crew._facade_mirror`): routes, the handlers package and tests import and patch every terminal name through it, and a patch reaches every owner module that holds the name. It keeps the WebSocket route `api_terminal_ws` with the bash readiness hook it installs, the completion route `api_terminal_complete` with the listing it ranks, the redaction route, the title poller, the shell cwd and title probes (the trusted-path `lsof` fallback) and the code-fence shell list. New work goes to the owner of its responsibility: `terminal_session.py` (`_TerminalSession`, the session registry, the terminal config gate), `terminal_shell.py` (the shell, cwd and child environment a session starts with), `terminal_transport.py` (which socket owns a session, and the PTY's bytes to and from it, forwarded unchanged while the readiness marker is matched), `terminal_lifecycle.py` (the shell child's liveness, exit, failure notice and teardown, and the orphan reaper), `terminal_complete.py` (the vetting steps of path completion) and `terminal_routes.py` (the create, list and delete routes). `test/test_terminal_facade.py` pins the owner set and its layer order. **Usage handler owners**: `dashboard/handlers/usage.py` is a mirror facade (`kiro_crew._facade_mirror`): routes, the gateway's turn writers and tests import and patch every usage name through it, and a patch reaches every owner module that holds the name. It keeps the usage routes, the model a turn is recorded under (`read_turn_model` and the wrapper walk behind it) and the token-record writer (`persist_token_record`, the family's one `kiro_crew.acp` importer). New work goes to the owner of its responsibility: `usage_store.py` (the shard store and its data-home overrides, the reporting window, slot categories, the row readers), `usage_spend.py` (a slot's spend, the per-day credits, one slot's per-turn usage), `usage_context.py` (context-window occupancy and a session's context trace), `usage_cost.py` (the Telemetry spend breakdown), `usage_history.py` (the Usage page's token history) and `usage_sessions.py` (the Usage page's kiro-cli session activity and its cached parse). `test/test_usage_facade.py` pins the owner set and its layer order. **Dashboard prompt and skill route owners**: `dashboard/handlers/prompts.py` is the facade every prompt and skill route, the handlers package, `handlers/discover.py` and tests import and patch. It is a `_facade_mirror` facade over sibling `prompts_.py` owners and keeps no code of its own: a patch of `prompts.` reaches every owner that binds the name, and no owner imports the facade. New work goes to the owner of its family: `prompts_caller_gates.py` (the owner and app-slot checks, the project a requesting chat names, the prompt-write refusals and their audit), `prompts_paths.py` (the prompt roots, the linked-root refusal, the pinned directory and the platform capabilities the pinned create relies on), `prompts_reading.py` (`GET /api/prompts` and the descriptions it shows), `prompts_authoring.py` (`GET`/`PUT`/`DELETE /api/prompts/{name}`, `POST /api/prompts` and the name limits), `prompts_skill_catalog.py` (`GET /api/skills`, project trust, a skill's file tree), `prompts_skill_edits.py` (a skill's detail and edits, pin, inject-on-trigger, `POST /api/skills`) and `prompts_skill_pending.py` (staged candidates). `test/test_prompts_handler_facade.py` pins the family. diff --git a/docs/system-specs/modules/memory-skills-hooks.md b/docs/system-specs/modules/memory-skills-hooks.md index 52563f6a0cc..f6f84d8c6d1 100644 --- a/docs/system-specs/modules/memory-skills-hooks.md +++ b/docs/system-specs/modules/memory-skills-hooks.md @@ -5366,6 +5366,13 @@ alias was deleted or a symlink retargeted. Repeat calls avoid a re-walk only thr to `atomic_write`, so it must land for all three surfaces at once rather than by reverting one of them to an in-place write that a mid-write failure or a full disk would turn into data loss. + Directory upload (`/api/directory-upload`) is not one of these surfaces: it is + create-only, so it never replaces a file. It creates the file through + `pinned_fs.write_new_file_pinned` (an `O_EXCL` create under the pinned + directory, with an identity-checked rollback), and an existing name answers + 409 `name_collision`. The bytes are written under the final name, so a reader + can see a partially written file while the upload runs, and a failed write + removes it (see [learn-cron-dashboard.md](learn-cron-dashboard.md)). - `delete_skill(name)` — removes entire skill directory - All three address the leaf relative to a descriptor pinning the parent chain (`pinned_fs`) where the platform has the descriptor-relative syscalls, so an diff --git a/src/kiro_crew/dashboard/file_api/uploads.py b/src/kiro_crew/dashboard/file_api/uploads.py index e474d3d8848..34565bfc317 100644 --- a/src/kiro_crew/dashboard/file_api/uploads.py +++ b/src/kiro_crew/dashboard/file_api/uploads.py @@ -1,11 +1,14 @@ -"""Composer upload intake: ``POST /api/upload/file``, its per-type content checks and the media streaming path.""" +"""Upload intake: ``POST /api/upload/file`` (composer attachments) and ``POST /api/directory-upload`` (Files panel), their per-type content checks and the media streaming path.""" from __future__ import annotations import asyncio +import errno import hashlib +import ntpath import os import re +import stat as _stat_mod import uuid import zipfile from pathlib import Path @@ -21,6 +24,7 @@ _ALLOWED_IMAGE_EXT, _ALLOWED_TEXT_EXT, _ALLOWED_VIDEO_EXT, + _CONTROL_CHARS_RE, _HEIF_BRANDS, _MAGIC_PREFIXES, _MAX_UPLOAD_FILES, @@ -32,12 +36,21 @@ _UPLOAD_DIR, _VIDEO_HINT_EXT, _ZIP_CONTAINER_EXTS, + OWNER_ONLY_FILE_MODE, SNIFF_BYTES, + WINDOWS_DEVICE_STEMS, + _PathProbeBusy, + _probe_busy_response, + _run_path_probe, _sel, + _validate_dashboard_path, data_home, ensure_directory, + is_owner_only_target, logger, part_stream, + pinned_fs, + platform_compat, sniff_raster_mime, upload_max_bytes, ) @@ -511,3 +524,572 @@ def _sniff_media_type(header: bytes) -> str | None: if header[:4] == b"RIFF" and header[8:12] == b"WAVE": return "audio/wav" return None + + +async def api_directory_upload(request: web.Request) -> web.Response: + """POST /api/directory-upload?dir= — write an uploaded file into a + directory already visible in the Files panel. + + Sibling of :func:`api_upload_file`, for a different destination: that + endpoint always writes into the fixed chat-attachment scratch directory + (``_upload_dir()``); this one writes into whatever directory the caller + named -- a drag-and-drop target row, the full-pane drop overlay's root, or + the "Upload files..." row-menu action -- which may be anywhere the + dashboard's own path gate (``_validate_dashboard_path`` / + ``is_sensitive_path``) permits, the same path gate ``/api/file-write`` + trusts for writing INTO the workspace. Like ``/api/file-write``, the + handler opens with the dashboard owner gate + (``require_owner_dashboard_request``) before any query parse, path probe or + multipart read, so a non-owner holding a channel-created dashboard session + cannot use it to plant files into the workspace. + + Reuses the existing admission gates rather than inventing new ones: the + SAME extension allowlist and the SAME magic-byte content-signature check + (``_content_matches_ext``) + ``api_upload_file`` applies to a chat attachment. The per-file size cap + here is ``dashboard.upload_max_mb`` (``upload_max_bytes``) for EVERY admitted file, video + containers included: unlike ``api_upload_file``, which streams a video + against the larger ``_MAX_VIDEO_UPLOAD_BYTES`` (512 MB), this endpoint + applies no media-specific cap, so a video or audio file the shared + allowlist admits is still refused past that limit (``file_too_large``). There is no + credential/secret content scan here, deliberately matching + ``api_upload_file``'s own boundary rather than the OUTBOUND ``file_send`` + gate's (``_gate_upload_file``'s credential scan): that scan protects + content leaving the workspace toward a third party, which is the opposite + trust direction from a user dropping their OWN file into their OWN + workspace, so porting it here would be inventing a new gate rather than + reusing an existing one. The ``file_delivery_consent`` module the handlers + module already imports is not the missing piece either: its one class + (``CLASS_OWNER_DASHBOARD``) gates handing workspace content OUT to the + dashboard, so consulting it here would be asking permission for the + opposite direction of travel. + + Unlike ``api_upload_file``'s aggressive ``[^\\w.\\-]`` character + replacement (harmless there -- every scratch upload is UUID-prefixed and + never user-facing by name), the filename here is the file's permanent, + user-visible identity in their own workspace, so only the traversal- + relevant part is stripped (``Path.name``) and the rest is kept as typed. + An empty name, ``.``, ``..`` or a name with a control character is + refused with ``invalid_filename`` on every platform. On a Windows host a + name Windows cannot store (``<>:"\\|?*``, a trailing dot or space, or a + device name such as ``CON``) is refused with the same 400 before any + write; a Linux or macOS host stores such a name as typed, like every + other tool that writes into the workspace. + + One file per request, by design: the frontend sends one multipart POST + per dropped/picked file (mirroring the Knowledge Base upload), which is + what makes per-file name-collision handling possible -- a batch endpoint + would have to decide the collision policy for the whole batch at once. + + Uploads are create-only: an existing file at the destination name is + never replaced. A name collision is always refused with + ``code: "name_collision"`` and a 409, so the frontend can tell the user + the file already exists. + + **Destination gating, not just directory gating.** ``dir`` alone passing + ``_validate_dashboard_path`` is not enough: the crew-home ROOT is + deliberately unfenced (only its leaves -- ``security_policy.json``, + ``profiles/``, ``admission_policy.json``, ``computer_use.json`` -- are, per + ``security/paths.py``), so ``dir=`` with a ``file`` part named + ``security_policy.json`` would otherwise create the keystone where it is + absent -- exactly the ``api_file_write`` sibling's own gate, which runs + the FULL path (filename included). ``dest`` is therefore re-validated here + the identical way, after the filename is known and before either the + extension check or the body is read, so a sensitive destination is refused + before any byte of the upload is even accepted off the wire. + + **Ancestor-swap and partial-write safety.** ``pinned_fs.pin_parent`` + walks ``target_dir`` -- already realpath-canonicalized by + ``_validate_dashboard_path`` above, so this walks that RECORDED chain + component-by-component with ``O_NOFOLLOW`` rather than re-resolving it (a + fresh resolution here would just as happily follow a swap that landed after + validation as the original lookup would). ``pin_parent`` on the parent, not + ``open_dir_pinned`` on the target -- ``open_dir_pinned`` resolves its own + parent chain internally, which is exactly the re-resolution this avoids. A + single ``os.open(dest, ..., O_NOFOLLOW)`` only guards the FINAL component; + the parents on the way to it are not covered by any flag on that one call. + The file is then created by ``pinned_fs.write_new_file_pinned``: an + ``O_EXCL`` create under the pinned descriptor, so an existing name is + never replaced, and an identity-checked rollback when any write, flush or + close fails. The bytes are written under the final name, so a reader can + see a partially written file while the upload runs, and a failed write + removes it. The existing-target lstat that decides + collision/symlink/directory happens through the SAME pinned descriptor, + so it is not a second by-name resolution the ancestor pin does not cover. + + Windows cannot pin a directory descriptor at all (no ``O_NOFOLLOW`` + + ``dir_fd`` support -- see ``pinned_fs.supports_pinned_walk``), so the + ancestor-swap protection above is NOT available in that FORM there -- but + the property it buys IS still available, through a different primitive: + ``platform_compat.pin_directory(target_dir)`` opens a handle without + ``FILE_SHARE_DELETE``, and Windows refuses to rename or delete that + directory OR ANY DIRECTORY ABOVE IT for as long as the handle stays open + (see ``TestPinDirectory::test_a_pinned_directory_cannot_be_renamed_or_removed`` + in ``test_platform_compat_filesystem.py``). The open refuses a link or + junction at ``target_dir``'s own name and pins a reparse directory that + holds its own contents, such as a OneDrive cloud-files folder + (``links_only=True``). The handle's kernel-reported final path + must first match the already-validated target, so an ancestor swap that + redirects the open itself is refused rather than trusted as a lock on the + wrong directory. Held across the whole classify-then-publish below, the + verified handle prevents a later ancestor swap, so the SAME by-name + classification and publish the POSIX floor already uses become safe on + Windows too -- ``platform_compat.is_link_or_junction`` still catches a + planted symlink OR junction at the final name (which + ``path.is_symlink()`` alone would miss), and the same ``O_EXCL`` create, + write and close run by name, with the failed-write rollback going through + ``pinned_fs.unlink_verified_by_name``. Where NEITHER + ancestor-pinning primitive exists on a platform (unreachable in the + current CI matrix: every one of Linux/macOS/Windows takes one branch or + the other), the upload refuses outright with ``ancestor_pin_unsupported`` + rather than publish with no ancestor guard at all. This mirrors the + documented residual in ``pinned_fs.write_file_pinned``: the + everywhere-floor is "a planted NAME is never followed," not "no + same-process race exists." + """ + from kiro_crew.dashboard.handlers._shared import require_owner_dashboard_request + + # Ahead of the query parse, the path probe and the multipart read. This route + # creates files anywhere off the sensitive floor -- which includes + # the steering documents, the skills and the MCP config whose own write routes are + # owner-gated -- so leaving it open would hand a non-owner (a channel-allowlisted + # user's dashboard session, created by send_dashboard_link) every file those gates + # protect. It is the same gate, and the same ordering, api_file_write uses. Ahead + # of the probe too, so whether a path exists is not a non-owner's to learn. + owner_denied = await require_owner_dashboard_request(request, "upload.directory") + if owner_denied is not None: + return owner_denied + + caller = request.get("user", "dashboard") + # NOT stripped: a POSIX directory name may legitimately end in a space, and + # trimming one would resolve the upload to the space-less sibling instead + # -- silently writing the file into a different directory than the client + # named. An untrimmed value that is blank or bad is rejected below. + raw_dir = request.query.get("dir", "") + + def _denied(code: str, message: str, status: int, *, resources: str = "") -> web.Response: + _sel().log_api_access( + caller=caller, + operation="upload.directory", + outcome="rejected", + source="dashboard", + resources=resources or f"reason:{code}", + ) + # Dispatched through literal-status branches, EACH with its own + # dict-literal body, rather than one `web.json_response(body, + # status=status)` call: the error-code contract gate + # (test/test_error_code_contract.py) statically reads a call's + # `status=` keyword and its body argument, and either one arriving as + # a variable is indistinguishable from a status/body that is + # genuinely runtime-decided -- which the gate correctly cannot verify + # and buckets as a violation-shaped finding on every call, new or + # not. Every status this handler ever passes is one of these five + # literals (400/403/409/413/501), so branching on the value keeps `_denied` as the one + # place that assembles `code`/``message`` while still giving the gate + # a literal status AND a literal, transparent, ``code``-bearing dict + # it can verify at each site. + if status == 400: + return web.json_response({"error": message, "code": code}, status=400) + if status == 403: + return web.json_response({"error": message, "code": code}, status=403) + if status == 409: + return web.json_response({"error": message, "code": code}, status=409) + if status == 413: + return web.json_response({"error": message, "code": code}, status=413) + if status == 501: + return web.json_response({"error": message, "code": code}, status=501) + raise ValueError(f"api_directory_upload._denied: unhandled status {status!r}") + + if not raw_dir: + return _denied("missing_required_fields", "dir is required", 400) + if not (request.content_type or "").startswith("multipart/"): + return _denied("not_multipart", "expected multipart/form-data", 400) + + def _validate_directory(raw: str) -> str: + """``_validate_dashboard_path`` plus the existence check, off the loop. + + Returns the validated directory, or ``""`` when it is refused or absent. + + Both are blocking filesystem calls -- ``validate_file_path`` resolves + ``realpath`` and walks ``is_sensitive_path``'s candidate forms, + ``os.path.isdir`` is a ``stat`` -- so they go through + :func:`_run_path_probe`, this module's only sanctioned route for + filesystem work on a caller-supplied path. Not ``asyncio.to_thread``: + that is the loop's shared default executor, and a thread wedged in an + uninterruptible ``stat`` on a dead mount never returns, so a caller + naming one repeatedly would retire a shared worker per request until + every unrelated ``to_thread`` user queued behind them. On the probe + pool the same caller exhausts that pool and nothing else, and the + refusal surfaces as a 503 rather than a silent stall. + """ + validated = _validate_dashboard_path(raw) + if not validated or not os.path.isdir(validated): + return "" + return validated + + try: + target_dir = await _run_path_probe(_validate_directory, raw_dir) + except _PathProbeBusy: + return _probe_busy_response( + resource=raw_dir, operation="upload.directory", caller=caller, source="dashboard" + ) + if not target_dir: + return _denied( + "invalid_directory", "invalid or forbidden directory", 400, resources=raw_dir + ) + + reader = await request.multipart() + part = await reader.next() + if part is None or not isinstance(part, BodyPartReader) or part.name != "file": + return _denied("missing_required_fields", "file is required", 400) + + filename = part.filename or "" + # Strip any directory components the client sent (Path.name) -- the only + # traversal-relevant sanitization a filename needs. It also means `dest` + # below (`target_dir / safe_name`) is LEXICALLY always a direct child of + # `target_dir`: safe_name never carries a `/` or a `..` segment, so there + # is nothing left for a resolve()-based containment check to catch that + # candidate_dest's own re-validation just below does not already cover. + safe_name = Path(filename).name + if ( + not safe_name + or safe_name in (".", "..") + or _CONTROL_CHARS_RE.search(safe_name) + or ( + platform_compat.IS_WINDOWS + and ( + any(ch in '<>:"\\|?*' for ch in safe_name) + or safe_name.endswith((".", " ")) + or safe_name.split(".", 1)[0].lower() in WINDOWS_DEVICE_STEMS + ) + ) + ): + return _denied("invalid_filename", "invalid filename", 400, resources=filename) + + # Re-gate the FULL destination -- directory validation above is not + # enough on its own: the crew-home ROOT is deliberately not itself + # sensitive (only its leaves are -- security_policy.json, profiles/, + # admission_policy.json, computer_use.json), so dir= passes + # target_dir's own check and a filename naming one of those leaves would + # otherwise reach the keystone unfenced. This mirrors api_file_write, + # which runs its caller-supplied path (filename included) through this + # SAME gate -- the destination gets no lighter a check here just because + # part of it was already validated. Checked before the extension check + # and before any byte of the body is read, so a sensitive destination is + # refused as cheaply as every other early rejection in this handler. + # Through the probe pool for the same reason target_dir's own validation is + # above -- never the shared default executor. + candidate_dest = os.path.join(target_dir, safe_name) + try: + dest_ok = await _run_path_probe(_validate_dashboard_path, candidate_dest) + except _PathProbeBusy: + return _probe_busy_response( + resource=candidate_dest, + operation="upload.directory", + caller=caller, + source="dashboard", + ) + if not dest_ok: + return _denied( + "sensitive_destination", + "destination path is not allowed", + 403, + resources=candidate_dest, + ) + + ext = Path(safe_name).suffix.lower() + allowed = ( + _ALLOWED_IMAGE_EXT + | _ALLOWED_TEXT_EXT + | _ALLOWED_DOC_EXT + | _ALLOWED_VIDEO_EXT + | _ALLOWED_AUDIO_EXT + ) + if ext not in allowed: + detail = f"Unsupported file type: {ext}" + if ext in _VIDEO_HINT_EXT: + accepted = ", ".join(sorted(_ALLOWED_VIDEO_EXT)) + detail = f"{detail} — accepted video containers: {accepted}" + return _denied("unsupported_file_type", detail, 400, resources=filename) + + # ``dashboard.upload_max_mb``, read off the loop like api_upload_file. + max_upload_bytes = await asyncio.to_thread(upload_max_bytes) + data = bytearray() + while True: + chunk = await part.read_chunk(8192) + if not chunk: + break + data.extend(chunk) + if len(data) > max_upload_bytes: + return _denied( + "file_too_large", + f"File too large (max {max_upload_bytes // 1024 // 1024}MB)", + 413, + resources=filename, + ) + raw = bytes(data) + if not raw: + return _denied("empty_upload", "no file content received", 400, resources=filename) + if not _content_matches_ext(ext, raw): + return _denied( + "content_signature_mismatch", + f"File content does not match its type: {ext}", + 400, + resources=filename, + ) + + # No dest.resolve().is_relative_to(target_dir) check here: safe_name is + # already traversal-free (see its own comment above), so dest is + # LEXICALLY always a direct child of target_dir, and candidate_dest -- + # the identical path -- was already re-validated through + # _validate_dashboard_path above. A resolve()-based containment check + # would add nothing that check does not already cover, while adding two + # more blocking realpath walks directly on the event loop. + dest = Path(target_dir) / safe_name + + def _write() -> str | None: + """Create ``dest`` holding ``raw``, ancestor-pinned, or leave nothing. + + See the handler's own docstring for why ``pin_parent`` walks the + already-resolved ``target_dir`` rather than re-resolving it. The + pinned branch needs what ``pinned_fs.write_new_file_pinned`` needs: + a descriptor-relative open, plus the descriptor-relative stat and + unlink its identity-checked rollback uses. + + The existing-target CLASSIFICATION below (missing/symlink/dir/file) + happens entirely through stat-shaped checks (``pinned_fs.stat_at``, + ``platform_compat.is_link_or_junction``, ``Path.is_dir``/``is_file``) + BEFORE any open is attempted, never by opening the destination and + branching on which exception comes back: opening an existing DIRECTORY + with write flags is ``IsADirectoryError`` on POSIX but a different type + on Windows, so classifying first keeps the 409 the same on both. + + The classification only describes the destination at the moment it + ran. The create itself is ``O_EXCL``, so of two concurrent same-name + uploads that both classified `missing`, exactly one creates the file + and the other gets ``FileExistsError``, reported as `name_collision`. + An existing `file` is a `name_collision` too. + """ + pinned = pinned_fs.supports_pinned_walk() and {os.unlink, os.stat}.issubset( + os.supports_dir_fd + ) + dir_fd: int | None = None + ancestor_guard_fd: int | None = None + if pinned: + try: + dir_fd = pinned_fs.pin_parent(target_dir, what="upload target directory") + except pinned_fs.PinnedPathRefusal: + return "symlink_refused" + except OSError: + return "invalid_directory" + elif platform_compat.IS_WINDOWS: + # Windows has no O_NOFOLLOW + dir_fd combination to pin a PARENT + # CHAIN with (pinned_fs.supports_pinned_walk() is False there), + # so the by-name classification and publish below are, on their + # own, exactly the by-name write an ancestor swapped for a + # junction after _validate_dashboard_path could still redirect -- + # reaching a keystone leaf would be a governance-ceiling bypass, + # which AGENTS.md treats as absolute. platform_compat.pin_directory + # closes that window a different way: the handle it returns is + # opened WITHOUT FILE_SHARE_DELETE, and for as long as it stays + # open, Windows refuses to rename OR delete the directory it + # names -- NOR ANY DIRECTORY ABOVE IT (its own docstring; pinned + # by TestPinDirectory:: + # test_a_pinned_directory_cannot_be_renamed_or_removed in + # test_platform_compat_filesystem.py). The handle's final path is compared + # with the already-validated target before trusting that lock: an + # ancestor swapped for a junction before the open can otherwise + # redirect the open itself and make it pin the attacker's target. + # Held across the whole classify-then-publish below, a verified + # handle prevents any later ancestor swap. The open refuses a link + # or junction already sitting at target_dir's own name and pins a + # reparse directory that holds its own contents, such as a OneDrive + # cloud-files folder. Any other refusal of the open (a sharing + # violation, EACCES) propagates and answers upload_failed. + try: + ancestor_guard_fd = platform_compat.pin_directory(target_dir, links_only=True) + except OSError as exc: + if isinstance(exc, NotADirectoryError) or exc.errno == errno.ELOOP: + return "symlink_refused" + raise + pinned_target = pinned_fs.fd_real_path(ancestor_guard_fd) + if pinned_target is None or ntpath.normcase( + ntpath.normpath(pinned_target) + ) != ntpath.normcase(ntpath.normpath(target_dir)): + os.close(ancestor_guard_fd) + ancestor_guard_fd = None + return "symlink_refused" + else: + # Neither ancestor-pinning primitive exists on this platform: no + # descriptor-relative walk (POSIX without O_NOFOLLOW/dir_fd + # support) and no Windows handle-based anti-rename lock either. + # Publishing by name here would be an unguarded ancestor-swap + # window with no mitigation at all, so this refuses the upload + # outright rather than risk a write landing wherever an ancestor + # was swapped to -- including a keystone leaf. In the current CI + # matrix (Linux, macOS, Windows) this branch is unreachable: all + # three either satisfy `pinned` above or are Windows, which takes + # the branch above instead. + return "ancestor_pin_unsupported" + try: + # 'missing' | 'symlink' | 'dir' | 'file' for whatever currently + # sits at the destination name. On POSIX this stats through the + # pinned dir_fd, so no ancestor is resolved by name. On Windows it + # checks by name, under the pin_directory handle that stays held + # across this check and the create below, so no ancestor can be + # renamed in between. platform_compat.is_link_or_junction catches + # a planted symlink OR junction, where ``Path.is_symlink()`` alone + # would miss a junction. + if dir_fd is not None: + st = pinned_fs.stat_at(dir_fd, safe_name) + if st is None: + kind = "missing" + elif _stat_mod.S_ISLNK(st.st_mode): + kind = "symlink" + elif _stat_mod.S_ISDIR(st.st_mode): + kind = "dir" + elif _stat_mod.S_ISREG(st.st_mode): + kind = "file" + else: + # FIFO / socket / device planted at the name: refused the + # same way a symlink is, never opened as a plain file. + kind = "symlink" + elif platform_compat.is_link_or_junction(dest): + kind = "symlink" + elif dest.is_dir(): + kind = "dir" + elif dest.is_file(): + kind = "file" + else: + kind = "missing" + + if kind == "symlink": + return "symlink_refused" + if kind == "dir": + return "is_a_directory" + if kind == "file": + return "name_collision" + + # Owner-only inside the data home, 0o666 under the umask elsewhere: + # the default mode atomic_write gives a new file. + mode = OWNER_ONLY_FILE_MODE if is_owner_only_target(dest) else 0o666 + try: + if dir_fd is not None: + pinned_fs.write_new_file_pinned(dir_fd, safe_name, raw, mode=mode, fsync=True) + return None + # Windows: no dir_fd to create relative to. The directory and + # its ancestors are held by ancestor_guard_fd, so the by-name + # O_EXCL create lands in the validated directory, and the + # rollback removes the name only while it is still this file. + fd = os.open( + dest, + os.O_WRONLY + | os.O_CREAT + | os.O_EXCL + | getattr(os, "O_NOFOLLOW", 0) + | getattr(os, "O_BINARY", 0), + mode, + ) + except FileExistsError: + return "name_collision" + created: os.stat_result | None = None + try: + created = os.fstat(fd) + written = 0 + while written < len(raw): + written += os.write(fd, raw[written:]) + os.fsync(fd) + fd, open_fd = -1, fd + os.close(open_fd) + except BaseException: + if fd >= 0: + if created is None: + # Re-ask the identity through the descriptor (a + # transient EIO usually answers the second time); with + # no identity the name stays, as in write_new_file_pinned. + try: + created = os.fstat(fd) + except OSError: + pass + try: + os.close(fd) + except OSError: + pass + if created is not None: + pinned_fs.unlink_verified_by_name( + Path(target_dir), + safe_name, + (created.st_dev, created.st_ino), + links_only=True, + ) + raise + return None + finally: + if dir_fd is not None: + os.close(dir_fd) + if ancestor_guard_fd is not None: + os.close(ancestor_guard_fd) + + # transfer=True: this holds a worker for the length of the write, so it + # belongs on the transfer pool rather than the probe pool a burst of these + # would otherwise starve validation behind. + try: + outcome = await _run_path_probe(_write, transfer=True) + except _PathProbeBusy: + return _probe_busy_response( + resource=str(dest), operation="upload.directory", caller=caller, source="dashboard" + ) + except OSError as exc: + # An unwritable destination, a full disk, or a Windows pin refused for a + # reason other than a link; _write maps every other OSError to an outcome. + logger.exception("upload.directory write failed for %s", dest) + _sel().log_api_access( + caller=caller, + operation="upload.directory", + outcome="failure", + source="dashboard", + resources=str(dest), + error=exc.strerror or type(exc).__name__, + ) + return web.json_response( + {"error": "failed to write file", "code": "upload_failed"}, status=500 + ) + if outcome == "name_collision": + return _denied( + "name_collision", + f"A file named {safe_name!r} already exists", + 409, + resources=str(dest), + ) + if outcome == "is_a_directory": + return _denied( + "is_a_directory", + "a directory with that name already exists", + 409, + resources=str(dest), + ) + if outcome == "symlink_refused": + return _denied( + "symlink_refused", "refusing to write through a symlink", 403, resources=str(dest) + ) + if outcome == "invalid_directory": + return _denied( + "invalid_directory", "invalid or forbidden directory", 400, resources=str(dest) + ) + if outcome == "ancestor_pin_unsupported": + return _denied( + "ancestor_pin_unsupported", + "this platform cannot guard the upload directory's ancestors " + "against being swapped mid-request", + 501, + resources=str(dest), + ) + + _sel().log_api_access( + caller=caller, + operation="upload.directory", + outcome="success", + source="dashboard", + resources=str(dest), + ) + return web.json_response({"ok": True, "path": str(dest), "name": safe_name}) diff --git a/src/kiro_crew/dashboard/handlers/__init__.py b/src/kiro_crew/dashboard/handlers/__init__.py index a30eb833d67..57be4e6e3d7 100644 --- a/src/kiro_crew/dashboard/handlers/__init__.py +++ b/src/kiro_crew/dashboard/handlers/__init__.py @@ -177,6 +177,7 @@ def sel(): api_browse_files, api_channel_upload_file, api_dashboard_config, + api_directory_upload, api_file_diff, api_file_download, api_file_grep, diff --git a/src/kiro_crew/dashboard/handlers/files.py b/src/kiro_crew/dashboard/handlers/files.py index 7f222f431e1..54eaf9d02c6 100644 --- a/src/kiro_crew/dashboard/handlers/files.py +++ b/src/kiro_crew/dashboard/handlers/files.py @@ -79,6 +79,7 @@ LINK_PATTERNS_MAX, link_pattern_url_ok, ) +from kiro_crew.constants import WINDOWS_DEVICE_STEMS # noqa: F401 from kiro_crew.dashboard import git_repo_touches as _git_repo_touches # noqa: F401 from kiro_crew.dashboard import part_stream, upload_destination # noqa: F401 from kiro_crew.dashboard.body_numbers import body_int # noqa: F401 @@ -290,6 +291,7 @@ _stream_media_part, _upload_dir, _write_file_restricted, + api_directory_upload, api_upload_file, ) from kiro_crew.dashboard.file_api.workspaces import ( # noqa: F401 @@ -347,7 +349,11 @@ from kiro_crew.messaging.display_safety import redact_for_display # noqa: F401 from kiro_crew.messaging.outbound_files import OutboundFile # noqa: F401 from kiro_crew.messaging.raster import SNIFF_BYTES, sniff_raster_mime # noqa: F401 -from kiro_crew.owner_only_files import ensure_directory # noqa: F401 - owner: uploads +from kiro_crew.owner_only_files import ( # noqa: F401 - owner: uploads + OWNER_ONLY_FILE_MODE, + ensure_directory, + is_owner_only_target, +) from kiro_crew.pdf_extract import PdfExtraction, extract_pdf_segments # noqa: F401 from kiro_crew.platform import binary_content_is_flagged # noqa: F401 from kiro_crew.platform import wide_content_is_flagged # noqa: F401 diff --git a/src/kiro_crew/dashboard/routes/taskrunner.py b/src/kiro_crew/dashboard/routes/taskrunner.py index 927aeb11429..24b84d77802 100644 --- a/src/kiro_crew/dashboard/routes/taskrunner.py +++ b/src/kiro_crew/dashboard/routes/taskrunner.py @@ -59,6 +59,7 @@ def register(app: web.Application) -> None: app.router.add_get("/api/project/tree", handlers.api_project_tree) app.router.add_post("/api/upload", handlers.api_upload) app.router.add_post("/api/upload/file", handlers.api_upload_file) + app.router.add_post("/api/directory-upload", handlers.api_directory_upload) app.router.add_post("/api/slack/upload-file", handlers.api_slack_upload_file) app.router.add_post("/api/channel/upload-file", handlers.api_channel_upload_file) app.router.add_post("/api/slack/pins", handlers.api_slack_pins) diff --git a/src/kiro_crew/pinned_fs_files.py b/src/kiro_crew/pinned_fs_files.py index 4a769f5f628..0cd365d6e64 100644 --- a/src/kiro_crew/pinned_fs_files.py +++ b/src/kiro_crew/pinned_fs_files.py @@ -740,6 +740,7 @@ def unlink_verified_by_name( expect: tuple[int, int], *, on_error: Callable[[OSError], None] | None = None, + links_only: bool = False, ) -> bool: """Unlink *parent/name* only while it holds ``(st_dev, st_ino)`` *expect*. @@ -748,9 +749,10 @@ def unlink_verified_by_name( the parent, delegates to :func:`unlink_verified` on POSIX, and checks the regular-file identity under the pin before unlinking on Windows. An absent or mismatched name is a deliberate refusal and returns ``False`` without - deleting anything. + deleting anything. ``links_only`` is passed to the parent's + :func:`pin_directory`, so a cloud-files parent is pinned rather than refused. """ - pin = pin_directory(parent) + pin = pin_directory(parent, links_only=links_only) try: if os.name != "nt": return unlink_verified(pin, name, expect, on_error=on_error) diff --git a/src/kiro_crew/platform_compat_links.py b/src/kiro_crew/platform_compat_links.py index 2736b4ac4b4..4808648c5c3 100644 --- a/src/kiro_crew/platform_compat_links.py +++ b/src/kiro_crew/platform_compat_links.py @@ -225,7 +225,7 @@ def unlink_link_or_junction(path: str | os.PathLike) -> None: _WIN_FILE_ATTRIBUTE_REPARSE_POINT = 0x00000400 -def pin_directory(path: str | os.PathLike) -> int: +def pin_directory(path: str | os.PathLike, *, links_only: bool = False) -> int: """Open *path* as a directory and return a descriptor that PINS it. For code that must let a child process write to ``/`` by path: @@ -250,6 +250,11 @@ def pin_directory(path: str | os.PathLike) -> int: point raises ``NotADirectoryError``; a POSIX symlink fails with whichever of ``ENOTDIR`` / ``ELOOP`` the kernel reports for ``O_DIRECTORY | O_NOFOLLOW``. Release with ``os.close``. + + ``links_only`` narrows the Windows reparse refusal to a reparse point that + stands for another name (:func:`win_fd_is_link`), so a cloud-files or dedup + directory is pinned as the directory it is. POSIX is unaffected: a link is + the only thing ``O_NOFOLLOW`` refuses there. """ if IS_POSIX: return os.open(os.fspath(path), pinned_dir_flags()) @@ -257,7 +262,9 @@ def pin_directory(path: str | os.PathLike) -> int: fd = _win_open_without_following(path) try: attrs = getattr(os.fstat(fd), "st_file_attributes", 0) - if not attrs & _WIN_FILE_ATTRIBUTE_DIRECTORY or attrs & _WIN_FILE_ATTRIBUTE_REPARSE_POINT: + if not attrs & _WIN_FILE_ATTRIBUTE_DIRECTORY or _win_reparse_refused( + fd, attrs, links_only=links_only + ): raise NotADirectoryError(errno.ENOTDIR, "not a real directory", os.fspath(path)) except BaseException: os.close(fd) diff --git a/temp-screenshots/files-panel-upload/drop-overlay.png b/temp-screenshots/files-panel-upload/drop-overlay.png new file mode 100644 index 00000000000..df45cd82ab6 Binary files /dev/null and b/temp-screenshots/files-panel-upload/drop-overlay.png differ diff --git a/temp-screenshots/files-panel-upload/upload-exists.png b/temp-screenshots/files-panel-upload/upload-exists.png new file mode 100644 index 00000000000..ec01741b379 Binary files /dev/null and b/temp-screenshots/files-panel-upload/upload-exists.png differ diff --git a/temp-screenshots/files-panel-upload/upload-progress.png b/temp-screenshots/files-panel-upload/upload-progress.png new file mode 100644 index 00000000000..cbb266c851d Binary files /dev/null and b/temp-screenshots/files-panel-upload/upload-progress.png differ diff --git a/temp-screenshots/files-panel-upload/upload-success.png b/temp-screenshots/files-panel-upload/upload-success.png new file mode 100644 index 00000000000..c878147b0f2 Binary files /dev/null and b/temp-screenshots/files-panel-upload/upload-success.png differ diff --git a/test/link_screen_sites.py b/test/link_screen_sites.py index 7d27022a8c6..af31e87d42d 100644 --- a/test/link_screen_sites.py +++ b/test/link_screen_sites.py @@ -114,6 +114,7 @@ ("cron_inflight.py", "_readable_running_dir"), ("dashboard/chat_tag_grants.py", "_quarantine_store"), ("dashboard/chat_tag_grants.py", "_store_dir"), + ("dashboard/file_api/uploads.py", "api_directory_upload._write"), ("dashboard/handlers/__init__.py", "_prompt_dir_entry"), ("dashboard/handlers/discover.py", "api_skills_discover_install._write_bundle"), ("dashboard/handlers/prompts_authoring.py", "_api_prompt_write._apply_locked"), diff --git a/test/test_dashboard_files_composition_contract.py b/test/test_dashboard_files_composition_contract.py index abec1c0f155..9457d568454 100644 --- a/test/test_dashboard_files_composition_contract.py +++ b/test/test_dashboard_files_composition_contract.py @@ -388,10 +388,10 @@ def test_the_routes_and_the_package_reach_the_facade_objects() -> None: if getattr(files, getattr(route.handler, "__name__", ""), None) is not None and route.handler.__module__ == _FACADE ] - assert len({id(h) for h in served}) == 34 + assert len({id(h) for h in served}) == 35 assert [h.__name__ for h in served if getattr(files, h.__name__) is not h] == [] reexports = _package_reexports() - assert len(reexports) == 36 + assert len(reexports) == 37 assert [n for n in reexports if getattr(handlers_pkg, n) is not getattr(files, n)] == [] diff --git a/test/test_handlers_files_directory_upload.py b/test/test_handlers_files_directory_upload.py new file mode 100644 index 00000000000..8f8cae75177 --- /dev/null +++ b/test/test_handlers_files_directory_upload.py @@ -0,0 +1,1177 @@ +"""Tests for ``api_directory_upload`` (POST /api/directory-upload). + +Backs the Files panel's drag-and-drop and "Upload files..." row-menu actions: +unlike ``api_upload_file`` (which always writes into the fixed chat-attachment +scratch directory), this endpoint writes into a caller-supplied directory +anywhere the dashboard's own path gate permits. + +Covers: the happy path, the create-only name-collision contract (an existing +name is always a 409, even for a request that still sends ``overwrite=1``), +the reused extension allowlist and size cap, the magic-byte content-signature +gate, invalid/sensitive/missing directories, that a path-traversal attempt via the uploaded filename lands +inside the target directory rather than escaping it, that a sensitive +DESTINATION is refused even when the directory alone would pass (the crew-home +keystone leaves), and that a symlink/junction planted at the destination name +is refused rather than followed on every platform. +""" + +from __future__ import annotations + +import errno +import os +import shutil +import stat +import threading +from pathlib import Path +from unittest.mock import MagicMock, patch +from urllib.parse import quote + +import aiohttp +import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer + +import kiro_crew.dashboard.handlers.files as files_mod +from kiro_crew import platform_compat +from kiro_crew.atomic_write import atomic_write +from kiro_crew.dashboard.handlers.files import api_directory_upload + + +def _make_app() -> web.Application: + """The directory-upload route as it is wired live: behind the dashboard owner gate. + + ``as_owner`` installs the standalone-local owner identity plus a stand-in for + the token middleware, so a request with no ``X-Test-User`` reads as the owner + (the default these tests want) and one carrying ``X-Test-User`` reads as that + non-owner subject -- the shape ``slack/allowlist.py::send_dashboard_link`` + mints for a channel-allowlisted user. + """ + from dashboard_owner_helpers import as_owner + + app = web.Application() + app.router.add_post("/api/directory-upload", api_directory_upload) + return as_owner(app) + + +# Kept as a named alias for the owner-gate tests that read as the non-owner. +_make_owner_app = _make_app + + +@pytest.mark.asyncio +async def test_directory_upload_refuses_a_non_owner(tmp_path: Path, mock_sel) -> None: + """A non-owner dashboard session cannot plant a file into the workspace. + + ``/api/directory-upload`` creates files anywhere off the + sensitive floor -- steering docs, ``SKILL.md`` and the MCP config included -- + so it holds the same owner boundary its sibling ``/api/file-write`` holds. + The refusal must land before the path probe, so a denied caller cannot even + learn whether the directory exists. + """ + steering = tmp_path / ".kiro" / "steering" + steering.mkdir(parents=True) + + with patch.object(files_mod, "_run_path_probe", wraps=files_mod._run_path_probe) as probe: + async with TestClient(TestServer(_make_owner_app())) as client: + form = aiohttp.FormData() + form.add_field("file", b"planted", filename="rules.md", content_type="text/plain") + resp = await client.post( + f"/api/directory-upload?dir={steering}", + data=form, + headers={"X-Test-User": "U0NONOWNER"}, + ) + body = await resp.json() + + assert resp.status == 403 + assert body.get("code") == "owner_only" + # Refused ahead of the path probe, so a non-owner learns nothing about the tree. + probe.assert_not_called() + assert not (steering / "rules.md").exists() + + +@pytest.mark.asyncio +async def test_directory_upload_still_writes_for_the_owner(tmp_path: Path, mock_sel) -> None: + """Positive control: the owner passes the gate and the upload lands.""" + target = tmp_path / "docs" + target.mkdir() + + async with TestClient(TestServer(_make_owner_app())) as client: + resp = await _upload( + client, + directory=str(target), + content=b"hello", + filename="note.md", + ) + + assert resp.status == 200 + assert (target / "note.md").read_bytes() == b"hello" + + +@pytest.fixture() +def mock_sel(): + """Stub the SEL audit sink so upload audit calls don't blow up.""" + with patch("kiro_crew.dashboard.handlers.files._sel") as m: + instance = MagicMock() + m.return_value = instance + yield instance + + +async def _upload( + client: TestClient, + *, + directory: str, + content: bytes, + filename: str, +) -> aiohttp.ClientResponse: + form = aiohttp.FormData() + form.add_field("file", content, filename=filename, content_type="text/plain") + return await client.post(f"/api/directory-upload?dir={directory}", data=form) + + +async def _upload_raw_filename( + client: TestClient, + *, + directory: str, + content: bytes, + filename: str, +) -> aiohttp.ClientResponse: + """Send the filename verbatim instead of applying FormData's quoting.""" + boundary = "kirocrew-directory-upload-test" + body = ( + f"--{boundary}\r\n".encode() + + ( + 'Content-Disposition: form-data; name="file"; ' + f"filename*=UTF-8''{quote(filename, safe='')}\r\n" + ).encode() + + b"Content-Type: text/plain\r\n\r\n" + + content + + f"\r\n--{boundary}--\r\n".encode() + ) + return await client.post( + f"/api/directory-upload?dir={directory}", + data=body, + headers={"Content-Type": f"multipart/form-data; boundary={boundary}"}, + ) + + +@pytest.mark.asyncio +@pytest.mark.skipif( + platform_compat.IS_WINDOWS, + reason="Win32 strips trailing spaces from a path component, so the two " + "directories this test needs to tell apart cannot both exist there", +) +async def test_trailing_space_in_dir_is_not_trimmed_to_a_sibling(tmp_path: Path, mock_sel) -> None: + """``dir`` is used verbatim: a directory whose name ends in a space is a + different directory from its space-less sibling, and trimming the query + value would land the upload in the sibling while the client is told the + name it asked for succeeded.""" + spaced = tmp_path / "reports " + sibling = tmp_path / "reports" + spaced.mkdir() + sibling.mkdir() + async with TestClient(TestServer(_make_app())) as client: + form = aiohttp.FormData() + form.add_field("file", b"payload", filename="a.txt", content_type="text/plain") + resp = await client.post(f"/api/directory-upload?dir={quote(str(spaced))}", data=form) + assert resp.status == 200, await resp.text() + body = await resp.json() + assert body["path"] == str(spaced / "a.txt") + assert (spaced / "a.txt").read_bytes() == b"payload" + assert not (sibling / "a.txt").exists() + + +@pytest.mark.asyncio +async def test_uploads_a_file_into_the_target_directory(tmp_path: Path, mock_sel) -> None: + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"hello world", filename="notes.txt" + ) + assert resp.status == 200, await resp.text() + body = await resp.json() + assert body["ok"] is True + assert body["name"] == "notes.txt" + written = tmp_path / "notes.txt" + assert written.read_bytes() == b"hello world" + assert body["path"] == str(written) + assert mock_sel.log_api_access.call_args.kwargs["outcome"] == "success" + # The mode is the one atomic_write's default gives a new file in the same place. + sibling = tmp_path / "sibling.txt" + atomic_write(sibling, b"x") + assert stat.S_IMODE(written.stat().st_mode) == stat.S_IMODE(sibling.stat().st_mode) + + +@pytest.mark.asyncio +async def test_windows_pin_final_path_must_match_validated_directory( + tmp_path: Path, mock_sel, monkeypatch +) -> None: + """A Windows pin that resolves elsewhere is refused before publication.""" + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + opened: list[int] = [] + closed: list[int] = [] + guard_fd = 1_000_000_000 + + def _pin_redirected_directory(path, **_kwargs): + opened.append(guard_fd) + return guard_fd + + real_close = os.close + + def _track_guard_close(fd, *args, **kwargs): + if fd in opened: + closed.append(fd) + return None + return real_close(fd, *args, **kwargs) + + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", _pin_redirected_directory) + monkeypatch.setattr( + files_mod.pinned_fs, + "fd_real_path", + lambda fd: str(tmp_path / "redirected"), + ) + monkeypatch.setattr(os, "close", _track_guard_close) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(tmp_path), + content=b"must not be published", + filename="redirected.txt", + ) + assert resp.status == 403, await resp.text() + body = await resp.json() + + assert body["code"] == "symlink_refused" + assert opened and opened == closed + assert not (tmp_path / "redirected.txt").exists() + + +@pytest.mark.asyncio +async def test_directory_validation_runs_off_the_event_loop_thread( + tmp_path: Path, mock_sel, monkeypatch +) -> None: + """``_validate_dashboard_path`` (``realpath`` plus ``is_sensitive_path``'s + candidate-form walk) and the directory-existence check are blocking + filesystem calls. AUTOSDE's ``no-blocking-call-on-event-loop`` rule + (``blocking: true`` -- see ``AUTOSDE.yaml``) is authoritative here: a + stalled network-mounted ``dir`` resolving or stat-ing directly on the + gateway's event loop would freeze every other request and the heartbeat + for as long as that mount takes to time out. Both validation calls + (``target_dir``'s own, and the destination re-check) must run on a + worker thread, never on the thread driving this test's own event loop. + """ + main_thread_ident = threading.get_ident() + seen_idents: list[int] = [] + + real_validate = files_mod._validate_dashboard_path + + def _recording_validate(raw: str) -> str | None: + seen_idents.append(threading.get_ident()) + return real_validate(raw) + + monkeypatch.setattr(files_mod, "_validate_dashboard_path", _recording_validate) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"hello", filename="c.txt") + assert resp.status == 200, await resp.text() + + # target_dir's own validation, plus the destination re-check -- both + # calls this handler makes to _validate_dashboard_path. + assert len(seen_idents) == 2 + assert all(ident != main_thread_ident for ident in seen_idents) + + +@pytest.mark.asyncio +async def test_name_collision_is_refused(tmp_path: Path, mock_sel) -> None: + existing = tmp_path / "report.txt" + existing.write_bytes(b"original") + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"new content", filename="report.txt" + ) + assert resp.status == 409, await resp.text() + body = await resp.json() + assert body["code"] == "name_collision" + assert existing.read_bytes() == b"original" + + +@pytest.mark.asyncio +async def test_name_collision_is_refused_even_when_overwrite_is_requested( + tmp_path: Path, mock_sel +) -> None: + """The endpoint never replaces a file: a request that sends the old + ``overwrite=1`` query parameter still gets ``409 name_collision`` and the + original bytes stay as they were.""" + existing = tmp_path / "report.txt" + existing.write_bytes(b"original") + form = aiohttp.FormData() + form.add_field("file", b"replacement", filename="report.txt", content_type="text/plain") + async with TestClient(TestServer(_make_app())) as client: + resp = await client.post(f"/api/directory-upload?dir={tmp_path}&overwrite=1", data=form) + assert resp.status == 409, await resp.text() + body = await resp.json() + assert body["code"] == "name_collision" + assert existing.read_bytes() == b"original" + + +@pytest.mark.asyncio +async def test_concurrent_create_lands_only_the_winner( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch +) -> None: + """A same-name race between two concurrent CREATE uploads: both classify + the destination as missing, but a rival claims the name before THIS + request's create runs. The create is ``O_EXCL``, so it fails with + ``FileExistsError`` (reported as ``name_collision``) and the rival's bytes + stay as they were. + + The rival's file is planted from ``is_owner_only_target``, the last call + before the create on both the pinned and the by-name branch, so the REAL + ``O_EXCL`` open meets a genuinely present file. + """ + real_is_owner_only_target = files_mod.is_owner_only_target + + def _rival_wins_then_create(path): + Path(path).write_bytes(b"rival's bytes") + return real_is_owner_only_target(path) + + monkeypatch.setattr(files_mod, "is_owner_only_target", _rival_wins_then_create) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"mine", filename="race.txt") + assert resp.status == 409, await resp.text() + body = await resp.json() + assert body["code"] == "name_collision" + assert (tmp_path / "race.txt").read_bytes() == b"rival's bytes" + + +@pytest.mark.asyncio +async def test_existing_directory_name_is_refused(tmp_path: Path, mock_sel) -> None: + (tmp_path / "report.txt").mkdir() + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(tmp_path), + content=b"x", + filename="report.txt", + ) + assert resp.status == 409, await resp.text() + body = await resp.json() + assert body["code"] == "is_a_directory" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "filename", + [ + "report?.txt", + "a None: + """On a Windows host a name Windows cannot store answers a clean 400 + instead of the create's OSError.""" + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload_raw_filename( + client, + directory=str(tmp_path), + content=b"must not be published", + filename=filename, + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "invalid_filename" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("windows", [False, True], ids=["posix", "windows"]) +@pytest.mark.parametrize("control", ["\x01", "\x7f", "\x85"], ids=["c0", "del", "c1"]) +async def test_control_character_filename_is_rejected_on_every_platform( + tmp_path: Path, mock_sel, monkeypatch, windows: bool, control: str +) -> None: + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", windows) + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload_raw_filename( + client, + directory=str(tmp_path), + content=b"must not be published", + filename=f"x{control}y.txt", + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "invalid_filename" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "disposition", + ['form-data; name="file"; filename=""', 'form-data; name="file"'], + ids=["empty-filename", "no-filename"], +) +async def test_empty_filename_is_rejected_as_invalid_filename( + tmp_path: Path, mock_sel, disposition: str +) -> None: + boundary = "kirocrew-directory-upload-test" + body = ( + f"--{boundary}\r\nContent-Disposition: {disposition}\r\n" + "Content-Type: text/plain\r\n\r\nmust not be published\r\n" + f"--{boundary}--\r\n" + ).encode() + async with TestClient(TestServer(_make_app())) as client: + resp = await client.post( + f"/api/directory-upload?dir={tmp_path}", + data=body, + headers={"Content-Type": f"multipart/form-data; boundary={boundary}"}, + ) + assert resp.status == 400, await resp.text() + payload = await resp.json() + assert payload["code"] == "invalid_filename" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +@pytest.mark.skipif( + platform_compat.IS_WINDOWS, reason="the host filesystem cannot store these names" +) +@pytest.mark.parametrize("filename", ["a:b.txt", "a?b.txt", "con.md"]) +async def test_windows_only_names_upload_on_posix(tmp_path: Path, mock_sel, filename: str) -> None: + """A Linux or macOS host stores a name only Windows refuses, as typed.""" + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload_raw_filename( + client, + directory=str(tmp_path), + content=b"payload", + filename=filename, + ) + assert resp.status == 200, await resp.text() + body = await resp.json() + assert body["path"] == str(tmp_path / filename) + assert (tmp_path / filename).read_bytes() == b"payload" + + +@pytest.mark.asyncio +@pytest.mark.skipif( + platform_compat.IS_WINDOWS, reason="the trailing-dot name rule applies on Windows" +) +async def test_trailing_dot_is_not_a_name_refusal_on_posix(tmp_path: Path, mock_sel) -> None: + """On POSIX ``notes.`` passes the name rule; its empty suffix is then + refused by the extension allowlist, like any extension-less name.""" + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload_raw_filename( + client, + directory=str(tmp_path), + content=b"payload", + filename="notes.", + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "unsupported_file_type" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +async def test_unsupported_extension_is_rejected_before_any_write(tmp_path: Path, mock_sel) -> None: + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"MZ\x90\x00", filename="payload.exe" + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "unsupported_file_type" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +async def test_size_limit_is_enforced(tmp_path: Path, mock_sel, monkeypatch) -> None: + monkeypatch.setattr("kiro_crew.dashboard.handlers.files.upload_max_bytes", lambda: 8) + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"way too many bytes", filename="big.txt" + ) + assert resp.status == 413, await resp.text() + body = await resp.json() + assert body["code"] == "file_too_large" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +async def test_content_signature_mismatch_is_rejected(tmp_path: Path, mock_sel) -> None: + """A ``.png`` extension whose bytes are not really PNG is refused + (CWE-434) — the same magic-byte gate ``api_upload_file`` applies.""" + async with TestClient(TestServer(_make_app())) as client: + form = aiohttp.FormData() + form.add_field( + "file", + b"not actually a png", + filename="fake.png", + content_type="image/png", + ) + resp = await client.post(f"/api/directory-upload?dir={tmp_path}", data=form) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "content_signature_mismatch" + assert not any(tmp_path.iterdir()) + + +@pytest.mark.asyncio +async def test_non_multipart_request_is_rejected_with_coded_400(tmp_path: Path, mock_sel) -> None: + """Malformed upload requests are rejected before aiohttp parses multipart.""" + async with TestClient(TestServer(_make_app())) as client: + resp = await client.post( + f"/api/directory-upload?dir={tmp_path}", + json={"file": "not multipart"}, + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body == {"error": "expected multipart/form-data", "code": "not_multipart"} + + +@pytest.mark.asyncio +async def test_missing_directory_is_rejected(tmp_path: Path, mock_sel) -> None: + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(tmp_path / "does-not-exist"), + content=b"x", + filename="a.txt", + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "invalid_directory" + + +@pytest.mark.asyncio +async def test_directory_that_is_actually_a_file_is_rejected(tmp_path: Path, mock_sel) -> None: + a_file = tmp_path / "not-a-dir.txt" + a_file.write_text("x") + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(a_file), content=b"x", filename="a.txt") + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "invalid_directory" + + +@pytest.mark.asyncio +async def test_sensitive_directory_is_rejected(tmp_path: Path, mock_sel, monkeypatch) -> None: + """The same ``is_sensitive_path`` gate ``/api/file-write`` trusts must + also cover the new write direction: a directory ``hooks.validate_file_path`` + flags as sensitive is refused before any multipart body is even read. + Faked via the shared gate (rather than a real ``~/.ssh``) so the test does + not depend on this host's actual home-directory layout.""" + sensitive_dir = tmp_path / "sensitive" + sensitive_dir.mkdir() + monkeypatch.setattr( + "kiro_crew.hooks.is_sensitive_path", + lambda p: os.path.realpath(p) == os.path.realpath(str(sensitive_dir)), + ) + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(sensitive_dir), content=b"x", filename="id_rsa.pub" + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "invalid_directory" + + +@pytest.mark.asyncio +async def test_missing_dir_query_param_is_rejected(tmp_path: Path, mock_sel) -> None: + async with TestClient(TestServer(_make_app())) as client: + form = aiohttp.FormData() + form.add_field("file", b"x", filename="a.txt", content_type="text/plain") + resp = await client.post("/api/directory-upload", data=form) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "missing_required_fields" + + +@pytest.mark.asyncio +async def test_path_traversal_in_filename_lands_inside_target_dir(tmp_path: Path, mock_sel) -> None: + """A filename carrying ``../`` components must never escape the + validated target directory -- the upload lands, but as a plain file + named after its OWN basename, inside ``tmp_path`` only. + + Built as a raw multipart body (rather than ``aiohttp.FormData``) because + the high-level FormData writer percent-encodes a ``/`` inside a + ``filename=`` parameter before it ever reaches the wire, which would + neutralize the traversal attempt in the TEST CLIENT rather than in the + handler under test — this sends the literal, unencoded ``../`` an + attacker-controlled client can send. + + The upload target is a CHILD of ``tmp_path`` (not ``tmp_path`` itself), so + a single ``../`` resolves to ``tmp_path`` -- still inside pytest's + auto-pruned basetemp -- rather than to ``tmp_path.parent``, which is + outside this test's own temp tree and not pytest's to clean up. The + assertion stays exactly as meaningful: it still fails loudly if the + traversal guards regress, since a real escape would land the file at + ``target_dir``'s PARENT (``tmp_path``), which is exactly what is checked + below. + """ + target_dir = tmp_path / "target" + target_dir.mkdir() + outside_marker = tmp_path / "escaped.txt" + outside_marker_existed = outside_marker.exists() + boundary = "kctestboundary" + body = ( + f"--{boundary}\r\n" + 'Content-Disposition: form-data; name="file"; filename="../escaped.txt"\r\n' + "Content-Type: text/plain\r\n\r\n" + "pwned?\r\n" + f"--{boundary}--\r\n" + ).encode("utf-8") + async with TestClient(TestServer(_make_app())) as client: + resp = await client.post( + f"/api/directory-upload?dir={target_dir}", + data=body, + headers={"Content-Type": f"multipart/form-data; boundary={boundary}"}, + ) + assert resp.status == 200, await resp.text() + response_body = await resp.json() + assert response_body["name"] == "escaped.txt" + assert (target_dir / "escaped.txt").read_bytes() == b"pwned?" + # Nothing was ever written outside target_dir (i.e. one level up, at + # tmp_path.parent, which this test never touches). + assert outside_marker.exists() == outside_marker_existed + + +@pytest.fixture() +def crew_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """A ``KIROCREW_HOME`` override, exercising the REAL ``is_sensitive_path`` + gate rather than a monkeypatched stand-in: the keystone leaves + (``security_policy.json``, ``profiles``, ``admission_policy.json``, + ``computer_use.json``) live directly under this directory once the + env var is set, the same way they would under a real crew data home.""" + home = tmp_path / "crew_home" + home.mkdir() + monkeypatch.setenv("KIROCREW_HOME", str(home)) + return home + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "leaf_name", + ["security_policy.json", "computer_use.json", "admission_policy.json", "profiles"], +) +async def test_keystone_destination_is_refused(crew_home: Path, mock_sel, leaf_name: str) -> None: + """The crew-home ROOT passes ``_validate_dashboard_path`` on its own (it + is deliberately not itself sensitive -- only its leaves are), so naming a + keystone leaf as the UPLOADED FILENAME while ``dir`` is the crew-home root + must still be refused by the re-gated destination check. This is the + exact escape the destination re-validation in ``api_directory_upload`` + closes: ``dir=`` with a ``file`` part named after an absent + keystone leaf must never create it.""" + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(crew_home), + content=b"forged content", + filename=leaf_name, + ) + assert resp.status == 403, await resp.text() + body = await resp.json() + assert body["code"] == "sensitive_destination" + # Nothing was written: the keystone leaf must not even come to exist. + assert not (crew_home / leaf_name).exists() + + +@pytest.mark.asyncio +async def test_keystone_destination_is_refused_when_leaf_already_exists( + crew_home: Path, mock_sel +) -> None: + """Same refusal, but against an EXISTING keystone file with real content -- + the destination re-check runs before any collision logic, so the + original ceiling is provably untouched, not merely "not yet created".""" + policy = crew_home / "security_policy.json" + policy.write_text('{"real": "ceiling"}') + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(crew_home), + content=b'{"forged": "ceiling"}', + filename="security_policy.json", + ) + assert resp.status == 403, await resp.text() + body = await resp.json() + assert body["code"] == "sensitive_destination" + assert policy.read_text() == '{"real": "ceiling"}' + + +@pytest.mark.asyncio +async def test_crew_home_root_as_dir_cannot_reach_a_nested_keystone_leaf( + crew_home: Path, mock_sel +) -> None: + """``profiles`` is gated as a whole DIRECTORY, so a file inside it is just + as much a keystone write as the bare leaf name above -- prove the same + re-gate closes that path too, by asking for the upload directly on the + (pre-existing) ``profiles`` directory rather than the crew-home root.""" + profiles_dir = crew_home / "profiles" + profiles_dir.mkdir() + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(profiles_dir), + content=b"rogue profile", + filename="rogue.json", + ) + assert resp.status == 400, await resp.text() + body = await resp.json() + # Caught by the DIRECTORY gate this time (dir itself is sensitive), not the + # destination re-check -- either gate refusing is correct; this pins that + # at least one of them does, so nothing can reach a leaf through the + # crew-home root, whether by naming the leaf directly or by walking into + # its directory. + assert body["code"] == "invalid_directory" + assert not any(profiles_dir.iterdir()) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("existing_kind", "expected_status", "expected_code"), + [ + ("dir", 409, "is_a_directory"), + ("file", 409, "name_collision"), + ("missing", 200, None), + ], +) +async def test_by_name_floor_still_classifies_the_destination_correctly( + tmp_path: Path, + mock_sel, + monkeypatch, + existing_kind: str, + expected_status: int, + expected_code: str | None, +) -> None: + """Forces the platform to the "cannot pin a directory descriptor" floor + (``pinned_fs.supports_pinned_walk`` returning False) -- the branch Windows + takes for real, since it has no ``O_NOFOLLOW`` + ``dir_fd`` support to pin + with -- and pins that the by-name existing-target classification + (``platform_compat.is_link_or_junction`` / ``Path.is_dir()`` / + ``Path.is_file()``) still gets missing/dir/file right, matching the + dir-fd-pinned classification's outcomes exactly. + + This is a real regression: the PRE-REWRITE ``_write()`` used a single + ``os.open(dest, O_EXCL/O_TRUNC | O_NOFOLLOW, ...)`` and branched on the + POSIX-specific exceptions it raised (``IsADirectoryError`` etc.) -- exactly + the design CI's Windows shard caught opening a directory with write flags, + where the OS does not raise that same exception type, so the branch never + matched and the exception surfaced as an unhandled 500 instead of the + intended 409. Detecting the existing target's kind BEFORE ever attempting + to open it (as this rewrite does) does not depend on which exception type + a given OS raises for which failure, so it does not reproduce that gap -- + which is what this test, run for real on every platform CI exercises, + pins. + """ + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + # Also forces IS_WINDOWS so the handler takes its Windows ancestor-guard + # branch (platform_compat.pin_directory) rather than failing closed with + # ancestor_pin_unsupported -- that branch's own real POSIX pin_directory + # implementation still works fine here (IS_POSIX is untouched), so this + # reaches the same by-name classify/publish floor the test's docstring + # describes, guarded exactly as Windows would guard it for real. + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + monkeypatch.setattr( + files_mod.pinned_fs, + "fd_real_path", + lambda fd: str(tmp_path), + ) + target = tmp_path / "report.txt" + if existing_kind == "dir": + target.mkdir() + elif existing_kind == "file": + target.write_bytes(b"original") + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, + directory=str(tmp_path), + content=b"new content", + filename="report.txt", + ) + assert resp.status == expected_status, await resp.text() + if expected_code is not None: + body = await resp.json() + assert body["code"] == expected_code + + if expected_status == 200: + assert target.read_bytes() == b"new content" + elif existing_kind == "file": + assert target.read_bytes() == b"original" + + +@pytest.mark.asyncio +async def test_ancestor_swapped_after_directory_validation_is_refused( + tmp_path: Path, mock_sel, monkeypatch +) -> None: + """Mirrors ``test_a_grandparent_swapped_after_canonicalization_is_refused`` + in ``test_dashboard_pinned_write_migration.py`` (the ``api_file_write`` + sibling's own coverage of the identical race): ``pinned_fs.pin_parent`` + walks ``target_dir``'s OWN already-resolved component chain with + ``O_NOFOLLOW`` rather than re-resolving it, so an ancestor swapped for a + symlink AFTER validation is refused rather than followed. + + ``_validate_dashboard_path`` is frozen to the pre-swap canonical directory + here (bypassing a fresh, live re-resolution) to model a request that is + already past validation at the moment the swap lands -- a live + re-resolution at request time would simply resolve through the new + symlink and validate a different, equally legitimate directory, which is + a different scenario than the one this protects against. + """ + if not files_mod.pinned_fs.supports_pinned_walk(): + pytest.skip("platform without pinned directory descriptors") + + named = tmp_path / "named" + (named / "mid" / "leaf").mkdir(parents=True) + canonical_target = str(Path(os.path.realpath(named / "mid" / "leaf"))) + + # The tree an attacker wants the write redirected into, holding a file + # that must survive with its own bytes. + victim = tmp_path / "victim" + (victim / "leaf").mkdir(parents=True) + (victim / "leaf" / "sentinel.txt").write_text("PROTECTED") + + shutil.rmtree(named / "mid") + (named / "mid").symlink_to(victim, target_is_directory=True) + + monkeypatch.setattr(files_mod, "_validate_dashboard_path", lambda raw: canonical_target) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=canonical_target, content=b"attacker body", filename="a.txt" + ) + assert resp.status == 403, await resp.text() + body = await resp.json() + assert body["code"] == "symlink_refused" + assert not (victim / "leaf" / "a.txt").exists() + assert (victim / "leaf" / "sentinel.txt").read_text() == "PROTECTED" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("branch", ["pinned", "by-name"]) +async def test_a_write_that_fails_mid_body_leaves_nothing_behind( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch, branch: str +) -> None: + """The file is created, the first ``os.write`` lands half the body and the + second fails with ``ENOSPC``: the rollback removes the partly written + file, so the destination is empty and a retry is not refused 409. Run on + the pinned branch and on the by-name branch Windows takes.""" + if branch == "pinned" and not files_mod.pinned_fs.supports_pinned_walk(): + pytest.skip("platform without pinned directory descriptors") + if branch == "by-name": + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + monkeypatch.setattr(files_mod.pinned_fs, "fd_real_path", lambda fd: str(tmp_path)) + payload = b"0123456789" * 8 + half = len(payload) // 2 + real_write = os.write + calls: list[int] = [] + + def _half_then_disk_full(fd, data): + if bytes(data) in (payload, payload[half:]): + calls.append(fd) + if len(calls) == 1: + return real_write(fd, payload[:half]) + raise OSError(errno.ENOSPC, "No space left on device") + return real_write(fd, data) + + monkeypatch.setattr(os, "write", _half_then_disk_full) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=payload, filename="report.txt" + ) + assert resp.status == 500, await resp.text() + body = await resp.json() + assert body["code"] == "upload_failed" + assert len(calls) == 2 + assert not (tmp_path / "report.txt").exists() + + +@pytest.mark.asyncio +async def test_a_failed_first_identity_probe_still_rolls_back_on_the_by_name_branch( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch +) -> None: + """On the by-name branch Windows takes, the ``O_EXCL`` create succeeds but + the first ``os.fstat`` fails (a transient EIO). The rollback re-asks the + identity through the descriptor, so the empty file is removed and a retry + of the same name is not refused 409.""" + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + monkeypatch.setattr(files_mod.pinned_fs, "fd_real_path", lambda fd: str(tmp_path)) + real_fstat = os.fstat + target = tmp_path / "report.txt" + failed: list[int] = [] + + def _first_probe_fails(fd, *args, **kwargs): + if not failed and target.exists(): + failed.append(fd) + raise OSError(errno.EIO, "Input/output error") + return real_fstat(fd, *args, **kwargs) + + monkeypatch.setattr(os, "fstat", _first_probe_fails) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"body", filename="report.txt" + ) + assert resp.status == 500, await resp.text() + assert (await resp.json())["code"] == "upload_failed" + assert failed, "the first identity probe never ran on the created file" + assert not target.exists() + retry = await _upload( + client, directory=str(tmp_path), content=b"body", filename="report.txt" + ) + assert retry.status == 200, await retry.text() + assert target.read_bytes() == b"body" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "refusal", + [ + NotADirectoryError(errno.ENOTDIR, "not a real directory"), + OSError(errno.ELOOP, "reparse point at the final component"), + ], + ids=["not-a-directory", "eloop"], +) +async def test_windows_without_pinned_walk_guards_ancestors_via_pin_directory( + tmp_path: Path, mock_sel, monkeypatch, refusal: OSError +) -> None: + """Windows has no O_NOFOLLOW + dir_fd combination to pin a parent CHAIN + with (``pinned_fs.supports_pinned_walk()`` is False there), so a bare + by-name write there would leave open the exact ancestor-swap window this + fix closes: ``platform_compat.pin_directory(target_dir)`` is called and + its handle held across the whole classify-then-publish -- which, for + real on Windows, blocks renaming ANY ancestor while the handle lives + (``TestPinDirectory::test_a_pinned_directory_cannot_be_renamed_or_removed`` + in ``test_platform_compat.py``). + + Simulated here (this test runs on POSIX dev machines) by forcing the + "not pinned" branch plus ``IS_WINDOWS``, and replacing ``pin_directory`` + with a fake that raises exactly as the real one does when a link or + junction already sits at the name. Before this fix there was no such call + and no such branch at all: the handler fell straight through to an + unguarded by-name write, so this fake would never even be reached and + the upload would have SUCCEEDED instead of being refused. + """ + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + + calls: list[str] = [] + + def _fake_pin_directory(path, **_kwargs): + calls.append(str(path)) + raise refusal + + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", _fake_pin_directory) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"attacker", filename="a.txt") + assert resp.status == 403, await resp.text() + body = await resp.json() + assert body["code"] == "symlink_refused" + assert calls == [str(tmp_path)] + assert not (tmp_path / "a.txt").exists() + + +@pytest.mark.asyncio +async def test_windows_without_pinned_walk_still_uploads_when_pin_succeeds( + tmp_path: Path, mock_sel, monkeypatch +) -> None: + """Companion to the refusal test above: when ``pin_directory`` succeeds, + the upload still proceeds normally (by name, exactly as the by-name + floor already did) with the guard held for the duration and released + afterward -- the fix adds a held guard, not a new failure mode for the + ordinary case.""" + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + monkeypatch.setattr( + files_mod.pinned_fs, + "fd_real_path", + lambda fd: str(tmp_path), + ) + + real_pin_directory = files_mod.platform_compat.pin_directory + opened: list[int] = [] + closed: list[int] = [] + + def _tracking_pin_directory(path, **kwargs): + fd = real_pin_directory(path, **kwargs) + opened.append(fd) + return fd + + real_close = os.close + + def _tracking_close(fd, *a, **k): + if fd in opened: + closed.append(fd) + return real_close(fd, *a, **k) + + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", _tracking_pin_directory) + monkeypatch.setattr(os, "close", _tracking_close) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"hello", filename="b.txt") + assert resp.status == 200, await resp.text() + assert (tmp_path / "b.txt").read_bytes() == b"hello" + assert opened and opened == closed + + +def _cloud_files_pin(real_pin): + """A ``pin_directory`` stand-in for a OneDrive / cloud-files folder on Windows. + + Such a folder carries a reparse attribute whose tag is not a name surrogate, + so the default pin refuses it as the real one does and ``links_only=True`` + pins it. + """ + calls: list[bool] = [] + + def _pin(path, *, links_only=False): + calls.append(links_only) + if not links_only: + raise NotADirectoryError(errno.ENOTDIR, "not a real directory", str(path)) + return real_pin(path, links_only=True) + + return _pin, calls + + +def _force_windows_branch(monkeypatch: pytest.MonkeyPatch, target: Path) -> None: + monkeypatch.setattr(files_mod.pinned_fs, "supports_pinned_walk", lambda: False) + monkeypatch.setattr(files_mod.platform_compat, "IS_WINDOWS", True) + monkeypatch.setattr(files_mod.pinned_fs, "fd_real_path", lambda fd: str(target)) + + +@pytest.mark.asyncio +async def test_windows_upload_into_a_cloud_files_folder_lands_the_file( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch +) -> None: + """A folder under a OneDrive sync root is pinned with ``links_only`` and the + upload lands, rather than being refused as a symlink.""" + _force_windows_branch(monkeypatch, tmp_path) + pin, calls = _cloud_files_pin(files_mod.platform_compat.pin_directory) + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", pin) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"hello", filename="c.txt") + assert resp.status == 200, await resp.text() + assert (tmp_path / "c.txt").read_bytes() == b"hello" + assert calls == [True] + + +@pytest.mark.asyncio +async def test_windows_pin_refused_for_another_reason_answers_upload_failed( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch +) -> None: + """A pin the filesystem refuses for a reason that is not a link (a sharing + violation, ``EACCES``) is a failed write, 500 ``upload_failed``, never a + 403 ``symlink_refused``.""" + _force_windows_branch(monkeypatch, tmp_path) + + def _denied_pin(path, **_kwargs): + raise PermissionError(errno.EACCES, "Permission denied", str(path)) + + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", _denied_pin) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=b"hello", filename="d.txt") + assert resp.status == 500, await resp.text() + assert (await resp.json())["code"] == "upload_failed" + assert not (tmp_path / "d.txt").exists() + assert mock_sel.log_api_access.call_args.kwargs["outcome"] == "failure" + + +@pytest.mark.asyncio +async def test_windows_failed_write_into_a_cloud_files_folder_still_rolls_back( + tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch +) -> None: + """The rollback re-pins the folder through ``unlink_verified_by_name``; in a + cloud-files folder that pin uses ``links_only`` too, so the partly written + file is removed.""" + _force_windows_branch(monkeypatch, tmp_path) + pin, _calls = _cloud_files_pin(files_mod.platform_compat.pin_directory) + rollback_pin, rollback_calls = _cloud_files_pin(files_mod.pinned_fs.pin_directory) + monkeypatch.setattr(files_mod.platform_compat, "pin_directory", pin) + monkeypatch.setattr(files_mod.pinned_fs, "pin_directory", rollback_pin) + payload = b"0123456789" * 8 + real_write = os.write + + def _disk_full_after_half(fd, data): + if bytes(data) == payload: + real_write(fd, payload[: len(payload) // 2]) + raise OSError(errno.ENOSPC, "No space left on device") + return real_write(fd, data) + + monkeypatch.setattr(os, "write", _disk_full_after_half) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload(client, directory=str(tmp_path), content=payload, filename="e.txt") + assert resp.status == 500, await resp.text() + assert (await resp.json())["code"] == "upload_failed" + assert rollback_calls == [True] + assert not (tmp_path / "e.txt").exists() + + +@pytest.mark.asyncio +async def test_no_file_part_is_rejected(tmp_path: Path, mock_sel) -> None: + async with TestClient(TestServer(_make_app())) as client: + form = aiohttp.FormData() + form.add_field("not_file", b"x", filename="not_file.txt", content_type="text/plain") + resp = await client.post(f"/api/directory-upload?dir={tmp_path}", data=form) + assert resp.status == 400, await resp.text() + body = await resp.json() + assert body["code"] == "missing_required_fields" + + +class TestWriteFailureIsACodedError: + """A write the filesystem refuses answers a coded JSON 500, never aiohttp's bare one. + + The create raises ``PermissionError`` for a destination the gateway cannot + write (read-only or root-owned) and ``OSError`` for ``ENOSPC``; the rail + renders ``body.error`` verbatim, so an unhandled raise shows "Internal + Server Error" with no ``code`` to switch on. The refusal is injected at the + file flush, which both the pinned and the by-name branch run. + """ + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "exc", + [ + PermissionError(errno.EACCES, "Permission denied"), + OSError(errno.ENOSPC, "No space left on device"), + ], + ids=["permission-denied", "disk-full"], + ) + async def test_write_oserror_answers_json_500_with_code( + self, tmp_path: Path, mock_sel, monkeypatch: pytest.MonkeyPatch, exc: OSError + ) -> None: + def _refuse_flush(_fd: int) -> None: + raise exc + + monkeypatch.setattr(os, "fsync", _refuse_flush) + + async with TestClient(TestServer(_make_app())) as client: + resp = await _upload( + client, directory=str(tmp_path), content=b"payload", filename="notes.txt" + ) + assert resp.status == 500, await resp.text() + assert resp.content_type == "application/json" + body = await resp.json() + + assert body["code"] == "upload_failed" + assert body["error"] + assert not (tmp_path / "notes.txt").exists() + assert mock_sel.log_api_access.call_args.kwargs["outcome"] == "failure" + assert mock_sel.log_api_access.call_args.kwargs["resources"] == str(tmp_path / "notes.txt") diff --git a/test/test_platform_compat_filesystem.py b/test/test_platform_compat_filesystem.py index 6e1c91775dd..d8aaca5cbf2 100644 --- a/test/test_platform_compat_filesystem.py +++ b/test/test_platform_compat_filesystem.py @@ -259,6 +259,74 @@ def test_a_pinned_directory_cannot_be_renamed_or_removed(self, tmp_path): assert (tmp_path / "swapped").is_dir() +class TestPinDirectoryLinksOnly: + """``pin_directory(links_only=...)`` on the Windows branch, with its two reads stubbed. + + A OneDrive / cloud-files folder is a directory carrying + ``FILE_ATTRIBUTE_REPARSE_POINT`` with a tag that is NOT a name surrogate, and + no CI host can make one, so the attribute read (``os.fstat``) and the tag read + (``win_fd_is_link``) are replaced and the open hands back a plain descriptor. + """ + + @pytest.fixture + def windows_pin(self, tmp_path, monkeypatch): + import kiro_crew.platform_compat_links as links + + stand_in = tmp_path / "stand-in" + stand_in.write_text("x") + opened: list[int] = [] + state = { + "attrs": links._WIN_FILE_ATTRIBUTE_DIRECTORY | links._WIN_FILE_ATTRIBUTE_REPARSE_POINT, + "is_link": False, + } + real_fstat = os.fstat + + def _open(path, **_kwargs): + opened.append(os.open(stand_in, os.O_RDONLY)) + return opened[-1] + + def _fstat(fd, *args, **kwargs): + info = real_fstat(fd, *args, **kwargs) + if fd not in opened: + return info + return types.SimpleNamespace( + **{ + name: getattr(info, name) + for name in dir(info) + if name.startswith("st_") and name != "st_file_attributes" + }, + st_file_attributes=state["attrs"], + ) + + monkeypatch.setattr(links, "IS_POSIX", False) + monkeypatch.setattr(links, "_win_open_without_following", _open) + monkeypatch.setattr(links, "win_fd_is_link", lambda _fd: state["is_link"]) + monkeypatch.setattr(os, "fstat", _fstat) + return links, state + + def test_default_refuses_a_cloud_files_directory(self, windows_pin, tmp_path): + links, _state = windows_pin + with pytest.raises(NotADirectoryError): + links.pin_directory(tmp_path / "OneDrive") + + def test_links_only_pins_a_cloud_files_directory(self, windows_pin, tmp_path): + links, _state = windows_pin + fd = links.pin_directory(tmp_path / "OneDrive", links_only=True) + os.close(fd) + + def test_links_only_still_refuses_a_junction(self, windows_pin, tmp_path): + links, state = windows_pin + state["is_link"] = True + with pytest.raises(NotADirectoryError): + links.pin_directory(tmp_path / "junction", links_only=True) + + def test_links_only_still_refuses_a_file(self, windows_pin, tmp_path): + links, state = windows_pin + state["attrs"] = links._WIN_FILE_ATTRIBUTE_REPARSE_POINT + with pytest.raises(NotADirectoryError): + links.pin_directory(tmp_path / "placeholder.txt", links_only=True) + + # --------------------------------------------------------------------------- # POSIX-branch coverage for the new platform_compat helpers. The # tests below deliberately exercise the ``if IS_POSIX:`` / Linux ``/proc`` paths diff --git a/website/src/api/client/files.ts b/website/src/api/client/files.ts index 7edcf833422..505d4c119dc 100644 --- a/website/src/api/client/files.ts +++ b/website/src/api/client/files.ts @@ -2,7 +2,7 @@ * Files and projects on the gateway host: worktree creation, recent * projects, directory, drive and file browsing, project git status/log/tree, * named workspaces, reveal/open, the file picker, diff, fuzzy search, path - * completion, composer uploads and screenshots. + * completion, composer and Files-panel uploads and screenshots. */ import { COMPOSER_UPLOAD_PATH, uploadComposerFiles } from '../../utils/composerUpload' @@ -193,6 +193,35 @@ export function createFilesEndpoints({ post, put, del, j, jfetch: fetch, checkSe checkSessionExpired(res) return res }), + /** + * Upload one OS file into an existing workspace directory (Files panel + * drag-and-drop / "Upload files…" row menu). + * + * Sibling of `uploadFiles` above: that one always lands in the fixed + * chat-attachment scratch directory; this one targets whatever directory + * the caller resolved (a drop target row, or the tree root). One request + * per file — the caller loops for multiple files — so a name collision is + * reported per file rather than for a whole batch at once. Returns a + * discriminated result rather than throwing, so a `name_collision` (409) + * reads as data the caller can report instead of an exception to catch. + */ + uploadToDirectory: async ( + dir: string, file: File, + ): Promise< + { ok: true; path: string; name: string } | { ok: false; status: number; code?: string; error: string } + > => { + const fd = new FormData() + fd.append('file', file) + const q = new URLSearchParams({ dir }) + const res = await fetch(`/api/directory-upload?${q}`, { method: 'POST', body: fd }) + checkSessionExpired(res) + let body: { path?: string; name?: string; error?: string; code?: string } + try { body = await res.json() } catch { body = {} } + if (!res.ok) { + return { ok: false, status: res.status, code: body.code, error: body.error || res.statusText } + } + return { ok: true, path: body.path ?? '', name: body.name ?? file.name } + }, screenshot: () => post('/api/screenshot').then(j) as Promise<{ path: string }>, } diff --git a/website/src/components/ChatDropOverlay.tsx b/website/src/components/ChatDropOverlay.tsx index 5c05c2feb75..c8c335dff4f 100644 --- a/website/src/components/ChatDropOverlay.tsx +++ b/website/src/components/ChatDropOverlay.tsx @@ -104,8 +104,12 @@ export function useChatFileDrop( export default function ChatDropOverlay({ active, + label, }: { active: boolean + /** Overrides the default "Drop to attach" copy — the Files rail reuses + * this same overlay for its own drop target with its own wording. */ + label?: string }) { const reduceMotion = useReducedMotion() @@ -147,7 +151,7 @@ export default function ChatDropOverlay({ }} /> - {i18nT('components.chatInput.drop_to_attach')} + {label ?? i18nT('components.chatInput.drop_to_attach')} diff --git a/website/src/i18n/locales/bn.json b/website/src/i18n/locales/bn.json index b37dc4a1e2a..6db9f0f1bf8 100644 --- a/website/src/i18n/locales/bn.json +++ b/website/src/i18n/locales/bn.json @@ -11961,6 +11961,8 @@ "content_searching": "খোঁজা হচ্ছে…", "ctx_add_to_chat": "চ্যাটে যোগ করুন", "ctx_download": "ডাউনলোড", + "ctx_upload_files": "ফাইল আপলোড করো…", + "drop_to_upload": "{{folder}}-এ আপলোড করতে এখানে ছাড়ো", "filter_placeholder": "ফাইল ফিল্টার করুন…", "git_status_failed": "Git স্ট্যাটাস পড়া যায়নি", "mode_content": "বিষয়বস্তু", @@ -11972,7 +11974,15 @@ "search_names": "ফাইলের নাম খুঁজুন", "tree_mode": "ফাইল তালিকা মোড", "content_engine_slow": "এই হোস্টে ripgrep নেই, তাই অনুসন্ধানটি ধীরগতির অন্তর্নির্মিত স্ক্যানে চালানো হয়েছে।", - "content_one_row_per_file": "প্রতি ফাইলে একটি সারি" + "content_one_row_per_file": "প্রতি ফাইলে একটি সারি", + "upload_exists": "“{{name}}” এই ফোল্ডারে ইতিমধ্যে আছে। তোমার ফাইলের নাম বদলাও বা আগে থেকে থাকা ফাইলটি সরিয়ে দাও, তারপর আবার আপলোড করো।", + "upload_failed": "{{name}} আপলোড করা যায়নি", + "uploading_name": "{{name}} আপলোড হচ্ছে…", + "uploading_count_one": "{{count}}টি ফাইল আপলোড হচ্ছে…", + "uploading_count_other": "{{count}}টি ফাইল আপলোড হচ্ছে…", + "uploaded_name": "{{name}} {{folder}}-এ আপলোড হয়েছে।", + "uploaded_count_one": "{{count}}টি ফাইল {{folder}}-এ আপলোড হয়েছে।", + "uploaded_count_other": "{{count}}টি ফাইল {{folder}}-এ আপলোড হয়েছে।" }, "filesHome": { "no_project_dir": "এই চ্যাটের জন্য কোনো প্রজেক্ট ডিরেক্টরি সেট করা নেই", diff --git a/website/src/i18n/locales/de.json b/website/src/i18n/locales/de.json index 313ebebd055..7e96e408c91 100644 --- a/website/src/i18n/locales/de.json +++ b/website/src/i18n/locales/de.json @@ -11961,6 +11961,8 @@ "content_searching": "Suche läuft…", "ctx_add_to_chat": "Zum Chat hinzufügen", "ctx_download": "Herunterladen", + "ctx_upload_files": "Dateien hochladen…", + "drop_to_upload": "Ablegen, um in {{folder}} hochzuladen", "filter_placeholder": "Dateien filtern…", "git_status_failed": "Git-Status konnte nicht gelesen werden", "mode_content": "Inhalt", @@ -11972,7 +11974,15 @@ "search_names": "Dateinamen durchsuchen", "tree_mode": "Dateilisten-Modus", "content_engine_slow": "Auf diesem Host ist ripgrep nicht vorhanden, daher wurde die langsamere integrierte Suche verwendet.", - "content_one_row_per_file": "eine Zeile pro Datei" + "content_one_row_per_file": "eine Zeile pro Datei", + "upload_exists": "„{{name}}“ existiert bereits in diesem Ordner. Benenne deine Datei um oder entferne die vorhandene und lade dann erneut hoch.", + "upload_failed": "Hochladen von {{name}} fehlgeschlagen", + "uploading_name": "{{name}} wird hochgeladen…", + "uploading_count_one": "{{count}} Datei wird hochgeladen…", + "uploading_count_other": "{{count}} Dateien werden hochgeladen…", + "uploaded_name": "{{name}} wurde in {{folder}} hochgeladen.", + "uploaded_count_one": "{{count}} Datei wurde in {{folder}} hochgeladen.", + "uploaded_count_other": "{{count}} Dateien wurden in {{folder}} hochgeladen." }, "filesHome": { "no_project_dir": "Für diesen Chat ist kein Projektverzeichnis festgelegt", diff --git a/website/src/i18n/locales/en-XA.json b/website/src/i18n/locales/en-XA.json index 45f9b9bbedc..bd7279de28f 100644 --- a/website/src/i18n/locales/en-XA.json +++ b/website/src/i18n/locales/en-XA.json @@ -11702,6 +11702,8 @@ "content_searching": "[Şèàŕçĥìñğ… ···············]", "ctx_add_to_chat": "[Àðð ţø çĥàţ ··········]", "ctx_download": "[Ðøẁñĺøàð ············]", + "ctx_upload_files": "[Ùþĺøàð ƒìĺèş… ············]", + "drop_to_upload": "[Ðŕøþ ţø ùþĺøàð ìñţø {{folder}} ··················]", "filter_placeholder": "[Ƒìĺţèŕ ƒìĺèş… ············]", "git_status_failed": "[Çøùĺðñ'ţ ŕèàð ţĥè ğìţ şţàţùş ····················]", "mode_content": "[Çøñţèñţ ···········]", @@ -11713,7 +11715,15 @@ "search_names": "[Şèàŕçĥ ƒìĺè ñàɱèş ···············]", "tree_mode": "[Ƒìĺè ĺìşţ ɱøðè ·············]", "content_engine_slow": "[Ţĥìş ĥøşţ ĥàş ñø ŕìþğŕèþ, şø ţĥè şèàŕçĥ ţøøķ ţĥè şĺøẁèŕ ƀùìĺţ-ìñ şçàñ. ·························]", - "content_one_row_per_file": "[øñè ŕøẁ þèŕ ƒìĺè ··············]" + "content_one_row_per_file": "[øñè ŕøẁ þèŕ ƒìĺè ··············]", + "upload_exists": "[“{{name}}” àĺŕèàðý èẋìşţş ìñ ţĥìş ƒøĺðèŕ. Ŕèñàɱè ýøùŕ ƒìĺè, øŕ ŕèɱøṽè ţĥè èẋìşţìñğ øñè, ţĥèñ ùþĺøàð àğàìñ. ·····························]", + "upload_failed": "[Çøùĺðñ'ţ ùþĺøàð {{name}} ··············]", + "uploading_name": "[Ùþĺøàðìñğ {{name}}… ··········]", + "uploading_count_one": "[Ùþĺøàðìñğ {{count}} ƒìĺè… ··············]", + "uploading_count_other": "[Ùþĺøàðìñğ {{count}} ƒìĺèş… ···············]", + "uploaded_name": "[Ùþĺøàðèð {{name}} ţø {{folder}}. ·············]", + "uploaded_count_one": "[Ùþĺøàðèð {{count}} ƒìĺè ţø {{folder}}. ·················]", + "uploaded_count_other": "[Ùþĺøàðèð {{count}} ƒìĺèş ţø {{folder}}. ··················]" }, "filesHome": { "no_project_dir": "[Ñø þŕøĵèçţ ðìŕèçţøŕý ìş şèţ ƒøŕ ţĥìş çĥàţ ·····················]", diff --git a/website/src/i18n/locales/en.manual.json b/website/src/i18n/locales/en.manual.json index e0fc4415dea..64a8b546866 100644 --- a/website/src/i18n/locales/en.manual.json +++ b/website/src/i18n/locales/en.manual.json @@ -5304,6 +5304,8 @@ "content_searching": "Searching…", "ctx_add_to_chat": "Add to chat", "ctx_download": "Download", + "ctx_upload_files": "Upload files…", + "drop_to_upload": "Drop to upload into {{folder}}", "filter_placeholder": "Filter files…", "git_status_failed": "Couldn't read the git status", "mode_content": "Content", @@ -5315,7 +5317,15 @@ "search_names": "Search file names", "tree_mode": "File list mode", "content_engine_slow": "This host has no ripgrep, so the search took the slower built-in scan.", - "content_one_row_per_file": "one row per file" + "content_one_row_per_file": "one row per file", + "upload_exists": "“{{name}}” already exists in this folder. Rename your file, or remove the existing one, then upload again.", + "upload_failed": "Couldn't upload {{name}}", + "uploading_name": "Uploading {{name}}…", + "uploading_count_one": "Uploading {{count}} file…", + "uploading_count_other": "Uploading {{count}} files…", + "uploaded_name": "Uploaded {{name}} to {{folder}}.", + "uploaded_count_one": "Uploaded {{count}} file to {{folder}}.", + "uploaded_count_other": "Uploaded {{count}} files to {{folder}}." }, "filesHome": { "no_project_dir": "No project directory is set for this chat", diff --git a/website/src/i18n/locales/es.json b/website/src/i18n/locales/es.json index ae22e2af441..35b63f1d2b2 100644 --- a/website/src/i18n/locales/es.json +++ b/website/src/i18n/locales/es.json @@ -12144,6 +12144,8 @@ "content_searching": "Buscando…", "ctx_add_to_chat": "Añadir al chat", "ctx_download": "Descargar", + "ctx_upload_files": "Subir archivos…", + "drop_to_upload": "Suelta para subir a {{folder}}", "filter_placeholder": "Filtrar archivos…", "git_status_failed": "No se pudo leer el estado de git", "mode_content": "Contenido", @@ -12155,7 +12157,17 @@ "search_names": "Buscar nombres de archivo", "tree_mode": "Modo de lista de archivos", "content_engine_slow": "Este host no tiene ripgrep, por lo que la búsqueda usó el escaneo interno más lento.", - "content_one_row_per_file": "una fila por archivo" + "content_one_row_per_file": "una fila por archivo", + "upload_exists": "“{{name}}” ya existe en esta carpeta. Cambia el nombre de tu archivo o elimina el existente y vuelve a subirlo.", + "upload_failed": "No se pudo subir {{name}}", + "uploading_name": "Subiendo {{name}}…", + "uploading_count_one": "Subiendo {{count}} archivo…", + "uploading_count_many": "Subiendo {{count}} archivos…", + "uploading_count_other": "Subiendo {{count}} archivos…", + "uploaded_name": "Se subió {{name}} a {{folder}}.", + "uploaded_count_one": "Se subió {{count}} archivo a {{folder}}.", + "uploaded_count_many": "Se subieron {{count}} archivos a {{folder}}.", + "uploaded_count_other": "Se subieron {{count}} archivos a {{folder}}." }, "filesHome": { "no_project_dir": "Este chat no tiene un directorio de proyecto", diff --git a/website/src/i18n/locales/fr.json b/website/src/i18n/locales/fr.json index ee3500f75c4..2be09826805 100644 --- a/website/src/i18n/locales/fr.json +++ b/website/src/i18n/locales/fr.json @@ -12144,6 +12144,8 @@ "content_searching": "Recherche…", "ctx_add_to_chat": "Ajouter à la conversation", "ctx_download": "Télécharger", + "ctx_upload_files": "Importer des fichiers…", + "drop_to_upload": "Dépose pour importer dans {{folder}}", "filter_placeholder": "Filtrer les fichiers…", "git_status_failed": "Impossible de lire l’état git", "mode_content": "Contenu", @@ -12155,7 +12157,17 @@ "search_names": "Rechercher dans les noms de fichiers", "tree_mode": "Mode liste de fichiers", "content_engine_slow": "Cet hôte n'a pas ripgrep, la recherche a donc utilisé le balayage interne plus lent.", - "content_one_row_per_file": "une ligne par fichier" + "content_one_row_per_file": "une ligne par fichier", + "upload_exists": "« {{name}} » existe déjà dans ce dossier. Renomme ton fichier ou supprime celui qui existe, puis importe-le à nouveau.", + "upload_failed": "Impossible d'importer {{name}}", + "uploading_name": "Importation de {{name}}…", + "uploading_count_one": "Importation de {{count}} fichier…", + "uploading_count_many": "Importation de {{count}} fichiers…", + "uploading_count_other": "Importation de {{count}} fichiers…", + "uploaded_name": "{{name}} importé dans {{folder}}.", + "uploaded_count_one": "{{count}} fichier importé dans {{folder}}.", + "uploaded_count_many": "{{count}} fichiers importés dans {{folder}}.", + "uploaded_count_other": "{{count}} fichiers importés dans {{folder}}." }, "filesHome": { "no_project_dir": "Aucun répertoire de projet n'est défini pour cette discussion", diff --git a/website/src/i18n/locales/hi.json b/website/src/i18n/locales/hi.json index 9d00885e72e..78b4e907040 100644 --- a/website/src/i18n/locales/hi.json +++ b/website/src/i18n/locales/hi.json @@ -11961,6 +11961,8 @@ "content_searching": "खोज रहे हैं…", "ctx_add_to_chat": "चैट में जोड़ें", "ctx_download": "डाउनलोड करें", + "ctx_upload_files": "फ़ाइलें अपलोड करो…", + "drop_to_upload": "{{folder}} में अपलोड करने के लिए यहाँ छोड़ो", "filter_placeholder": "फ़ाइलें फ़िल्टर करें…", "git_status_failed": "Git स्थिति पढ़ी नहीं जा सकी", "mode_content": "सामग्री", @@ -11972,7 +11974,15 @@ "search_names": "फ़ाइल नाम खोजें", "tree_mode": "फ़ाइल सूची मोड", "content_engine_slow": "इस होस्ट पर ripgrep नहीं है, इसलिए खोज धीमे अंतर्निहित स्कैन से की गई।", - "content_one_row_per_file": "प्रति फ़ाइल एक पंक्ति" + "content_one_row_per_file": "प्रति फ़ाइल एक पंक्ति", + "upload_exists": "“{{name}}” इस फ़ोल्डर में पहले से मौजूद है। अपनी फ़ाइल का नाम बदलो या मौजूदा फ़ाइल हटाओ, फिर दोबारा अपलोड करो।", + "upload_failed": "{{name}} अपलोड नहीं हो सकी", + "uploading_name": "{{name}} अपलोड हो रही है…", + "uploading_count_one": "{{count}} फ़ाइल अपलोड हो रही है…", + "uploading_count_other": "{{count}} फ़ाइलें अपलोड हो रही हैं…", + "uploaded_name": "{{name}} {{folder}} में अपलोड हो गई।", + "uploaded_count_one": "{{count}} फ़ाइल {{folder}} में अपलोड हो गई।", + "uploaded_count_other": "{{count}} फ़ाइलें {{folder}} में अपलोड हो गईं।" }, "filesHome": { "no_project_dir": "इस चैट के लिए कोई प्रोजेक्ट डायरेक्टरी सेट नहीं है", diff --git a/website/src/i18n/locales/it.json b/website/src/i18n/locales/it.json index 779a6cec5bf..0f1e6581adf 100644 --- a/website/src/i18n/locales/it.json +++ b/website/src/i18n/locales/it.json @@ -12144,6 +12144,8 @@ "content_searching": "Ricerca in corso…", "ctx_add_to_chat": "Aggiungi alla chat", "ctx_download": "Scarica", + "ctx_upload_files": "Carica file…", + "drop_to_upload": "Rilascia per caricare in {{folder}}", "filter_placeholder": "Filtra file…", "git_status_failed": "Impossibile leggere lo stato di git", "mode_content": "Contenuto", @@ -12155,7 +12157,17 @@ "search_names": "Cerca nei nomi dei file", "tree_mode": "Modalità elenco file", "content_engine_slow": "Questo host non ha ripgrep, quindi la ricerca ha usato la scansione interna più lenta.", - "content_one_row_per_file": "una riga per file" + "content_one_row_per_file": "una riga per file", + "upload_exists": "«{{name}}» esiste già in questa cartella. Rinomina il tuo file o rimuovi quello esistente, quindi caricalo di nuovo.", + "upload_failed": "Impossibile caricare {{name}}", + "uploading_name": "Caricamento di {{name}}…", + "uploading_count_one": "Caricamento di {{count}} file…", + "uploading_count_many": "Caricamento di {{count}} file…", + "uploading_count_other": "Caricamento di {{count}} file…", + "uploaded_name": "{{name}} caricato in {{folder}}.", + "uploaded_count_one": "{{count}} file caricato in {{folder}}.", + "uploaded_count_many": "{{count}} file caricati in {{folder}}.", + "uploaded_count_other": "{{count}} file caricati in {{folder}}." }, "filesHome": { "no_project_dir": "Nessuna directory di progetto impostata per questa chat", diff --git a/website/src/i18n/locales/ja.json b/website/src/i18n/locales/ja.json index 4bf64c70399..e8710c66ff1 100644 --- a/website/src/i18n/locales/ja.json +++ b/website/src/i18n/locales/ja.json @@ -11778,6 +11778,8 @@ "content_searching": "検索中…", "ctx_add_to_chat": "チャットに追加", "ctx_download": "ダウンロード", + "ctx_upload_files": "ファイルをアップロード…", + "drop_to_upload": "ここにドロップすると「{{folder}}」にアップロードします", "filter_placeholder": "ファイルを絞り込む…", "git_status_failed": "Git の状態を読み取れませんでした", "mode_content": "内容", @@ -11789,7 +11791,13 @@ "search_names": "ファイル名を検索", "tree_mode": "ファイル一覧モード", "content_engine_slow": "このホストには ripgrep がないため、検索は低速な内蔵スキャンで実行されました。", - "content_one_row_per_file": "1 ファイルにつき 1 行" + "content_one_row_per_file": "1 ファイルにつき 1 行", + "upload_exists": "「{{name}}」はこのフォルダーに既に存在します。アップロードするファイルの名前を変更するか、既存のファイルを削除してから、もう一度アップロードしてください。", + "upload_failed": "{{name}} をアップロードできませんでした", + "uploading_name": "{{name}} をアップロードしています…", + "uploading_count_other": "{{count}} 個のファイルをアップロードしています…", + "uploaded_name": "{{name}} を「{{folder}}」にアップロードしました。", + "uploaded_count_other": "{{count}} 個のファイルを「{{folder}}」にアップロードしました。" }, "filesHome": { "no_project_dir": "このチャットにはプロジェクトディレクトリが設定されていません", diff --git a/website/src/i18n/locales/ko.json b/website/src/i18n/locales/ko.json index 83028e5b6a3..f71d7bd6f9d 100644 --- a/website/src/i18n/locales/ko.json +++ b/website/src/i18n/locales/ko.json @@ -11778,6 +11778,8 @@ "content_searching": "검색 중…", "ctx_add_to_chat": "채팅에 추가", "ctx_download": "다운로드", + "ctx_upload_files": "파일 업로드…", + "drop_to_upload": "여기에 놓으면 ‘{{folder}}’에 업로드합니다", "filter_placeholder": "파일 필터링…", "git_status_failed": "Git 상태를 읽을 수 없습니다", "mode_content": "내용", @@ -11789,7 +11791,13 @@ "search_names": "파일 이름 검색", "tree_mode": "파일 목록 모드", "content_engine_slow": "이 호스트에는 ripgrep이 없으므로 검색이 더 느린 내장 스캔으로 실행되었습니다.", - "content_one_row_per_file": "파일당 한 행" + "content_one_row_per_file": "파일당 한 행", + "upload_exists": "‘{{name}}’이(가) 이 폴더에 이미 있습니다. 업로드할 파일의 이름을 바꾸거나 기존 파일을 삭제한 다음 다시 업로드하세요.", + "upload_failed": "{{name}} 업로드에 실패했습니다", + "uploading_name": "{{name}} 업로드 중…", + "uploading_count_other": "파일 {{count}}개 업로드 중…", + "uploaded_name": "{{name}}을(를) ‘{{folder}}’에 업로드했습니다.", + "uploaded_count_other": "파일 {{count}}개를 ‘{{folder}}’에 업로드했습니다." }, "filesHome": { "no_project_dir": "이 채팅에는 프로젝트 디렉터리가 설정되어 있지 않습니다", diff --git a/website/src/i18n/locales/pt.json b/website/src/i18n/locales/pt.json index 12bbc9ea116..e04ec6c8186 100644 --- a/website/src/i18n/locales/pt.json +++ b/website/src/i18n/locales/pt.json @@ -12144,6 +12144,8 @@ "content_searching": "Buscando…", "ctx_add_to_chat": "Adicionar à conversa", "ctx_download": "Baixar", + "ctx_upload_files": "Enviar arquivos…", + "drop_to_upload": "Solte para enviar para {{folder}}", "filter_placeholder": "Filtrar arquivos…", "git_status_failed": "Não foi possível ler o status do git", "mode_content": "Conteúdo", @@ -12155,7 +12157,17 @@ "search_names": "Buscar nomes de arquivo", "tree_mode": "Modo de lista de arquivos", "content_engine_slow": "Este host não tem ripgrep, portanto a busca usou a varredura interna mais lenta.", - "content_one_row_per_file": "uma linha por arquivo" + "content_one_row_per_file": "uma linha por arquivo", + "upload_exists": "“{{name}}” já existe nesta pasta. Renomeie o seu arquivo ou remova o existente e envie novamente.", + "upload_failed": "Não foi possível enviar {{name}}", + "uploading_name": "Enviando {{name}}…", + "uploading_count_one": "Enviando {{count}} arquivo…", + "uploading_count_many": "Enviando {{count}} arquivos…", + "uploading_count_other": "Enviando {{count}} arquivos…", + "uploaded_name": "{{name}} enviado para {{folder}}.", + "uploaded_count_one": "{{count}} arquivo enviado para {{folder}}.", + "uploaded_count_many": "{{count}} arquivos enviados para {{folder}}.", + "uploaded_count_other": "{{count}} arquivos enviados para {{folder}}." }, "filesHome": { "no_project_dir": "Nenhum diretório de projeto definido para este chat", diff --git a/website/src/i18n/locales/ru.json b/website/src/i18n/locales/ru.json index b50bc8e21ac..777c95aa784 100644 --- a/website/src/i18n/locales/ru.json +++ b/website/src/i18n/locales/ru.json @@ -12327,6 +12327,8 @@ "content_searching": "Поиск…", "ctx_add_to_chat": "Добавить в чат", "ctx_download": "Скачать", + "ctx_upload_files": "Загрузить файлы…", + "drop_to_upload": "Отпусти, чтобы загрузить в {{folder}}", "filter_placeholder": "Фильтр файлов…", "git_status_failed": "Не удалось прочитать статус git", "mode_content": "Содержимое", @@ -12338,7 +12340,19 @@ "search_names": "Искать в именах файлов", "tree_mode": "Режим списка файлов", "content_engine_slow": "На этом хосте нет ripgrep, поэтому поиск выполнен более медленным встроенным способом.", - "content_one_row_per_file": "по одной строке на файл" + "content_one_row_per_file": "по одной строке на файл", + "upload_exists": "«{{name}}» уже есть в этой папке. Переименуй свой файл или удали существующий, затем загрузи снова.", + "upload_failed": "Не удалось загрузить {{name}}", + "uploading_name": "Загрузка {{name}}…", + "uploading_count_one": "Загрузка {{count}} файла…", + "uploading_count_few": "Загрузка {{count}} файлов…", + "uploading_count_many": "Загрузка {{count}} файлов…", + "uploading_count_other": "Загрузка {{count}} файла…", + "uploaded_name": "Файл {{name}} загружен в {{folder}}.", + "uploaded_count_one": "{{count}} файл загружен в {{folder}}.", + "uploaded_count_few": "{{count}} файла загружены в {{folder}}.", + "uploaded_count_many": "{{count}} файлов загружено в {{folder}}.", + "uploaded_count_other": "{{count}} файла загружено в {{folder}}." }, "filesHome": { "no_project_dir": "Для этого чата не задан каталог проекта", diff --git a/website/src/i18n/locales/zh-CN.json b/website/src/i18n/locales/zh-CN.json index d86947962a1..f9dd9338da0 100644 --- a/website/src/i18n/locales/zh-CN.json +++ b/website/src/i18n/locales/zh-CN.json @@ -11778,6 +11778,8 @@ "content_searching": "正在搜索…", "ctx_add_to_chat": "添加到聊天", "ctx_download": "下载", + "ctx_upload_files": "上传文件…", + "drop_to_upload": "拖放到此处，上传到“{{folder}}”", "filter_placeholder": "筛选文件…", "git_status_failed": "无法读取 Git 状态", "mode_content": "内容", @@ -11789,7 +11791,13 @@ "search_names": "搜索文件名", "tree_mode": "文件列表模式", "content_engine_slow": "此主机没有 ripgrep，因此搜索使用了较慢的内置扫描。", - "content_one_row_per_file": "每个文件一行" + "content_one_row_per_file": "每个文件一行", + "upload_exists": "此文件夹中已存在“{{name}}”。重命名你的文件，或删除已有的同名文件，然后重新上传。", + "upload_failed": "无法上传 {{name}}", + "uploading_name": "正在上传 {{name}}…", + "uploading_count_other": "正在上传 {{count}} 个文件…", + "uploaded_name": "已将 {{name}} 上传到“{{folder}}”。", + "uploaded_count_other": "已将 {{count}} 个文件上传到“{{folder}}”。" }, "filesHome": { "no_project_dir": "此聊天未设置项目目录", diff --git a/website/src/i18n/pluralKeys.json b/website/src/i18n/pluralKeys.json index 42da3368ef0..bf36866a7c3 100644 --- a/website/src/i18n/pluralKeys.json +++ b/website/src/i18n/pluralKeys.json @@ -293,5 +293,7 @@ "pages.settings.chatPanel.follow_default_more", "pages.membersPage.goals_heartbeat_per_day", "appSdk.newMessagesDivider.new_messages", - "pages.chatPage.jump_to_new" + "pages.chatPage.jump_to_new", + "pages.chat.fileBrowserRail.uploading_count", + "pages.chat.fileBrowserRail.uploaded_count" ] diff --git a/website/src/pages/chat/FileBrowserRail.tsx b/website/src/pages/chat/FileBrowserRail.tsx index e5a72fe59d9..30a8f7a73fe 100644 --- a/website/src/pages/chat/FileBrowserRail.tsx +++ b/website/src/pages/chat/FileBrowserRail.tsx @@ -5,7 +5,9 @@ import { useTranslation } from 'react-i18next' import { Files, Diff, Search, X, RefreshCw, FileText, Folder } from 'lucide-react' import { api } from '../../api/client' import { fileGrep, type FileGrepHit } from '../../api/fileGrep' +import ChatDropOverlay, { useChatFileDrop } from '../../components/ChatDropOverlay' import ErrorNotice from '../../components/ErrorNotice' +import StatusNotice from '../../components/StatusNotice' import { gitFilterRefusalCause, gitFilterRefusalCopyKey, @@ -26,6 +28,8 @@ import { PierreWorkspaceTree, type TreeReveal } from '../../pierre/tree' import { mergeTreeMatches } from '../../lib/treeSearchMatches' import { findReport, reportForError } from '../../utils/errorReport' import { errMessage } from '../../utils/thunkError' +import { pathBasename, stripTrailingSeparators } from '../../utils/pathBasename' +import { useDirectoryUpload } from './useDirectoryUpload' /** Rail width bounds; the grip clamps between them. */ const RAIL_MIN_W = 300 @@ -541,6 +545,26 @@ export default function FileBrowserRail({ projectDir, onFileOpen, onAddToContext )}` : t('pages.chat.fileBrowserRail.changed') + // A drop anywhere on the rail uploads into the project root: the overlay + // covers the tree, so no row under the pointer is visible to choose. A + // specific folder is reached through its row menu's "Upload files…". The + // tree computes the same fallback (`tree?.root ?? projectDir`) because the + // backend answers with the realpath, which can differ from a symlinked + // projectDir. + const root = treeData?.root ?? projectDir + // The name the Files tab header shows for the project (FilesHomePanel). + const rootName = pathBasename(stripTrailingSeparators(projectDir)) || projectDir + + const { + uploadFiles, pickAndUpload, progress: uploadProgress, error: uploadError, dismissError, + success: uploadSuccess, dismissSuccess, fileInput, + } = useDirectoryUpload(projectDir, root) + const { active: dragOver, dropTargetProps } = useChatFileDrop((dataTransfer) => { + const files = Array.from(dataTransfer.files) + if (!files.length) return + void uploadFiles(root, files) + }) + // Both queries poll (10s tree / 5s status); this is the "I changed something // outside the app, show me now" escape hatch. `refetchQueries` (not // `invalidateQueries`) so `refreshing` tracks the actual network round trip @@ -618,9 +642,11 @@ export default function FileBrowserRail({ projectDir, onFileOpen, onAddToContext /> )}
+
{/* All/Changed scopes the TREE, and Content mode has no tree. Left rendered it kept its "Changed" highlight while the content results @@ -799,6 +825,30 @@ export default function FileBrowserRail({ projectDir, onFileOpen, onAddToContext />
)} + {(uploadProgress || uploadSuccess) && ( +
+ + +
+ )} + {/* Files rail, no draft to lose on hand-off: the tree itself holds no + unsaved input, so askAgent is safe to leave on. */} + {uploadError && ( +
+ +
+ )}
{contentMode ? ( onFileOpen(abs, changedMode)} onAddToContext={onAddToContext} + onUploadRequest={pickAndUpload} searchQuery={nameSearch ? null : query || null} selectedPath={selectedPath ?? null} onOpenFolder={onOpenFolder ? folder => onOpenFolder(`${projectRoot}/${folder}`) : undefined} @@ -858,6 +909,7 @@ export default function FileBrowserRail({ projectDir, onFileOpen, onAddToContext )}
+ {fileInput} ) } diff --git a/website/src/pages/chat/useDirectoryUpload.tsx b/website/src/pages/chat/useDirectoryUpload.tsx new file mode 100644 index 00000000000..22cb48825b4 --- /dev/null +++ b/website/src/pages/chat/useDirectoryUpload.tsx @@ -0,0 +1,221 @@ +import { useCallback, useEffect, useRef, useState, type ChangeEvent, type ReactNode } from 'react' +import { useMutation, useQueryClient } from '@tanstack/react-query' +import { api } from '../../api/client' +import { i18nT } from '../../i18n/t' +import { backslashIsSeparator, pathBasename, stripTrailingSeparators } from '../../utils/pathBasename' + +export interface DirectoryUploadHandle { + /** Upload one or more OS files into `absDir` (the tree root for a drop on + * the rail). A name that already exists in + * `absDir` is reported in `error`; the existing file is never replaced. + * A name already uploading into `absDir` is skipped, not sent again. */ + uploadFiles: (absDir: string, files: File[]) => Promise + /** Opens the OS file picker and uploads the selection into `absDir` — the + * "Upload files…" row-menu action's click path. */ + pickAndUpload: (absDir: string) => void + /** "Uploading report.md…" / "Uploading 3 files…" while a batch runs, or null. */ + progress: string | null + /** The most recent upload failure, or null. */ + error: string | null + dismissError: () => void + /** "Uploaded report.md to docs." after a batch with no failures, or null. */ + success: string | null + dismissSuccess: () => void + /** Render once in the host's JSX: the hidden file-picker input. */ + fileInput: ReactNode +} + +/** `p` with `/` separators and no trailing separator, for comparing paths. */ +function comparablePath(p: string): string { + return stripTrailingSeparators(backslashIsSeparator(p) ? p.replace(/\\/g, '/') : p) +} + +interface RunningBatch { + id: number + names: string[] +} + +/** + * Upload orchestration shared by the Files rail's drop overlay and its + * "Upload files…" row-menu action — one place for the collision message, the + * query invalidation that makes a new file appear in every file listing and + * search, and the progress, success and error surface, so the two entry points + * cannot drift apart. + * + * `rootDir` is the directory the tree lists (the backend's realpath of + * `projectDir`, which can differ for a symlinked project); the success line + * names a target folder relative to it, and the root itself by the project's + * folder name. + */ +export function useDirectoryUpload(projectDir: string, rootDir: string = projectDir): DirectoryUploadHandle { + const qc = useQueryClient() + const [error, setError] = useState(null) + const [success, setSuccess] = useState(null) + const [running, setRunning] = useState([]) + // A chat-slot switch changes projectDir in place without remounting the + // rail (React's adjust-state-on-prop pattern, synchronous before paint): + // one project's notices must not show in another project's panel. + const [errorDir, setErrorDir] = useState(projectDir) + if (errorDir !== projectDir) { + setErrorDir(projectDir) + setError(null) + setSuccess(null) + setRunning([]) + } + // The project the rail shows now; an upload batch that finishes after a + // switch reads it to keep its failures out of the new project's panel. + const currentDirRef = useRef(projectDir) + useEffect(() => { + currentDirRef.current = projectDir + }, [projectDir]) + const inputRef = useRef(null) + const pendingDirRef = useRef(null) + // `${dir}/${name}` of every file being sent now, across batches. + const inFlightRef = useRef(new Set()) + const batchIdRef = useRef(0) + + // Every cached listing or search of workspace files an upload can add a row + // to. The first three are keyed by `projectDir`. The rest are keyed by a + // root this hook cannot know, so they are invalidated by prefix: the rail's + // filename search uses the backend's realpath of the project, and the folder + // tab's listing and search use whatever folder that tab is showing. + const invalidate = useCallback(() => { + qc.invalidateQueries({ queryKey: ['project-tree', projectDir] }) + qc.invalidateQueries({ queryKey: ['git-status', projectDir] }) + qc.invalidateQueries({ queryKey: ['file-grep', projectDir] }) + qc.invalidateQueries({ queryKey: ['rail-name-search'] }) + qc.invalidateQueries({ queryKey: ['browse-files'] }) + qc.invalidateQueries({ queryKey: ['folder-file-search'] }) + }, [qc, projectDir]) + + // The actual write is a server-state mutation, so it goes through + // useMutation/mutateAsync rather than a bare fetch call (website/AGENTS.md: + // "Data fetching is React Query"). + const uploadMutation = useMutation({ + mutationFn: ({ absDir, file }: { absDir: string; file: File }) => + api.uploadToDirectory(absDir, file), + }) + + // api.uploadToDirectory wraps fetch(); a transport failure (offline, DNS, + // an aborted request) REJECTS that promise instead of resolving an + // {ok:false} result -- and mutateAsync propagates that same rejection. + // Every caller here fires uploadFiles without awaiting it + // (`void uploadFiles(...)`), so an unwrapped rejection becomes an unhandled + // promise rejection: no error state, no invalidate, and the drop or picker + // selection appears to do nothing. Catching here folds a thrown error into + // the SAME {ok:false} shape the API already returns for a non-2xx + // response, so uploadOne's own handling covers both without a second + // error path. + const attemptUpload = useCallback(async ( + absDir: string, file: File, + ): Promise>> => { + try { + return await uploadMutation.mutateAsync({ absDir, file }) + } catch (err) { + return { ok: false, status: 0, error: err instanceof Error ? err.message : String(err) } + } + }, [uploadMutation]) + + const uploadOne = useCallback(async (absDir: string, file: File): Promise => { + const result = await attemptUpload(absDir, file) + if (result.ok) return null + if (result.code === 'name_collision') { + return i18nT('pages.chat.fileBrowserRail.upload_exists', { name: file.name }) + } + return `${i18nT('pages.chat.fileBrowserRail.upload_failed', { name: file.name })}: ${result.error}` + }, [attemptUpload]) + + // The folder a success line names: its path under the tree root, or the + // project's folder name (what the Files tab header shows) for the root. + const folderLabel = useCallback((absDir: string): string => { + const dir = comparablePath(absDir) + const root = comparablePath(rootDir) + if (dir === root || dir === comparablePath(projectDir)) { + return pathBasename(comparablePath(projectDir)) || projectDir + } + if (dir.startsWith(`${root}/`)) return dir.slice(root.length + 1) + return pathBasename(dir) || dir + }, [rootDir, projectDir]) + + const uploadFiles = useCallback(async (absDir: string, files: File[]) => { + // A name still uploading into this folder (typically dropped again because + // the first drop seemed to do nothing) is not sent a second time: the server + // would refuse it as a collision with the copy it is still writing. The + // running batch's progress line already covers it. + const dirKey = comparablePath(absDir) + const batch: File[] = [] + const keys: string[] = [] + for (const file of files) { + const key = `${dirKey}/${file.name}` + if (inFlightRef.current.has(key)) continue + inFlightRef.current.add(key) + keys.push(key) + batch.push(file) + } + if (batch.length === 0) return + const id = ++batchIdRef.current + setError(null) + setSuccess(null) + setRunning(prev => [...prev, { id, names: batch.map(f => f.name) }]) + const failures: string[] = [] + for (const file of batch) { + const failure = await uploadOne(absDir, file) + if (failure) failures.push(failure) + } + for (const key of keys) inFlightRef.current.delete(key) + setRunning(prev => prev.filter(b => b.id !== id)) + if (currentDirRef.current === projectDir) { + if (failures.length > 0) { + setError(failures.join('; ')) + } else { + const folder = folderLabel(absDir) + setSuccess(batch.length === 1 + ? i18nT('pages.chat.fileBrowserRail.uploaded_name', { name: batch[0].name, folder }) + : i18nT('pages.chat.fileBrowserRail.uploaded_count', { count: batch.length, folder })) + } + } + invalidate() + }, [uploadOne, invalidate, projectDir, folderLabel]) + + const runningNames = running.flatMap(b => b.names) + const progress = runningNames.length === 0 + ? null + : runningNames.length === 1 + ? i18nT('pages.chat.fileBrowserRail.uploading_name', { name: runningNames[0] }) + : i18nT('pages.chat.fileBrowserRail.uploading_count', { count: runningNames.length }) + + const pickAndUpload = useCallback((absDir: string) => { + pendingDirRef.current = absDir + inputRef.current?.click() + }, []) + + const onInputChange = useCallback((e: ChangeEvent) => { + const dir = pendingDirRef.current + const files = e.target.files ? Array.from(e.target.files) : [] + // Reset so picking the SAME file again still fires a change event. + e.target.value = '' + if (dir && files.length) void uploadFiles(dir, files) + }, [uploadFiles]) + + const fileInput = ( + + ) + + return { + uploadFiles, + pickAndUpload, + progress, + error, + dismissError: () => setError(null), + success, + dismissSuccess: () => setSuccess(null), + fileInput, + } +} diff --git a/website/src/pierre/PierreWorkspaceTreeImpl.tsx b/website/src/pierre/PierreWorkspaceTreeImpl.tsx index 787891d4e55..c3da2d6809e 100644 --- a/website/src/pierre/PierreWorkspaceTreeImpl.tsx +++ b/website/src/pierre/PierreWorkspaceTreeImpl.tsx @@ -20,7 +20,7 @@ import type { ContextMenuOpenContext as FileTreeContextMenuOpenContext, } from '@pierre/trees' import { FileTree, useFileTree } from '@pierre/trees/react' -import { AtSign, Download, FileDiff, FolderDot, FolderLock, FolderOpen, Undo2 } from 'lucide-react' +import { AtSign, Download, FileDiff, FolderDot, FolderLock, FolderOpen, Undo2, Upload } from 'lucide-react' import { api } from '../api/client' import { ApiError } from '../api/apiError' import ErrorNotice from '../components/ErrorNotice' @@ -120,15 +120,20 @@ function expandedDirectoriesOf( * anchor, the outside-click wash, and open/close (the portal root's * `data-file-tree-context-menu-root` marker is the library's documented way to * keep a portaled surface counting as "inside"); this renders only the item - * list: the built-in "Add to chat" action, plus any row an installed app - * contributes for the `tree-context` surface (row click already opens a file, - * so the menu deliberately carries no Open duplicate). Every action closes the - * menu itself so focus returns to the row. */ -function TreeContextMenu({ item, context, root, onAddToContext, contribItems, onError }: { + * list: the built-in "Add to chat" action, the built-in "Upload + * files…" action (DIRECTORY rows only), plus any + * row an installed app contributes for the `tree-context` surface (row click + * already opens a file, so the menu deliberately carries no Open duplicate). + * Every action closes the menu itself so focus returns to the row. */ +function TreeContextMenu({ item, context, root, onAddToContext, onUploadRequest, contribItems, onError }: { item: FileTreeContextMenuItem context: FileTreeContextMenuOpenContext root: string onAddToContext?: (absPath: string, kind: TreeEntryKind) => void + /** Right-click "Upload files…" on a directory row: hands the host the + * ABSOLUTE directory path so it can open the OS file picker and upload + * the selection into it. Absent → the row is not rendered. */ + onUploadRequest?: (absDirPath: string) => void /** Contributed `tree-context` rows, resolved by the PARENT (which already holds * the `['apps']` query) and passed down. Deliberately a prop rather than a hook * call here: this component mounts inside Pierre's context-menu slot, and a @@ -175,6 +180,11 @@ function TreeContextMenu({ item, context, root, onAddToContext, contribItems, on context.close() run() } + // "Upload files…" renders only on a directory row, and only when the host + // supplied the handler: this same component instance answers every row the + // tree opens a menu for, and a FILE row must never show the directory-only + // upload action even when the host wired it for other rows. + const showUpload = isDir && !!onUploadRequest // Focus the first item on open. Pierre's own open path calls `item.focus()` // on the tree ROW, never on this slotted content, so a keyboard user who // opens the menu (Shift+F10 / the ContextMenu key) would otherwise be left @@ -193,13 +203,15 @@ function TreeContextMenu({ item, context, root, onAddToContext, contribItems, on ?? menuRef.current?.querySelector('[role="menuitem"]'))?.focus() }, []) // The shared role="menu" keyboard contract (#6231). The item count is no longer - // fixed: the built-in row is gated on `onAddToContext` and an installed app may - // contribute any number of rows its `when` admits, so this menu can hold one row - // or several and the arrows do real navigation whenever it holds more than one. - // Honouring the contract per-surface-by-item-count is how surfaces drift anyway — - // an arrow inside an open menu must be consumed rather than scrolling the tree - // behind it, Tab must stay contained (#2533), and IME composition keys must not - // reach the menu at all, all true whatever the count. `enabled: true` + // fixed: the built-in "Add to chat" row is gated on `onAddToContext`, the + // built-in "Upload files…" row is gated on `onUploadRequest` AND a directory + // row, and an installed app may contribute any number of rows its `when` + // admits — so this menu can hold one row or several and the arrows do real + // navigation whenever it holds more than one. Honouring the contract + // per-surface-by-item-count is how surfaces drift anyway — an arrow inside + // an open menu must be consumed rather than scrolling the tree behind it, + // Tab must stay contained (#2533), and IME composition keys must not reach + // the menu at all, all true whatever the count. `enabled: true` // unconditionally because Pierre only mounts this component while the menu is open. // focusFirstOnOpen: false — the firstItemRef effect above already owns focus // entry (it must, because Pierre focuses the tree ROW, not this slotted @@ -280,14 +292,15 @@ function TreeContextMenu({ item, context, root, onAddToContext, contribItems, on } }, [context]) // Render nothing rather than an empty bordered popup: with no host row, no - // Download (a directory), AND no app row that its `when` admits for this - // node, there is nothing to show and no menuitem for the focus effect to land - // on. + // Download (a directory with no upload handler), no upload row (absent + // handler, or a file row), AND no app row that its `when` admits for this + // node, there is nothing to show and no menuitem for the focus effect to + // land on. // A file row also carries a built-in Download (retrieve the bytes onto the // machine running the browser). Directories do not: the ask is file rows // only, and /api/file-download serves a single file, not a folder. const canDownload = !isDir - if (!hasBuiltinChatRow && !canDownload && rows.length === 0) return null + if (!hasBuiltinChatRow && !canDownload && !showUpload && rows.length === 0) return null return
)} + {/* Built-in Upload for a directory row: hands the absolute directory + path to the host, which opens the OS file picker and POSTs to + /api/directory-upload. Directory rows only (mutually exclusive with + Download above, which is file-only), so the firstItemRef handoff + cannot collide. */} + {showUpload && ( +
onUploadRequest?.(abs))} + onKeyDown={activate(() => onUploadRequest?.(abs))} + > + + {i18nT('pages.chat.fileBrowserRail.ctx_upload_files')} +
+ )} {/* App-contributed rows (contributes.fileMenuItems, surface 'tree-context'). An installed app declares these in its manifest; core POSTs the node context to the app's endpoint on activation and never imports app code. @@ -364,7 +395,7 @@ function TreeContextMenu({ item, context, root, onAddToContext, contribItems, on return (
void /** Right-click "Add to context" on a row: hands the host the ABSOLUTE path @@ -431,6 +462,10 @@ export function PierreWorkspaceTreeImpl({ projectDir, onFileOpen, onAddToContext * rendered, and the context menu opens only if an app contributes a * 'tree-context' row this node matches. */ onAddToContext?: (absPath: string, kind: TreeEntryKind) => void + /** Right-click "Upload files…" on a DIRECTORY row: hands the host the + * ABSOLUTE directory path so it can open the OS file picker and upload the + * selection into it. Absent → no upload row on any menu. */ + onUploadRequest?: (absDirPath: string) => void /** Forwarded into the tree's search session (null clears it). */ searchQuery?: string | null /** 'all' renders the full workspace; 'changed' renders only the files with @@ -1346,6 +1381,8 @@ export function PierreWorkspaceTreeImpl({ projectDir, onFileOpen, onAddToContext // query, so it must come through the ref too. const onAddToContextRef = useRef(onAddToContext) onAddToContextRef.current = onAddToContext + const onUploadRequestRef = useRef(onUploadRequest) + onUploadRequestRef.current = onUploadRequest const rootRef = useRef(root) rootRef.current = root @@ -1430,6 +1467,7 @@ export function PierreWorkspaceTreeImpl({ projectDir, onFileOpen, onAddToContext context={ctx} root={rootRef.current} onAddToContext={onAddToContextRef.current} + onUploadRequest={onUploadRequestRef.current} contribItems={treeItemsRef.current} onError={setActionError} /> @@ -1736,8 +1774,9 @@ export function PierreWorkspaceTreeImpl({ projectDir, onFileOpen, onAddToContext // Always wired: a file row now always carries a built-in Download, so // the menu is useful for every file even with no host and no app rows. // The per-node `TreeContextMenu` still renders NOTHING (returns null) for - // a node with no action — a directory with no host and no app row — so a - // right-click there opens no bordered popup despite the menu being wired. + // a node with no action — a directory with no host, no upload handler and + // no app row — so a right-click there opens no bordered popup despite the + // menu being wired. renderContextMenu={renderContextMenu} />
diff --git a/website/src/pierre/tree.tsx b/website/src/pierre/tree.tsx index 18f36db6753..07c9a21e01e 100644 --- a/website/src/pierre/tree.tsx +++ b/website/src/pierre/tree.tsx @@ -42,12 +42,15 @@ export interface TreeReveal { nonce: number } -export function PierreWorkspaceTree({ projectDir, onFileOpen, onAddToContext, searchQuery, mode, selectedPath, persistExpansion, onOpenFolder, reveal, onRevealed, active }: { +export function PierreWorkspaceTree({ projectDir, onFileOpen, onAddToContext, onUploadRequest, searchQuery, mode, selectedPath, persistExpansion, onOpenFolder, reveal, onRevealed, active }: { projectDir: string onFileOpen?: (absPath: string) => void /** Right-click "Add to context" on a row — forwards the ABSOLUTE path and * whether it is a file or a directory to the host composer. */ onAddToContext?: (absPath: string, kind: 'file' | 'dir') => void + /** Right-click "Upload files…" on a directory row — forwards the ABSOLUTE + * directory path so the host can open the OS file picker for it. */ + onUploadRequest?: (absDirPath: string) => void /** Forwarded into the tree's search session (null clears it). */ searchQuery?: string | null /** 'all' (default) = full workspace; 'changed' = only working-tree changes. */ @@ -77,7 +80,7 @@ export function PierreWorkspaceTree({ projectDir, onFileOpen, onAddToContext, se const key = persistExpansion ? [mode ?? 'all', projectDir].join('\u0000') : (mode ?? 'all') return ( }> - + ) } diff --git a/website/src/test/ApiClient.refactor.surface.test.ts b/website/src/test/ApiClient.refactor.surface.test.ts index 686d71e000a..01106322f11 100644 --- a/website/src/test/ApiClient.refactor.surface.test.ts +++ b/website/src/test/ApiClient.refactor.surface.test.ts @@ -151,7 +151,7 @@ const API_KEY_ORDER = [ 'setUpdateChannel', 'restartGateway', 'armUpdate', 'armStatus', 'dismissUpdateArm', 'cancelUpdate', 'simulateUpdate', 'pickFiles', 'fileDiff', 'fileSearch', 'pathComplete', 'uploadFiles', - 'screenshot', 'themes', 'dashboardConfig', 'featureVideoNext', + 'uploadToDirectory', 'screenshot', 'themes', 'dashboardConfig', 'featureVideoNext', 'featureVideoFeedback', 'featureVideoProbe', 'featureVideoStatus', 'featureVideoFetchAll', 'updateDashboardConfig', 'createTheme', 'installTheme', 'updateTheme', 'deleteTheme', 'themeDetail', 'themeBoot', 'updateThemeConfig', @@ -240,7 +240,7 @@ const NO_SESSION_KEY = [ 'pendingQuestions', 'logLevel', 'taskRunnerStatus', 'renameTaskRun', 'updateTask', 'refineStatus', 'planContext', 'checkUpdate', 'changelog', 'releases', 'armStatus', 'fileDiff', - 'fileSearch', 'pathComplete', 'uploadFiles', 'themes', + 'fileSearch', 'pathComplete', 'uploadFiles', 'uploadToDirectory', 'themes', 'dashboardConfig', 'themeDetail', 'themeBoot', 'voiceConfig', 'voiceVoices', 'voiceSystemVoices', 'awsConsent', 'fileDeliveryConsent', 'fileDeliveryConsentArmStatus', 'credentialRedaction', 'channelsList', 'channelPresets', @@ -254,6 +254,7 @@ const ARGS: Record = { memoryRecords: ['member-reviewer', { q: 'contact', kind: 'fact' }, 0, 50], memoryRecordHistory: ['member-reviewer', { kind: 'fact', id: 'user.contact' }, 25, 0], uploadFiles: [[new File(['abc'], 'a.txt', { type: 'text/plain' })]], + uploadToDirectory: ['/tmp/proj', new File(['abc'], 'a.txt', { type: 'text/plain' })], uploadCrewAvatar: ['crew-a', new File(['png'], 'a.png', { type: 'image/png' })], sttTranscribe: [new Blob(['wav'], { type: 'audio/wav' })], // Takes only an AbortSignal; its deadline helper throws on a string before it fetches. diff --git a/website/src/test/FileBrowserRail.test.tsx b/website/src/test/FileBrowserRail.test.tsx index 97d09a3d8bb..e1f9143ddc9 100644 --- a/website/src/test/FileBrowserRail.test.tsx +++ b/website/src/test/FileBrowserRail.test.tsx @@ -25,6 +25,7 @@ const H = vi.hoisted(() => ({ projectTree: vi.fn(), projectGitStatus: vi.fn(), fileSearch: vi.fn(), + uploadToDirectory: vi.fn(), }, fileGrep: vi.fn(), })) @@ -44,6 +45,7 @@ vi.mock('../pierre/tree', () => ({ onOpenFolder?: (folder: string) => void reveal?: { path: string; nonce: number } | null active?: boolean + onUploadRequest?: (absDir: string) => void }) => ( <> {/* Stands in for the limit row under a cut folder: present only when @@ -65,6 +67,16 @@ vi.mock('../pierre/tree', () => ({ > tree + {/* Stands in for a directory row's "Upload files…" context-menu action: + PierreWorkspaceTreeImpl.test.tsx pins that click wired to this same + callback, so this probe only needs to pin that FileBrowserRail + forwards it and reacts correctly when it fires. */} + ), })) @@ -334,6 +346,121 @@ describe('FileBrowserRail file opens', () => { }) }) +function fileTransfer(name = 'a.txt'): DataTransfer { + const file = new File(['x'], name, { type: 'text/plain' }) + return { + types: ['Files'], + items: [{ kind: 'file', type: file.type, getAsFile: () => file }], + files: [file], + dropEffect: 'none', + } as unknown as DataTransfer +} + +describe('FileBrowserRail upload', () => { + beforeEach(() => { + H.api.uploadToDirectory.mockReset().mockResolvedValue({ ok: true, path: `${DIR}/a.txt`, name: 'a.txt' }) + }) + + const railEl = () => rail() + + it('shows the drop overlay naming the project root for an OS file drag over the rail', () => { + mount() + fireEvent.dragEnter(railEl(), { dataTransfer: fileTransfer() }) + expect(screen.getByText('Drop to upload into repo')).toBeInTheDocument() + }) + + it('uploads a pane-wide drop (no specific row) into the tree root', async () => { + mount() + fireEvent.drop(railEl(), { dataTransfer: fileTransfer() }) + + await waitFor(() => expect(H.api.uploadToDirectory).toHaveBeenCalledWith(DIR, expect.any(File))) + }) + + it('uploads a drop released over a folder row or a file row into the tree root', async () => { + // The overlay hides the tree, so the row under the pointer must not pick + // the destination. These rows carry the attributes @pierre/trees renders. + mount() + const folderRow = document.createElement('div') + folderRow.setAttribute('data-item-path', 'src/') + folderRow.setAttribute('data-item-type', 'folder') + const fileRow = document.createElement('div') + fileRow.setAttribute('data-item-path', 'src/a.ts') + fileRow.setAttribute('data-item-type', 'file') + fileRow.setAttribute('data-item-parent-path', 'src/') + tree().append(folderRow, fileRow) + + fireEvent.drop(folderRow, { dataTransfer: fileTransfer('one.txt') }) + fireEvent.drop(fileRow, { dataTransfer: fileTransfer('two.txt') }) + + await waitFor(() => expect(H.api.uploadToDirectory).toHaveBeenCalledTimes(2)) + expect(H.api.uploadToDirectory.mock.calls.map(([dir, file]) => [dir, (file as File).name])).toEqual([ + [DIR, 'one.txt'], + [DIR, 'two.txt'], + ]) + }) + + it('forwards onUploadRequest to the tree and uploads the picker selection into that directory', async () => { + mount() + fireEvent.click(screen.getByTestId('upload-request')) + const input = screen.getByLabelText('Upload files…') as HTMLInputElement + const file = new File(['y'], 'picked.txt', { type: 'text/plain' }) + + fireEvent.change(input, { target: { files: [file] } }) + + await waitFor(() => expect(H.api.uploadToDirectory).toHaveBeenCalledWith(`${DIR}/target-dir`, file)) + }) + + it('shows an upload failure inline and lets it be dismissed', async () => { + H.api.uploadToDirectory.mockResolvedValue({ ok: false, status: 400, code: 'unsupported_file_type', error: 'Unsupported file type: .exe' }) + mount() + + fireEvent.drop(railEl(), { dataTransfer: fileTransfer('a.exe') }) + + const notice = await screen.findByTestId('file-browser-rail-upload-error') + expect(notice).toHaveTextContent('Unsupported file type: .exe') + + fireEvent.click(within(notice).getByLabelText('Dismiss')) + expect(screen.queryByTestId('file-browser-rail-upload-error')).toBeNull() + }) + + it('shows a polite uploading status while a drop uploads, then a dismissible success line', async () => { + let resolveUpload: (value: unknown) => void = () => {} + H.api.uploadToDirectory.mockReturnValueOnce(new Promise((resolve) => { resolveUpload = resolve })) + mount() + + fireEvent.drop(railEl(), { dataTransfer: fileTransfer('report.md') }) + + const status = await screen.findByTestId('file-browser-rail-upload-progress') + expect(status).toHaveAttribute('role', 'status') + expect(status).toHaveTextContent('Uploading report.md…') + + resolveUpload({ ok: true, path: `${DIR}/report.md`, name: 'report.md' }) + const done = await screen.findByTestId('file-browser-rail-upload-success') + expect(done).toHaveAttribute('role', 'status') + expect(done).toHaveTextContent('Uploaded report.md to repo.') + expect(screen.queryByTestId('file-browser-rail-upload-progress')).toBeNull() + + fireEvent.click(within(done).getByLabelText('Dismiss')) + expect(screen.queryByTestId('file-browser-rail-upload-success')).toBeNull() + }) + + it('does not send a second drop of a file that is still uploading', async () => { + let resolveUpload: (value: unknown) => void = () => {} + H.api.uploadToDirectory.mockReturnValueOnce(new Promise((resolve) => { resolveUpload = resolve })) + mount() + + fireEvent.drop(railEl(), { dataTransfer: fileTransfer('report.md') }) + await screen.findByTestId('file-browser-rail-upload-progress') + fireEvent.drop(railEl(), { dataTransfer: fileTransfer('report.md') }) + + expect(H.api.uploadToDirectory).toHaveBeenCalledTimes(1) + resolveUpload({ ok: true, path: `${DIR}/report.md`, name: 'report.md' }) + await screen.findByTestId('file-browser-rail-upload-success') + expect(H.api.uploadToDirectory).toHaveBeenCalledTimes(1) + expect(screen.queryByTestId('file-browser-rail-upload-error')).toBeNull() + }) +}) + describe('FileBrowserRail refresh', () => { it('hands the agent the journaled server report, not the translated sentence', async () => { // The notice shows a composed line ("Couldn't load the file tree — Refresh to retry"), which is diff --git a/website/src/test/PierreWorkspaceTreeImpl.contextMenu.test.tsx b/website/src/test/PierreWorkspaceTreeImpl.contextMenu.test.tsx index 7480130c031..cbe307255ad 100644 --- a/website/src/test/PierreWorkspaceTreeImpl.contextMenu.test.tsx +++ b/website/src/test/PierreWorkspaceTreeImpl.contextMenu.test.tsx @@ -459,6 +459,103 @@ describe('PierreWorkspaceTreeImpl — row context menu Download', () => { }) }) +describe('PierreWorkspaceTreeImpl — row context menu "Upload files…"', () => { + const openMenu = (item: MenuItem) => { + const close = vi.fn() + const context: MenuContext = { + anchorElement: document.createElement('div'), + anchorRect: document.createElement('div').getBoundingClientRect(), + close, + restoreFocus: vi.fn(), + } + const node = treeMock.fileTreeProps.at(-1)!.renderContextMenu!(item, context) + return { close, ...render(<>{node}) } + } + + it('offers Upload files… on a directory row, wired to the absolute directory path', async () => { + const onUploadRequest = vi.fn() + renderTree({ onUploadRequest }) + await waitForTree() + + const { close } = openMenu({ kind: 'directory', name: 'a', path: 'src/a' }) + + fireEvent.click(screen.getByRole('menuitem', { name: 'Upload files…' })) + expect(onUploadRequest).toHaveBeenCalledWith(`${ROOT}/src/a`) + expect(close).toHaveBeenCalledTimes(1) + }) + + it('never offers Upload files… on a file row, even when the handler is wired', async () => { + const onUploadRequest = vi.fn() + renderTree({ onUploadRequest }) + await waitForTree() + + openMenu({ kind: 'file', name: 'b.ts', path: 'src/a/b.ts' }) + + expect(screen.queryByRole('menuitem', { name: 'Upload files…' })).not.toBeInTheDocument() + }) + + it('offers only the built-in Download on a file row when only onUploadRequest is wired', async () => { + // Upload is directory-only, but a file row still carries its built-in + // Download row, so the menu renders with exactly that one action. + const onUploadRequest = vi.fn() + renderTree({ onUploadRequest }) + await waitForTree() + + openMenu({ kind: 'file', name: 'b.ts', path: 'src/a/b.ts' }) + + const items = screen.getAllByRole('menuitem') + expect(items).toHaveLength(1) + expect(items[0]).toHaveTextContent('Download') + expect(screen.queryByRole('menuitem', { name: 'Upload files…' })).not.toBeInTheDocument() + }) + + it('offers both actions on a directory row, Add to chat first, activating independently', async () => { + const onAddToContext = vi.fn() + const onUploadRequest = vi.fn() + renderTree({ onAddToContext, onUploadRequest }) + await waitForTree() + + openMenu({ kind: 'directory', name: 'a', path: 'src/a' }) + + const items = screen.getAllByRole('menuitem') + expect(items.map(i => i.textContent)).toEqual(['Add to chat', 'Upload files…']) + + fireEvent.click(screen.getByRole('menuitem', { name: 'Upload files…' })) + expect(onUploadRequest).toHaveBeenCalledWith(`${ROOT}/src/a`) + expect(onAddToContext).not.toHaveBeenCalled() + }) + + it('focuses Add to chat first when both actions are present on a directory row', async () => { + renderTree({ onAddToContext: vi.fn(), onUploadRequest: vi.fn() }) + await waitForTree() + + openMenu({ kind: 'directory', name: 'a', path: 'src/a' }) + + expect(screen.getByRole('menuitem', { name: 'Add to chat' })).toHaveFocus() + }) + + it('focuses Upload files… when it is the only action on a directory row', async () => { + renderTree({ onUploadRequest: vi.fn() }) + await waitForTree() + + openMenu({ kind: 'directory', name: 'a', path: 'src/a' }) + + expect(screen.getByRole('menuitem', { name: 'Upload files…' })).toHaveFocus() + }) + + it('activates Upload files… from the keyboard', async () => { + const onUploadRequest = vi.fn() + renderTree({ onUploadRequest }) + await waitForTree() + + openMenu({ kind: 'directory', name: 'a', path: 'src/a' }) + const menuitem = screen.getByRole('menuitem', { name: 'Upload files…' }) + + fireEvent.keyDown(menuitem, { key: 'Enter' }) + expect(onUploadRequest).toHaveBeenCalledWith(`${ROOT}/src/a`) + }) +}) + describe('PierreWorkspaceTreeImpl — row context menu keyboard contract (#6231)', () => { // The DEGENERATE case of the shared `role="menu"` contract: this menu hosts // exactly ONE menuitem, so every focus-move assertion is vacuously true — diff --git a/website/src/test/useDirectoryUpload.test.tsx b/website/src/test/useDirectoryUpload.test.tsx new file mode 100644 index 00000000000..36f5fa7fd6b --- /dev/null +++ b/website/src/test/useDirectoryUpload.test.tsx @@ -0,0 +1,435 @@ +/** + * `useDirectoryUpload` — the upload orchestration shared by the Files rail's + * drop overlay and its "Upload files…" row-menu action. + * + * Rendered through a small harness (not `renderHook`) because the hook hands + * back JSX (`fileInput`) that must actually mount for the hidden picker input + * to be interactive. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' + +vi.mock('../api/client', () => ({ api: { uploadToDirectory: vi.fn() } })) + +import { api } from '../api/client' +import { useDirectoryUpload } from '../pages/chat/useDirectoryUpload' + +const PROJECT_DIR = '/repo' +const REAL_PROJECT_DIR = '/private/repo' +const TARGET_DIR = '/repo/dir' + +function Harness({ projectDir = PROJECT_DIR, rootDir }: { projectDir?: string; rootDir?: string }) { + const { uploadFiles, pickAndUpload, progress, error, dismissError, success, dismissSuccess, fileInput } = + useDirectoryUpload(projectDir, rootDir) + return ( +
+ + + + + + {progress &&
{progress}
} + {success &&
{success}
} + {success && } + {error &&
{error}
} + {error && } + {fileInput} +
+ ) +} + +function mount(projectDir = PROJECT_DIR, rootDir?: string) { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const invalidateSpy = vi.spyOn(qc, 'invalidateQueries') + const view = render( + + + , + ) + const rerenderWith = (nextDir: string) => view.rerender( + + + , + ) + return { qc, invalidateSpy, rerenderWith } +} + +beforeEach(() => { + vi.mocked(api.uploadToDirectory).mockReset() +}) + +describe('useDirectoryUpload — uploadFiles', () => { + it('invalidates every file listing and search query an upload can change after a successful batch', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ ok: true, path: `${TARGET_DIR}/a.txt`, name: 'a.txt' }) + const { qc, invalidateSpy } = mount() + // The keys the Files rail, its tree and the folder tab read, each built + // with the root its reader uses: the rail's filename search with the + // backend's realpath of the project, the folder tab with its own folder. + const listingKeys = [ + ['project-tree', PROJECT_DIR], + ['project-tree', PROJECT_DIR, 'dir', 'dir'], + ['git-status', PROJECT_DIR], + ['file-grep', PROJECT_DIR, 'needle'], + ['rail-name-search', REAL_PROJECT_DIR, 'a.txt'], + ['browse-files', TARGET_DIR], + ['folder-file-search', PROJECT_DIR, 'a.txt', 50], + ] + for (const key of listingKeys) qc.setQueryData(key, { seeded: true }) + + fireEvent.click(screen.getByTestId('upload')) + + await waitFor(() => + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['project-tree', PROJECT_DIR] }), + ) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['git-status', PROJECT_DIR] }) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['file-grep', PROJECT_DIR] }) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['rail-name-search'] }) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['browse-files'] }) + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['folder-file-search'] }) + for (const key of listingKeys) { + expect(qc.getQueryState(key)?.isInvalidated, JSON.stringify(key)).toBe(true) + } + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('surfaces a non-collision failure', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ + ok: false, status: 400, code: 'unsupported_file_type', error: 'Unsupported file type: .txt', + }) + mount() + + fireEvent.click(screen.getByTestId('upload')) + + await waitFor(() => + expect(screen.getByTestId('error')).toHaveTextContent("Couldn't upload a.txt: Unsupported file type: .txt"), + ) + }) + + it('surfaces every refusal from one multi-file upload batch', async () => { + vi.mocked(api.uploadToDirectory) + .mockResolvedValueOnce({ + ok: false, status: 400, code: 'unsupported_file_type', error: 'Unsupported file type: .txt', + }) + .mockResolvedValueOnce({ + ok: false, status: 413, error: 'File is too large', + }) + mount() + + fireEvent.click(screen.getByTestId('upload-two')) + + await waitFor(() => { + const notice = screen.getByTestId('error') + expect(notice).toHaveTextContent("Couldn't upload a.txt: Unsupported file type: .txt") + expect(notice).toHaveTextContent("Couldn't upload b.exe: File is too large") + }) + }) + + it('dismisses a shown error', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ ok: false, status: 500, error: 'boom' }) + mount() + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(screen.getByTestId('error')).toBeInTheDocument()) + + fireEvent.click(screen.getByTestId('dismiss')) + + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('clears a failure notice when projectDir changes in place', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ ok: false, status: 500, error: 'boom' }) + const { rerenderWith } = mount() + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(screen.getByTestId('error')).toBeInTheDocument()) + + rerenderWith('/other-repo') + + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('drops the failures of an upload that finishes after projectDir changes', async () => { + let resolveUpload: (value: Awaited>) => void = () => {} + vi.mocked(api.uploadToDirectory).mockReturnValue( + new Promise((resolve) => { resolveUpload = resolve }), + ) + const { invalidateSpy, rerenderWith } = mount() + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(api.uploadToDirectory).toHaveBeenCalledTimes(1)) + + rerenderWith('/other-repo') + resolveUpload({ ok: false, status: 500, error: 'boom' }) + + await waitFor(() => + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['project-tree', PROJECT_DIR] }), + ) + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('clears an earlier failure notice when a later upload succeeds', async () => { + vi.mocked(api.uploadToDirectory) + .mockResolvedValueOnce({ ok: false, status: 500, error: 'boom' }) + .mockResolvedValueOnce({ ok: true, path: `${TARGET_DIR}/a.txt`, name: 'a.txt' }) + const { invalidateSpy } = mount() + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(screen.getByTestId('error')).toBeInTheDocument()) + invalidateSpy.mockClear() + + fireEvent.click(screen.getByTestId('upload')) + + await waitFor(() => + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['project-tree', PROJECT_DIR] }), + ) + expect(api.uploadToDirectory).toHaveBeenCalledTimes(2) + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('reports a name collision with the already-exists message and never retries', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ + ok: false, status: 409, code: 'name_collision', error: 'already exists', + }) + const { invalidateSpy } = mount() + + fireEvent.click(screen.getByTestId('upload')) + + await waitFor(() => + expect(screen.getByTestId('error')).toHaveTextContent( + '“a.txt” already exists in this folder. Rename your file, or remove the existing one, then upload again.', + ), + ) + expect(api.uploadToDirectory).toHaveBeenCalledTimes(1) + expect(vi.mocked(api.uploadToDirectory).mock.calls[0]).toEqual([TARGET_DIR, expect.any(File)]) + expect(screen.queryByRole('dialog')).toBeNull() + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['project-tree', PROJECT_DIR] }) + }) + + it('reports a collision alongside another file\'s failure in one batch', async () => { + vi.mocked(api.uploadToDirectory) + .mockResolvedValueOnce({ ok: false, status: 409, code: 'name_collision', error: 'already exists' }) + .mockResolvedValueOnce({ ok: false, status: 400, code: 'unsupported_file_type', error: 'Unsupported file type: .exe' }) + mount() + + fireEvent.click(screen.getByTestId('upload-two')) + + await waitFor(() => { + const notice = screen.getByTestId('error') + expect(notice).toHaveTextContent('“a.txt” already exists in this folder.') + expect(notice).toHaveTextContent("Couldn't upload b.exe: Unsupported file type: .exe") + }) + expect(api.uploadToDirectory).toHaveBeenCalledTimes(2) + }) + + it('surfaces a rejected upload call (e.g. a transport failure) as an error instead of an unhandled rejection', async () => { + vi.mocked(api.uploadToDirectory).mockRejectedValue(new Error('Failed to fetch')) + mount() + + fireEvent.click(screen.getByTestId('upload')) + + await waitFor(() => + expect(screen.getByTestId('error')).toHaveTextContent("Couldn't upload a.txt: Failed to fetch"), + ) + }) +}) + +type UploadResult = Awaited> + +/** Makes the next upload call hang until the returned resolver is called. */ +function deferUpload(): (value: UploadResult) => void { + let resolveUpload: (value: UploadResult) => void = () => {} + vi.mocked(api.uploadToDirectory).mockReturnValueOnce( + new Promise((resolve) => { resolveUpload = resolve }), + ) + return (value) => resolveUpload(value) +} + +const OK: UploadResult = { ok: true, path: `${TARGET_DIR}/a.txt`, name: 'a.txt' } + +describe('useDirectoryUpload — progress and success', () => { + it('shows the uploading file while its upload runs and drops the line when it ends', async () => { + const resolve = deferUpload() + mount() + + fireEvent.click(screen.getByTestId('upload')) + + expect(await screen.findByTestId('progress')).toHaveTextContent('Uploading a.txt…') + resolve(OK) + await waitFor(() => expect(screen.queryByTestId('progress')).toBeNull()) + }) + + it('counts the files of a running multi-file batch', async () => { + const resolve = deferUpload() + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount() + + fireEvent.click(screen.getByTestId('upload-two')) + + expect(await screen.findByTestId('progress')).toHaveTextContent('Uploading 2 files…') + resolve(OK) + await waitFor(() => expect(screen.queryByTestId('progress')).toBeNull()) + }) + + it('names the file and its folder, relative to the project, after a successful upload', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount() + + fireEvent.click(screen.getByTestId('upload')) + + expect(await screen.findByTestId('success')).toHaveTextContent('Uploaded a.txt to dir.') + expect(screen.queryByTestId('error')).toBeNull() + }) + + it('names the project folder for an upload into the tree root, and a subfolder under a realpath root', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount(PROJECT_DIR, REAL_PROJECT_DIR) + + fireEvent.click(screen.getByTestId('upload-root')) + expect(await screen.findByTestId('success')).toHaveTextContent('Uploaded a.txt to repo.') + + fireEvent.click(screen.getByTestId('upload-real-sub')) + await waitFor(() => expect(screen.getByTestId('success')).toHaveTextContent('Uploaded a.txt to sub.')) + }) + + it('counts the files of a successful multi-file batch', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount() + + fireEvent.click(screen.getByTestId('upload-two')) + + expect(await screen.findByTestId('success')).toHaveTextContent('Uploaded 2 files to dir.') + }) + + it('shows no success line for a batch with a failure', async () => { + vi.mocked(api.uploadToDirectory) + .mockResolvedValueOnce(OK) + .mockResolvedValueOnce({ ok: false, status: 413, error: 'File is too large' }) + mount() + + fireEvent.click(screen.getByTestId('upload-two')) + + await waitFor(() => expect(screen.getByTestId('error')).toHaveTextContent("Couldn't upload b.exe")) + expect(screen.queryByTestId('success')).toBeNull() + }) + + it('does not send a name that is still uploading into the same folder', async () => { + const resolve = deferUpload() + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount() + + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(api.uploadToDirectory).toHaveBeenCalledTimes(1)) + fireEvent.click(screen.getByTestId('upload')) + + expect(api.uploadToDirectory).toHaveBeenCalledTimes(1) + expect(screen.getByTestId('progress')).toHaveTextContent('Uploading a.txt…') + resolve(OK) + expect(await screen.findByTestId('success')).toHaveTextContent('Uploaded a.txt to dir.') + expect(api.uploadToDirectory).toHaveBeenCalledTimes(1) + expect(screen.queryByTestId('error')).toBeNull() + + // Once the first upload has ended the same name is sent again (and the + // server answers for it). + fireEvent.click(screen.getByTestId('upload')) + await waitFor(() => expect(api.uploadToDirectory).toHaveBeenCalledTimes(2)) + }) + + it('clears the success line on dismiss and when the next batch starts', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + mount() + fireEvent.click(screen.getByTestId('upload')) + await screen.findByTestId('success') + + fireEvent.click(screen.getByTestId('dismiss-success')) + expect(screen.queryByTestId('success')).toBeNull() + + fireEvent.click(screen.getByTestId('upload')) + await screen.findByTestId('success') + const resolve = deferUpload() + fireEvent.click(screen.getByTestId('upload')) + + await screen.findByTestId('progress') + expect(screen.queryByTestId('success')).toBeNull() + resolve(OK) + await screen.findByTestId('success') + }) + + it('clears the success line and the running batch line when projectDir changes in place', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValueOnce(OK) + const { invalidateSpy, rerenderWith } = mount() + fireEvent.click(screen.getByTestId('upload')) + await screen.findByTestId('success') + + rerenderWith('/other-repo') + expect(screen.queryByTestId('success')).toBeNull() + + const resolve = deferUpload() + fireEvent.click(screen.getByTestId('upload-two')) + await screen.findByTestId('progress') + rerenderWith(PROJECT_DIR) + expect(screen.queryByTestId('progress')).toBeNull() + + // A batch that finishes after the switch reports nothing in the new panel. + vi.mocked(api.uploadToDirectory).mockResolvedValue(OK) + resolve(OK) + await waitFor(() => + expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['project-tree', '/other-repo'] }), + ) + expect(screen.queryByTestId('success')).toBeNull() + expect(screen.queryByTestId('progress')).toBeNull() + }) +}) + +describe('useDirectoryUpload — pickAndUpload', () => { + it('opens the hidden file picker scoped to the requested directory', () => { + mount() + const input = screen.getByLabelText('Upload files…') as HTMLInputElement + const clickSpy = vi.spyOn(input, 'click') + + fireEvent.click(screen.getByTestId('pick')) + + expect(clickSpy).toHaveBeenCalledTimes(1) + }) + + it('uploads the picker selection into the pending directory', async () => { + vi.mocked(api.uploadToDirectory).mockResolvedValue({ ok: true, path: `${TARGET_DIR}/b.txt`, name: 'b.txt' }) + mount() + fireEvent.click(screen.getByTestId('pick')) + const input = screen.getByLabelText('Upload files…') as HTMLInputElement + const file = new File(['y'], 'b.txt', { type: 'text/plain' }) + + fireEvent.change(input, { target: { files: [file] } }) + + await waitFor(() => expect(api.uploadToDirectory).toHaveBeenCalledWith(TARGET_DIR, file)) + }) + + it('does nothing when the picker is dismissed with no selection', () => { + mount() + fireEvent.click(screen.getByTestId('pick')) + const input = screen.getByLabelText('Upload files…') as HTMLInputElement + + fireEvent.change(input, { target: { files: [] } }) + + expect(api.uploadToDirectory).not.toHaveBeenCalled() + }) +})