* feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash
Implements docs/superpowers/specs/2026-09-12-projects-mvp-phase2-design.md
(issue #5160, tracker #5129) in the slice order of the spec's §16.
Slices:
- A: ProjectsConfig + write-time 422 UTF-8 byte cap; PROJECT_CONTEXT_KEY
admission pinning (both server-owned sets + worker hoist); latest-only
request-scoped <project> block via DynamicContextMiddleware
wrap_model_call/awrap_model_call (idempotent reassembly, reserved ID
prefix + marker + provenance, never persisted); journal audit
fingerprints; Instructions tab.
- B: ProjectDocumentRow + migration 0023; ProjectDocumentRepository with
locked check-and-set; hash-qualified immutable shelf storage with
Paths helpers; upload/list/content/delete-to-trash routes; project
delete trashes the shelf in-transaction; request-scoped bounded
<documents> index with honest count/shown + actionable overflow note;
list_project_documents/read_project_document tools registered only on
pinned runs; PAT allowlist + drift guards; blocking-IO anchors.
- C: shared thread-upload ingestion service (uploads router refactored to
parity); POST from-thread with provenance; attach-to-thread with
lock-staged copy (archived source allowed); read-only thread-files
view with per-group truncation reporting.
- D: restore (restored/merged/not_found/no_target/content_missing; no
file moves), purge (continuous row lock across unlink/delete/commit,
retryable on FS errors), retention sweep (lazy + startup, 24h orphan
guard, row-side reconciliation never deletes).
- E: Documents tab (shelf + conversation-files browser, provenance,
archived banner, content-missing rows), /workspace/trash route,
sidebar entry, composer attach handoff, i18n (en-US/zh-CN), e2e mocks
+ specs.
Review hardening folded in (10 rounds, all with tests):
- force active shelf content (HTML/XML family) to download; nosniff on
artifact + content responses; unified unsandboxed-iframe PDF preview
(fixes the pre-existing Chromium sandbox blank in the artifact viewer)
- scope document trash to the URL project under the document lock
- atomic no-overwrite filename reservation for ALL ingestion (seeded
claims + os.link commit with suffix retry; same-name re-upload now
unique-names instead of replacing); hidden staging only, no visible
placeholders; lease cleanup on setup failure
- serialize conversion under the document lock with post-lock active
revalidation; drain locked filesystem work on cancellation; preserve
bytes when an insert's commit state is uncertain (including trashed
rows)
- original-integrity checks before serving text or cached conversions;
content_missing surfaced in list responses (UI reads the flag, no
409-probe); downloads always serve original bytes
- bounded streaming document reads with cached char counts; shelf limits
declared in middleware release identity
- thread-root confinement for from-thread sources; config fallback
rejects fractional/infinite values; composer counts staged
attachments; pending attachments persist until submission or removal;
in-flight instruction/rename edits survive save refetches; shelf and
trash pagination; conversation-file and thread-files pages stay
subscribed to refetches
Docs: README/README_zh, backend API.md/ARCHITECTURE.md, AGENTS.md
contracts, config.example.yaml projects block.
Review follow-ups (head b4807477 → this revision):
- The trash retention sweep is split so repeated lazy triggers stay
bounded: the indexed expiry purge still runs on every trigger
(GET /api/trash/documents, POST /api/trash/purge) while the
O(all rows + all files) reconciliation is throttled to one run per
user per 15 minutes (process-local, per-user window). The startup
sweep now runs as a background task instead of blocking gateway
readiness, and shutdown awaits it (bounded).
- The export scrub (stripInternalMarkers) is fence- and indentation-aware
like the render path, so a pasted, fenced <project>/<documents> snippet
survives markdown export while real injected blocks (never fenced) are
still removed. Fence regexes moved to a dependency-free leaf module to
avoid the messages↔streamdown import cycle.
- The artifact viewer's PDF iframe no longer carries an added title
attribute (the upstream e2e contract locates it via :not([title])), and
the upstream artifact-preview spec now pins the new contract: PDFs
render unsandboxed, images keep sandbox="".
* fix(projects): round-2 review — cancel an overrun trash sweep, restore the PDF frame title
- Shutdown cancelled only the shield around the background startup sweep,
so an all-users reconciliation that outlived the 5s budget kept walking
rows and files while the document repo and DB engine were disposed
underneath it. The wait now lives in `_shutdown_startup_trash_sweep`,
which cancels the task and drains it before worker exit: the shield
keeps the wait bounded, the cancel makes it final (CancelledError lands
at the sweep's next await, and `_run_startup_trash_sweep` only catches
`Exception`, so nothing swallows it).
- The browser-preview iframe lost `title={getFileName(filepath)}` in the
previous fix round, leaving the PDF frame without an accessible name
while its siblings keep theirs. Restore it (WCAG frame titles), assert
it in the DOM test, and anchor the e2e on `iframe[title="report.pdf"]`
instead of `iframe:not([title])`.
* fix(projects): round-3 review — report the sweep's late finish, not a phantom cancel
`Task.cancel()` returns False when the sweep already finished inside the
window between the deadline firing and the cancel, so the shutdown log
claimed a cancellation that never happened. Branch on that outcome: the
warning stays for a real cancel, a late finish is logged at info, and both
paths still reap the task before worker exit.
* fix(projects): round-4 review — make Empty trash delete what it confirms
`POST /api/trash/purge` only ran the retention sweep, and the sweep's
candidate selection is age-gated, so a freshly trashed document survived
"Empty trash" even though the confirmation promises that every listed
document is permanently deleted. With one trashed row the route answered
`{"purged": 0}` and left it in place; `GET /api/trash/documents` sweeps
expired rows before listing, so the visible rows were normally ineligible
for the action by construction.
Empty trash now drives `purge_all_trashed`: the caller's trashed rows
(`list_all_trashed`, no age filter) each go through the same guarded,
row-locked `purge` as the single-document delete — bytes first, then the
row, in one transaction — so a row restored mid-flight is skipped instead of
force-deleted, and an unlink failure rolls that row back and answers 500 with
a retryable message. Retention expiry stays where it was: the sweep's
`purge_candidates` is now the only age-gated selection, and the lazy
retention sweep still runs on the listing and at startup.
Tests: the router suite replaces the retention-gated expectation with the
reviewer's repro (fresh row purged, bytes unlinked, shelf and other users'
trash untouched, a failing unlink stays retryable and 500); a blocking-I/O
anchor drives the new entry point through the offload; the mocked e2e covers
the action end to end; a new real-backend spec performs it against the real
gateway and re-reads `GET /api/trash/documents`. README, API, ARCHITECTURE
and the phase-2 design docs (en+zh) state the age-independent contract.
* fix(frontend): restore sanitization in custom streamdown rehype chains
Streamdown 2.5 replaces its entire default rehype chain
[rehype-raw, rehype-sanitize, rehype-harden] with whatever array the
caller passes via the rehypePlugins prop. Every custom chain in this
repo therefore rendered LLM/stored markdown without any sanitization:
- Artifact markdown previews (markdown-preview-plugins.ts +
artifact-file-detail.tsx) parse raw HTML via rehypeRaw, so a
generated .md artifact could inject <style>/<iframe>/on* handlers
into the workspace DOM (stored XSS; only javascript: anchors were
blocked by the ArtifactLink component).
- The memory settings summary (memory-settings-page.tsx) spread the
shared preset without component overrides, so a hostile
<a href="javascript:..."> in stored memory content rendered as a
clickable anchor.
Fix strategy:
- Add rehype-sanitize (already resolved in the lockfile via streamdown)
as a direct dependency and re-insert a [rehypeSanitize, schema] step
in the shared preset (core/streamdown/plugins.ts). It runs after
rehypeRaw (raw HTML must be parsed into hast before it can be
cleaned) and before rehypeKatex/rehypeSlug (their output is trusted
and would otherwise be filtered or clobbered) - the same
raw -> sanitize -> math ordering streamdown itself uses.
- The schema extends rehype-sanitize's GitHub-style defaultSchema (the
base of streamdown's own sanitize schema) so legitimate authored
artifact HTML (tables, details, images, alignment/size attributes)
keeps working while script/iframe/style, on* handlers and
non-allow-listed URL schemes (javascript:, data:, ...) are dropped.
The only extensions are tel: hrefs and the math-inline/math-display
class markers remark-math emits and rehype-katex detects.
- Position rehypeSlug after the sanitize step in the artifact chain so
sanitize's id clobbering (id="x" -> id="user-content-x") cannot break
the heading anchors it creates.
- Pass a: createMarkdownLinkComponent() on the memory settings page as
defense in depth, matching the chat rendering path.
Unit tests feed a hostile payload (<a href="javascript:...">,
<img onerror>, <script>, <iframe>, <style>, ontoggle) through both
render paths and assert no executable/clickable equivalent survives,
plus regression guards for heading anchors, legitimate HTML and KaTeX
math rendering.
* fix(frontend): keep the sanitize clobber prefix on heading anchors; minimal lockfile
Review follow-ups on the sanitization change:
- Heading anchors: rehypeScopedSlug replaces rehype-slug in the artifact
chain. It runs after the sanitize step (so raw-HTML headings are also
anchored) but keeps rehype-sanitize's user-content- id prefix — an
untrusted heading like "## current" cannot mint an unprefixed
id="current" (the DOM-clobbering shape the sanitizer guards against).
In-page fragment links are translated to the prefixed anchors so they
still resolve; external URLs, bare "#", already-prefixed fragments and
sanitize-prefixed raw-HTML ids are left untouched.
- Lockfile: regenerated as a minimal diff — only the two direct-dependency
importer entries (rehype-sanitize, github-slugger for the scoped slug)
are added; the libc platform selectors on the 64 native package records
are preserved byte-for-byte instead of being dropped by lockfile
normalization.
Full frontend suite: 1034 tests passing; tsc and prettier clean.
* style: reorder github-slugger import ahead of the hast type import
* test(e2e): expect the clobber-prefixed heading anchor in artifact preview
The scoped slug plugin gives generated heading ids rehype-sanitize's
user-content- prefix and translates fragment links to match, so the
anchor-scroll test must locate the prefixed id.
* fix(frontend): reset the scoped slugger per tree; keep footnote anchors single-prefixed
Review follow-ups:
- The scoped slug attacher holds one GithubSlugger, but streamdown
caches the unified processor by plugin name, so the instance survived
across parses and repeated renders of the same heading grew -1/-2
suffixes (the artifact-anchor e2e could not find the id on re-render).
The transformer now resets the slugger per tree, as rehype-slug does;
a regression test renders identical artifact markdown twice.
- remark-rehype emits GFM footnote anchors already clobber-prefixed
(user-content-fn-1); the sanitize step prefixed those ids again while
their hrefs stayed single-prefixed, breaking footnote navigation in
every chain built on the shared preset. A new rehypeClobberFragments
step runs right after sanitize: double-prefixed ids are normalized
back to one prefix, and unprefixed fragment hrefs are translated to
the prefixed form (already-prefixed and external links untouched).
The artifact slug now inserts after this step; covered by a footnote
regression test on the shared render path.
Unit suite 1036 passing; artifact-preview e2e verified locally
(9/9, including the heading-anchor scroll test).
* fix(frontend): harden artifact and markdown rendering
* fix(#4117): restore allow-scripts for scroll-restore, fix citation XSS bypass
willem-bd identified these issues:
1. [REGRESSION] sandbox removed allow-scripts which broke HTML artifact
scroll-restoration — the injected postMessage script could not run.
The prior config (allow-scripts allow-forms without allow-same-origin,
i.e. opaque origin) was already safe. Restored with corrected comment.
2. [XSS bypass] CitationLink rendered <a href={href}> directly before
isSafeHref ran, so prompt-injected [citation:x](javascript:...)
bypassed the safety check. Moved citation block after the guard.
Also:
- Added mailto: and tel: to SAFE_HREF_PROTOCOLS
- Kept anchor-only attributes off the <span> fallback (React DOM warning)
Note: the loadMessages re-throw originally in this commit was dropped
during the rebase — upstream #4065 rewrote useThreadHistory as a
TanStack useInfiniteQuery that already surfaces fetch failures.
* test(e2e): update sandbox assertion to match allow-scripts allow-forms
The artifact preview iframe's sandbox was restored to 'allow-scripts allow-forms'
for scroll-restoration. Update the E2E test to expect the corrected value.
* fix(#4117): address review — ArtifactLink XSS guard, urlOfArtifact sandbox e2e
- Apply the isSafeHref guard to ArtifactLink (artifact-link.tsx) so a
prompt-injected javascript:/data: href in a .md artifact preview cannot
reach a real anchor in the main document, matching the guard in
createMarkdownLinkComponent.
- Add an e2e asserting the urlOfArtifact iframe (non-code
browser-previewable files like PDF) keeps its empty sandbox.
Note: the loadMessages error-surfacing changes originally in this commit
were dropped during the rebase — upstream #4065 rewrote useThreadHistory
as a TanStack useInfiniteQuery whose queryFn already throws on
!response.ok and surfaces the failure via a toast.
* fix(frontend): let write_file non-code previewable artifacts render in sandboxed iframe
When a write_file artifact such as PDF is clicked in chat, the component receives a path that forced isCodeFile=true, hiding the sandboxed iframe behind the code editor. Now non-code browser-previewable files are detected early so the sandboxed iframe renders correctly.
Fixes the E2E test: renders sandboxed iframe for a browser-previewable non-code file.
* fix(#4117): align PDF artifact route pattern with convention (drop /mock/ prefix)
The test used /mock/api/threads/... for the PDF artifact content route,
but the urlOfArtifact helper generates /api/threads/... (without /mock/)
when isMock=false. The other tests (e.g. presented artifacts) already use
the correct /api/threads/... pattern.
* test(frontend): add render-level coverage for unsafe markdown/artifact links
- Render MarkdownLink and ArtifactLink via renderToStaticMarkup and
assert an unsafe javascript: href produces a disabled <span> (never an
<a>), including through the citation-labelled branch, and that safe
https hrefs render hardened anchors (target=_blank, rel=noopener).
- The new ArtifactLink render test caught the unsafe href leaking onto
the fallback <span> through the {...rest} spread; drop the spread in
both span fallbacks so anchor-only attributes (href/target/rel) and
react-markdown's node prop never reach the span.
- Cover mailto:/tel: in the isSafeHref unit cases and fix the stale
SAFE_HREF_PROTOCOLS docstring that still claimed http/https-only.
* fix(frontend): route the agent save-hint through the safe localStorage facade
Export safeLocalStorage from core/settings/local and use it for the
agent-create save-hint read/write so blocked browser storage (Safari
private mode, strict containers, embedded WebViews) cannot throw from
the effect. core/agents/feature-cache.ts already guards its
localStorage access with try/catch, so settings + agent pages are now
consistently best-effort.
* fix(frontend): allow scheme-less relative markdown links in isSafeHref
Closes#3192
Root cause
----------
The artifact preview header is driven by a Radix Select whose <SelectValue>
renders the label of the <SelectItem> matching the current value. The option
list was built solely from `artifacts` in ArtifactsContext, which is only
synced from `thread.values.artifacts` (chat-box.tsx). Artifacts surfaced via
the message-layer `present_files` tool call are never written back to
`thread.values.artifacts`, so when such a file is selected its filepath has
no matching <SelectItem>. Radix then renders an empty trigger and the header
filename appears blank, even though the preview body loads correctly.
Fix
---
Compute `artifactOptions` as a defensive union: if the currently selected
filepath is missing from `artifacts`, prepend it so a matching <SelectItem>
always exists. This keeps the header label in sync with the active file
without changing context semantics or coupling the UI to a future
auto-discovery mechanism (see existing TODO in chat-box.tsx).
Tests
-----
Add an e2e case that mocks a thread whose only artifact is delivered via
`present_files` (thread.values.artifacts = []) and asserts both the header
title and preview content render. All 21 e2e tests pass.
