Support local markdown images in chat

[sleepinginsummer]

Summary\n- render local markdown image paths through an authenticated session image fetch\n- add hub local-image endpoint backed by existing session file RPC\n- bound persisted generated-image cache across CLI restarts\n\n## Testing\n- bun run --cwd cli test src/modules/common/generatedImages.test.ts\n- bun run --cwd hub test src/web/routes/git.test.ts\n- bun typecheck\n- bun run build:web

[github-actions]

Findings

• [Major] Linux absolute image paths never use the authenticated image fetch. normalizeLocalImagePath only accepts file:///, macOS /Users//Volumes, and Windows drive paths, so markdown like ![shot](/tmp/project/out.png) or /home/user/project/out.png falls through to the raw <img src>. From the web app that URL points at the hub origin and 404s instead of calling /api/sessions/:id/local-image, even though the new hub test covers /tmp/project/out.png. Evidence: web/src/components/assistant-ui/markdown-text.tsx:655.

Summary
Review mode: initial
One Major regression found in the local markdown image rendering path. Residual risk: I did not run the full suite in this review pass.

Testing
Not run (automation)

[github-actions]

[Major] Linux absolute image paths never use the authenticated image fetch.

This regex excludes ordinary Linux paths such as /tmp/project/out.png and /home/user/project/out.png, so those markdown images fall through to the raw <img src> and resolve against the hub origin instead of getSessionLocalImageBlob. That breaks the feature on Linux, and it contradicts the new hub route test that uses /tmp/project/out.png.

Suggested fix:

const LOCAL_IMAGE_PATH_PATTERN = /^(?:file:\/\/\/|\/(?:Users|Volumes|home|tmp|var|opt|mnt|media|workspace|work)\/|[A-Za-z]:[\\/]).+\.(?:png|jpe?g|gif|webp|avif|bmp|svg)$/i

A more robust option is to classify any root-absolute POSIX path (/^\//) as local here and let the backend workspace/path validation reject anything outside the session.

[github-actions]

Findings

• [Major] Imported-source sync stops matching after the first fork - codexSourceSessionId is now preserved, but import target selection still only matches metadata.codexSessionId; after HAPI resumes the import and rewrites codexSessionId to the fork id, syncing the original Codex transcript again can create a duplicate HAPI session instead of updating the existing one, evidence hub/src/web/routes/codexDesktop.ts:841.
  Suggested fix:

  const relatedCandidates = candidates
      .filter((candidate) => {
          const metadata = candidate.metadata
          return metadata?.codexSessionId === codexSessionId
              || metadata?.codexSourceSessionId === codexSessionId
      })
      .sort((a, b) => b.updatedAt - a.updatedAt)

• [Major] Selected Codex import parses every transcript before filtering - the selected-ID path calls listLocalCodexSessionsWithMessages(Number.MAX_SAFE_INTEGER) and only then filters, so importing one session from a large Codex history can read/parse all JSONL files with full message bodies and hit the 120s RPC timeout, evidence cli/src/api/apiMachine.ts:228.
  Suggested fix:

  export function listLocalCodexSessionsWithMessagesByIds(ids: Set<string>): LocalCodexSessionWithMessages[] {
      return listLocalCodexSessionSummaries(Number.MAX_SAFE_INTEGER)
          .filter((session) => ids.has(session.id))
          .map((session) => parseCodexLocalSession(session.file, true))
          .filter((session): session is LocalCodexSessionWithMessages => Boolean(session))
  }
  
  const allSessions = requestedIds
      ? listLocalCodexSessionsWithMessagesByIds(requestedIds)
      : listLocalCodexSessionSummaries()

• [Minor] Malformed file URI can crash markdown rendering - decodeURI runs during render and throws on invalid percent-encoding, so a message containing something like file:///%E0%A4%A.png can break the chat view instead of falling back to the raw image src, evidence web/src/components/assistant-ui/markdown-text.tsx:659.
  Suggested fix:

  export function normalizeLocalImagePath(src: string | undefined): string | null {
      if (!src || !LOCAL_IMAGE_PATH_PATTERN.test(src)) return null
      if (!src.startsWith('file://')) return src
      try {
          return decodeURI(src.slice('file://'.length))
      } catch {
          return null
      }
  }

Summary

• Review mode: initial
• Found import matching, selected-import scalability, and malformed markdown-image URI issues in the new Codex import/local image paths.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

[MAJOR] This preserves the original Codex thread id, but the import target lookup below still filters only by metadata.codexSessionId === codexSessionId. After the first HAPI resume, CodexSession.onSessionFound rewrites codexSessionId to the fork id, so syncing/importing the original source transcript again will miss the existing HAPI session and create a duplicate. Match codexSourceSessionId as well when selecting the import target.

Suggested fix:

const relatedCandidates = candidates
    .filter((candidate) => {
        const metadata = candidate.metadata
        return metadata?.codexSessionId === codexSessionId
            || metadata?.codexSourceSessionId === codexSessionId
    })
    .sort((a, b) => b.updatedAt - a.updatedAt)

[github-actions]

[MAJOR] The selected-ID path reads and parses every Codex JSONL transcript with full message bodies before filtering by the requested ids. On machines with a large Codex history, importing one session can scan thousands of large files and hit the 120s RPC timeout; nonexistent ids also force the same worst-case work. Filter to matching summaries/files before parsing messages.

Suggested fix:

export function listLocalCodexSessionsWithMessagesByIds(ids: Set<string>): LocalCodexSessionWithMessages[] {
    return listLocalCodexSessionSummaries(Number.MAX_SAFE_INTEGER)
        .filter((session) => ids.has(session.id))
        .map((session) => parseCodexLocalSession(session.file, true))
        .filter((session): session is LocalCodexSessionWithMessages => Boolean(session))
}

const allSessions = requestedIds
    ? listLocalCodexSessionsWithMessagesByIds(requestedIds)
    : listLocalCodexSessionSummaries()

[github-actions]

[MINOR] decodeURI can throw for malformed percent-encoding, and this runs during React render. A chat message with an invalid file URL such as file:///%E0%A4%A.png can crash the markdown view instead of simply not using the local-image fetch path.

Suggested fix:

export function normalizeLocalImagePath(src: string | undefined): string | null {
    if (!src || !LOCAL_IMAGE_PATH_PATTERN.test(src)) return null
    if (!src.startsWith('file://')) return src
    try {
        return decodeURI(src.slice('file://'.length))
    } catch {
        return null
    }
}

[github-actions]

Findings

• No issues found.

Summary

• Review mode: follow-up after new commits
• Prior bot findings appear addressed: imported-source matching now checks codexSourceSessionId, selected imports use ID-filtered transcript loading, and malformed file URIs are guarded.
• Residual risk: large Codex histories and multi-runner routing remain integration-heavy; coverage exists for the fixed paths, but I did not run the full suite in this review pass.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

Findings

• [Major] Bound automatic local-image reads before crossing RPC — the new markdown image route fetches any absolute image-looking path through readSessionFile, but that handler reads the entire file and returns base64. Because Image now auto-fetches these paths during chat render, an agent can emit /workspace/huge.png and force the CLI, hub socket, and browser to move the full file without a size guard. Evidence: hub/src/web/routes/git.ts:181.
  Suggested fix:

  const MAX_LOCAL_IMAGE_BYTES = 25 * 1024 * 1024
  
  // Plumb maxBytes through SyncEngine/RpcGateway into the ReadFile RPC and
  // enforce it with stat(resolvedPath) before readFile(resolvedPath).
  const result = await runRpc(() => engine.readSessionFile(
      sessionResult.sessionId,
      parsed.data.path,
      { maxBytes: MAX_LOCAL_IMAGE_BYTES }
  ))

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Found one regression risk in the current full diff: local markdown image rendering needs a byte cap before reading/encoding files over RPC. The only commit since the last bot review changes theme cleanup; this finding is from the full current PR diff as requested.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

[Major] Bound automatic local-image reads before crossing RPC

This route is now reached automatically from markdown image rendering for any absolute path that looks like an image. It delegates to readSessionFile, whose CLI handler reads the whole file and base64-encodes it before replying, so a large .png in the workspace can make opening the chat push an unbounded payload through CLI memory, Socket.IO, hub memory, and the browser. Please add a local-image byte limit that is enforced on the CLI side before readFile.

Suggested fix:

const MAX_LOCAL_IMAGE_BYTES = 25 * 1024 * 1024

// Plumb maxBytes through SyncEngine/RpcGateway into the ReadFile RPC and
// enforce it with stat(resolvedPath) before readFile(resolvedPath).
const result = await runRpc(() => engine.readSessionFile(
    sessionResult.sessionId,
    parsed.data.path,
    { maxBytes: MAX_LOCAL_IMAGE_BYTES }
))

[github-actions]

Findings

• [Major] Preserve the selected runner when importing Codex history — /codex/sync-session reads transcripts from the machine resolved by listCodexSessionsViaMachine, but importSingleCodexSession discards that remote.machineId and re-infers metadata.machineId from transcript.cwd. For runners without workspace roots, ambiguous roots, or hub/CLI running on different hosts, resolveImportMachineId(...) returns undefined; the newly created Hapi session then has no machine id and hub-local host metadata, so the new-session flow immediately calls resumeSession and can fail with “No machine online” even though the same runner just served the transcript. Evidence hub/src/web/routes/codexDesktop.ts:1663, triggered by web/src/components/NewSession/index.tsx:692.
  Suggested fix:

  function importSingleCodexSession(options: {
      // ...
      machineId?: string | null
  }): ScriptLaunchResponse {
      // ...
      const resolvedMachineId = options.machineId
          ?? resolveImportMachineId(transcript.cwd, options.namespace, engine)
          ?? undefined
      const metadata = buildImportedSessionMetadata(
          transcript,
          asRecord(existingStored?.metadata),
          resolvedMachineId,
          options.yolo ? 'yolo' : undefined
      )
  }
  
  const result = await importSelectedCodexSessions({
      // ...
      localSessions: remote.sessions,
      machineId: remote.machineId,
      model: parsed.model,
      modelReasoningEffort: parsed.modelReasoningEffort,
      yolo: parsed.yolo
  })

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Found one remaining Major issue in the full current diff. The previous bot finding about bounding local-image reads appears addressed by the follow-up maxBytes plumbing.

Testing

• Not run (automation)

HAPI Bot

[github-actions]

[Major] Preserve the selected runner when importing Codex history

/codex/sync-session reads the transcript from the machine resolved by listCodexSessionsViaMachine, but this import path discards remote.machineId and re-infers the target machine from transcript.cwd. For runners without workspace roots, ambiguous roots, or hub/CLI running on different hosts, that inference returns undefined; the created Hapi session then has no metadata.machineId, so the new-session path immediately calling resumeSession can fail with “No machine online” even though the same runner just served the transcript.

Suggested fix:

function importSingleCodexSession(options: {
    // ...
    machineId?: string | null
}): ScriptLaunchResponse {
    // ...
    const resolvedMachineId = options.machineId
        ?? resolveImportMachineId(transcript.cwd, options.namespace, engine)
        ?? undefined
    const metadata = buildImportedSessionMetadata(
        transcript,
        asRecord(existingStored?.metadata),
        resolvedMachineId,
        options.yolo ? 'yolo' : undefined
    )
}

const result = await importSelectedCodexSessions({
    // ...
    localSessions: remote.sessions,
    machineId: remote.machineId,
    model: parsed.model,
    modelReasoningEffort: parsed.modelReasoningEffort,
    yolo: parsed.yolo
})