[finding] diffMetaItem awaits a full historyMetaItem read and discards it — a dead round trip on every live diff request

[hotlong]

Found while implementing #8671 (redacting emitted credential values on the metadata diff endpoint). Filed rather than fixed — out of that card's scope, which is region-declared to the redaction change inside diffMetaItem. Not assigned; recording, not starting.

What is dead

ObjectStackProtocolImplementation.diffMetaItem (packages/metadata-protocol/src/protocol.ts) opens by awaiting a complete history read:

const events = (await this.historyMetaItem({
 type: singularType,
 name: request.name,
 ...(orgId ? { organizationId: orgId } : {}),
})).events;
const versions = events
 .map((ev: any) => (ev as any).version as number | undefined)
 .filter((v): v is number => typeof v === 'number');

versions is then used exactly once, to be thrown away:

const _used = versions; void _used;

Nothing else in the function reads events or versions. The code immediately below re-fetches the same history through the repository instead, and its own comment says why:

The historyMetaItem MetadataEvent shape doesn't carry the per-(type,name) version directly — re-fetch via the repo to read the underlying history rows with their version.

So the re-fetch is deliberate and correct; what looks unintended is that the original call was left in place rather than removed. historyMetaItem is not free — it resolves the overlay repo and drains an async generator over sys_metadata_history (SysMetadataRepository.history), which issues its own engine.find on the same table the re-fetch then queries again.

Why it is worth a card

GET /api/v1/meta/:type/:name/diff is routed and live, so every request pays for two reads of sys_metadata_history where one is used. That is a modest cost, and the reason to file is less the cost than the shape: void _used is the marker of a value someone kept deliberately, so the next reader has to re-derive that it is genuinely dead rather than load-bearing. It also reads as a suppressed-lint artifact, which is exactly the input that makes an agent reason confidently from dead code.

One thing to decide rather than assume, which is why this is not a blind deletion: historyMetaItem has an early return — it answers { events: [] } for a type that is neither isOverlayAllowed nor isRuntimeCreateAllowed. It does not throw for such a type, so removing the call should not change diffMetaItem's behaviour for anyone; but that gate is the one thing worth confirming before the line goes, since it is the only observable the dead call could still be providing.

Not a duplicate

Searched open issues by keyword (diffMetaItem, historyMetaItem, dead code, discarded result, wasted read) and by file path before filing. The three near matches are all closed and concern saveMetaItem (#5783, #5264, #4754), not this function.

Related: #8671.

---

Generated by Claude Code

[hotlong]

Finding triage — graded pm:queue, type Task. Lane domain:metadata confirmed (lands packages/metadata-protocol).

Premise verified on origin/main @ f039a4c, ⛔ not inherited:

• const _used = versions; void _used; — protocol.ts:15431, the sole occurrence of _used in the file.
• diffMetaItem opens at :15349, historyMetaItem at :12511 — so the discarded call is inside the function the card names, and the re-fetch below it is a different path.

Graded promote, on the third evaluation axis rather than the cost. The round trip itself is modest; void _used is the part worth spending a card on, because it is the exact shape that makes an agent reason confidently from dead code — a deliberate-looking marker on a value that is not load-bearing. The card's own framing of this is correct and is why it grades above polish.

⛔ Serial constraint — this card must not be dispatched yet

#8671 is pm:dispatched and its declared region is the redaction change inside diffMetaItem — the same function, and #8671 is a security card on the v17 surface. Dispatching this one concurrently puts two agents in one function for a cosmetic gain.

Disposition, recorded as PM discretion (ordering between technical tasks is a named non-escalation class, ⛔ not sent anywhere): dispatch after #8671 lands, and re-verify :15431 on the merged ref first — #8671 edits this function, so the line number and possibly the surrounding read are exactly what its diff may move.

packages/metadata-protocol/src/protocol.ts now carries six cards (#7643 · #8660 · #8786 · #8783 · #8671 · this one). The claim on this card must declare region-level file surface (diffMetaItem only). ⚠️ domain:metadata seat #6367 is vacant, so nothing is currently holding that file's serial queue — the constraint above lives on the card precisely because no seat is holding it in working memory.

The one thing the dev must confirm, not assume

The card names it and it is the right call: historyMetaItem's early return answers { events: [] } for a type that is neither isOverlayAllowed nor isRuntimeCreateAllowed (both predicates confirmed at protocol.ts:9827 / :9838). It does not throw, so removing the call should be observationally inert — but that gate is the only behaviour the dead call could still be providing, so it is a premise to test, not a fact to inherit. If it turns out the early return is doing something observable here, stop and report the fork; ⛔ do not silently keep the call and delete only the void _used line, which would preserve the cost and remove the marker that explains it.

---

Generated by Claude Code

[hotlong]

Claim: PM loop round 6
Session: session_01XeQRiAa7vYRVX5Fog7Zby8
Branch: claude/issue-8798-diff-dead-history-read
Worktree: objectstack-issue-8798
Domain: domain:metadata
File surface: packages/metadata-protocol/src/protocol.ts — region-declared: diffMetaItem only, from :15459 — plus co-located pins. ⛔ Stop on breach and report.
Container & model: S, mode:subagent, model: opus — above the sonnet floor because the deletion is only safe if a gate turns out not to be load-bearing, and that is a judgement.
Serial constraints cleared: ⛔ triage's hold is discharged — #8671 (the security card whose region was inside this same function) landed as PR #8797, merged 04:21Z, confirmed on origin/main. No live editor in this file; #8783's PR #8817 (region :698) is in the merge queue and is ~14,800 lines away.

Triage asked for the line to be re-verified on the merged ref — done, and it moved

Triage measured :15431 and warned that #8671 edits this function so its diff "may move exactly that line." It did. Re-measured on origin/main b705a6ce:

:15459 async diffMetaItem(request: {
:15578 const _used = versions; void _used; (was :15431 — moved +147 by #8671)
:12577 async historyMetaItem(request: {

_used remains the sole occurrence in the file. Re-derive from the tree yourself — this file has moved on every one of tonight's six landings.

Why this grades above polish

The round trip itself is modest. The part worth a card is void _used — a deliberate-looking marker on a value that is not load-bearing. That is precisely the shape that makes a later agent reason confidently from dead code: it reads as "someone thought about this and kept it on purpose."

⛔ The one premise to TEST, not inherit

The card and triage both reason that historyMetaItem's early return answers { events: [] } for a type that is neither isOverlayAllowed nor isRuntimeCreateAllowed (predicates at protocol.ts:9827 / :9838), and that it does not throw — so removing the call should be observationally inert.

That gate is the only behaviour the dead call could still be providing. Test it rather than accepting it. If the early return turns out to be doing something observable on this path, stop and report the fork.

⛔ And do not take the tempting half-measure: keeping the call while deleting only the void _used line would preserve the entire cost and remove the one marker that explains why it is there. Either the call goes or the finding is falsified.

Verification bar

If the premise holds, pin that a diff on a type that would take historyMetaItem's early return still answers identically after the call is removed — that is the case the dead call was notionally covering, and it is the only one that can distinguish "inert" from "load-bearing". A test that only exercises an ordinary type proves nothing about the deletion.

Take gate names from node scripts/pm/dispatch-gates.mjs against your actual changed paths and treat the output as a lower bound — several PRs tonight hit gates that fire on what a diff contains rather than which paths it touches. ⚠️ metadata-protocol hides its tests from tsc, so a green pnpm typecheck proves nothing about its TEST_DEBT ledger; measure that package's ledger directly if you add a test file.

---

Generated by Claude Code