Support task finish images in Lark cards

[danielwpz]

Summary:

• Fix task-thread image attachment delivery to reply to the task status card message.
• Add optional finish_task images and carry them through task settlement events.
• Upload finish images for Lark task cards and render them as card image elements with cached image keys.

Tests:

• pnpm preflight

[claude]

PR Review: Support task finish images in Lark cards

This is a well-structured PR that adds optional image attachments to finish_task and renders them as images in Lark task cards. The end-to-end flow — tool schema → validation → signal extraction → lifecycle → outbound events → card rendering — is wired up correctly and consistently. Tests cover the main happy paths. I found one meaningful issue and two minor observations.

1. Image key caching never actually works

The resolveTaskCardImages function is designed to cache uploaded image keys in binding metadata so that subsequent card renders can reuse them without re-uploading. However, the cache is never read back in practice.

Root cause: buildBindingMetadata only preserves sequenceFloor from existing metadata — it drops all other fields including taskResultImages. When resolveTaskCardImages receives the result of buildBindingMetadata, the cache is always empty.

The caller in deliverTaskCard (outbound.ts ~1034):

let metadataJson = buildBindingMetadata({...base...}, existing?.metadataJson ?? null);
// ^^^ drops taskResultImages from old metadata

const taskCardImages = await resolveTaskCardImages({
  ...state, metadataJson,  // <-- metadataJson has NO taskResultImages
});

Inside resolveTaskCardImages:

const metadata = parseBindingMetadata(input.metadataJson);
const cachedImages = readCachedTaskResultImages(metadata.taskResultImages);
// ^^^ always [] because buildBindingMetadata dropped the cache

The image keys ARE persisted into the database (via ensureInteractiveCardReferenceBinding / updateDeliveryState), but on the next card update they are discarded by buildBindingMetadata and re-uploaded.

Impact: In practice, task cards are rarely re-rendered for a terminal state (the structure signature guards against duplicate renders), so the practical impact is low. However, any reconciliation or forced re-render will re-upload images unnecessarily — wasting the caching code entirely.

Fix direction: The simplest fix is to pass existing?.metadataJson (rather than the fresh buildBindingMetadata result) as the metadataJson input to resolveTaskCardImages, so it can read cached keys. Then merge the returned image cache into the base metadata. Alternatively, have resolveTaskCardImages accept the existing binding/metadata directly for cache reading, separate from the metadata it returns.

2. Promise.all on permission-requiring operations may only surface one rejection

In finish-task.ts, the image validation loop uses Promise.all:

const images = await Promise.all(
  (args.images ?? []).map(async (image) => {
    const absolutePath = requireFilesystemAccess(context, {...});
    ...
  }),
);

Each requireFilesystemAccess call creates a FilesystemAccessController which may throw toolRecoverableError if approval is needed. Under Promise.all, the first rejection causes the whole batch to fail, hiding subsequent permission denials from the user. If a task has 3 images and none are pre-approved, the user would need 3 separate calls to resolve them one at a time. This matches existing patterns in the codebase (other multi-file tools have the same behavior), so it is not a new problem — but worth noting.

3. Duplicate type definitions

TaskCompletionImageAttachment (in task-completion.ts) and TaskRunResultImageAttachment (in outbound-events.ts) are structurally identical:

{ path: string; displayPath: string; alt?: string; }

Defining them separately risks divergence if one is updated without the other. Consider exporting a single shared type (e.g., from task-completion.ts) and re-using it in outbound-events.ts.

---

Verdict: Approve with the caching bug noted. The caching issue is low-severity in practice but represents code that looks correct yet silently does not work as intended — it should be fixed in a follow-up to avoid confusing future maintainers. The rest of the PR is clean, well-tested, and follows project conventions.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3dfa180c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[danielwpz]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3dfa180c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3dfa180c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[danielwpz]

Addressed the review feedback before merge: ebd2aca fixes task image cache reuse, batched finish-image permission reporting, and duplicate image attachment typing; 515d5bd retries task completion cards after transient finish-image upload failures and logs the task-status-card race fallback per current scope. All review threads are resolved, local preflight passed, and CI is green.