feat(web,hub): scratchlist v2.2 hub attachment storage (#921)

[heavygee]

Summary

Adds scratchlist v2.2 image/file attachment support on top of hub-synced scratchlist (#896).

• Hub stores attachment bytes under HAPI_HOME/scratchlist-attachments/; SQLite holds AttachmentMetadata JSON on session_scratchlist.attachments (schema V14 to V15; scratchlist table remains V11 to V12 from feat(web,hub): scratchlist v2 - hub sync via typed table + session-updated piggyback #896).
• REST: upload, serve, and limits endpoints; entry create/update accept optional attachments; delete cleans hub files.
• Web: composer scratchlist mode can park text + images; drawer shows float thumbnails; promote-to-composer rehydrates attachments; promote-to-queue stages hub bytes to CLI upload for send.
• Copy action tooltip clarifies text-only (not images).

Closes #921.

Test plan

• bun typecheck and bun run test green on this branch
• Fresh hub DB reaches schema 15 with session_scratchlist.attachments
• Existing V14 DB migrates and gains attachments without dropping scratchlist rows
• Scratchlist mode: attach image, Send enabled, entry appears with thumbnail
• Promote entry to composer rehydrates image preview
• Promote to queue sends text + attachment into the session
• Attachment-only entry (no text) allowed within configured limits
• Oversize / disallowed MIME rejected with clear API/UI error
• Cross-device: entry + attachment bytes available from a second client against the same hub

Notes

• Depends on feat(web,hub): scratchlist v2 - hub sync via typed table + session-updated piggyback #896 (merged). Schema ladder: V11 to V12 scratchlist table, V12 to V14 message_epochs reconciliation, V14 to V15 attachments column.
• Configurable via HAPI_SCRATCHLIST_MAX_ATTACHMENT_* / HAPI_SCRATCHLIST_ALLOWED_ATTACHMENT_MIMES.

[github-actions]

Findings

• [Major] Attachment metadata is trusted without proving session ownership — POST/PUT /scratchlist accepts client-supplied id/path/size and only checks size/MIME, then the download route authorizes the current session but reads match.path; a forged metadata row can point at another session/namespace's hub file if the path is known. Evidence hub/src/web/routes/sessions.ts:856.
• [Major] Scratchlist upload quota ignores uncommitted files — upload writes bytes immediately, but the per-session limit is computed only from persisted scratchlist entries, so repeated upload calls that are never attached to an entry can fill disk outside maxBytesPerSession. Evidence hub/src/sync/syncEngine.ts:600.

Summary
Review mode: initial
Found two Major issues in the new hub-backed attachment storage path. Both need server-side fixes; tests should cover forged attachment metadata and abandoned uploads.

Testing
Not run (automation; avoided executing PR code). Suggested tests: forged cross-session/cross-namespace attachment metadata is rejected; repeated unattached uploads cannot exceed session quota and are cleaned up.

HAPI Bot

[github-actions]

Review mode: follow-up after new commits. Found two attachment-cleanup regressions in the new scratchlist attachment path.

[github-actions]

Findings

• [Major] Promote-to-composer rehydrates attachments while scratchlist mode is still active, so assistantApi.composer().addAttachment(file) runs against createScratchlistAttachmentAdapter(...) instead of the normal upload adapter. That leaves hapi-hub:scratchlist/... paths in the composer, and the later chat send will format them as @path references the agent cannot read. web/src/components/SessionChat.tsx:326
  Suggested fix:

  const handlePromoteToComposer = useCallback(async (entry: ScratchlistEntry) => {
      assistantApi.composer().setText(entry.text)
      props.onExitScratchlistMode()
  
      if (entry.attachments?.length) {
          window.requestAnimationFrame(() => {
              void rehydrateScratchlistAttachmentsToComposer(
                  props.api,
                  props.sessionId,
                  entry.attachments,
                  assistantApi.composer()
              )
          })
      }
  }, [assistantApi, props.api, props.onExitScratchlistMode, props.sessionId])

• [Major] Canceling a scratchlist attachment while its hub upload is in flight leaves the newly written blob orphaned, because the post-upload cancel branch returns without deleting the server-side file. Those bytes stay in HAPI_HOME/scratchlist-attachments, count against the session quota, and nothing points back to them for cleanup. web/src/lib/scratchlistAttachmentAdapter.ts:60
  Suggested fix:

  const result = await api.uploadScratchlistAttachment(sessionId, file.name, content, contentType)
  if (cancelledAttachmentIds.has(id)) {
      if (result.success && result.attachment) {
          await api.deleteScratchlistAttachment(sessionId, result.attachment.id).catch(() => {})
      }
      return
  }

Summary
Review mode: follow-up after new commits
Two Major regressions remain in the new scratchlist attachment flow. The server-side trust/quota issues from the prior pass appear addressed; the remaining gaps are in the web-side composer handoff and cancel race.

Testing
Not run (automation). Suggested tests: promote-to-composer uses the normal upload adapter after leaving scratchlist mode; removing an attachment during upload deletes any successfully written hub blob.

[github-actions]

Findings

• [Major] Partial attachment IDs can delete referenced files — scratchlist files are named ${uuid}-${filename}, but the delete helper removes the first filename that starts with ${attachmentId}-; because UUIDs contain hyphens, a request using only the first UUID segment bypasses the route's exact stillReferenced check and deletes a file still referenced by an entry. Evidence hub/src/scratchlistAttachments/storage.ts:255 and hub/src/web/routes/sessions.ts:984.
  Suggested fix:

  const SCRATCHLIST_ATTACHMENT_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
  
  function isScratchlistAttachmentId(id: string): boolean {
      return SCRATCHLIST_ATTACHMENT_ID_RE.test(id)
  }
  
  if (!isScratchlistAttachmentId(claimed.id)) {
      return { ok: false, error: 'Invalid scratchlist attachment id' }
  }
  if (!isScratchlistAttachmentId(attachmentId)) return false

• [Major] Hub-only scratchlist uploads can be sent to chat after toggling the drawer off — when scratchlistMode is on, the composer uploads attachments to hapi-hub:scratchlist/...; if the user turns scratchlist mode off before pressing Send, the normal chat path receives that hub-private path unchanged, and the CLI only formats attachment paths as local @path references. Evidence web/src/components/SessionChat.tsx:1191 and web/src/lib/scratchlistAttachmentAdapter.ts:130.
  Suggested fix:

  const prepareChatAttachments = async (items?: AttachmentMetadata[]) => {
      const hubItems = (items ?? []).filter((att) => isHubScratchlistAttachmentPath(att.path))
      if (hubItems.length === 0) return items
  
      const normalItems = (items ?? []).filter((att) => !isHubScratchlistAttachmentPath(att.path))
      const staged = await stageScratchlistAttachmentsForComposeSend(
          props.api,
          props.session.id,
          hubItems
      )
      return [...normalItems, ...staged]
  }
  
  return props.onSend(text, await prepareChatAttachments(attachments), scheduledAt)

Questions

• None.

Summary
Review mode: follow-up after new commits
Found two Major issues in the current scratchlist attachment flow. The earlier bot concerns around cross-session metadata and abandoned-upload quota are mostly addressed, but the current ID matching still has an exactness bug, and hub-staged attachments need conversion before they enter the normal chat send path.

Testing
Not run (automation; avoided executing PR code). Suggested coverage: DELETE with a UUID prefix must not remove an in-use attachment; toggling scratchlist mode off after a hub upload must stage the file through normal upload before chat send.

HAPI Bot

[github-actions]

Findings

• [Major] Text-only scratchlist edits can fail after a session-id transfer because existing attachment paths still point at the old session id.
• [Major] Deleting a session removes the DB rows but leaves hub-side scratchlist attachment bytes on disk.
• [Major] Attachment validation still accepts partial UUID ids, letting direct REST callers persist aliases that bypass the in-use check.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The previous bot findings were addressed, but the current attachment storage still has lifecycle and identity issues around session transfer, session deletion, and server-side metadata validation.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

Findings

• [Major] Attach-before-toggle files are routed to scratchlist with non-hub paths, so the hub rejects the send. Evidence web/src/components/SessionChat.tsx:531.
  Suggested fix:

  export function shouldRouteToScratchlist(
      scratchlistMode: boolean,
      attachments: AttachmentMetadata[] | undefined,
      scheduledAt: number | null | undefined,
  ): boolean {
      if (!scratchlistMode) return false
      if (scheduledAt != null) return false
      return (attachments ?? []).every((att) => isHubScratchlistAttachmentPath(att.path))
  }

• [Major] Replacing attachments can falsely exceed the per-session byte cap because removed bytes are still counted during PUT validation. Evidence hub/src/web/routes/sessions.ts:937.
  Suggested fix:

  const removedAttachments = existing.attachments.filter(
      (old) => !nextAttachments.some((next) => next.id === old.id)
  )
  const removedBytes = removedAttachments.reduce((sum, att) => sum + att.size, 0)
  const diskBytesAfterRemoval = Math.max(0, diskBytes - removedBytes)
  const sessionBytesBefore = Math.max(0, diskBytesAfterRemoval - entryBytes)

• [Major] Deleting one scratchlist entry can delete attachment bytes still referenced by another entry. Evidence hub/src/sync/syncEngine.ts:572.
  Suggested fix:

  const remainingIds = new Set(
      this.store.scratchlist
          .list(sessionId)
          .flatMap((entry) => entry.attachments.map((att) => att.id))
  )
  const orphaned = existing.attachments.filter((att) => !remainingIds.has(att.id))
  void import('../scratchlistAttachments/storage').then(({ deleteScratchlistAttachmentFiles, getHapiHomeDir }) =>
      deleteScratchlistAttachmentFiles(getHapiHomeDir(), orphaned)
  )

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The prior ID/path validation fixes are present, but the current scratchlist attachment lifecycle still has routing, byte-accounting, and shared-reference deletion issues.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

Findings

• [Major] Canonicalize attachment filenames before persisting/serving them — resolveScratchlistAttachmentForSession verifies the id/path/size, but then stores claimed.filename; the attachment GET route later interpolates that value into Content-Disposition at hub/src/web/routes/sessions.ts:789. A direct REST caller can therefore persist CR/LF/quote/control characters and make future attachment fetches fail or produce malformed headers. Evidence hub/src/scratchlistAttachments/storage.ts:216.
  Suggested fix:

  function sanitizeFilename(filename: string): string {
      const sanitized = filename
          .replace(/[�-��"\/]/g, '_')
          .replace(/\.\./g, '_')
          .replace(/\s+/g, '_')
          .slice(0, 255)
      return sanitized || 'upload'
  }
  
  const storedFilename = sanitizeFilename(fileName.slice(`${claimed.id}-`.length))
  
  return {
      ok: true,
      attachment: {
          id: claimed.id,
          filename: storedFilename,
          mimeType: claimed.mimeType,
          size: info.size,
          path: toHubScratchlistAttachmentPath(storageKey),
      },
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• One attachment metadata canonicalization issue remains; the previous bot findings appear addressed in the current diff.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

Findings

• [Major] Serialize the scratchlist upload quota check — the upload path reads sessionBytes, validates against maxBytesPerSession, then awaits the file write. Two concurrent uploads for the same session can both observe the same pre-write total and both write, bypassing the configured session cap. Evidence hub/src/sync/syncEngine.ts:617.
  Suggested fix:

  private readonly scratchlistUploadLocks = new Map<string, Promise<void>>()
  
  private async withScratchlistUploadLock<T>(namespace: string, sessionId: string, fn: () => Promise<T>): Promise<T> {
      const key = `${namespace}\0${sessionId}`
      const previous = this.scratchlistUploadLocks.get(key) ?? Promise.resolve()
      let release!: () => void
      const current = new Promise<void>((resolve) => { release = resolve })
      const tail = previous.catch(() => undefined).then(() => current)
      this.scratchlistUploadLocks.set(key, tail)
      await previous.catch(() => undefined)
      try {
          return await fn()
      } finally {
          release()
          if (this.scratchlistUploadLocks.get(key) === tail) {
              this.scratchlistUploadLocks.delete(key)
          }
      }
  }
  
  return await this.withScratchlistUploadLock(namespace, sessionId, async () => {
      const sessionBytes = await sumScratchlistAttachmentBytesOnDisk(hapiHome, namespace, sessionId)
      const validation = validateScratchlistAttachmentsForWrite([provisional], limits, sessionBytes)
      if (!validation.ok) return { success: false, error: validation.error, code: validation.code }
      const attachment = await writeScratchlistAttachmentFile(hapiHome, namespace, sessionId, filename, mimeType, buffer)
      return { success: true, attachment }
  })

• [Major] Delete hub blobs after successful chat fallback staging — when a user uploads while scratchlist mode is on, then toggles it off before sending, this code stages the hub-resident blobs into normal CLI uploads but leaves the original hub scratchlist files unreferenced. Those files still count in sumScratchlistAttachmentBytesOnDisk, so repeated fallback sends can exhaust the scratchlist session cap with no UI path to clear them. Evidence web/src/components/SessionChat.tsx:545.
  Suggested fix:

  if (hubItems.length > 0) {
      const normalItems = list.filter((att) => !isHubScratchlistAttachmentPath(att.path))
      const staged = await stageScratchlistAttachmentsForComposeSend(props.api, props.session.id, hubItems)
      const accepted = await props.onSend(text, [...normalItems, ...staged], scheduledAt)
      if (accepted) {
          await Promise.allSettled(
              hubItems.map((att) => props.api.deleteScratchlistAttachment(props.session.id, att.id))
          )
      }
      return accepted
  }
  return props.onSend(text, attachments, scheduledAt)

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The previous filename canonicalization finding appears addressed, but the latest full diff still has a quota race and an orphaned-blob path around scratchlist attachment uploads.

Testing

• Not run (automation). Suggested coverage: concurrent scratchlist uploads at the session cap; scratchlist-mode upload toggled back to chat send deletes the original hub blob only after accepted send.

HAPI Bot

[github-actions]

Review mode: initial.

Two issues found: the scratchlist update schema rejects clearing the last attachment, even though the PUT handler is written to replace the attachment list; and the scratchlist-to-queue staging path does not roll back already-uploaded files if a later attachment fails, which can leave orphaned uploads.

Not run (review only).

[github-actions]

Findings

• [Major] Re-key scratchlist attachment files during session merges — attachment storage and validation are now scoped by namespace/sessionId, but existing merge paths only transfer session_scratchlist rows. After a dedup/resume merge, moved entries can still point at files under the old session directory, so sumScratchlistAttachmentBytesOnDisk(newSessionId) misses those bytes and future attachment PUT validation rejects the old paths as outside the new session. Evidence hub/src/scratchlistAttachments/storage.ts:196 with merge context hub/src/sync/sessionCache.ts:932.
  Suggested fix:

  const movedScratchlist = this.store.scratchlist.transfer(oldSessionId, newSessionId)
  if (movedScratchlist.moved > 0) {
      const { getHapiHomeDir, moveScratchlistAttachmentFilesForSession } = await import('../scratchlistAttachments/storage')
      for (const entry of this.store.scratchlist.list(newSessionId)) {
          const attachments = await moveScratchlistAttachmentFilesForSession(
              getHapiHomeDir(),
              namespace,
              oldSessionId,
              newSessionId,
              entry.attachments,
          )
          if (attachments.some((att, i) => att.path !== entry.attachments[i]?.path)) {
              this.store.scratchlist.update(newSessionId, entry.entryId, { attachments })
          }
      }
      this.emitScratchlistChanged(newSessionId)
  }

• [Minor] Re-check the final PUT payload after attachment clearing — the schema now allows any attachments-only patch, so PUT { "attachments": [] } against an attachment-only entry with empty text passes validation and stores an empty scratchlist row. That breaks the create/update invariant that an entry has text or attachments. Evidence hub/src/web/routes/sessions.ts:933 and shared/src/apiTypes.ts:296.
  Suggested fix:

  if (nextText.trim().length === 0 && nextAttachments.length === 0) {
      return c.json({
          error: 'Scratchlist entry requires text or attachments',
          code: 'scratchlist_entry_empty',
      }, 400)
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The prior quota-race and fallback cleanup issues appear addressed, but the full diff still has a merge-path attachment storage bug and a direct-API empty-entry validation gap.

Testing

• Not run (automation). Suggested coverage: merge/dedup scratchlist entries with attachments and verify paths/quota still use the new session id; PUT clearing the last attachment from an attachment-only entry returns 400.

HAPI Bot

[github-actions]

Findings

• [Major] Reuse restored scratchlist uploads instead of uploading again — composer drafts persist the already-uploaded attachment path for remount/reload recovery, but the new scratchlist adapter ignores restored upload metadata and always reads the file and calls the scratchlist upload endpoint again. The original hub blob is then orphaned and still counts toward the per-session disk cap; for a large attachment near the cap, the restored draft can fail to upload on reload even though the bytes are already present. Evidence web/src/lib/scratchlistAttachmentAdapter.ts:40, with draft persistence context web/src/components/AssistantChat/HappyComposer.tsx:309.
  Suggested fix:

  import { getRestoredUploadMetadata } from '@/lib/composer-attachment-drafts'
  import { parseHubScratchlistAttachmentPath } from '@hapi/protocol'
  
  const restored = getRestoredUploadMetadata(file)
  if (restored) {
      const key = parseHubScratchlistAttachmentPath(restored.path)
      const storedName = key?.split('/').pop()
      const hubId = storedName?.slice(0, 36)
      if (hubId) {
          const hubAttachment = {
              id: hubId,
              filename: file.name,
              mimeType: contentType,
              size: file.size,
              path: restored.path,
          }
          yield {
              id,
              type: 'file',
              name: file.name,
              contentType,
              file,
              status: { type: 'requires-action', reason: 'composer-send' },
              path: restored.path,
              hubAttachment,
              previewUrl: restored.previewUrl,
          } as PendingScratchlistAttachment
          return
      }
  }

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The prior bot findings appear addressed. The remaining issue is a reload/remount path that can strand scratchlist attachment bytes and block restored drafts under quota pressure.

Testing

• Not run: bun is not installed in this runner image (bun typecheck failed with command not found). Suggested coverage: restore a composer draft containing a hub scratchlist attachment and assert the adapter does not call uploadScratchlistAttachment, preserves metadata for send, and can delete the existing hub blob on removal.

HAPI Bot

[github-actions]

Findings

• None.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• No issues found in the current diff. Prior bot feedback about restored scratchlist uploads appears addressed in this head. Residual risk: attachment queue/promotion flows are covered by focused unit tests, but I did not run the full suite in this automation pass.

Testing

• Not run (automation).

HAPI Bot

[heavygee]

Proof: promote-to-queue (Send) on a scratchlist entry with a hub attachment disgorges the file into the chat/queue path.

921-scratchlist-attachment-disgorge.mp4: https://github.com/heavygee/hapi/releases/download/pr-attach-proof-1205/proof-2026-07-28T21-01-40-921-scratchlist-attachment-disgorge.mp4

_{asset: heavygee/hapi@pr-attach-proof-1205/proof-2026-07-28T21-01-40-921-scratchlist-attachment-disgorge.mp4 · delete release after merge if desired}