Preflight tool-call counting fix: amend late posts, per-process counted ids, longer TTLs

What they did

The author fixed issue #1040 in newrelic-experimental/preflight, where a tool call could be counted twice, under the wrong session, or zero times. They started from a TLA+ model, HookDrainPairing.tla, which found four interleavings that double-count or misattribute a call, and reproduced each on the real modules against upstream/main 6d173ace. The fix went through seven revisions, each reviewed, with regressions found and fixed: the first dropped late posts and turned successes into permanent timeouts; later rounds fixed an approved long Bash call counted as rejected, a post with no session id failing to pair, repeated or unmatched amends pushing the success rate above 1, and a restart re-counting a late post whose id had left memory or was older than the newest 256 on disk. The final design: a late post amends the record its pre emitted via onRecordAmend and amendRecord() in src/storage/types.ts, so outcome, duration and error type come from the post while intake fields like activeSkill survive; SessionTracker amends by record id through one applyOutcome() function covering success and error counts, toolErrorsByType, Bash exit codes and the timeline entry. TTLs grew: Bash pends 11 minutes, Agent and Task 30, plus the 5-minute prompt wait on top. shutdown() stops the event processor before the final persistSession() so in-flight calls save as timeouts. Counted ids reach disk only after their record is saved, via persistCountedIdsOfSavedSessions(), and each process writes counted/<sessionId>.<pid>.json with readers taking the union. A store swap keeps the adopted session's pres pending and emits every other session's pending pre as a timeout under its own session. A second TLA+ model, PendingEngineCounting.tla, verified the result.

How it went

The PR ships with a test plan where each new test failed on an earlier revision and passes on this branch: an Agent pre amended 31 minutes later reaching onWorkflowAgent as timeout then success, a store swap emitting another session's pre as one timeout while its own Bash call pairs as one 4000 ms success, a Bash call approved after a 4-minute prompt and run 8 minutes counted as success at 720000 ms, and a restarted processor dropping late posts of already-counted calls. PendingEngineCounting.tla holds every safety rule and every quiescence rule except the known #980 two-writer limit. The author kept the PR within a size budget by moving pending-engine changes into stacked #1048.

Worth knowing

Several gaps remain by design. A call in flight across a restart or another session's call at a store swap counts once as a timeout, not with its real outcome. A rejected Bash call is counted after 16 minutes, not 5, because the processor cannot distinguish an approved running call from a rejected one. A failed session write can still mark ids counted, so a crash after a full disk drops a late post and counts zero. A lost post keeps a long-running call pending for its whole TTL, so an API failure in that window reads duringToolExecution with no tool running. A late post with no toolUseId amends the oldest timed-out pre of its tool, which can land on the wrong call. New Relic AiToolCall events are not amended, and OTel spans, TaskDetector, LatencyTracker, RetryDetector, TurnTracker, cost attribution, alert windows and QualityProxyTracker keep the timeout record. A pending engine still counts drained calls as main does until #1048.

Wiring

Work happens in the preflight repo on the hook event pipeline. HookEventProcessor emits a pre record when a tool call starts and, on a late post for a pre already emitted as timeout or rejected, fires onRecordAmend with the earlier record id. amendRecord() lives in src/storage/types.ts next to ToolCallRecord and merges the post onto the stored record; LocalSessionAggregator and the MCP toolCallBuffer replace the record in place, and an amended Agent call also reaches WorkflowRunTracker and the subagent type fallback. SessionTracker is constructed with a cap (DEFAULT_MAX_PENDING, matching the event processor) and keeps amendable timed-out and rejected records, applying changes through applyOutcome(). TTLs are configured per tool: Bash 11 minutes, Agent and Task 30, plus a 5-minute prompt wait. shutdown() calls the event processor's stop before persistSession(). persistSession() calls persistCountedIdsOfSavedSessions(sessionIds) right after each session save, and each process writes counted/<sessionId>.<pid>.json with a union read on lookup and a daily GC. replaceStore() keeps the adopted session's pres pending and flushes other sessions' pres as timeouts through the shutdown-flush path. Verification runs the TLA+ models HookDrainPairing.tla and PendingEngineCounting.tla (pec-fix2-*) alongside the vitest suites event-processor.test.ts and local-session-aggregator.test.ts.

Try it yourself

In this repo, fix the tool-call counting bug where a call can be counted twice, under the wrong session, or zero times. Read the TLA+ models HookDrainPairing.tla and PendingEngineCounting.tla first and list the interleavings that break EachCallCountedOnce. Then: (1) add onRecordAmend(record) to HookEventProcessor and amendRecord() beside ToolCallRecord in src/storage/types.ts, merging outcome, duration and errorType from the post while keeping intake fields like activeSkill; (2) make SessionTracker amend by record id through a single applyOutcome() that updates success and error counts, toolErrorsByType, Bash exit codes and the timeline entry; (3) raise TTLs to 11 minutes for Bash and 30 for Agent/Task, with the 5-minute prompt wait on top; (4) stop the event processor before the final persistSession() in shutdown(); (5) write counted ids to counted/<sessionId>.<pid>.json only after the session save, with readers taking the union; (6) at replaceStore(), keep the adopted session's pres pending and emit every other session's pending pre as a timeout under its own session. Add tests that fail on the previous revision for each path, and run the TLA+ models to confirm the safety and quiescence rules.