Skip to content

cost-tracker.sh: no dedup key on the metrics append (same bug shape as Vercel Workflow SDK #2376) #117

Closed

Description

@wasikarn

Context

Found during a 5-agent drill-down comparing the Vercel Workflow SDK launch
article against this repo (2026-08-31 session). One angle checked whether any
of matt-harness's own retry-prone surfaces share the exact bug shape publicly
filed against Vercel's SDK (vercel/workflow#2376):
a completion lease that dies the moment an operation finishes, so a late
retry re-does the work from scratch instead of recognizing it already ran.

The gap

hooks/stop/cost-tracker.sh is stateless by design — it re-derives cumulative
token totals from the full transcript on every Stop event (see the file's
own header comment). The append at line 158:

[[ -n "$rows" && ! -L "$metrics_file" ]] && printf '%s\n' "$rows" >> "$metrics_file"

has no completion marker or dedup key. If the same Stop event ever fires
twice for one turn, this appends the same row twice, silently inflating the
cumulative totals mh:cost-report aggregates.

hooks/session/instructions-loaded-journal.sh has the identical shape
(unbounded append, no dedup key) — already a known, accepted tradeoff per its
own line-9 comment, not a new finding.

Why this is parked, not ready-for-agent

Severity is low (a log double-write, not a duplicated real-world side effect)
and there's no evidence Stop actually double-fires in this harness today —
building a fix for a theoretical gap would be exactly the kind of speculative
work this repo's own doctrine argues against. Revisit if/when there's an
actual observed instance of Stop firing twice for one turn (e.g. a spend
report with implausibly doubled totals for a session).

If it ever needs fixing

Minimal shape: key each row by (session_id, transcript_path, byte-offset-at-write-time)
and skip the append if that key was already written this run.

Activity

  1. added
    bugSomething isn't working
    parkedWorth revisiting later, distinct from wontfix (never)
    on Aug 31, 2026
  2. wasikarn commented on Aug 31, 2026

    @wasikarn
    OwnerAuthor

    Scope correction (same-day deep-audit, independently re-verified): the original body said cost-tracker.sh and instructions-loaded-journal.sh were the only two files with this shape. That was under-searched. A repo-wide grep for the same pattern (stateless re-derive + >> append + no dedup key) turns up at least 3 more, two of which explicitly self-document the precedent:

    • hooks/session/skill-usage-telemetry.sh — own comment: "ponytail: unbounded append, same precedent as instructions-loaded-journal.sh and hooks/stop/cost-tracker.sh"
    • hooks/session/precompact-state-flush.sh — same shape, plain >> append with no dedup key
    • hooks/stop/nudge-compliance-tracker.sh — closest analog: same stateless-rederive-every-Stop pattern as cost-tracker.sh, explicit "same guard as cost-tracker.sh" comment for its symlink check, but no equivalent comment acknowledging the dedup gap itself

    Lower-severity, structurally different (bounded via trim-after-append, but unbounded file count per session/tool combo): hooks/advisory/loop-repeat-nudge.sh, hooks/advisory/mcp-failure-nudge.sh.

    Still parked, not ready-for-agent — same reasoning as the original body: no evidence any of these hooks' triggering event actually double-fires in practice. If this ever gets picked up, the minimal fix (dedup key = (session_id, transcript_path, byte-offset-at-write-time), skip re-append if already written this run) should be applied to whichever of these 5 files turns out to actually need it, not applied speculatively to all of them.

  3. wasikarn commented on Sep 21, 2026

    @wasikarn
    OwnerAuthor

    Fixed in 145b808 + a4380e3: hooks/stop/cost-tracker.sh now keys a per-session completion marker on (transcript_path, transcript byte size) before re-deriving/appending. An unchanged transcript size for the same session_id means this Stop event was already processed, so a repeat is skipped instead of double-writing the row. Marker is only set when the state was actually a no-op or the row was actually persisted -- a code-review pass caught an inverted version of the same bug (marker set even when the pre-existing symlink guard blocked a real append), fixed in the follow-up commit with its own regression test. 31/31 tests pass in tests/hooks/test-session-stop.sh.

  4. wasikarn commented on Sep 21, 2026

    @wasikarn
    OwnerAuthor

    Correction to the harm statement, from a fresh-context deep-audit of the fix (2026-09-21):

    A duplicate identical row does not inflate the totals mh:cost-report prints. The report keeps the latest row per session (skills/meta/cost-report/references/data-model.md: rows are cumulative totals, latest-row aggregation), so two identical rows collapse to one. Only raw-row consumers see the duplicate, e.g. the CSV mode of cost-report-dedup.js, which prints the last 100 rows as-is.

    The completion marker is still worth having for the raw path. Two follow-ups landed in the same audit: the marker write now carries the same symlink guard as the costs.jsonl append, and the comment states the marker covers sequential duplicates only (the Stop hook is async and the check is check-then-write, no lock). Ships with v1.1.109.

  5. added a commit that references this issue on Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingparkedWorth revisiting later, distinct from wontfix (never)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions