Skip to content

feat(runtime): recall the files a message carried - #5409

Open
Joob1n wants to merge 1 commit into
apache:mainfrom
Joob1n:feat/recall-materials
Open

Joob1n wants to merge 1 commit into
apache:mainfrom
Joob1n:feat/recall-materials

Conversation

@Joob1n

@Joob1n Joob1n commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Follows #5368. Third of the three layers in #5343: attachments become reachable.

The gap

A user message of 你看看 plus a screenshot carried no searchable text at all. The message was unreachable by any term a person would think of, and the file was invisible to the model even while its own transcript was being read back.

Read already accepts maka://runtime/attachments/<artifactId>, so nothing new is needed to open a file — what was missing was a way to learn that one exists.

What this adds

Recall matches a message on the names of the files it carried, and returns those files beside the passage as metadata:

{
  "message_id": "", "role": "user", "text": "你看看",
  "materials": [{
    "name": "pipeline-failure.png", "kind": "image",
    "mime_type": "image/png", "bytes": 2048,
    "resource": "maka://runtime/attachments/art_01HQ…"
  }]
}

Names are matched but not folded into the returned text — the user never typed them.

The Session boundary

readAttachmentResource refuses an artifact whose record.sessionId differs from the calling Session. That is a deliberate boundary, and recall returns passages from other Sessions, so resource is present exactly when the material is reachable from the Session asking. Elsewhere the file is named without an address, which is the honest answer rather than an invitation to a call that can only fail. Widening that boundary would be its own change, with its own review.

Details worth naming

  • A file name is user-authored text, so it leaves through the same redaction the passage body does, and a credential-shaped name can no more be matched than returned.
  • An attachment that does not match the current shape is skipped rather than projected as a material named undefined; the per-message count is bounded by the platform's own MAX_ATTACHMENT_COUNT rather than a second constant invented here.
  • A message whose whole content was a pasted file has no text of its own and used to be dropped from every passage — which would have made the file unreachable in exactly the case this layer exists for. It now projects with empty text and its materials.
  • The candidate scan is unchanged. A file name is a JSON string value of the event that carried it, so a payload scan still offers every Session that could match; the end-to-end test proves this through the real ledger rather than a double.
  • No RecallMaterial tool. The plan in Recall: a knowledge-retrieval layer over Session history #5343 sketched one, but Read already resolves the address, so a second tool would only restate it. The envelope also carries materials per message rather than as one top-level list: the passage already says which message carried what, and a separate list would duplicate it.

Verification

Core 46/46, storage and the ledger corpus suite green. Each new behaviour was checked in reverse — reverting it alone makes exactly its own test fail (5 ablations: matching on names, keeping a text-less message, withholding the cross-Session address, skipping a malformed attachment, redacting a name).

Gates pass, no epoch bump. runtime and runtime-host carry the same failures as the merge base; two process-lifecycle tests flake only under parallel load and pass when their file runs alone.

https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J

A screenshot pasted under "have a look" left no trace in the transcript
text, so the message was unreachable by any term a person would think to
search. Recall now matches a message on the names of the files it
carried and returns those files as metadata beside the passage.

Materials are metadata only — name, kind, media type, size. A material
carries the address `Read` accepts exactly when it is reachable from the
Session asking: an attachment read resolves against the calling Session
and refuses one stored elsewhere, so offering the address across a
Session boundary would invite a call that can only fail. Elsewhere the
file is named but has no address, which is the honest answer.

A file name is user-authored text, so it leaves through the same
redaction the passage body does, and a credential-shaped name can no
more be matched than it can be returned. An attachment that does not
match the current shape is skipped rather than projected as a material
named `undefined`.

A message whose whole content was a pasted file has no text of its own
and used to be dropped from every passage, which would have made the
file unreachable in exactly the case this layer exists for; such a
message now projects with empty text and its materials.

No new tool: `Read` already accepts `maka://runtime/attachments/<id>`.
The candidate scan needs no change either — a file name is a JSON string
value of the event that carried it, so a payload scan still offers every
Session that could match.

Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 16, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff plus the surrounding authority (attachments.ts, events.ts, artifact-attachments.ts, artifact-coordinator.ts, conversation-copy.ts). The Session-boundary reasoning is the part I most wanted spelled out, and it is — the findings below are about edges of that same invariant.

