fix: reconcile production fixes onto the reorganized main - #166
Draft
tnunamak wants to merge 278 commits into
Draft
fix: reconcile production fixes onto the reorganized main#166tnunamak wants to merge 278 commits into
tnunamak wants to merge 278 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tnunamak
changed the title
A registered, owner-actionable connector was invisible on /sources/add. isRunnableAddOffer paired the preview tier with only `experimental_opt_in`, which held until the first preview-tier local-collector connector shipped: enrollment issues a code the owner redeems on their own machine, so it resolves to `available_now` and matched neither arm of the condition. The backend reported it registered and owner_actionable the whole time; only the console withheld it. Key the preview arm on availability rather than on the one disposition that broke. `available_now` already means "the owner can add an account now from a shipped surface", which is exactly the property this gate wants, so a future preview disposition resolving to it cannot reintroduce the same invisibility. `requires_server_setup` and `not_available_here` still withhold the offer. Adds a tier x disposition matrix test asserting the observable contract -- whether an entry is offered -- rather than restating the branches. Under the old condition exactly the preview local-collector case fails and the other six pass, so it pins the defect rather than the implementation. Also updates connection-modality's pinned local-collector allowlist, which had not been regenerated since the connector was added. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 9b19aa3e8649853d062251ee2a9f36dc622c0877)
The uploader built its error text with String(body.error ?? statusText).
The RI host always shapes that field as an object -- pdppError writes
{code, message, type} -- so String() on it produced the literal
"[object Object]" for every host-side failure.
On this instance that discarded the cause of 24 quarantined Gmail
attachment gaps, each recorded as
"blob upload failed (503): [object Object]". The 503 itself may well be
transient, but nothing downstream could tell, because the only evidence
was a stringified object.
Read .message out of the object shape, falling back to a plain string
field and then to statusText. The uploader is shared by groupme,
imessage, signal, whatsapp and apple_photos, so all of them stop
throwing away host error detail.
This restores diagnosability; it does not retry the existing quarantined
gaps. Those are sticky by design and need an explicit requeue once this
ships, at which point the real 503 cause becomes visible for the first
time.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 457e23e93ac25f158f0838137c02258849335e03)
The Sources view told the owner "hit a transient read interruption, retrying automatically before showing an error", auto-retried exactly once, then parked on a manual Retry button. The owner reported that message sitting on screen for an hour. The read was never the problem. "Destination stream closed early" comes from React's RSC streaming writer when the HTTP response closes while the Flight stream is still being written -- the /_ref/connectors read had already returned 200 in ~1.2s. Most plausibly the page poller's router.refresh() superseding an in-flight render, or the tab throttling the connection. There is nothing to fix at the fetch layer; refFetch already awaits its json and sets cache: no-store. So the boundary renders the same skeleton loading.tsx uses, and once a last-good timestamp exists, a dimmed "Updated Xm ago" beneath it. A teardown now looks like an ordinary page load. Retry is unbounded with capped backoff (300ms to 15s) -- no terminal give-up state, no manual Retry button. The retry counter lives at module scope, not in state: the boundary remounts fresh on every catch, so a useState counter resets to zero each time and never actually backs off. The invariants test pinned the old copy. It now pins the property the copy was violating -- the sources view renders no failure language in any state. Seven sibling segments (/syncs, /audit, /grants, /event-subscriptions, /device-exporters, /schedules, /deployment) plus the console root share this pattern and mostly do not even auto-retry. Same fix applies; not in this commit. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit fb10d94810349eb32fd2b019343fe8f2201d486d)
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)
fb10d9481 fixed /sources. Seven sibling segments and the console root had the same defect and mostly worse: a full-viewport takeover with "Couldn't load X" and a Try again button, and unlike /sources, no auto-retry at all. A stream teardown the owner should never have learned about became a dead end requiring a click. Each leaf boundary now renders the skeleton its own loading.tsx uses -- list or detail, matched per route -- and retries unbounded with capped backoff. A teardown is indistinguishable from an ordinary load. The root boundary is deliberately NOT unbounded. The dashboard already fault-isolates its own reads through safeRead, so anything reaching the root is either the same stream race or a genuine unhandled fault, and this codebase has no error reporting beyond console.error. Retrying a real crash forever would erase the only signal an operator has. It stays quiet for five attempts, then falls back to the existing panel -- strictly better than showing that panel immediately, without hiding a hard failure. Only the backoff arithmetic is shared. Each route keeps its own module-scope counter: one shared counter object would let a failure on /grants corrupt the backoff on /schedules. The counter must live at module scope because Next remounts the boundary on every catch, so useState resets to zero and never actually backs off. components/segment-error.tsx is now unused by every former caller. Left in place; deleting shared files belongs in its own change. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 29b81745cf307a33024ddfde577d0384ab43c1b2)
An H-E-B run failed with connector_error_json message "heb_session_failed: [REDACTED]". The cause was the string [REDACTED]. The redactor exists for a good reason -- it strips bare tokens of 24+ characters from connector-authored messages so an unlabelled API key in a stack trace never lands in a durable row. But it matches on shape, not content, and "heb_verification_code_not_provided" is 35 characters of clean snake_case. The rule ate the answer. The unredacted channel already existed: TerminalError.code flows to connector_error_code without passing through the redactor, and boundConnectorErrorCode already validates it against a strict charset. Nothing populated it on this path. establishSession's catch -- the one choke point every connector's ensureSession failure crosses -- now promotes a thrown message to code when it matches that same charset. A compound message with spaces or colons still fails the test and still gets only the redacted message, so nothing PII-shaped rides through. Because the fix is at the shared boundary it also covers amazon and chase, which throw the same convention. USAA throws human sentences and is deliberately untouched; changing its error text is its own change. The real cause survived only in spine_events.known_gaps, which is where I had to go to read it. That is the actual lesson: the durable row a person reads first was the one place the answer was destroyed. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 46887c2e84667872d7d55af1f7488649222b2469)
design-notes/ has been tracked since the initial contribution, but six notes were sitting untracked -- including two from August 7 and 17. They would not survive a worktree cleanup, and nobody but their author could see them. Three are from today's production work and record findings that outlived the incidents that produced them: - failure-diagnosability: five failures in one day whose causes were destroyed before anyone could read them. Includes the corrected scale (three real sites, not the 246 a bad grep suggested) and the finding that the redactor is an entropy heuristic, not a PII control -- it eats clean reason codes and passes an email address through untouched. - source-state-truth: why a source holding 2.4M live records renders as "Not measured", and why a finished manual import can never be green under a model where every condition is required. - summary-evidence-projection-controller: an independent reviewer's terminal design for the maintenance sweep, after four starvation bugs of the same shape in twenty-four hours. The other three predate today and were simply never committed. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 54d20febc5bf59c8e56fe3269ffc2bb72273bc42)
… need Nine pending gaps across five streams, four distinct causes. Two are fixed here; the other two need one instrumented run, which this also arms. Credit-card billing and balances each carried two gaps created by a single crashed run. The next full run scraped both cards successfully, but unlike transactions -- which has a complete serve-then-recover lifecycle -- these two streams had no recovery-emission path at all, so the gaps could never clear no matter how many successful runs followed. Generalized the transactions pattern to cover them. The inbox stream identified 13 rows and covered zero. Every row is dropped when date_short is empty, and all 13 failing at once reads as column-index drift in the fixed-position scraping rather than 13 bad rows -- but the correct mapping cannot be guessed without the real DOM. It now emits an inbox_rows_unresolved skip result instead of failing silently, and captures the listing DOM so the next run answers it. Statements identified 10 of 10 and downloaded none, four times, with the download listeners armed and seeing nothing for 45 seconds. The listeners are page-scoped and this file already documents that Playwright downloads never reach context-level listeners, so the likely cause is the Download menuitem opening a page the listeners cannot see. Added a context-level new-page watcher, which tests that directly. Credit-card export was already instrumented and needs no change; it was simply never run with capture enabled. Worth recording: collectUsaa never calls reportStreamFailure, so all three defects surface as skip results and the run reports success. Capture must therefore be armed with PDPP_CAPTURE_FIXTURES, not PDPP_CAPTURE_ON_FAILURE, which would delete the evidence for exactly this run. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 1a0e297ef60ff573a9952daeeb0cef866f43982f)
…n failed The owner re-authenticated ChatGPT repeatedly over days. His session cookie is valid until 2026-11-16. He was never logged out. navigateAndProbeSession fired /api/auth/session exactly once, three seconds after page load, and checkSession collapsed every failure -- network blip, non-2xx, Cloudflare interstitial -- into "logged out". One transient false opened the full credential-login flow, which hit an app-approval prompt on an account that was already authenticated. Sometimes that auto-resolved; sometimes it hung the whole 900-second assistance budget and failed the run. Either way the owner got a notification he had to act on. Now the probe retries four times, 1.5s apart, before concluding a session is dead. Six seconds worst case against a fifteen-minute hang plus a login. Two diagnosability defects found underneath it: run_history.failure_reason was hardcoded null on every completed run -- buildSuccessOrFailureRecord read a type that never declared the runtime's failure_message even though the object always carried it. That is why every failed row all day had an empty reason and the cause had to be dug out of spine_events. Widened the type and read the field. The runtime's own resolution never carried a specific assistance-timeout message either; that lived only in the terminal spine event via duplicated logic that had drifted from its sibling. Unified both onto one helper and corrected a timeout mislabelled as failure_origin: connector when the runtime caused it. The RS ingest path classified per-record storage failures onto an in-memory outcome object and logged nothing, so a deterministic 503 on a 500-record batch was invisible. It now logs the real cause, connector instance, run and stream, without leaking into the client 503 or the mutation.rejected event. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit daace02700e0a8a61b6fcd992a0b60504fa4bde1)
Every published @pdpp/local-collector from 1.5.1 to 1.5.4 crashes on any invocation, including --version. Line 1 of its packed local-device-client.js imports @pdpp/reference-contract, which is not in dependencies and does not exist on the registry. It resolves through the pnpm workspace link, so it works for every developer and no user. Three of the owner's collector machines were broken by it today. Two gates should have caught it and neither could: validate-package.ts asserted no @pdpp/* appears in the DECLARED dependency sections, and separately checked that RELATIVE import specifiers resolve inside the tarball. Nothing looked at bare specifiers in the packed code, so an undeclared workspace import passed clean. @pdpp/cli and @pdpp/mcp-server had the identical hole. pack-install-run already existed -- it packs, installs into a clean project outside the workspace, and executes the binary. It would have caught this outright. It was not in local-collector's verify. mcp-server had the same omission; cli and read-core were already wired. Now every packed .js/.mjs/.d.ts is scanned for static imports, re-exports, dynamic import() and require(), each bare specifier resolved against Node builtins and the manifest's real dependencies. A private @pdpp/* culprit names this incident in the failure message. The check is generic on purpose; a @pdpp/-specific regex would only catch the one we already know about. Proven on the real artifact rather than a mock: reintroduced the offending import, built, and watched the packed-tarball validator fail; reverted, rebuilt, watched it pass. Two pre-existing drifts fixed because they blocked the verification: signal missing from the expected-connector lists, and mcp-server not handling npm 12's object-keyed pack --json shape, which local-collector already handled. Disclosed and not fixed: pack-install-run's iMessage sample smoke reports records_seen 0 while the outbox shows sent 1. Reproduced on a clean tree, so it predates today. It is the last thing standing between local-collector and a fully green verify. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit 7ad9fe610101b66d67e9cd52c7922dd5feb6c6d3)
Reddit collected on a 12-hour schedule from April through 2026-08-05. Since
credentials were re-added on 08-17 every run has failed at session
establishment, zero records, seven in a row.
The failure was reddit_session_failed: reddit_login_post_submit_failed on a
fully automated run with no owner interaction requested at all -- the form
filled and submitted correctly, then the connector never found a session.
Post-submit, the 2FA check asked whether an OTP field was visible using a
hardcoded one-second isVisible probe, while the pre-submit username field
gets a ten-second waitFor. Reddit paints the OTP step in a second client-side
render pass that routinely takes longer than a second after
domcontentloaded. When it does, the connector concludes there is no 2FA,
never asks the owner for the code, and spins out a dead ninety-second cookie
poll before failing. The owner is never prompted, so from the outside it
looks like the login simply did not work.
Wait for it properly, five seconds, matching the pattern the username field
already uses. The regression test scripts the OTP field attaching at 1.2s:
before the fix zero interaction requests are sent and the run throws; after,
the owner is asked for the code.
Also fixed the shared makeLocator fixture, whose fake waitFor ignored the
state option -- a genuinely hidden field would have passed a
waitFor({state:"visible"}) check, which would have made this test lie.
The separate manual-login failures look like a real Cloudflare challenge on
a browser profile whose cookies were wiped by the 08-16 delete and recreate.
That is an external condition, not a defect, and the existing manual handoff
is the right mechanism for it.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit b28cb8c24e9a168d825612c3c36189c0e5317e3c)
Three connections labelled "historical archive N of M" sit under "Needs you -- requires your input before collection can continue" on /syncs, each saying "Reconnect this account and collection resumes." That sentence is false. These are the residue of a delete: on 2026-08-16 the connections were removed with a proper cascade and tombstones written, then re-created under a different binding-key prefix so the resurrection guard would not fire. What is left has records from real past runs, no credential row, and no schedule. Reconnecting would not resume anything -- it would create a new connection. The owner has said plainly these are history. CredentialsValid is right that no credential exists. The bug is the remediation copy derived from it, and where that copy is shown. The server already computes source_visibility: "hidden_from_sources" for exactly this shape, and the Sources list already honors it. That exclusion was simply scoped to one consumer, so the same row still generated a live prompt on /syncs and the dashboard. Read the field in the work-item builder too. No health condition changes and no server change -- the right primitive already shipped, one more caller just needed to use it. Distinct from the finished-manual-import case (Google Maps, WhatsApp), which needs a genuine terminal state because its question is unanswerable rather than merely misrouted. A companion test proves a visible needs-owner connection with the identical verdict still surfaces, so the fix suppresses the fragment and nothing else. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit bbbf780b272e1a611dce40a4a78e1c3109c19454)
An engineer migrating this instance read the diagnosability note and found
the same defect in their own verification gate within five minutes:
tgt_rows=$(psql -Atqc "SELECT count(*) FROM ${tbl};" 2>/dev/null || echo "ERROR")
That is [object Object] in bash. Permission denied, a missing table, a
dropped connection and genuine data loss all collapse to one string, and the
gate exits non-zero having destroyed the reason. It appeared twice. Worth
recording because it proves the rule is not about TypeScript or error
objects -- it is about any transform that keeps the fact of a failure and
discards which failure.
Their session also reframes the scope. Four of their five findings were
nothing-reported-it rather than it-broke: Postgres crash-recovering behind
RestartCount 0 and green healthchecks, meilisearch crash-looping behind a
healthcheck that probed only its own port, promtail dying on a full disk
silently, and pg_dump exiting 0 with an unrestorable dump. Our five were the
narrower shape -- something reported a failure and destroyed its cause. The
wider half is worse, because the narrow one at least leaves a row to
investigate.
So the invariant gains a second clause: a component whose failure is
survivable must still be observable. A supervisor that restarts or a probe
that recovers does not make a process healthy, and something must say so.
This does not widen the remediation. Still three sites; the 439-catch
measurement and the 97%-benign classification stand. It widens what counts
as evidence that a component is healthy, which is a monitoring question.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit df8cf31d157e6279f32b3cf5bee1ae8a8cc90057)
The engineer who contributed the second variant produced the third an hour later by making the mistake himself. Checking whether the sweep's backlog had drained, he grouped by record_snapshot_state, saw 29 of 29 current, and reported the board clean. The real backlog was nine rows and the sweep had logged seventy consecutive no-progress passes. dirty is a different column. Nothing was hidden, destroyed, or masked. The failure was recorded correctly, in a field he did not read. That is the same error I made this morning from the other direction -- reporting 15 of 21 green from evidence columns while the Sources page disagreed. connector_summary_evidence carries seven signals and none of them is the verdict; the verdict is isHealthyConditionSet, ten conditions evaluated together. Any reader who samples one column gets a plausible answer that is not the answer. So: a system with N independent health signals and no single authoritative one invites every reader to pick a different signal and be confidently wrong. The remedy is not better logging -- the logging was perfect. A verdict needs exactly one source, and the raw signals must be hard to mistake for it. record_snapshot_state reads like the state of the record snapshot, which it is, and like the state of the row, which it is not. All three variants end with an operator holding a wrong conclusion. Only the first destroys anything and only the second hides anything. The third needs neither -- it is enough to offer several true answers to slightly different questions and let the reader choose. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit b5d2f6f9a6bc1ef4376f6a1dcc68c291100eba47)
…rminal The sweep sat at 70+ consecutive no-progress passes with rows dirty and candidates_inspected 0. Deleting the fleet-wide record count (a5505bb59) was necessary and did not fix it. Two failures remained, both from one cause: spine_events has an index covering four terminal event types and nothing covering the rest. Discovery's maxLifecycleSeq query groups MAX(event_seq) by connection with no event_type filter, so it falls outside that partial index and does a parallel seq scan -- measured 1.1s against 1.4M rows on production, against a 500ms per-unit floor. It is cancelled every pass, and because discovery's seven queries share one failure boundary, the cancellation aborts the batch before anything is classified. The second is deterministic and was the dominant one. Enrolling Signal today created a connection with zero spine_events and no evidence row. Its repair runs MAX(event_seq) WHERE connector_instance_id = -- same missing index -- and times out even with zero matching rows, because an unindexed predicate cannot be pruned by absence of matches. The row therefore never gets evidence, and every walk-tranche page reaching it throws "Cannot publish connector list summary without canonical evidence", which isExpectedProjectionRace does not forgive. On walkFirst ticks that killed the acceleration tranche outright -- the only path that reads the dirty rows. Add the general (connector_instance_id, event_seq) index on both backends, following the precedent the terminal-scoped index already set. EXPLAIN confirms both queries now plan index scans. SQLite needed a follow-on: with two candidate indexes and no partial-index selectivity stats, its planner started preferring the general one for terminal-scoped folds, about 1.5x slower. The three terminal fold queries now carry an explicit INDEXED BY hint. Disclosed: connector-summary-sweep-stuck-page-starvation.test.ts now fails its FAIL-BEFORE case because the queries are genuinely fast enough that its hardcoded 100k-row backlog no longer starves anything. The starvation it pins is less severe rather than gone; the calibration needs revisiting by whoever owns it. Left untouched rather than guessed at. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit bb0e02ef139d0e1895fa73a87f384499247818e3)
Two sources holding 419k records between them -- a Google Maps Timeline export and a WhatsApp archive -- render as "Not measured · Freshness has not been measured yet". Both are source_kind=manual, paused, with zero runs ever. They are finished one-shot imports that will never refresh, so Fresh and SourceCoverageComplete, which both presuppose a future run, can never be satisfied. The owner reads that as broken. The carve-out was tried twice and reverted (e47ffc632, 8b1838e85) because it contradicts the generation fence, and a test locks the current behavior in as correct. So this does not force them green. Fresh becomes not_applicable -- not true, since calling a 2023 export fresh would be a second lie -- and coverage stays required. not_applicable already existed as a condition status whose own doc comment says it exists "so the projection stops encoding certainty as doubt", but it was confined to presentation. This makes it load-bearing: a condition that does not apply counts as satisfied, one that is merely unknown does not, and inapplicability may only come from durable evidence that the question is meaningless. That last clause is the whole safety property, and two tests pin it -- a source with the identical shape but no acquisition declaration stays grey. Inapplicability is derived from source_kind, a CHECK-constrained fact written once at creation, never inferred from an absent run. A second gap the design note had not anticipated: rendered-verdict and owner-state read the raw freshness axis and evidence.source directly, so they would have rendered "Not measured" regardless of the condition. They now read the typed Fresh condition instead. These two rows will not resolve today. terminal_facts_historical forces ProjectionReliable false at step 1 of 14 and returns before the healthy classification at step 14 is reached. That is a separate upstream defect in the evidence fold, disclosed rather than worked around. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit b75b48bde74501e62638944e58e19b079fe3740e)
Signal's manifest says preview and that is correct -- it is live in production with 6448 records and shows under Preview on /sources/add. The conformance roster still listed it as development-only and its own integration test asserted the same, failing three tests against a manifest that was right. Steam's mutation table still expected steam_response_malformed when GetRecentlyPlayedGames omits games entirely. Commit 3ccca8000 deliberately made that valid: the documented shape for an account that has played nothing in two weeks is a response with total_count and no games array. The test asserted behavior the code had intentionally shed. Replaced with two explicit cases so the fix cannot silently widen -- absent means empty, present-and-not -an-array still throws. The iMessage smoke was the interesting one. records_seen 0 while the outbox showed sent 1 looked like a miscount; it was neither a miscount nor a connector defect. The synthetic fixture pinned its dates to a fixed Apple epoch base around March 2023. On 2026-08-09, two days after that fixture was written, an undeclared-scope default of 30 days landed -- deliberate, owner-protective behavior that gives an enrollment declaring no boundary a recent-history window. Every one of the fixture's 500 rows fell outside it and was correctly filtered. The single sent item was the checkpoint, not records. So a correct feature silently invalidated a correct fixture, and the smoke had been failing on a true negative ever since. The fixture now computes its base from wall clock rather than a constant. pnpm verify for local-collector is green end to end for the first time: 204 unit tests, validate:package, and all six fixture smokes -- three of which had never run at all, because iMessage crashed before reaching them. Flagged, not fixed: google_messages and google_takeout pin timestamps the same way and are equally time-scopable. They pass today by luck of the window. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit c4d83f85e1e58d7dfa7e1cb89502e61de3c6af17)
The owner saw "Connector code needs a fix" on his own sources and said it was a terrible thing to show a user. He is right, and it was in four places. Three faults, not one. It is developer language on an owner surface -- he is running self-hosted software, so being told the code is broken is both unhelpful and faintly accusatory about software he chose to run. It names who is at fault instead of what happens next. And it reads as total failure on a source that is still holding real data and still collecting most of its streams. The file already had the right register one line away: "Coverage gap needs review" for the softened case. The hard case now says "Some data from this source can't be collected" -- a fact about his data rather than a diagnosis of ours. The unmeasured case drops "a connector update is needed" for "Some data from this source isn't being measured yet", which is also more honest, since evidence can still arrive. The line worth getting right is that some of these states genuinely are our defect and not something he can act on. The copy must not invite an action he cannot take, and must not imply everything is fine when coverage really is partial. Naming the consequence does both. Vaguer is not kinder -- "Something went wrong" would be worse than what was there. These stay specific. One of the four is a string-equality guard that branches on the CTA text, so it had to move in lockstep. That coupling is pre-existing and fragile; left as found rather than refactored under a copy change. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit ece557fa43b1b1d5fc65a2cce4bbf622e55404c9)
The owner asked where Venmo was. It has never been addable: no connection, no tombstone, no records. The manifest ships and the connector is complete, but public_listing.tier was development, and three separate gates withhold that tier outright -- sourceSetupAvailability returns not_available_here, isRunnableAddOffer never reaches an offer branch, and sourceSetupAction returns null, so even a rendered card would have no button. Confirmed against the live page: chase and signal appear in the markup, venmo does not. Development was an honest tier when it was set -- the connector's own header comment says no live network call has proven it against a real account. That is still true. But it is the same situation Signal was in, and Signal is listed at preview with a rationale saying exactly that, so the owner can perform the first real run. Venmo has 70 passing tests across parsing, schema, redaction, fixtures, and the auth-flow retry-safety guards. So: preview, not supported. The Preview disclosure already tells the owner these paths have not completed live validation, which is the honest framing for a connector that is code-complete and unproven. Field-by-field against chase, the closest browser-bound comparator, the tier was the only structural gap. Everything else is legitimate connector-specific content. One existing test pinned Venmo at development to verify the legacy-UAT exposure gate. Repointed it at spotify, which is genuinely development-tier, so the gate still has a valid subject. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit ea26e6fa3178f327cc80bc78737e89fadf3b369c)
The owner re-authenticated ChatGPT repeatedly today and asked why. This is why: PDPP_BROWSER_PROFILE_ROOT is /var/lib/pdpp/browser-profiles, baked into the image, and the owner stack mounted nothing there. Every browser profile lived in the container's ephemeral writable layer and was destroyed on each restart. Twelve deploys in one day, twelve logins. It affects every browser-backed connector -- ChatGPT, Reddit, Amazon, Chase -- not just the one he noticed. And it compounded a separate bug fixed earlier today, where a single-shot session probe manufactured re-auth demands against a live cookie: that one cried wolf, this one made the wolf real. deploy/docker/docker-compose.yml already mounts pdpp-data:/var/lib/pdpp. The owner stack simply never did, and neither declared the volume. Both now match. Recovered rather than reset: the running container's 110MB of live profiles, including a Reddit session the owner had just solved a captcha for, were archived out and restored into the new volume before the mount was added. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit ef06f3e65b995f77313830f358c824a74c2751ba)
Two finished imports holding 419k records render "Not measured" and cannot recover. Both have zero terminal run events, ever, fleet-wide -- verified against spine_events. The fold's own documentation says such a row converges to current via the bootstrap path and never reaches terminal_facts_historical at all. These sit at historical anyway. The bootstrap path is unreachable in production. The sweep folds pages of about 25 instances at a time, and almost any page contains some connection with real terminal history, so the scope's max terminal seq is never null and the zero-history branch never fires. The existing lock-in test only exercised a singleton scope, which is why it never caught this. Then seedFoldState seeds each row's generation-current flag from its own stored reason code. Once anything stamps a zero-event row historical, that seed reads false on every later pass, and a row with no events has an empty drain, so nothing can ever flip it back. It rewrites the same wrong verdict forever. Seed true when the instance has no attributable terminal events at any generation, overriding a stale stored reason. That restores the documented behavior without touching the generation fence: a row with genuine historical events still seeds false and still forces ProjectionReliable false, which the lock-in test verifies. The alternative -- exempting classifyUnreliableProjection for sources whose acquisition is complete -- is the carve-out reverted twice already. It would let a real generation-mismatched fact pass as reliable for any manual source, which is the thing the fence exists to stop. No migration needed. Both rows are already behind the fleet high-water, so they re-enter on the next pass and self-heal. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit c1c6c2c019d357bd3b99a9a54ac40478f75405ad)
The owner solved a captcha in the streamed browser, clicked continue, and the run failed 315 milliseconds later with reddit_login_unexpected_ui. He did this twice today. Both times the work was wasted. His session was fine. I pulled the Chromium cookie DB out of the live profile and read it: token_v2, loid and session_tracker all present, last_access stamped at exactly the moment of failure. Nothing in the connector or the browser launcher clears cookies or resets a profile on failure. The login succeeded; the connector just never looked again. manualBrowserLogin calls its probe exactly once, immediately after the owner's interaction resolves. Reddit wires isSessionLive straight into that. With no credentials configured, isSessionLive does a real navigation to old.reddit.com and checks for a logout link -- which takes time right after a captcha redirect settles. The single check read a transient not-yet as a permanent no. Poll instead: 15 seconds, 3 second interval, overridable for tests. Same shape as the OTP fix earlier today, in the manual-handoff path rather than the automated one. Scoped to Reddit's two call sites rather than the shared browser-handoff helper, which chase, usaa and venmo also use and which has its own passing contract test. They may want the same treatment; that is a separate change with its own evidence. Sized by precedent, not measurement -- I could not reproduce a live captcha without the owner. If Reddit serves a different post-captcha shape, this fixes the timing but not that. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit dda456e8c30631bd604442182a1c631efa845382)
Signal collected 6448 records cleanly today -- evidence fresh, all seven
state columns current, all 67 ingest batches accepted -- and shows "Can't
collect" anyway.
The verdict comes from the heartbeat, not the evidence. Its outbox carries
one gap row from a failed attempt at 19:22:45 ("spawn tsx ENOENT"),
superseded by the successful run a minute later. The collector reports any
nonzero backlog as blocked, and deriveOutboxAxisFromHeartbeat's blocked
branch looked only at dead letters, so a single piece of debris from a
superseded attempt was indistinguishable from a genuinely unreadable
exporter.
Carve out the bounded case: dead letters zero, backlog known and at most
three, no pending records, heartbeat not stale. Any of those failing keeps
the old classification. An unknown backlog never qualifies, so an older
collector build stays conservative.
Also adds unfillableAccounted to the coverage evidence, so a caller that can
prove every outstanding gap is permanently uncollectable -- a recorded byte
size against a recorded cap, not an attempt count -- can report coverage
complete with an honest reason rather than blocking forever on data that
cannot be fetched. Retry exhaustion is deliberately NOT such proof: 117
failed attempts show the strategy has not worked, not that the item is
impossible. Three anti-false-green tests pin that.
Nothing populates unfillableAccounted yet; the read model that would is
owned elsewhere. The runtime accepts the evidence, which is the slice that
belongs here.
Signal will not resolve until its collector heartbeats again -- its last is
now stale past the threshold, and staleness still blocks, correctly.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
(cherry picked from commit 73f1f78705d9ef25ee1b45734b60cf1b2e43ce47)
Two sources holding 419k records between them can never be green. Both are manual imports with zero runs ever and zero rows in connector_detail_gaps, run_history or acquisition_batches. Coverage evidence comes from a collection run; nothing will run again. The adjacent Gmail case looks identical from the pill and is not. Gmail has 32 terminal gaps each carrying observed_size_bytes against a recorded cap -- it measured its shortfall and can enumerate it. These imports never measured at all. Measured-and-provably-impossible and never-measured-and-never-will-be are different states and must not share a signal; asserting the former over an empty set is the false-green the anti-green tests exist to catch. I had been treating them as one design. An agent refused that framing on the evidence and was right. The line already in the code holds: Fresh can be satisfied by not_applicable, SourceCoverageComplete cannot -- it is gated by conditionIsTrue at connection-health.ts:1830. A completed import buys exemption from a freshness proof, never from proving it ingested what it claimed. So the answer is not to exempt coverage but to let an import prove it. connector-coverage-policy.ts already declares a snapshot_import_receipt strategy that nothing emits -- the placeholder is the design, unbuilt. It needs a receipt written at upload time, a manifest declaration, and a read-side branch. Until then these two stay honestly red, which is correct. Green by exemption would mean the page can no longer tell a source that proved its completeness from one that never tried. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> (cherry picked from commit c44c179bb0804c638835531c5c2dda98e1325721)
…an't keep Google Maps Timeline Import (cin_50f5bf4b7ecbc7acd6f4c254, a one-time, never-refreshing import) rendered "coverage unknown" with copy that implied measurement was merely delayed: "Coverage has not been measured yet.", "Some data from this source isn't being measured yet." Both phrasings say "yet" — a promise that a future run will resolve it. This import's own freshness copy already says otherwise: "This is a one-time import. It finished and will not refresh." There is no future run, so "not measured yet" is false for this shape. The underlying connector logic was already correct (DETAIL_COVERAGE is emitted only once a full parse pass completes; see the runtime network-retry fix in fix/googlemaps-import-coverage-0822 for why that pass never completed). This is purely a copy defect: the forward_statement, per-stream statement, and maintainer required_action CTA that fire on unmeasured coverage never checked whether the connection is a finished one-time import before choosing "yet" wording. Fix: branch each of the three copy sites on freshnessNotApplicable (the same signal the freshness annotation already uses to say "will not refresh") to say precisely why coverage can't be measured, instead of implying a future run will fix it. Ordinary still-refreshing sources keep the existing "yet" wording unchanged. Extracted the branching into two named helpers (unmeasuredCoverageForwardStatement, unmeasuredRequiredStreamsCta) to keep buildForwardStatement/ buildRequiredActions under the cognitive-complexity ceiling. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…tLabel The reference server has legitimately emitted "Archived" (ref-control.ts:6619) and "Setup never completed" (ref-control.ts:6629) pills since the orphaned-record-visibility and venmo-surface lanes landed, but the console's hand-maintained mirror of the server's VerdictLabel union (runtime/rendered-verdict.ts) in ref-client.ts:893 never picked up either string — a pre-existing gap flagged, but not fixed, during that work. A strictly-typed caller switching on RefVerdictPill.label would silently fail to narrow either real server value. Widened the union to the exact ten labels runtime/rendered-verdict.ts's VerdictLabel declares, and added a source-scan test (ref-client.ts cannot be imported by node:test — it pulls in server-only transitively, same constraint ref-client-pagination.test.ts documents) that pins every required label so the two unions cannot silently re-diverge again. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…rod-0822 Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
… sources peregrine Claude Code renders "Can't collect" with one dead-lettered record out of 10,001 — correct per the terminal-only freshness carve-out already deployed (0785345). But the record's identity/error is invisible: device_source_instances.last_error_json is null even though the collector sends it. Root cause: markSourceInstanceHeartbeat unconditionally overwrites last_error_json on every heartbeat. The local collector (vendor/pdpp-collector-runtime) only attaches last_error on its run-completion and corrective heartbeats; its far more frequent "starting"/skip-scan heartbeats omit it, nulling out the dead-letter detail on the very next heartbeat after it's recorded — while the aggregate dead_letter count (which does survive every heartbeat) still says the record is stuck. The owner is left with "1 failed upload" and no way to see what it was, contradicting the no-invisible-data principle. Preserve last_error_json across a heartbeat that omits last_error only when the incoming outbox diagnostics still report dead_letter > 0; clear it when the count drops to 0 (recovered) or diagnostics are absent, matching the existing conservative-on-missing-evidence default. Investigated but not changed: peregrine Signal Desktop's "Freshness has not been measured yet" is an honest read, not a bug — its collector's last heartbeat is ~13 hours old (30-minute staleness threshold), and its device rows already correctly rebind to one connector_instance_id across re-enrollments. No attribution defect found. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…facts_json Investigating run_1787406305278 (google-maps, 2026-08-22) showed `controller_terminated_before_run_finished` was correctly stamped -- the container was replaced in a rolling deploy (predecessor container SIGTERMed at 13:55:28Z, one second after the run's last live batch), and the new boot's reconciler correctly adjudicated it as a prior-epoch orphan. But confirming that required correlating raw spine events against container logs across the deploy: `docker inspect`'s RestartCount stayed 0 because Docker doesn't count a container recreation as a restart, only an in-place one. All 30 controller_terminated_before_run_finished runs and the 15 legacy controller_restarted runs since 2026-08-15 checked out the same way -- none were mislabeled. The real gap is that the terminal reason alone is an unverifiable claim. The boot reconciler and the adjudicate-orphaned-runs repair script both already write the epoch-transition evidence (original_boot_epoch, reconciled_by_boot_epoch, etc.) into the spine event's data_json, but neither ever projected it into run_history -- both hand-roll their own run_history UPDATE outside the general run-history-writer.ts path, so it never picked up facts_json. Pass it through so a reader can confirm the claim from the row alone. The drift-repair queries (which reconcile ANY terminal event type against a stale 'running' row) now adopt the terminal event's data_json wholesale, for the same reason status and terminal_reason already do. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Investigated the confirmed regression: a fresh backfill on 2026-08-17 captured only 25 of 101 cancelled Amazon orders that a 2026-06-02 backfill of the same account had captured, with the 76 missing rows 100% delivery_status=Cancelled, 100% Subscribe & Save-shaped, on recurring monthly dates from 2018-2023. No code regression was found. Every line between page.goto's URL/query params, list-page pagination (PAGE_LIMIT/START_INDEX_STEP), year discovery/planning, the DOM card selector, parseOrderCard/findOrderId, and the shape-check schema is byte-identical between the 2026-06-02 baseline (45e8ff8) and the 2026-08-17 deploy — confirmed by direct diff, not by trusting the connector's own accounting. The connector's own evidence backs this: the run's `considered` denominator matches what shipped with zero drop accounting, spine_events holds zero run.stream_skipped events of any reason for this connection ever (list_page_shape_check_failed, unparseable_order_date, selector_drift, amazon_empty_history_after_prior_orders), and none of the 907 detail-gap events from that run's per-run detail budget overlap the 76 missing order ids. record_rejections is a different, RS-ingest-level table and was the wrong place to look; spine_events.run.stream_skipped is the connector's own SKIP_RESULT trail, and it is empty too. The missing rows are not correlated with page position (roughly uniform across all 10 slots of a paginated page) or with a year/date cutoff (intact years surround the affected 2018-2023 band on both sides, which instead correlates with cancelled-order volume: 3-23 cancelled orders/year in the gap vs 1-5 outside it). The August run was also not one atomic pass — it was 8 runs across ~5 hours including 3 failures — but the entire years cursor (2005-2024) was written by a single run (19:54-20:08), ruling out a stale cross-run resume as the cause. The strongest fit for all of this evidence is Amazon-side: these specific old, high-volume, never-shipped orders rendered differently on Amazon's page in August than they did in June, in a way that parseOrderCard's `.yohtmlc-order-id` lookup does not match — dropping the card with literally no trace anywhere in the connector's diagnostics. That silent-drop behavior itself is not new: the pre-TypeScript-migration browser-evaluate code (commit 70f637235's parent) had the identical `if (!orderId) return null` / `.filter(Boolean)` shape, so this isn't a recent regression either. Regardless of root cause, a card that matches `.order-card`/ `.js-order-card` but produces no order id should not vanish silently — this fix adds visibility so a future occurrence (this account's or any other) is diagnosable instead of invisible. countOrderCardsWithoutOrderId (parsers.ts) counts cards parseOrdersListDom silently drops for lacking a parseable order id; extractAndShapeCheckOrders (index.ts) emits a new SKIP_RESULT (list_page_order_id_not_found) when that happens on a page that also proved at least one real order. The `rawOrders.length > 0` gate is required: Amazon's "Buy it again" recommendation carousel reuses the `.js-order-card` class on a genuinely empty year (orders-list-empty-year-with-carousel fixture), and that pre-existing, already-tested case must keep emitting zero diagnostics. Proof: parsers.test.ts adds a card missing `.yohtmlc-order-id` next to a normal card and asserts parseOrdersListDom still silently drops it (the unfixed behavior) while countOrderCardsWithoutOrderId reports the loss; the test was run against a countOrderCardsWithoutOrderId that always returns 0 to confirm it fails (0 !== 1) before the real implementation was restored. integration.test.ts adds an end-to-end scrapeListPage case over the same shape asserting the new SKIP_RESULT fires with dropped_card_count: 1, and confirms the pre-existing carousel-only empty-year test still emits no diagnostic. Full amazon suite (165 tests) and tsc -p . pass. Does not affect the year-partitioned proven-empty guard (6f9143f58): this fix only adds a diagnostic on pages that already produced ≥1 real order, so a year containing only cancelled orders is unaffected either way by this change. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…6-consolidated-0822
…-0822 Signed-off-by: Tim Nunamaker <tnunamak@gmail.com> # Conflicts: # apps/console/src/app/(console)/sources/sources-view-model.ts
…med_out A scheduler wall-clock timeout discarded the connector's real terminal counts. runTimedOutError() built a fresh error object with only classification fields (failure_reason/terminal_reason/message/run_id/ trace_id), dropping records_emitted, known_gaps, reported_records_emitted, and checkpoint_summary even though runConnector's own result already carried the correct values from durable ingest accounting. Every timed-out run therefore reported records_emitted: 0 and known_gaps: [] regardless of how much work it had durably committed before being killed. Reproduced against production: run_1787407222861 (Slack) durably wrote 402,494 messages, 91,202 reactions, 81,857 message_attachments, and 19,936 files, and still recorded records_emitted: 0, known_gaps_json: []. The fix is to forward the fields runConnector already computed instead of re-synthesizing a narrower object; a genuinely empty run still reports 0, since the value now comes from truth rather than a hardcoded default. Separately, the same run was killed by the external-walk wall-clock ceiling (maxRunWallClockMs) while slackdump's own archive walk had already finished and only the local sqlite-to-Postgres read-and-emit phase remained -- a phase with no provider rate limit and no external dependency. Give connectors a way to declare that transition: a `phase_boundary: "local_only_phase_started"` PROGRESS marker (added via a local type augmentation over the vendored @pdpp/connector-protocol package, since its ProgressExtra type has no room for new fields) that permanently disarms the attempt watchdog for the rest of that run. The Slack connector now emits this marker right before its local archive read phase. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…y the full test run The seven-branch consolidation (fix/pr166-consolidated-0822) merged cleanly at the git level but left five real defects only the test suites caught: - fused-source-status.ts: SEVERITY_BY_KIND was missing archived/setup_failed, the two SourceStatusKind variants added by the setupFailed-field branches, causing a TS2739 typecheck failure. Ranked both at severity 0, matching source-actionability.ts's own "ranked first / ranked alongside archived" ordering for those states. - amazon.json: the cancelled-bisect branch added a new SKIP_RESULT reason (list_page_order_id_not_found) without a registered owner-facing display message, which reason-display-messages.test.ts enforces so owners never see a raw reason code. - receipt.ts: a new Postgres-only conformance test (from the quiet-expiry/ instance-remediation work) was never added to the skip-accounting allowlist, tripping the "unexplained skip" gate. - requeue-fabricated-too-large-detail-gaps.ts: this new Gmail-incident repair script hardcoded the connector id as a compile-time literal, violating the RI's zero-connector-knowledge architectural boundary (ri-zero-connector-knowledge-conformance.test.ts). Converted --connector-id and --stream to required runtime arguments with no default, preserving the tool's fail-closed safety model without any hardcoded identity literal. - requeue-quarantined-detail-gaps.source.ts: a new regression test in this branch introduced "gmail" literals into what the conformance scanner treats as production code (the file is discovered via a generated .test.mjs stub rather than a .test.ts suffix). Replaced with a generic placeholder connector id; the test exercises pure CLI arg-parsing, not Gmail-specific behavior. Also removed a stray /tmp/package.json (declaring "type": "commonjs") left over from an unrelated prior session on this machine, which was shadowing Node's ESM detection for every mkdtempSync(tmpdir())-based child-process test and caused 4 unrelated false failures in undici-parser-assertion-containment.test.ts. Full suites now pass clean: reference-implementation 9,525/9,525, apps/console 2,078/2,078, polyfill-connectors 4,650/4,650 (10 skipped, Postgres-only). Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
`commitState` runs only from `handleDoneClose`. A run whose CONTROLLER is
replaced mid-walk never reaches DONE, so it commits no cursor however long
it ran. A walk longer than the interval between deploys therefore never
converges -- a completeness failure, not a slowness one. Measured on the
owner's instance: 45 runs ended this way between 2026-08-15 and 2026-08-22,
concentrated in the longest walks (9 Slack, 4 Gmail, 3 YNAB, plus Amazon
and Google Maps).
The spec forbade this outright. `spec-collection-profile.md` said the
runtime MUST NOT persist STATE from a `failed` run, and closed the
abandoned loophole explicitly: "a process exit without valid DONE, or
cancellation MUST preserve the default fail-closed rule and persist no
staged STATE." A restart is exactly that, so there was no loophole to
exploit and the spec had to change. It now carves ONE narrow exception,
"Restart abandonment", and keeps fail-closed as the default everywhere else.
The exception preserves the invariant it carves out of -- a cursor must
never advance past records whose coverage was not proven -- structurally
rather than by trust:
1. Only a CONTROLLER-lifecycle death qualifies. A connector that reported
its own failure has an unproven state map by definition. Keyed on
terminal REASON, never status: restart-killed runs are stored under
TWO statuses (`abandoned` and `failed`), so a status-based check
misses 17 of the 45.
2. A stream that is a declared detail-coverage PARENT is excluded, since
it can face a DONE-time shortfall verdict. Eligibility is read from
the MANIFEST -- a connector cannot declare itself eligible (ruling
D10: qualification is proven, never self-declared).
3. A pending detail gap or unproven prefix coverage excludes the stream.
The uncertain case fails CLOSED.
The run stays failed/abandoned and every withheld stream stays eligible for
retry, so nothing here makes a run look more complete than it was.
Mutation-proven, three mutants, all killed: dropping the detail-parent
guard fails 2 tests (this is the safety property -- if it survived, the
guard would be theater); accepting any terminal reason fails 1; dropping
the pending-gap and unproven-coverage guards fails the suite. Restored:
7/7 pass.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…wn_gaps on timeout) Merges fix/run-honesty-0822 (4acc0ff) into the pr166-consolidated head (66999ff) so the fix for runTimedOutError() discarding real durable ingest counts (records_emitted, known_gaps, reported_records_emitted, checkpoint_summary) on a scheduler wall-clock timeout ships alongside production. No conflicts with the base. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…nt across controller replace) Merges feat/resumable-checkpoints-direct-0822 (2c44e57) into the run-honesty-merged head so the Restart-abandonment eligibility function and spec-collection-profile.md amendment ship alongside production. This commit adds the eligibility primitive and spec carve-out only; it is not yet wired into the restart/commitState call path, so runtime behavior is unchanged by this merge. No conflicts with the base. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
force-pushed
the
fix/sweep-fairness-and-transformer-bounds
branch
from
August 22, 2026 20:24
3988a51 to
feffd80
Compare
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