fix: reconcile production fixes onto the reorganized main - #166
Draft
tnunamak wants to merge 129 commits into
Draft
fix: reconcile production fixes onto the reorganized main#166tnunamak wants to merge 129 commits into
tnunamak wants to merge 129 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tnunamak
changed the title
…d archive Slack emitted a DETAIL_COVERAGE record for (state_stream=messages, stream=messages) inside runRequestedStreams, which the message-family fold calls once per scoped archive. The runtime rejects a repeated (state_stream, stream) pair, so any workspace with more than one scoped archive failed the run outright. Production has three, and the connection has eight failed runs and no successes at all. Reproduced on live data while diagnosing: a manually triggered run collected 2,030 records and then died on exactly this error. mergeScopedMessageArchivePasses already sums `considered` across the base archive and every scoped archive, so the correct denominator was being computed and then discarded. The emission is now hoisted out of the loop into declareMergedMessageCoverage, called once per run with that merged total; reactions and message_attachments ride the same call, since they declared the same parent boundary and had the identical defect. Single-archive runs emit exactly one record with the value they emitted before. Only the multi-archive case changes. The accompanying test builds two disjoint single-channel scoped archives and fails against unmodified source with the duplicate error. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 38d7f179c690a0c230044c7ee75028fcd1517b3d)
An unexpected store -- one the collector reported that the server's
descriptor table no longer declares -- was fatal to a coverage snapshot.
That is the normal result of a device running a build older than the
server, and it made a single stale store name discard an otherwise
complete proof.
Measured on production: connection cin_ece4bfe5096b8bf67a1468c2
("peregrine Codex") has 1,293,596 collected records, summary evidence
fresh with every component current, a current heartbeat and a fully
drained outbox, and displayed "Not measured". The only drift was one
legacy `logs` store. Nothing was missing; every declared store was
reported.
The asymmetry is the change. A collector reporting a store this build no
longer declares scanned MORE than was asked of it, which cannot weaken a
completeness claim -- and unexpected entries were already excluded from
the accounted rows, so they could never corrupt the proof either. Their
only effect was to fail the gate. A MISSING store is the opposite: the
collector did not account for something the server requires, so the
snapshot genuinely is not committed. Missing, duplicate and malformed
remain fatal.
unexpectedStores stays in the result, so the drift remains observable; it
is now informational rather than disqualifying.
Prior art agrees on the shape: restic exits 3 for "some source files
could not be read" while still creating a usable snapshot, borg separates
warning from error, and rclone check reports missing-on-dst and differ as
distinct named categories. None discards a result because it is imperfect
in one direction.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 292e02b9a0d530f08051ff5da6156253289bfa8b)
… out A bare "Timed out waiting for PostgreSQL bootstrap serialization lock" tells an operator nothing actionable. The server exits, the supervisor restarts it, the next attempt times out on the same unnamed holder, and the loop continues indefinitely. Observed cost today: a wedged DELETE FROM records, orphaned from a killed process, held a conflicting lock on connector_instances. Every boot queued behind it and the reference listener never bound its port, so the app served errors while the database was healthy and fully intact. Rolling the image back did not help -- and could not, because the blocker lived in Postgres rather than in the container, and survived every container restart. Diagnosing it by hand cost roughly 20 minutes of downtime. Postgres could have answered in one query. The timeout error now appends the holding sessions: pid, state, wait event, and how long the statement has run. Best-effort and deliberately non-fatal -- this runs on a path that is already failing, so a diagnostic that throws would replace a useful error with a worse one. It reports no query text, which can carry record values, and caps at five rows. Both branches exercised against live Postgres: with no advisory holder it reports that the contention is likely a table-level lock elsewhere; with a holder it renders "Held by: pid 2186703 (active, waiting on Timeout, 4s)". Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit f2409e2ee9a17d3414376dbdc91b18deb77c5e2e)
… read cursor Three sources whose records arrived outside a collection run each sat at checkpoint 0, and sinceSeq is the minimum across participants, so the fold restarted at the beginning of a 1,438,556-event log every pass. With a 2s budget it read zero qualifying events, wrote nothing, reported incomplete, and repeated -- leaving all 25 rows terminal_facts_historical indefinitely. The oldest checkpoint among the 22 sources that had collected was 1,350,342, so the real work was ~88k events. A participant with no checkpoint has no position to resume from and must not set the floor; when every participant lacks one the floor stays 0 so a fresh install is unaffected. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 0f5411a877896408a9a282c26f5bdae89cc23d47)
Steam: GetRecentlyPlayedGames omits `games` entirely when the account has
played nothing in the trailing two-week window; the documented shape is
{"response":{"total_count":0}}. Requiring an array failed the whole run
for an account that simply had not played recently. Absent now reads as
empty; a present non-array is still a protocol violation and still throws.
Gmail: attachment ids were pushed into DETAIL_COVERAGE.required_keys
unconditionally, so the same attachment observed twice in a run -- a
message re-observed across pages, or a retry re-walking a partially
hydrated thread -- produced a duplicate key and the runtime rejected the
whole coverage record. Record each key once; the terminal-outcome buckets
are already keyed by the same id and stay consistent.
Both surfaced on live runs 2026-08-17 while bringing sources back to
healthy.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 3ccca80005ecb74d61d4a92ab451763365b7282e)
…n data The deployed image had been iterated live to fold version 5, but every committed branch is still at 4. Building from fd8f617b1 therefore shipped a binary OLDER than the data it reads: 26 of 28 evidence rows carry stream_facts_fold_version=5, and the version guard correctly failed closed with fold_logic_version_incompatible_future rather than corrupting them. The visible result was ProjectionReliable=false on nearly every source and a fleet of grey pills, which reads identically to having no evidence at all. This restores the v5 read-model and evidence-engine captured from the running image (rescue/deployed-uncommitted-0816), including applyRecoveryGapClosureFacts, the round-robin drain, the component_stale repair reason, and publishConnectorListSummaryTerminalProjection -- the list-summary publisher whose absence from origin/main turned every source grey on an earlier deploy today. The checkpoint-less-participant fold fix is re-applied on top. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit b74242141b34af85b28b5946c5a28940b7be189c)
…sidecar Adds the Signal connector (message export via the sigtop CLI, ISC) using the same arms-length-subprocess pattern as slack/slackdump. Connector code and manifest come from the worktree-pdpp-signal-connector branch, which was independently reviewed this session; registration in orchestrator.ts is included, without which the connector is invisible to the runtime. sigtop publishes only a Windows binary on its releases, so Linux is built from the pinned v0.24.0 source tag in a throwaway Go stage. Only the resulting binary, its ISC license, and a commit-exact SOURCE_URL are copied into the final image; Go itself is not. Without this the connector would fail at runtime with a missing binary on every self-hosted deploy, which is what shipping it on origin/main alone would have done. Rebased onto the v5 fold lineage deliberately: the connector branch was cut from origin/main at fold logic version 4, and deploying that against version-5 evidence rows fails closed on every source. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit b7e40e33e96ac0f37af3e9b1d9ba8a72f9056dff)
…y pass rowNeedsFoldParticipation returned true for any row whose terminal_facts_state was not 'current'. A row refused as terminal_facts_historical has no attributable event at its own generation, so re-running the fold changes nothing until a new fact-carrying event lands -- which the checkpoint-lag predicate below already detects. Under the old predicate such a row rejoined every pass forever, converging to the identical verdict each time while consuming the shared budget. Observed in production 2026-08-17: seven sources whose records arrived outside a collection run held this reason permanently. The sweep ran 10.5s against its 2s budget with those seven as participants, so eight other rows that had genuinely just collected sat dirty and were never repaired. Same starvation shape as the checkpoint floor, one layer up. Historical-refused rows now fall through to the checkpoint-lag predicate: they rejoin the moment the log advances past their checkpoint, so a real new event still converges them, and pure silence no longer costs a pass. Every other non-current state -- fold failure, contention, incomplete replay -- is genuinely retryable and still participates unconditionally. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit c428a7042c1fb08ae213c62f2a0749eb40087929)
… the runtime glibc Adds the four registration files the connector needs beyond its own source: collector-registry, connector-conformance-roster, coverage-conformance drivers, and the no-await-in-loops allowlist. Without these the connector is only half-registered -- present but not wired into collection or conformance. Builds sigtop on golang:bookworm rather than golang:latest. The runtime image is bookworm (glibc 2.36) while golang:latest is trixie (2.38), so a binary built there loads on the builder and dies in the final image with 'GLIBC_2.38 not found'. Also installs libsecret-1-0 in the runtime stage: sigtop links it for Signal Desktop keyring access, and having only the -dev package in the builder produced a binary that compiled cleanly and failed on first use. The verification step is now a bare 'sigtop -v' with no '|| true', so a binary that cannot execute fails the build instead of shipping. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 3f92b72466553ff9350d06edc726cb0ee4eb6e91)
sigtop has no version or -v subcommand; both attempts failed the build with 'invalid command'. Invoking it bare prints usage and exits non-zero, which still proves the binary loads its shared libraries and parses arguments -- the actual thing this check exists to catch. Grepping for the usage banner keeps a genuine load failure fatal, since a GLIBC or libsecret mismatch prints a linker error rather than usage. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 4478e1740d8b68fa734173d9c81a04036e612b75)
The manifest's own rationale gated listing on an operator proving a real run and explicitly opting in. The sidecar half of that is now settled: sigtop is built from pinned v0.24.0 source into the Core image and smoke-verified at build time, so it is proven present and executable on the shipped runtime rather than assumed. Preview, not Supported: no repeatable live run against a real Signal account has been recorded yet, and only Linux is exercised. That matches how apple_contacts and groupme are tiered -- real evidence, not yet repeatable. The owner opted this into listing to perform that first run. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit a9bdaeb31be3138a9b9a34397d7578fe0fe8c3ed)
…s diagnosable The Signal manifest declared connector_id and manifest_uri under registry.pdpp.dev; every other shipped manifest uses registry.pdpp.org. The registry validates that connector_id resolves to connector_key against the expected host, so the connector was rejected on every boot and never registered -- it was present in the image and invisible to the product. The reconcile log made that undiagnosable: it printed only the error code, 'invalid_request', with no field and no message. Diagnosing this one rejection cost several build-and-deploy cycles of guessing at plausible causes (listing tier, reachability capability, missing setup block, stream coverage_policy) -- all wrong. The log now includes the param and the message, which named the real cause immediately: connector_id must match connector_key; use manifest_uri for registry or document provenance Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 0d1390d0800b88fbf5549b1787b65773e8cd7dbd)
The first fix guarded only NULL checkpoints, but a row that has never had a terminal event folded into it stores a literal 0. The shared floor therefore went back to 0 after deploy -- observed live with four participants and the sweep replaying from the start of a 1.44M-event log again. Two places needed the same idea. seedFoldState now skips any checkpoint that is null OR zero when computing the floor. rowNeedsFoldParticipation now excludes a historical-refused row whose checkpoint is zero: the carve-out added earlier routed it to the checkpoint-lag predicate, and 0 < maxSeq is trivially true, so it rejoined every pass anyway. Such a row re-enters only when something real changes for it -- a new event marks it dirty, or its checkpoint advances past zero. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 5bd5b665caf5191aea6b67199937b72a40d61ae9)
…side sidecar Signal cannot run server-side by construction, and the manifest did not say so. Its SQLCipher key is stored encrypted in config.json behind Electron safeStorage and unwraps only through a session-bound OS keyring (kwallet6/gnome-keyring, Keychain, DPAPI). Tested against real data on this host, a container fails at four successive layers: file absent, then 'cannot connect to D-Bus session bus', then EOF on a uid mismatch, then an AppArmor denial once uids matched. Copying the database is insufficient -- the key is not in the file. Three changes: - becomes a first-class binding in runtime_requirements.bindings, and sourceKindFromManifestBindings resolves it to local_device. The engine now refuses server-side placement up front, through the same mechanism that already keeps browser connectors off the collector profile, rather than leaving the next person to rediscover this four D-Bus layers deep. - signal.json declares that binding with its rationale. - The sigtop builder stage is removed from the Core image. Shipping a binary that cannot work there implies support that does not exist; the builder-stage pattern stays proven via slackdump. The packaging rule this establishes -- sidecar packaging keys off the connector's placement bindings, not one uniform mechanism -- is recorded in design-notes/connector-sidecar-packaging-2026-08-17.md, along with the basic_text edge case (a keyring-disabled Signal stores the key unwrapped, which is a documentation note rather than a reason to ship an image stage). Signal remains local-collector-only with PATH/SIGTOP_BIN resolution and a clear install error. Connector code must not fetch executables at runtime; that acquisition story belongs to the registry's signed, ABI-tagged artifacts. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 1bb41762bf1aced5a39423c6a5b3f8231b6c23a0)
Both compose files started Postgres with stock settings sized for a small development database. A PDPP node is a write-heavy record store, and the owner's instance shows what that costs: 5.4M records across 76 GB, where the default 1GB max_wal_size forced a checkpoint every 16-22 seconds. Postgres logged 'checkpoints are occurring too frequently' 8,209 times, ingest returned intermittent 503 ingest_batch_storage_error that failed whole connector runs, and two backend processes were killed under the write pressure -- each time Postgres auto-recovered, so the container never restarted and health checks stayed green while runs failed. Raising max_wal_size to 8GB, min_wal_size to 1GB and checkpoint_timeout to 15min removed the warnings outright on the live instance. shared_buffers and effective_cache_size move off their 128MB/4GB defaults to modest values that are safe on a 4 GB VPS. Every value is overridable via PDPP_POSTGRES_* so an operator with more RAM can raise shared_buffers toward 25% of memory without editing the file. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 142e9b172fa8324823dfe570817eb7ccfceab9fd)
…ty repair
11 of 27 production sources (GitHub, Gmail, USAA, Amazon, YNAB, ...) sat at
health state unknown/summary_evidence_dirty_backstop for over an hour despite
SUCCEEDED runs and current terminal facts. The maintenance sweep ran every
tick (candidates_inspected: 9-25) but reported repaired: 0, skipped: 0,
failed: 0, candidate_reason_counts: {} every time.
Root cause: runBoundedObservationPhases (connector-summary-read-model.ts) runs
a "missing"-only repair phase before the "generic" phase that classifies
dirty/stale/checkpoint-mismatch. missing's own batched discovery pays a fixed
cost over the whole requested scope regardless of how many rows are actually
missing -- including zero, which is the common case for an already-dirty,
already-observed row. That discovery cost is not reserved against the round's
shared cooperative deadline, so under load it can consume the entire budget
before generic's own discoverCandidates ever runs. generic is the only phase
that ever returns "dirty", so the round returns empty candidate counts and the
row stays dirty=1/state=stale forever, invisible to every counter.
Fixed the same way connector-maintenance-sweep.ts already closes the
structurally identical walk-vs-acceleration starvation: alternate which
phase gets first opportunity at the remaining deadline, giving generic a
hard 2-round bound on how long missing can deny it a turn. The fold that
existing participants depend on still always runs first, unconditionally,
preserving the separately-tested "fold before slow generic repairs" contract.
Added a regression test reproducing the exact production shape (many
missing-evidence sibling connections inflating missing's discovery cost,
one already-warm dirty row) and proving it converges within the alternation's
2-round bound.
Tests: connector-summary-dirty-priority-starvation.test.ts (12/12),
connector-summary-evidence-bounded-sweep.test.ts (8/8),
connector-summary-evidence-engine-scoped-consumer.test.ts (5/5),
connector-summary-source-revision.test.ts all pass; repeated runs show no
flakiness. biome check clean on both changed files (one pre-existing,
untouched runCursorWalk complexity warning remains, unrelated to this change).
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit a1f6f1784da1840093839261405020d8d5015ec2)
The owner reported reddit_session_failed on every run, but the login and
captcha genuinely succeeded and a real, working session cookie was
established each time. isSessionLive was deciding liveness by looking for
a single logout-link selector in old.reddit.com's HTML — that selector no
longer matches, so it reported the session dead even though the cookie
worked for everything downstream.
isSessionLive now probes the owner-only /user/{username}/saved.json
listing instead: a 200 is ground truth for a working session, and it's
exactly the data the connector needs anyway, so this doubles as real
coverage rather than a DOM guess. A genuinely dead session (403/redirect)
still fails with the same reddit_session_failed taxonomy. Falls back to
the old DOM probe only for the credential-less manual hand-off, where no
username is known yet.
Left a comment instead of implementing the manual-handoff retry gap noted
alongside this (ensureRedditManualSession/recoverRedditBlockedLogin give
one shot, unlike amazon.ts's final-verify retry) — three existing tests
deliberately assert sendInteraction is never called on the post-submit
poll-timeout path as defense-in-depth for the credential-safety invariant,
so that change needs its own pass rather than riding along here.
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit f346ba9e1efcceab125f7abfae0583565cae1c38)
Owner reported on a real phone: on the interactive run-stream page, page scroll/rotate worked but taps never reached the remote browser (blocking a Reddit captcha click). Desktop mouse input was unaffected. Root cause: the capture-phase pointerdown/pointerup gate in stream-viewer.tsx required `event.button === 0` for touch pointer events before forwarding them to the remote surface. Per the Pointer Events spec, `button`/`buttons` describe mouse-style button state and are not a reliable signal for touch — some touch input paths report a non-zero or non-mouse-like `button` on pointerup for a legitimate primary contact, so this gate silently dropped taps while leaving desktop mouse input (which does carry a meaningful button state) working. Scroll/rotate were unaffected because they don't route through this gate. Extracted the gate into stream-viewer-pointer-input.ts (matching the existing stream-viewer-geometry.ts extraction pattern) so it is unit-testable outside the component, and removed the touch/button check entirely — touch has no secondary button, so `button` must never gate touch forwarding. The mouse hover-move suppression (buttons === 0) is preserved unchanged. touch-action: manipulation was already correctly scoped to the remote-surface media element in stream.css, so it was not implicated. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit bee6a4f3805b04db26a10f9e2e9b7666bfa98201)
…rated setup flow Slack's manifest declares only network + filesystem bindings and setup.modality: static_secret -- it never places a managed browser surface under its current design. But browser_surface_leases/browser_surfaces still held 21 rows from Aug 1-3 (an earlier setup flow's "#browser-phase" session capture), all cleanly torn down: leases released, surfaces stopping, none unhealthy or active. getConnectorBrowserSurfaceProjection counts any historical row as "this connector has browser evidence," so it fell through projectConnectorBrowserSurfaceEvidence's unhealthy/ready checks (both false for a stopped surface) into BROWSER_SURFACE_UNKNOWN_PROJECTION -- forever, with no run ever able to clear it. Confirmed fleet-wide: chase and usaa (also browser-phase connectors with zero ready/non-terminal surfaces left) show the same axes.remote_surface: "unknown", collapsing connection_health to state: "unknown" / pill "Not measured" even with a fully successful, complete-coverage run. getConnectorBrowserSurfaceProjection now takes manifestHasBrowserBinding and short-circuits to no-evidence (not_applicable) when the connector's CURRENT manifest declares no browser binding at all, before consulting any historical row -- the same "manifest declaration is a stable required-capability fact" pattern manifestRequiresBrowserSessionRepair already uses. ref-control.ts wires it from runtime_requirements.bindings.browser !== undefined. This does not touch the existing retired-history behavior for connectors that DO use a browser (chatgpt/usaa/reddit still correctly project unknown from stale rows) -- only connectors whose manifest never mentions browser at all skip the history check entirely. Added a test proving the pre-fix call shape still reproduces the stuck-unknown bug, and that passing manifestHasBrowserBinding: false resolves a Slack-shaped legacy-lease fixture to a healthy, none-axis headline instead. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 640e5e47bd58957b5ecda5c914b592566deee287)
…minator
runAllMailPasses runs a historical page and a forward page in the same call,
both emitting messages records through the same emitRecord counter, so
collected already reflects both. But the messages DETAIL_COVERAGE was built
from historicalMessageCoverage alone, dropping the forward pass's
considered/covered -- the sibling message_bodies DETAIL_COVERAGE a few lines
above already correctly sums both passes. On the live instance this showed
up as messages: collected(451) > considered(431), reported as coverage:
"unknown" on an otherwise clean, fully-covered run.
Sum historicalMessageCoverage + forwardMessageCoverage for both considered
and covered, matching message_bodies. Extended the existing "scheduled runs
advance historical pages while forwarding new mail" test to assert the
messages DETAIL_COVERAGE equals the sum of both passes; reverting the fix
alone fails it (actual {considered: 1, covered: 1} vs expected {considered:
2, covered: 2}).
Not touched: attachments' terminal_gap. That's driven by 52 durable
connector_detail_gaps rows (16 quarantined + 4 temporary_unavailable, both
genuine provider-side gaps, plus 32 too_large rows stale since 2026-08-03
predating the size-cap policy that now correctly excludes too_large from
gap creation). The 32 stale rows permanently poison the terminal count even
though current collection behavior is correct -- this needs a product
decision on cleanup/lifecycle for orphaned terminal gaps, not a blind patch,
so it's flagged as a follow-up rather than fixed here.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 4161f5d7b78d45072916c3a0dad3c1321849307d)
…hange
openExportDialog clicked Export, slept a fixed 2500ms
(EXPORT_DIALOG_DELAY_MS), then took one immediate .count() read of the
date-range select. A dialog that rendered even slightly slower than
2500ms -- network jitter, client-side render variance -- read count 0,
which emitDialogUnexpectedShapeDiagnostic reports as
export_dialog_unexpected_shape. tryExportLadder treats that phase as fatal
and aborts every remaining candidate date-range retry on the first
occurrence, so the whole run gets classified source_structure_changed ->
export_affordance_missing, an "actionable" severity gap that
mapCoverageAxis turns into terminal_gap for the ENTIRE connection -- even
though accounts/account_stats/inbox_messages were complete and the
credit-card/statements gaps were unrelated, already-correctly-classified
retryable PDF-download timeouts.
Live evidence ruled out a genuine USAA site change: today's runs for this
connection alternated between clean success and export-affordance failure
across two different phases (no_export_affordance, then later
export_dialog_unexpected_shape) with inconsistent marker counts -- the
signature of flaky render timing, not a stable redesign, which would fail
the same way every run. Same class of defect as the already-fixed Chase
current_activity hydration-wait bug
(fix-chase-current-activity-hydration-wait): a fixed timing assumption
trusted as proof of "structure changed" rather than "hasn't rendered yet."
Replaced the fixed sleep + one-shot count with a bounded waitFor({ state:
"visible", timeout: EXPORT_DIALOG_DELAY_MS }) on the date-range select. A
slow-but-real render now succeeds; only a select that never appears within
the same time budget still falls through to the existing unexpected-shape
diagnostic and Escape path, unchanged. Added a regression test asserting a
slow-but-real render is treated as ready, not a structure change --
verified it fails against the pre-fix code and passes with the fix.
USAA suite: 233/233 (1 pre-existing skip) pass. tsc --noEmit clean. biome
check clean.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit b7c0e3df1844c7146a588705974f4dcc27a1615d)
…sentation-bearer expiry Interactive-login connectors (Reddit, Amazon) run Chromium+Xvfb inside the core image with no n.eko. The connector registers a direct-CDP streaming target once, via the run-target registry, when it emits its manual_action interaction — confirmed by production logs: PUT /admin/runs/:runId/interactions/:interactionId/streaming-target returns 200, and the first two mint attempts for that run succeeded with backend "cdp". The viewer's presentation-bearer session has its own short TTL (independent of the interaction's timeout_seconds, which can run up to 30 minutes). `schedulePresentationExpiry`'s timeout fired `invalidateForInteractionResolved` on bearer expiry, which defaults to force-unregistering the run-target registry entry via `terminalizePresentation`'s default `cleanupTarget`. That conflated "the viewer's token expired" with "the interaction is over": the manual_action interaction was still pending (owner hadn't responded), but the registry entry it depended on was deleted anyway. Every subsequent mint attempt then hit `streaming_companion_unavailable`, permanently, with no recovery short of re-running the connector — this is the reported "Open browser" 500. The code already had the right pattern one path away: the `stream_session_superseded` branch in `mintStreamSession` passes `cleanupTarget: () => Promise.resolve()` with a comment explaining the direct-CDP target is owned by the interaction, not the bearer/session. Apply the same no-op cleanup to the bearer-expiry path, so a fresh mint after expiry re-attaches a new companion to the same still-registered target. Added a regression test that registers a direct-CDP target, mints a session, advances a fake clock past the bearer TTL to fire the expiry timer, and asserts the registry entry survives (no `run_target_force_unregistered`) and a subsequent mint still succeeds. This is a general reference-server fix, not a neko-specific one — neko-backed runs already keep their lease-scoped surface alive independently and are unaffected (leases are keyed by run_id at BrowserSurfaceLeaseManager, not by the presentation bearer). Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 07416c6be6b3df84e6516b51540663988a108a14)
…lling to unknown
The reference server's run-interaction-stream mint route raises
StreamingCompanionUnavailableError as HTTP 503 with body code
streaming_companion_unavailable (reference-implementation/server/streaming/
routes.ts). The console's stream-reach-diagnostics classifyReason() only
mapped 401/409/410; every other status, including this 503, fell to the
`default: return "unknown"` branch, whose copy is "Couldn't reach the browser
stream after several tries." That phrasing reads as a network/reachability
problem and is exactly what hid the real, actionable
STREAMING_COMPANION_UNAVAILABLE error behind generic wording — the same
outage this branch is fixing on the server side.
Map 503 with probeCode "streaming_companion_unavailable" to the existing
companion_unavailable reason (its copy already says the right thing: "The
browser session is no longer running on the server. Start the browser step
again."). Other 503 codes (e.g. the managed n.eko window-settle probe's
managed_surface_window_settle_unavailable) still fall to unknown rather than
being collapsed into companion_unavailable, since they are a different,
currently-unclassified condition.
Also address the underlying pattern, not just this one instance: the
`unknown` reason now appends the observed status code to its message
("... (server responded 500)") so the next unmapped status is visible in the
UI instead of silently reusing identical generic copy the way this one did.
Updated the two existing tests that asserted the old exact-verbatim `unknown`
message.
Added tests: 503 + streaming_companion_unavailable classifies as
companion_unavailable; 503 with an unrelated/absent code still classifies as
unknown (so a different real 503 condition isn't mislabeled).
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 0ff275ca4e5474b4b2a1f576a27580a3f515f692)
…ust that it is Four production local-device connections (peregrine Claude Code/Codex, vivid fish Claude Code, Simon VM Claude Code) are stuck at coverage=unknown despite collecting continuously. Traced via connector_state (coverage_diagnostics) and git history, not guessed: their agent builds (0.0.0+d53f67fa2034 from 08-03, +378a2ba7a7ae from 07-25) predate 5e4493c/59557c1e5/4d9e6b7e4 (08-10), the commits that taught claude_code/codex to report coverage_diagnostics rows for derived_messages/derived_attachments/derived_memory_notes (claude_code) and derived_messages/derived_function_calls (codex) at all -- not merely to tolerate them once already reported (292e02b9a, already deployed). Their committed snapshots structurally cannot contain those rows, so parseCoverageDiagnosticsStateSnapshot's missingStores check correctly computes non-empty and deriveLocalCoverageAxis correctly refuses to call the connection reliable. This is genuinely-unproven coverage, not the server refusing to read a proven snapshot: pre-5e4493cdb, these three streams were never measured by ANY store for claude_code, not even the parent sessions/projects scan (confirmed by reading the commit's own before/after). ref-connectors-local-coverage-green's existing comment already documents this as intended behavior. Per the project's coverage-honesty rule, `unknown` is the correct verdict here, and fabricating `missingStores` tolerance for old builds would assert coverage that was never measured. The fix ships once these four devices' agents update past 4d9e6b7. Simon VM's absent run_history is a separate, non-bug observation: its enrollment revoked 2026-05-20 (dexp_90b7966a095308d7) was superseded by a still-active, still-heartbeating enrollment (dexp_9ed7a42ed38a505c, last heartbeat today) that has ingested continuously since; run_history simply never carries rows for push-based local-device ingest the way it does for scheduler-polled connectors, and its retention only reaches back to 2026-08-13 regardless. What IS fixed: deriveLocalCoverageAxis returned reliable:false with no way to tell WHICH precondition failed, so diagnosing this required manually tracing parseCoverageDiagnosticsStateSnapshot by hand. Added unreliableReason (invalid_cursor / generation_mismatch / malformed / no_authoritative_inventory / no_committed_snapshot / duplicate_stores / missing_stores / unexpected_stores), computed in the same short-circuit order the reliable conjunction already checks, so the reason always names the first failed precondition. Purely additive -- never changes what counts as reliable, only labels an existing refusal. New test asserts the very distinction this diagnosis needed: missing_stores (genuinely unproven) vs generation_mismatch (a different kind of refusal), plus that a reliable result carries no unreliableReason at all. node --test test/ref-connectors-local-coverage-green.test.ts: 31/31 pass. Full node --test test/*.test.ts: 8266/8499 pass, 18 failures -- all pre-existing (SQLite writer-lock contention, scheduler/ingest timing, and Signal-connector conformance WIP already present in this worktree before this change), none touching ref-control.ts coverage logic. tsc --noEmit clean. biome check --write clean. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 67c8730f3b8161634ac33c39038dea007c15ceec)
A paused connector instance (e.g. Chase) could not have its sign-in details edited: the credential-capture route's allowStatuses only admitted active/draft, so the resolver threw the generic connector_instance_inactive error for a perfectly ordinary owner action. Widen the allowlist to admit paused, and — since nothing else in the codebase ever un-pauses a connection — flip status back to active on a successful capture, mirroring owner-connection-reactivate's revoked->active pattern. revoked stays deliberately excluded; that remains reactivate's job. Assisted-by: AI (cherry picked from commit fd8f617b1aa792c6391cbd88573e848163005a4c) Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…eck added b914483 added assertBareSpecifiersResolve without narrowing two reads that `noUncheckedIndexedAccess` types as `string | undefined`, so `packages/mcp-server` stopped typechecking (4 errors: TS2322, TS2345 x2, TS18048). The pre-push typecheck gate covered reference-implementation, console, and polyfill-connectors but never this package, so it went unnoticed. bareSpecifierPackageName returned `segments[0]` directly. String.split always yields at least one element, so the undefined branch is unreachable, but the compiler cannot prove that; falling back to the whole specifier keeps the declared `string` return honest without asserting a non-null. bareImportSpecifiers passed a possibly-undefined capture group into isBareSpecifier(specifier: string). The static-import loop above it already guards with `specifier &&`; the dynamic-import/require loop did not. The same guard makes the two loops consistent and skips unmatched groups instead of calling .startsWith on undefined. No behavior change: both edits narrow types along paths unreachable at runtime. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…push on
Six redaction tests carried canaries written to look like real secrets --
`sk_live_51...`, `ghp_...`, `github_pat_...`. GitHub's push protection matches
on shape and cannot tell a canary from a live key, so it refused every push of
this branch on files whose entire purpose is proving secrets never escape.
The right fix depends on whether the shape is load-bearing, and it differs per
file, so each was decided separately rather than swept with one rule.
Three tests assert a STRUCTURAL property: `RecordsIngestSystemicFailureError`
builds a fixed template and never interpolates the underlying failure's text,
and the assertions are plain `includes` checks that never parse the marker.
Nothing matches those strings against a credential pattern, so the shape proves
nothing and reshaping cannot weaken the test. Those markers become
`canary_...`.
The other three are the opposite: the shape IS the test. `SECRET_CONDITION_PATTERN`
matches the literal prefix `ghp_`, the reporter's `GITHUB_PAT_PATTERN` needs
both the prefix and a >=20-char tail, and `LONG_OPAQUE_RE` decides purely by
length and character class. A reshaped canary there would stop tripping the
filter and the assertion would pass for the wrong reason -- a test that proves
nothing while looking green. Those keep their exact bytes and are assembled at
runtime with `.join("_")`, which leaves no matchable literal in the source
while the redactor still sees the identical string.
`cimd.test.ts` is the one genuine judgement call. Its event builder is a
structural allowlist that keeps only code/name and never copies `.message`, so
today the marker's shape is incidental and it could have been reshaped. It is
split instead because the case the test NAMES is "PAT-shaped", and if the
taxonomy ever grows a message-bearing field a reshaped marker would silently
stop covering it. Splitting costs nothing and keeps the name honest.
Each choice is recorded at the literal, so the next person to touch one knows
whether the bytes matter before changing them.
43 across cimd, stderr-redact-declared-reasons and run-tests-reporter-determinism.
Not verified: that the push actually succeeds. That needs a push, which this
branch is deliberately not doing yet.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
… code The apple_contacts hydration fix (a6681353f) moved every sequential await in that connector and added two more -- the chunked multiget, and the emitContactRecord call inside hydrateMissingBodies. The allowlist still pointed at the old line numbers, so its exception rows described code that had moved out from under them. That matters because the allowlist is matched on an exact (path, line, column) triple. A row pointing at the wrong line does not fail loudly; it detaches from the await it was meant to sanction, which is how a reviewed exception quietly becomes an unreviewed one. Five rows re-aimed and two added. The multiget entry is `provider_pacing_backpressure`: chunks are issued sequentially so one large change set cannot fan out unbounded parallel requests at iCloud. The hydrateMissingBodies entry is `ordered_protocol_emission`, matching the three sibling emit sites. After this the checker reports zero findings for apple_contacts, which is the whole claim being made here. Disclosed, and NOT fixed by this commit: the same checker reports 145 unlisted findings across sixteen connectors, and that gate is already red at HEAD. usaa and venmo alone account for 21 of them while being byte-identical to HEAD and carrying 20 and 4 allowlist rows -- so the drift predates this branch's work and belongs to whichever commits last moved those files. Fixing it is a mechanical sweep of its own and would bury this one-connector change. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Every Gmail run opens All Mail with a SELECT, and IMAP answers it with EXISTS -- the server's own count of the messages in the mailbox. The connector read uidValidity and uidNext off that same MailboxObject and dropped the count sitting next to them. The `labels` record body still carries the fossil of that decision: `message_count: null`. So the only mailbox-wide inventory total Gmail hands us was already in memory, measured at the provider boundary before a single UID is walked, and it cost nothing to keep. This binds it. Zero extra round-trips. The interesting decision is where NOT to put it. The obvious move is to make EXISTS the `considered` for the messages DETAIL_COVERAGE, and that would break the connector. `isHealthyBoundedContinuation` in reference-implementation/server/continuation-proof.ts:89 admits a bounded page only when `considered === covered` on same-page facts. Substituting a 140k-message mailbox total for a 500-message page denominator makes every run report partial forever and forfeits the continuation proof outright. That is a wrong denominator, not a better one, so the per-page fact is left exactly alone and the mailbox total is disclosed as its own PROGRESS fact beside it -- the server's number, next to how far the two cursors have actually walked. Validated fail-closed, following jellyfin's validateTotalRecordCount: missing, non-numeric, non-finite, fractional and negative each throw with their own reason rather than defaulting to a number. A total that silently reads as success is the bug this contract exists to prevent, so absence has to be louder than a wrong value, not quieter. The decrease check is the part that needed care. A shrinking count inside one UIDVALIDITY is real deletion or a server bug and throws. Across a re-key it is neither -- the server rebuilt the UID space, so the old count describes a different space and a lower number is expected. Comparing across that boundary would turn every legitimate re-key into a spurious failure, so the prior total is carried on the all_mail cursor next to the uidvalidity it was measured in and compared only within one epoch. That is the same re-key-versus-deletion distinction the messages cursor already draws a few lines above. 12 new tests driving the real runAllMailPasses against a stubbed client, so they fail if the total stops being bound, stops being validated, or stops being carried. One of them pins the per-page denominator at 2 rather than 1200 specifically so a future change cannot quietly route the mailbox total into the fact that must not hold it. 238 gmail. What this does not prove: that we hold everything the mailbox holds. It discloses the provider's number next to our boundary and makes the two comparable. Closing the gap is the cursors' job, and the contiguity work from f9a88e68a is what guards that. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…ssage count ChatGPT conversations declare `message_count_on_current_branch`, and it is worthless as a completeness anchor. That field is computed by our own countBranchMessages from the same `mapping` object, inside the same call that emits those messages. Both sides of the comparison read one in-memory graph, so checking emitted against declared is a tautology that can never fail. It is worse than useless, because it actively hides the defect it looks like it would catch. flattenTreeCurrentBranch walks parent pointers and stops silently when a parent is absent from the mapping. A truncated payload therefore yields a short branch AND a correspondingly short declared count -- the denominator shrinks to match the loss, the two numbers agree, and the conversation reads complete. A count-based check reports green on exactly the data it should flag. The provider assertion worth holding the payload to is structural rather than numeric. `current_node` plus each node's `parent` pointer declare a chain, and that chain makes two claims the payload can be measured against: the declared tip must be present in the mapping, and the chain must terminate at a real root rather than at a pointer into a node we never received. Both are read at the payload boundary, before any emit decision. Reconciled against 5,821 live conversations before choosing the contract. On-branch counts matched the declared count exactly for 5,815 and never once exceeded it, so equality is the right relation and a shortfall is real rather than noise. The six that did not match are the finding: three conversations in the paused archive are each short by exactly one message, and each is missing its opening user turn -- an assistant node whose parent names a user message that was never delivered. Reported as SKIP_RESULT, not DETAIL_GAP, for the same reason `empty_detail` is. The fetch succeeded and the conversation is genuinely hydrated; re-fetching returns the same truncated graph, so a retryable gap would spin forever. The honest report is that we reached it and what it gave us is internally inconsistent. Two shapes deliberately stay silent. Holding MORE than the current branch is normal -- 28% of live conversations carry off-branch alternatives, and a check that fired on those would be noise rather than signal. A conversation with no current_node declares nothing, so there is no claim to reconcile and inventing one would be a false positive. 7 new tests, including one that drives the truncated payload through the count comparison and asserts it passes -- the tautology pinned as a test so nobody reintroduces it as an improvement. 262 chatgpt. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
67 orders in this instance claim `item_count: 0` while the database holds 3 to 7 item rows for each of them. The record says the order was empty; the items are right there. The count was `Math.max(list.items.length, detail.items.length)` against a non-nullable `min(0)` schema. That shape cannot express "not measured", so a deferred detail fetch collapsed to a confident zero. Which orders it hits is not random, and the live signature is what makes the cause certain. Across 1,183 collected orders, `item_count === 0` never occurs on an order holding 1 or 2 items, and occurs on 53 holding 3 to 7. Amazon renders per-item titles on the list card only for small orders; past that it collapses them behind a "+N more items" affordance carrying no item markup. So for a 3+ item order the list contributes nothing and the count rests entirely on the order-detail page -- which the connector defers under its per-run attempt budget, its temporary-failure cap, or a latched session repair. Every one of those 53 orders also has a null shipping_address_summary, which is only populated from that same detail page. The page was missing, not the parse. item_count becomes nullable, in the record type, the zod schema and the manifest. A zero now means we opened the detail page and it listed nothing; a null means we never opened it. An order whose list card did render its titles still reports a real count without a detail fetch, because observed items are observed whichever surface showed them. The second half is a shortfall gap. Every item the detail page listed should survive into a record, so `merged.length < detail.items.length` means an item we were shown did not become a row. The denominator is counted at the parse boundary, before mergeOrderItems decides anything. The denominator choice is the part worth arguing with. It is the detail page's list only, never the list card's. The list card is not an authority on how many items an order has -- falling back to it would manufacture a shortfall out of ordinary deduplication on every collapsed order, which is the same wrong- denominator mistake in a new place. No detail page means no claim, so nothing is reported. 12 new tests across the two files. One pins that the list card must not become a denominator; one pins that a null validates, since the non-nullable schema is what forced the lie in the first place. 151 amazon, 2 skipped (pre-existing). Not verified: the 67 live records keep their false zero until Amazon next collects those orders. This fixes what future runs assert, not what is already stored. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…d item The upvoted stream held a stored cursor dated 2026-04-28 while the account's real upvote history runs back to 2011. Everything below that cursor had become unreachable, and the stream was wedged: each run halted on its first item and collected nothing. The stop rule assumed every Reddit listing is ordered by the item's own created_utc, so the first child at or below the cursor proves the rest are older and paging can stop. That is true for submitted and comments, where the listing IS the user's authorship timeline and creation order is listing order. It is false for saved, upvoted, downvoted and hidden. Those are ordered by when the OWNER acted, not by when the item was created. Upvoting a 2015 post today puts a 2015 created_utc at rank 1, and the created-based stop then halts the walk on item one. The stored cursor being future-dated relative to real history is not a corrupt value -- it is the correct high-water mark of a field that was never the sort key, which is why nothing looked wrong. Ordering becomes an explicit per-stream property rather than a global assumption. The four action-ordered streams walk the full listing to Reddit's own end and dedupe by fullname; submitted and comments keep the early stop, so incremental runs stay cheap. The default is "created", so a new stream has to opt IN to the full walk rather than inheriting it by accident. The full walk is affordable because it is bounded by end-of-listing, not by a page ceiling, and re-emitting is free -- records are keyed by fullname and take the upsert's noop disposition. Deduping happens before the caller counts considered/covered, so coverage reflects distinct items rather than repeat sightings when Reddit shifts items between pages mid-walk. Children arriving without a fullname are kept rather than collapsed. Treating a missing name as one shared identity would silently drop real data on a shape Reddit has not been observed to emit; downstream validation is the right place to judge them. The per-stream restart test now asserts different behavior per ordering rather than one shared expectation, and a new test walks an item far below the cursor to prove history is recovered instead of skipped -- the real-world shape of the defect. 134 reddit. Not verified without a live run: that upvoted actually drains its 2011-era backlog. The cursor is no longer able to hide it, which is the fix; how much is back there is only knowable after a real collection. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
GetOwnedGames returns `game_count` and GetRecentlyPlayedGames returns `total_count`, both alongside the array they serve. Both were parsed into the response object and never read. The coverage denominator was the served array's own length, which makes the proof circular: `considered` and `covered` both derive from whatever arrived, so a truncated response is fully covered by definition. Both calls are unpaginated, so there is no continuation evidence to catch it either -- a short array is simply the whole run. Binding the source's own count makes a short array a visible shortfall. If Steam says the account owns N games and served fewer, the stream reads partial instead of complete. Same posture jellyfin already takes with TotalRecordCount. Absent is tolerated: the field is optional in the wire contract, and a missing total falls back to the served length -- no worse than today. Malformed fails closed. A total below the served count is a contradiction rather than a shortfall and throws, because a denominator smaller than its numerator is nonsense and quietly clamping it would hide a real protocol break. 50 steam. Not verified: no live Steam run exercised a genuinely truncated response, since Steam does not truncate these on demand. The truncation case is proven against a stubbed short array, and the fallback and malformed paths against their own fixtures. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Attachment coverage reported `covered: totalAttachments` against `considered: totalAttachments` -- the same number on both sides, so the proof could not fail. Media the bounded-read policy refused was already counted in that number, so a chat whose oversized media was skipped still read fully covered. The runtime's coverage contract is explicit that a weighed-but-dropped item belongs to neither the collected nor the covered count. The connector already knew which files it dropped -- it emits a PROGRESS line saying how many exceeded the read policy -- and then added them to the numerator anyway. `covered` is now the discovered count minus the skipped count, threaded up from emitAttachmentRecords through emitParsedExport to the run-level total. `considered` stays the full discovered count, because those files were genuinely weighed. A chat with skipped media now reads partial, which is what happened. 69 whatsapp, 1 skipped (a pre-existing memory-growth countertest that needs --expose-gc). Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
403 of this owner's 973 Slack channels hold zero messages, 278 of them public. A channel slackdump never visited and a channel with genuinely nothing in it looked identical, so there was no way to tell an empty channel from a hole. slackdump records the answer itself. Its CHUNK table carries FINAL alongside NUM_REC, and TYPE_ID 0 is the MESSAGES chunk type per the archive's own TYPES table. A channel with a final messages chunk is one it walked to the end; a channel with only non-final chunks was cut short. That is the only per-channel completeness fact the archive exposes -- there is no message count and no has_more anywhere in the schema -- and it is the archive tool's bookkeeping, not anything this connector emitted. Across all archives, 552 channels carry a finalized message chunk against 973 in the inventory. That difference was entirely invisible before. Emitted as a one-directional set difference, inventory minus finalized, never a count comparison. A count cannot separate "short N channels" from "hold N extra" from "N duplicated", and for a preservation product the direction matters: a channel we hold history for that Slack has since archived or deleted must not read as loss. This asks only which channels the source lists that we cannot prove we finished. Channels we hold but Slack no longer lists never appear. An archive carrying no CHUNK bookkeeping at all emits nothing. An archive that cannot report finality cannot prove anything is missing either, and inventing a gap from absent evidence is the same defect as inventing coverage from it. This is deliberately a second diagnostic rather than an extension of the existing one. That one compares the archive against this connector's own prior state, so a channel never archived in the first place is invisible to it forever -- which is exactly the hole being closed here. Separately, the message family declared `covered: considered` in three places. A row whose Slack ts will not parse gets a fabricated sent_at from parseMessageRow's fallback; it is still emitted because the body is real, but it is not objectively accounted for. covered is now tallied per row from the row's own parse outcome at the enumeration site, so those read partial instead of the tautological complete. 213 slack. Not verified: no live re-archive was run, so the 421 unproven channels are a measurement of the current archives rather than a demonstration that a full re-archive closes them. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
All 156 of this owner's groups carry `messages_count: null`, in every version
ever collected, while the sibling members_count populates normally on the same
records. The connector reads a flat `messages_count` field. GroupMe's
documented GET /groups response nests it: `messages: { count,
last_message_id, last_message_created_at, preview }`. There is no flat field,
so the read never had anything to find.
The reader now accepts either shape. Non-integer, negative and non-finite
values are rejected rather than coerced, so a malformed provider value degrades
to unknown instead of becoming a fabricated anchor.
That count then anchors the group_messages walk, which previously had no
external reference at all. Two findings, kept separate on purpose. A group
where GroupMe reports more messages than the walk enumerated is a real gap. A
group where GroupMe reported no usable count has NO anchor, and saying so is
the honest alternative to treating an unanchored walk as proven -- which is
what reporting a false zero did.
One-directional. Only provider > walked is reported. Holding more than GroupMe
currently claims is preservation working: messages deleted upstream after we
collected them legitimately make our holdings larger, and flagging that as loss
would be backwards.
Scoped to walks that can actually be compared. The provider count describes a
group's whole history, so it is only checked against a cold start or an
explicit full_refresh. An incremental resume deliberately sees only new
messages, and holding a lifetime total against that window would report a false
shortfall on every healthy run. A since-scoped walk is excluded for the same
reason. A failed pass withholds both findings exactly as it already withholds
the cursor map, because a failed pass proves nothing about completeness.
Two existing tests changed expectations rather than behavior: the cold-start
fixture declares messages_count 10 while its stubbed pages supply 2 and 1, so
the anchor correctly reports both groups short. That is the check working
against the fixture's own numbers.
198 groupme, 2 skipped (pre-existing).
Stated ceiling, and it is a real one: this is a scalar, so it detects magnitude
only. It cannot tell "missing N distinct messages" from "collected N
duplicates". GroupMe exposes no cheap per-group message-id listing, so a set
comparison -- the shape used for slack and signal in this series -- is not
available here.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The messages cursor is a forward-only `sent_at > ?` watermark. Signal Desktop is a linked-device replica, so re-linking backfills rows carrying older sent_at values -- and every one of those lands at or below the watermark, where the filter can never revisit it. The gap was permanent and silent. Two facts now, deliberately not merged. The anchor is SELECT COUNT(*) over Signal Desktop's own messages table, measured at the source boundary before the emit pass runs, so it cannot be contaminated by anything this run emitted. It is reported as a stream-level fact and NOT folded into the window's considered, for the same reason as gmail's in this series: isHealthyBoundedContinuation admits a bounded window only when considered equals covered, so a stream-level total in the window denominator pins every incremental run to a false partial. The load-bearing part is a set difference: source ids at or below the watermark that no prior run ever emitted. A scalar cannot work here, and I checked numerically against the live 4,739-row database before choosing. sourceTotal minus belowWatermark is identically the in-window row count when both are measured from the same database in the same instant -- a tautology that cannot fire, not even on the backfill hole it would be written to catch. Counts also conflate missing with surplus with duplicated, and for a preservation product held-but-gone-upstream is correct behavior rather than loss. So the check asks only which source ids we have never emitted. A row deleted from Signal Desktop simply stops appearing and produces no finding. That requires a durable emitted-id set on the cursor, which needs two guards. A legacy cursor carries a watermark but no id set, so every below-watermark id would look never-emitted -- a false gap for rows we genuinely have. That first post-deploy run reports unproven and seeds the set; the next one checks properly. A cold start needs no special case: since is 0, so nothing is below the watermark. The set is capped at 200,000 ids, which holds this owner's whole ~4.7k history and refuses to grow without limit; when the cap binds the check degrades to cannot-prove rather than to a clean bill. Fails closed throughout. A malformed or unreadable count throws rather than defaulting to zero, because an unmeasurable boundary is not an empty one. The monotonicity rule jellyfin applies is deliberately omitted: Signal legitimately deletes rows as disappearing messages expire, so a decreasing total is ordinary here. 72 signal. Stated ceiling: this proves we hold everything the local replica holds. It cannot prove we hold everything the account ever had -- messages sent before this device was linked, or expired before it synced, are absent from the source table and therefore from the denominator too. No local anchor can close that, and this one does not claim to. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…ed on an unreadable directory Two defects in the local-file streams, both of which report absence as fact. The skip gate compared mtime. mtime is owner-controlled metadata, not a content fact: `git checkout` and `rsync --times` both restore a previous mtime onto genuinely different bytes. Reproduced directly -- a SKILL.md whose body changed completely, with its mtime restored, emitted zero records on the next run and would have stayed invisible indefinitely. The gate is now a SHA-256 of the bytes. These files are small and were already read in full to build their record, so the hash costs no extra I/O once the read moves ahead of the gate -- which it must, since the gate is a fact about content and the bytes have to be in hand to compute it. The stored value stays a number so the existing file_mtimes cursor shape is untouched: a 52-bit prefix is exact in a double and does not collide at these file counts. A cursor written by the old mtime build mismatches once and the file is re-emitted -- a one-time re-read, never a miss. Second, readdir failures were swallowed whole. A chmod 000 on a skills directory holding a real skill produced zero records, zero skips, no error, and a STATE checkpoint carrying an empty cursor -- the run silently recorded "this owner has no skills" as fact, and the checkpoint then suppressed those files on every subsequent run. A missing directory (ENOENT) is honestly empty; an unreadable one (EACCES, EPERM, ENOTDIR, EIO) means the enumeration did not happen. The filesystem is this connector's entire source of truth, so that is a source-boundary failure, not evidence of emptiness, and it now throws. The checkpoint is the part that made it durable rather than transient, so a failed scan no longer writes STATE at all. It emits source_unreadable instead, which says the contents are unknown for this run rather than that there are none. 140 claude_code, including a test that chmods a directory holding a real skill and asserts no empty cursor is checkpointed. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…iblings emitSkillsStream swallowed every readdir error, so a permission failure on the skills directory was indistinguishable from an owner who has no skills. Same class as the claude_code defect fixed alongside this: the filesystem is the whole source of truth here, so an unreadable directory is a source-boundary failure rather than evidence of emptiness. This connector already had the right answer in two other places. emitRulesStream and emitPromptsStream both route through listIfExists, which distinguishes ENOENT from a real failure. This aligns skills with them rather than inventing a third posture. ENOENT stays a legitimate empty enumeration. 130 codex. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…considered buildFullScanCoverageMessage sets covered equal to considered by construction, so notion's coverage proof could not fail. That would be harmless if no record could ever be rejected, but the schema is strict -- title bounded at 4000 chars, url at 4096, safe-text constraints -- so a real page can fail validation and was being claimed as covered anyway, purely because it had been enumerated. covered is now tallied per record from the same validateRecord verdict the runtime's emitRecord applies, and the coverage message is built with buildDetailCoverageMessage so the two numbers can differ. A page the schema rejects now reads a real partial. considered stays items.length, measured at the enumeration site, since the search result genuinely is the boundary. A page suppressed as unchanged still counts as covered: an earlier run's identical content already passed the real shape check, so it is genuinely accounted for rather than merely skipped. This stays deletion-safe, which is why the boundary is the right one to measure. Notion reports a deleted page in-band as `archived: true` rather than omitting it, so a deletion stays inside the enumerated set, validates like any other row, and counts as covered -- the runtime turns it into a tombstone. An upstream deletion is a covered fact, never a coverage gap, which is what keeps this proof from firing on PDPP's deliberate retention of records the provider has since removed. The same property is pinned for YNAB, which reaches it by the same in-band route -- its delta marks a deletion as a returned record with `deleted: true`. That connector was already correct; the tests are added because the property is load-bearing for this whole series and nothing was holding it. Live check when written: this instance holds 9 YNAB tombstones across several runs, 9 of the fleet's 11, so the in-band signal does reach durable storage. 15 notion, 88 ynab. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
An uploaded Timeline element the parser could not key was dropped silently. It never reached a record and it never reached the denominator either, so the run reported considered equal to covered and read complete -- the count was recomputed from the survivors rather than measured at the source. The boundary is now processStreamEvent, where one element event is exactly one element the file contained, counted before the parser's verdict. Three drop sites feed it: an element the parser yields nothing at all for, a point without a usable id or timestamp, and a segment without an id or start_time. All three were previously invisible. Attributing a yields-nothing element to a stream uses the element's own format, which is unambiguous -- a legacy `locations` element can only ever have been a point, a `semanticSegments` element only ever a segment. Unaccounted elements raise considered without raising covered, so the stream reads partial, and an element_unaccounted skip reports how many of how many. finishSegments stops using buildFullScanCoverageMessage when anything went unaccounted, since that helper forces covered equal to considered and would re-hide exactly the drop being counted. 81 google_maps, 1 skipped (pre-existing). Worth being precise about what this proves, because the name invites overclaiming. This is artifact reconciliation: it proves the run ingested everything the uploaded file contained. It says nothing about whether the file is a complete export of the owner's history -- Google decides what goes into it, and no check on this side can see past that. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Every other connector in this series gained a completeness anchor. GitHub deliberately gains none, and that is a finding rather than an omission -- the tempting scalars all measure something other than this stream's boundary, so binding one would assert a permanent false gap. Written into the code beside declareListConsidered, with a test pinning it, because the next person to audit this will otherwise see a gap where the reasoning should be and "fix" it. public_repos is the trap, because it is already stored on the user_stats record and looks authoritative. It counts a strict subset: 94 public against 575 held repositories, of which 355 are private and 465 belong to orgs -- neither counted. Binding it asserts a roughly 6x shortfall forever. public_gists matched its 8 public gists exactly while 43 secret gists sat outside it, which is the same defect wearing a passing test. Link rel="last" yields a page count, so it only becomes an item total under the assumption every page is full -- which the last page never is. total_count on /search/issues genuinely IS authoritative for its query, and is already consumed for cap detection. It stays out of the denominator because a search index is eventually consistent with the REST list this stream walks, so ordinary index lag would read as coverage loss. Underneath all four sits the constraint that actually decides it. PDPP retains records after the provider deletes them, and GitHub really does delete repos, issues and gists. A provider total describes the surviving account and is legitimately smaller than what we hold, so any two-way provider == held check flags successful preservation as a defect. The sound relation would be three-way -- provider total equals live holdings minus known tombstoned -- and this connector declares no tombstones at all, so the third term does not exist and the relation cannot be closed. If an anchor is wanted later, the shape is a set comparison, not a scalar: GitHub returns stable numeric ids on every one of these streams, so compare id sets and tombstone the upstream-absent ones rather than counting them as loss. That is the same shape slack and signal use in this series. 110 github, including a test asserting a private or org repository is collected while public_repos does not count it -- so the subset relation is pinned rather than merely asserted in prose. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Fourteen live rows record a statement's Ending Balance summary line as a transaction, amounts up to $52,334.41 that never happened. The newest was emitted three days ago on current code, so this is producing bad rows now, not historically. MODERN_TXN_LINE_RE matches a summary line as readily as a purchase: the row '02/04 Ending Balance -- -- $33,821.48' has the shape of a transaction and nothing downstream disagreed. Every completeness check in the fleet asks whether data is missing; none asks whether what we hold is real. A statement answers that. USAA prints a beginning and an ending balance for the period, so the transactions in between must satisfy ending - beginning == sum(amounts). Verified against four real statements, and the balances chain across consecutive months -- February's ending equals March's beginning. That is an equality, not a shortfall check, which is the point: it fails on an invented transaction exactly as it fails on a lost one. The other anchors bound tonight only detect absence. The date corruption reported earlier was already fixed and tombstoned -- both rows carry deleted = true and the live row reads 2025-12-26. Reproducing the parse from the stored January PDF, whose sha256 matches the record, confirms current code handles the two-date credit-card row correctly. No work was needed there. Honest ceiling: the anchor currently reports mismatched on every checking statement, because the multi-line table format means the parser extracts zero transactions from it. That is a true finding surfaced rather than suppressed, and the parser rewrite it calls for is not done. Credit-card statements print no balance summary and stay unanchored. 256 tests, 7 mutations each reverted alone and each killed by its own test. Settled from stored PDFs and both databases; no live run was used to reach these conclusions. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
contact_groups is a manifest-required stream holding zero records, ever, including deleted. It read as an empty account. It was blindness. iCloud stores a group as its own vCard carrying X-ADDRESSBOOKSERVER-KIND:group, with members listed as X-ADDRESSBOOKSERVER-MEMBER lines. The connector only ever read CATEGORIES on contact vCards, so contact_groups could not emit regardless of what the account held. Worse, the group's own resource was being emitted as a phantom contact and counted as covered -- the stream that could not see groups was inflating its own coverage with them. Bind the anchor to the enumerated collection: CardDAV returns every member URL for an empty sync token, so the id set is available and the comparison is a set difference rather than a count. That distinguishes an account with no groups from a connector that cannot see them, which is the distinction this stream has never been able to make. The fake CardDAV server never served a group vCard, which is why 92 tests passed while the connector was blind to the shape. It serves one now. Not settled: this account holds one contact and the credential is sealed, so whether iCloud has groups here is unknown until a run. The logic is proven end-to-end against a real group-vCard shape, not against this account. 92 tests, 9 mutations each reverted alone and each killed by its own test. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…'s own item count Chase picked since_last_statement whenever a cursor existed at all, without asking whether that cursor was still current. That window reaches back about thirty days, so once a cursor fell further behind than that, no scheduled run could ever cross the gap -- the export it chose could not see far enough, and choosing it was conditioned on nothing but the cursor's existence. Staleness now routes to a bounded date_range export. The per_account cursor was also never pruned, so an account that stopped appearing in discovery was skipped silently with no gap emitted. Pruning is now reported, and guarded so a failed discovery cannot erase every cursor at once. One existing test asserted the defect directly -- a cursor 165 days stale was expected to yield since_last_statement. Split into a fresh case and a stale case rather than deleted, so the passing half still pins the behaviour that is correct. No sound statement anchor exists for Chase and none was invented. Its statements are PDFs without a machine-readable balance summary, and QFX carries a point-in-time balance rather than a period pair, so there is nothing to reconcile against. HEB declares each order's item count on the list page while the items come from the detail page, so comparing them compares two independent surfaces rather than a number against itself. One-directional, because holding more than the provider still lists is preservation rather than loss. That anchor already has something to report: the tnunamak account's two orders declare 144 items between them and hold 89. The instance is not dormant as recorded -- it is blocked onboarding, with six runs in four days each emitting an identical 54 records and two failures. The connector behaved correctly throughout, refusing to declare terminal and leaving the checkpoint uncommitted. 190 chase and 156 heb tests; mutations reverted individually, each killed by its own test. Neither anchor has been exercised by a live run. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…red protocol The reorg moved `collector-definition`, `is-main-module`, `pdpp-safe-text` and the `EmittedMessage` protocol type out of `packages/polyfill-connectors/src/` and into the vendored `@pdpp/connector-protocol` package. Four files carried over from this branch still imported them by their old in-repo relative paths, which typechecked on the pre-reorg tree and fail with TS2307 on the new one. Each is repointed at the exact specifier every sibling connector already uses on main, so this adds no new convention: - connectors/signal/collector-definition.ts -> @pdpp/connector-protocol/collector-definition - connectors/signal/index.ts -> @pdpp/connector-protocol (isMainModule) - connectors/signal/schemas.ts -> @pdpp/connector-protocol/pdpp-safe-text - connectors/_conformance/change-feed-is-not-inventory.test.ts -> @pdpp/connector-protocol Import-path only; no behavior change. Kept as its own commit rather than squashed into the signal commit because two of the files are introduced later in the series, so the fix cannot apply at that point in history. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…d tree
`check:noAwaitInLoops-conformance` passes on origin/main (305 live locations,
305 entries) and was failing on this branch with 160 unlisted findings and 158
stale entries. Almost all of that is line-number drift: this branch's own
changes moved the code the allowlist pins by (path, line, column), and the
conflict resolution during the rebase kept one side's numbers rather than the
rebased tree's.
158 of the stale entries are the same intentional sequential awaits at new
coordinates. Each was remapped within its own file by matching the reported
column and preserving the existing `category` and `note`, so no entry silently
changes meaning. The one entry whose column also moved
(usaa/statement-pdfs.ts `hydrateOneStatement()`, 577:5 -> 657:7) was confirmed
against the source before remapping.
Two findings are genuinely new, introduced by this branch, and are added as
reviewed entries rather than renumbered:
- gmail/integration.test.ts:2073 — the backfill walk feeds each run's
committed cursor into the next, so the iterations are dependent by
construction; `test_assertion_sequencing`.
- src/auto-login/chatgpt.ts:300 — `checkSessionWithRetry` must stop on the
first successful probe, so the attempts cannot be issued together;
`bounded_retry_polling`.
Gate now reports 321 live locations against 321 entries with no new, stale, or
duplicate findings.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Both tests were written against the pre-reorg tree and fail on post-reorg main
for setup reasons that read, misleadingly, as product failures.
rs-ingest-systemic-failure-server-log.test.ts asserted 503 and got 404. The
manifest it registers omits `manifest_uri` and stream `selection`, which the
current source-declaration contract requires — registration was rejected with
`source.declaration_invalid` ("sourceId must be an absolute URI"), the
connector was never registered, and the ingest POST 404'd before reaching the
fault hook. `registerManifest` discarded the response, so the real error was
invisible. Fixed by matching the manifest its already-passing sibling
(rs-ingest-systemic-failure-redaction.test.ts) uses, and by asserting the
registration status so this cannot fail silently again.
coverage-conformance-drivers.ts failed all 6 reddit streams with
`reddit_http_0`. This branch's own reddit commit routes every listing fetch
through `ensureRedditJsonOrigin`, which reads `page.url()` and navigates when
the origin is wrong. The conformance mock implements only `evaluate`, so the
origin check saw an unnavigated page, short-circuited to `status: 0`, and never
reached the fetch stub. Reddit's own integration.test.ts oracle mock was
updated for this and the conformance driver was missed; the mock now carries
the same `goto`/`url` pair.
Coverage conformance: 29/29. The reddit streams now prove coverage rather than
throwing (18 streams proven, up from 12).
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Four local-collector connections sat unhealthy on coverage_unknown with no indication of what the owner should do. All four were running collector binaries predating the derived_messages/derived_attachments/ derived_memory_notes stores, so their coverage snapshots structurally could not contain what the parser requires -- missingStores is non-empty, the snapshot is refused, and the axis is honestly unknown. The health model was right to withhold green: coverage really was unproven. What it failed to do was distinguish "this collector is too old to prove coverage" from "no evidence yet", which reads identically to the owner and sends them looking at the server. Report coverage_unknown_stale_collector with an update_connector remediation when the refusal is specifically a missing-store shortfall. No condition is weakened; an out-of-date collector still cannot turn a source green, it just says so. One of these connections also never emitted CollectionSucceeded, for the same underlying reason -- its build has no terminal-collection call at all, so no run.completed event was ever written and terminal facts could never converge. That half needs no server change either; it self-heals on the first run after the binary is current. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit e5b1e3948a228f21f4dfd75c48275dbc2b21f968) (cherry picked from commit c6179c7)
tnunamak
force-pushed
the
fix/sweep-fairness-and-transformer-bounds
branch
from
August 20, 2026 19:00
7ca31d2 to
0cd898a
Compare
tnunamak
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR changes
Someone self-hosting this system to collect their own data from services like a bank or a mail provider — the "owner" — could be told that one of their connected accounts had "no coverage evidence yet", when the real situation was that the small program doing the collecting on their laptop was simply too old. "Coverage" here means proof that the collector actually retrieved everything the server expects for that account; it has nothing to do with test coverage. The two situations read identically on screen but call for opposite responses: the first means wait for the next scheduled run, the second means update the collector. Owners waited instead of updating, so the account never reached a healthy state.
That one behavioral fix is the whole payload of this PR. The other 127 commits are a replay of production history already reviewed on its own branch; they are here for the reasons below, not for fresh review.
Why a branch this size exists
The live PDP-Connect deployment is one running instance of this repository's server and web console. During several weeks of incident response it was patched directly, so it accumulated fixes never proposed back here. Two independent attempts were made to return them. The first is this PR, opened from a branch named
fix/sweep-fairness-and-transformer-bounds— that name describes only the first few fixes it originally carried and no longer describes its contents, but a pull request's branch cannot be renamed, so it stays. The second wasfix/preservation-fixes-0819, a later and larger branch that reconstructed the same production history commit by commit; it exists only to feed this PR and is not open for review separately.While both sat,
main— this repository's default branch, which both were meant to merge into — was reorganized: three packages (collector-runtime,connector-protocol, andlocal-collector, the collector program mentioned above) were deleted from this repository and are now consumed as prebuilt tarballs published fromdata-connect, a different repository despite the similar name. That reorganization is what left this PR's 26 original commits stranded on a pre-reorganizationmain, unmergeable.This PR is now the union of both attempts rebased onto the reorganized
main: 127 commits fromfix/preservation-fixes-0819, plus one commit recovered from the 26. The other 25 were dropped as already-present, with the evidence below. Currentmainis then merged in on top, which is required rather than cosmetic: that merge bringsvendor/pdpp-collector-runtime-0.0.1.tgz, a prebuilt archive of the collector code that now lives in thedata-connectrepository, and the current version fixes how the collector launches the Signal connector. Deploying this branch with the older archive would reintroduce that break, sovendor/is taken frommainin full — both archives and theirSHA256SUMSchecksum file are byte-identical tomain, and both verify against those checksums.How the duplicates were established
Git ancestry misleads here because two earlier squash-merges rewrote these commits into new objects, and
git patch-id— which fingerprints a commit by the content of its diff, so the same change stays recognizable across a rebase — misleads too, because squashing concatenates several diffs into one that matches none of the originals. Trusting those signals previously produced a nonsense count of 739 unmerged commits. Every one of the 26 was therefore compared by file content instead.Of the 82 files this PR originally touched, 53 now have exactly the same content as on
fix/preservation-fixes-0819, compared by git blob hash — an identity check, not a similarity score. That leaves 29. Three belong topackages/local-collector, which the reorganization deleted, so reapplying them would recreate files that no longer belong to this repository; that work is being ported todata-connectinstead. For the remaining 26 files the other branch carries a strictly newer version of the same fix. For example, the USAA bank connector — a connector being the module that logs into one provider and retrieves that provider's data — gained a statement-balance reconciliation after this PR was opened, and every one of the 245 lines this PR adds to its two files is already present in the newer version. That check is line-presence, weaker than the blob-hash identity used for the 53: it would not catch lines preserved but reordered into different functions, so I also read the differing regions of all 26 files directly.One removal deletes a hazard rather than a duplicate. This PR's copy of
design-notes/failure-diagnosability-2026-08-18.mdillustrates a point using a fake Stripe-style key beginningsk_live_; the other branch's copy replaced it with a placeholder. Keeping this PR's version would have reintroduced a string that GitHub's push-protection scanner blocks on push.The one commit that was kept
The commit "fix: say when coverage is unknown because the collector is out of date" (
c6179c7f5) was the only one carrying behavior the other branch lacked. It reports a distinct condition,coverage_unknown_stale_collector, together with an "update the collector" instruction, when the collector program on the owner's machine predates the data categories the server now asks it to measure — so it never gathered them and structurally cannot prove coverage.The other branch did land an equivalent fix earlier, but a later commit on it —
aa8019f1d, whose subject reads "fix: index spine_events by instance for every event type, not just terminal" — also deleted 32 lines of unrelated stale-collector logic, while leaving the test asserting that behavior in place. The result was a test asserting an outcome no production code could produce, sofix/preservation-fixes-0819was failing before this commit was restored: the test expectedcoverage_unknown_stale_collectorand got the genericcoverage_unknown. Restoring the commit restores the deleted logic, andreference-implementation/test/ref-connectors-local-coverage-green.test.tsnow passes all 32 of its test cases. I read that commit's message and full diff and found no stated intent to remove this behavior — and a deliberate removal would normally have deleted the test too — so I have treated it as accidental. I found no written record either way, so whoever made that change may want to confirm.Review notes
connection-health.tsandref-control.tseach conflicted because the two branches add a different optional field to the same shared type:unfillableAccounted, recording that every outstanding gap in collected data is backed by durable proof the missing item can never be retrieved, andunknownStaleCollectorBuild, recording that coverage is unmeasurable specifically because the collector is out of date. They describe unrelated situations, so both fields and both explanatory comments were kept and the enclosing function signatures widened to carry both.mainnow imports that same function from a vendored tarball. Git merges that cleanly and the result is a silently duplicated definition. This is why the threelocal-collectorfiles were dropped by hand rather than merged.reference-implementation(the server and scheduler this PR mostly touches),apps/console(the owner-facing web app),packages/polyfill-connectors(the connector modules), andpackages/mcp-server. Separately, scanning every added line forsk_live_/ghp_-style key patterns and for the author's email address returns no matches — the check that caught the design-note hazard above; no commit deletes a file thatmainstill has, ruling out a rebase silently reverting someone else's work; and all 128 non-merge commits carry aSigned-off-bytrailer (the Developer Certificate of Origin sign-off this project requires) from a single author.reference-implementationfiles, so it cannot reach them.mcp-servertest suites, only their typechecks. And the accidental-deletion reading ofaa8019f1dis an inference from its diff and message, not something I confirmed with its author.Assisted-by: AI