What I verified

  • No path leaves the process. recallMaterials surfaces only name/kind/mimeType/bytes plus the address formatAttachmentResourceRef builds (packages/core/src/attachments.ts:34), which requires ref.kind === 'session_file' and isCanonicalArtifactEntityId(ref.relativePath) (^[A-Za-z0-9_-]{1,128}$). So external_file.absolutePath / workspace_file.relativePath are never projected (the notes.md test covers it), and a crafted relativePath such as ../../etc/passwd cannot form an address at all.
  • reachable matches the real boundary. readAttachmentResource re-resolves against the caller (packages/storage/src/artifact-attachments.ts:63-66), and every writer I found either sets ref.sessionId to the owning Session (packages/runtime-host/src/server/artifact-coordinator.ts:622) or rewrites it on copy (packages/runtime/src/conversation-copy.ts:2068-2105, packages/runtime-host/src/server/workhub-message-attachments.ts:56-60). So passage.sessionId === activeSessionId is the right proxy, and the only production caller does pass activeSessionId (packages/runtime/src/recall-tools.ts:118), so resource is never silently always absent.
  • Redaction covers names in both branches (recall.ts:1278-1282) and matching still requires the term to survive redaction (recall.ts:948verify), so the "cannot probe for it" assertion exercises the real boundary rather than a coincidence.
  • Budget: materials ride inside overhead (recall.ts:1141), so a file-carrying message cannot overrun RECALL_PASSAGE_MAX_BYTES / RECALL_TOTAL_PAYLOAD_CAP_BYTES; the list itself is capped at MAX_ATTACHMENT_COUNT.
  • isPassageMessage (recall.ts:1258) and projectPassageMessage (recall.ts:1280) decide the same set — the "redaction never empties text" assumption holds for every pattern in packages/core/src/redaction.ts:58-64.

1. resource is offered for a file Read refuses (medium)

formatAttachmentResourceRef judges the ref; readAttachmentResource judges the record and refuses two cases: record.source !== 'user_upload' (artifact-attachments.ts:58-61) and record.kind === 'pdf'throw new Error('PDF attachments cannot be decoded by Read') (artifact-attachments.ts:71-73).

PDF is the normal path, not an edge case: attachmentKindFromMimeType maps application/pdf'pdf' (packages/core/src/attachments.ts:267) and the upload commit stores it as a 'pdf' artifact (artifact-coordinator.ts:606-610). So a PDF attached in the asking Session still yields resource: maka://runtime/attachments/…, while the new description sentence tells the model it "can be opened with Read" (recall-tools.ts:61-62) — exactly the call that can only fail, which is what the cross-Session half of this PR was careful to avoid. doc/other binaries fail more quietly: they fall through to readTextInSession, which is bytes.toString('utf8') (packages/storage/src/artifact-store.ts:765-771), so the model gets mojibake rather than an error.

Either withhold resource for material Read cannot decode (kind === 'pdf', or sniffably-binary mimes) or narrow the description to what Read actually returns (images, text-ish files). A test mirroring the cross-Session one — a pdf material inside the asking Session carries no resource — would pin the invariant the doc comment claims.

2. The private isAttachmentRef restates an existing predicate, and is looser (medium)

recall.ts:243-256 hand-writes a guard and asserts value is AttachmentRef, while packages/core/src/events.ts:359-376 already exports isAttachmentRef (kind union, Number.isSafeInteger(bytes) && bytes >= 0, full isStorageRef) and events.ts:378-386 isCanonicalAttachmentRef. events.ts:170-177 records the repo's own rule against restating such a predicate per layer (#4804).

It is not only duplication — the looser version changes behaviour on precisely the input the guard exists for (a record not of the current shape):

  • kind is only checked to be a string, so kind: 'video' projects a material whose kind is outside AttachmentRef['kind'], and the assertion keeps TS from noticing. It reaches the model verbatim (recall-tools.ts:243-248).
  • bytes: NaN | -1 | 1.5 passes typeof === 'number'; JSON.stringify turns that into "bytes": null in the tool result — a field declared number arriving as null.
  • ref is only checked to be a non-null object, so a ref with no kind is accepted. That happens to be safe only because formatAttachmentResourceRef then returns null.

Importing isAttachmentRef from ./events.js (already imported for the type) fixes all three. isCanonicalAttachmentRef would go further and also require canonical locators — reasonable, but a judgement call, since it would drop older non-canonical refs that today still project.

Nits

  • recall.ts:229attachments.slice(0, MAX_ATTACHMENT_COUNT) runs before the shape check, so eight malformed entries ahead of a valid one drop that valid one from both matching and projection. Filter first, then slice, so the cap means "at most 8 materials" rather than "at most 8 candidates". (The malformed test puts the malformed entries first, so it passes either way.)
  • recall.ts:224 — only UserMessage carries attachments (session.ts:778, events.ts:158); narrowing on message.type === 'user' would drop the as { attachments?: unknown } cast and let TS type the field.
  • Coverage: the first test asserts only the anchor's materials, so materials on a neighbour message (a file-carrying message rendered beside a text-matched anchor) are unasserted; and the text-less-message test runs scanDeps only, unlike its neighbour that loops both scan paths. The cross-Session and redaction behaviour is well covered, including end-to-end through the real ledger.
  • bytes is whatever the send recorded and is never re-checked against the artifact store; recall runs with includeArchived: true, so a material can address an artifact Session cleanup already removed. Fine as metadata — noting only that the address can be stale, and the model will see Attachment was not found in this Session with no way to distinguish that from "wrong Session".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants