fix: make first-run self-hosting reliable - #84
Open
tnunamak wants to merge 88 commits into
Open
Conversation
Project manifest setup metadata into shared owner-facing form contracts and remove misleading provider setup CTAs. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Clarify shared source setup, app access, and automatic-progress copy across operator-console first-user journeys. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Document Slackdump's xoxc and d-cookie contract, normalize credentials without decoding URL-encoded values, and require a confirmed first-sync run before showing setup status. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Make generic connection runs collection/active-only by default and require setup callers to opt into draft admission. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Use the registered owner-template projection as the live catalog authority and fail closed on unproven actions. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Remove the `isReadyProviderAuthorizationEntry` compensating branch and its call sites in source-setup-presentation.ts and source-setup-catalog.tsx. `provider_auth_connect` is now an explicit case in every disposition switch; `providerAuthConnectEntries` and `unsupportedNetworkEntries` filter by disposition directly. The compensating function existed because `provider_auth_connect` was absent from all switches and fell through to default/unknown handling. After the catalog authority fix (202bb289), the live add-source surface reads disposition exclusively from the server template projection — the compensating branch was dead code for that path. This commit removes it and makes `provider_auth_connect` a first-class named case everywhere. 1783/1783 console tests green. Zero type errors. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
YNAB has a proven personal-access-token static-secret capture form in its manifest but was not in the live-proven roster, causing the setup form to be proof-gated. The catalog picker showed "Not available here" at /sources/add despite the form being fully functional at /connect/static-secret/ynab. Root cause: add YNAB to STATIC_SECRET_LIVE_PROVEN_CONNECTOR_KEYS to restore catalog authority and unify proof with form availability. Test coverage: added YNAB-specific test to connection-setup-plan.test.ts verifying supportState/proofGate/disposition/validationMode, and extended connection-catalog.test.ts to confirm actionability and correct route. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
run_1786072649511 (UAT container pdpp-pr81-uat, connection
cin_c2f766b7166a6184adf021aa) failed transactions retryably:
download_button_click_failed, locator.click timeout 10000ms, with
locator('mds-button#download') resolved. mds-button is a custom
element whose accessible name and interactive target live in shadow
DOM, so a resolved host-element CSS locator only proves attached +
visible, not that Playwright's click actionability checks (stable,
hit-testable, enabled) can be satisfied against it.
clickActivityControl/clickFileTypeControl already use a CSS-id-first,
semantic-role-fallback strategy for the same reason (Chase's MDS
elements are documented as unreliable for pure CSS-locator
interaction). Apply the same two-tier strategy to the Download button
via a new clickDownloadButton() helper, reusing the getByRole locator
already registered as a diagnostic probe for this label but never
wired into the actual click path.
Root cause is inferred from Playwright/MDS shadow-DOM semantics plus
this connector's own established mitigation pattern for the identical
symptom on sibling controls; no DOM/trace/screenshot capture exists
for this run because PDPP_CAPTURE_FIXTURES/PDPP_CAPTURE_ON_FAILURE
were unset in the container, so this could not be confirmed against
a captured shadow-DOM snapshot of the failing state. Needs a real-account
retest with capture enabled to close the loop.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
REVISE of ddd67efad: that fix targeted the wrong mechanism (a redundant pre-poll ahead of fill(), which already auto-waits) and its test only proved the pre-poll, not the real defect. Reset to base 3b94075 and re-investigated from ground truth. Root cause, confirmed from the preserved localhost:3012 container's pdpp.sqlite (run_1786072363359, connection cin_77e45b073c12aa61fa93988a): the credential saved correctly (connector_instance_credentials row, credential_kind=username_password, status=active) and static-secret injection built the right REDDIT_USERNAME/REDDIT_PASSWORD fragment (traced statically through connection-scoped-run-env.ts -> controller.ts runNow -> runtime/index.ts's spawn env, merged last). The run's own spine_events show run.interaction_required fired 1.1s after run.started with reddit.ts's exact "did not render expected inputs and no Cloudflare challenge was detected" message — meaning ensureRedditSession read the credential, navigated, and its `userIn.count()` check read zero matches almost immediately, before ever calling fill(). The operator then completed the whole login by hand in the opened browser stream: paste worked because it was the operator pasting, not automation — matching the reported symptom. Locator.count() takes a single synchronous DOM snapshot; Playwright's own docs call it flaky for existence checks and point at waitFor/an auto-waiting action instead. On Reddit's client-rendered login page, a count() read taken right after domcontentloaded can race the page's own render and see zero matches that would appear moments later. The fix replaces that one-shot count() with `waitFor({state:"attached"})`, giving the render a bounded real chance — the same shape chase.ts/usaa.ts already use ahead of their first fill. Added a test with a locator that attaches after a delay: fill() must receive the real value once the field appears, and the operator must not be asked to intervene for a field that arrives within budget. Verified this discriminates: swapped in the pre-fix reddit.ts and the test failed with fillCalls=[] — proving the old code never called fill() at all in this scenario, the same shape as the ground-truth run's evidence. Did not touch Amazon: its existing fillWhenVisible already retries count() in a bounded polling loop (not a single snapshot), so it does not share this defect. ChatGPT has the same single-shot count() pattern but no corroborating evidence was available to prove or reproduce it here, so it is out of scope for this change. Full polyfill-connectors suite: 2760 passed, 0 failed, 6 skipped. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Six discrete UAT defects, all fixed by extending the existing setup-status/owner-state/assistance authority rather than adding timers or connector-specific conditionals: - Slack/YNAB stuck on "First sync pending": the setup-status route discarded a genuinely in-flight run_history row (status "running") whenever no controller_active_runs row existed yet for the connection, so the projection never converged past first_sync_pending until the run went fully terminal. - /sources at_interval_ms crash: already fixed on this branch by c0f64f4; verified no live regression, no code change needed. - connector_instance_inactive on draft retry: runConnectorNowAction's connection-scoped call never passed run_admission, so the server defaulted to the active-only allowlist. Threaded run_admission: "setup" through both Sources action call sites. - "No browser action is waiting" before assistance arrives, and a browser view that outlives a resolved H-E-B login: both were the same ambiguous fallback branch in renderNoAssistanceSurface. Added hasResolvedBrowserSurfaceAssistance to distinguish "never had a browser step" from "browser step already resolved," giving the latter its own truthful hand-off surface instead of waiting on full run completion. - Chase run.started-only window read as "needs you": the draft- lifecycle short-circuit in resolveOwnerStateResolver won unconditionally over the owner-attention/collecting checks, so an active run with no interaction yet misread as generic setup-in- progress. Reordered so open owner-attention and active-progress are checked before the generic draft fallback, and extracted the fallback's condition into isDraftAwaitingFirstActivity to keep cognitive-complexity mass at its pre-change baseline. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
… success
A ChatGPT connection with 9,163 collected records had status revoked but
source_binding_json still read {kind: browser_enrollment_shell, ...} —
Sources hid it (RETIRED_SETUP_SHELL_BINDING_KINDS) while Explore still
showed its records. Root cause: activateDraftConnection only ever flipped
status draft->active on first successful ingest; nothing ever moved the
binding kind off the temporary setup marker, so a later revoke (TTL sweep
for browser shells, explicit owner revoke for the other two setup kinds)
wrongly treated a real, fully-collected connection as retired setup residue.
The defect is generic across every RETIRED_SETUP_SHELL_BINDING_KINDS member,
not browser-specific: static_secret_draft and manual_upload_draft have the
same gap. Adds a shared connector-instance-store primitive
(promoteSetupBinding, both SQLite and Postgres) that atomically rewrites
source_binding_json to a durable sibling kind (browser_collector,
static_secret, manual_upload) alongside the status flip, guarded by the
binding's current kind so it is idempotent and never touches an unrelated
row. Preserves each kind's setup-specific durable metadata still needed for
future runs (setup_fields, import_dir/import_dir_env_var) and never touches
the row's identity tuple (connector_instance_id/owner/source_kind/
source_binding_key). Promotion fires only on first accepted ingest, never
merely because a run started.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
… binding-kind coverage
promoteSetupBinding's guard only checked binding kind, not status: an owner
revoke landing between activateDraftConnection's read and the promotion
UPDATE could be silently overwritten back to active, resurrecting a revoked
connection. Add `status = 'draft'` to both SQLite and Postgres UPDATE
predicates, closing the window entirely at the database level.
The store now returns { instance, promoted } so callers can distinguish a
real promotion from a guarded no-op. activateDraftConnection (extracted from
its inline closure so the branch is independently unit-testable) returns
null and never attaches an activation schedule when promoted is false.
Audited every consumer of source_binding_json.kind for the new static_secret
and manual_upload durable kinds via grep, not typechecking: found and fixed
a real gap in ref-static-secret-setup-status.ts's SETUP_KIND_BY_BINDING_KIND
map, which was missing `static_secret` (accidentally still correct via a
manifest-based legacy fallback, but not by design). All other consumers
(credential capture/repair, manual-upload run-env resolution, console
repair routing) were already correct or already kind-agnostic.
Trimmed several oversized explanatory comments down to their load-bearing
facts now that the types and tests carry most of the contract.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Add isolated slackdump-builder stage that downloads pre-built tarball from official GitHub release v4.4.2, verifies SHA256 checksum, and extracts binary. Key design: - Single-architecture path: TARGETARCH maps to correct tarball name (x86_64/arm64) - Extracts to /build/slackdump, then COPY to /usr/local/bin/slackdump - SHA256 verification against official upstream checksums - Preserves AGPL-3.0 LICENSE and upstream source URL reference - No build toolchain in final image (builder stage only) - Verifies slackdump works with version check at build time Architecture support: x86_64 (amd64) and arm64. Unsupported architectures fail at build time with clear error message. Tarball checksums (v4.4.2): - slackdump_Linux_x86_64.tar.gz: e2f386b2af30b0ba0ae98973f6a053225fba7d7127a20ad196cfdd96bf601052 - slackdump_Linux_arm64.tar.gz: 71d8b55b9132c0d39d6fe66e3542ee7d2ec6c032b7701928124c736611cc235e Slack connector is subprocess-wrapped (not linked), enabled by this binary. Upstream: https://github.com/rusq/slackdump/blob/v4.4.2 Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Change rationale from claiming "the Docker reference deployment includes the slackdump runtime" to specific upstream reference. New rationale includes: - Version pin: v4.4.2 - License: AGPL-3.0 - Upstream URL: https://github.com/rusq/slackdump/blob/v4.4.2 - Image target: core-browser Docker image Slack connector remains "proven" and "background_safe" for core-based deployments that include this bundle. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Add focused Docker integration test that builds core image once, then performs multiple inspections on the same image: 1. slackdump binary is executable at /usr/local/bin/slackdump 2. slackdump version check succeeds (v4.x.y format) 3. AGPL-3.0 license file present at expected path 4. Upstream source URL reference preserved 5. Image size sanity check (detects build bloat) Test gracefully skips if Docker unavailable. Single build + multi-inspect model avoids rebuilding huge image multiple times. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Fixes PR #84 maintainability defect: ownerActionable was computed once in buildOwnerConnectorCatalog but re-derived in isOwnerActionableEntry and re-derived again in isUnavailableSetupEntry, causing inconsistency risk. Changes: - isOwnerActionableEntry: when field is defined (live owner catalogs), return it directly; retain fallback rules only for demo/test entries from buildConnectorCatalog - isUnavailableSetupEntry: consolidate to single isOwnerActionableEntry call; removes duplicate static_secret condition logic - Add property tests proving all catalog fixtures have consistent helper/presentation agreement with authoritative ownerActionable field; mutations fail Test coverage: - ownerActionable field is sole authority for live owner catalogs - isOwnerActionableEntry respects fallback rules for demo/test entries - presentation consistency: helpers agree with ownerActionable authority - owner catalog: actionability converges across availability and action surfaces Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Keep Slackdump licensing documentation aligned with the shipped images and inspect the already-built core image in Docker CI. The focused gate verifies the pinned binary, corresponding-source disclosure, and builder-stage isolation without a duplicate image build or a platform-sensitive size ceiling. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
A red-team probe against PR #84 (owner run cancellation) proved a durable /v1/ingest write already admitted into the per-connector-instance write coordinator before an owner-cancel still committed to `records` after run_history recorded the run terminal. runtime/index.ts's own cancellation is a client-side AbortSignal on the fetch call; it cannot retroactively refuse a write the server already accepted for processing. Adds a run-bound admission fence inside the existing durable write transaction (SQLite: single-writer-connection atomicity; Postgres: `FOR UPDATE` on the run's run_history row, serializing against the terminal writer's own row-locking UPDATE) that refuses a write once its run is no longer 'running'. Fails closed on an unrecognized run_id (spoofed/mistyped/ foreign), since a genuine run-bound write is always preceded by an awaited run.started insert. Opt-in via options.runId — owner/API ingestion that never threads a run_id is unaffected. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…cement without verified identity An active connection that reached `active` via first-sync (no synchronous probe ever ran) or a no-probe connector's recapture had no durable verified_identity on its binding, and the identity-claim guard was either skipped entirely (no-probe path never called it) or silently passed when verified_identity was absent -- absence of proof was read as permission, letting a credential replacement silently retarget the connection to a different provider account. Replace the presence/absence inference with a terminal authority (assertStaticSecretActiveCredentialReplacementAllowed) that fires whenever an active, real-pipeline connection already has a stored credential: a replacement is allowed only when a synchronous provider probe's identity matches the durable verified_identity already on record, or the submitted secret's key-derived fingerprint exactly matches the one already stored. Owner-typed setup_fields are never trusted as identity proof on either side of that comparison, since they're trivially resubmittable alongside a stolen secret -- only a value a live provider probe just returned, compared against verified_identity written from an earlier successful probe, counts. No match on either channel fails closed with a typed static_secret_identity_unverified_replacement error directing the owner to disconnect and create a new connection. Legacy pre-pipeline bindings and the very first capture on an active row (no credential yet) are unaffected. Also corrects two stale complexity-mass-ratchet justifications surfaced by touching these files (connector-instance-credential-store.ts was already measuring 13 against a justification frozen at 7 before this change) and adds the new terminal-authority function's justification. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
PR #84 red-team P2: local-collector logout could only delete its local profile — there was no route letting a device revoke its own credential server-side, so the token stayed live indefinitely after "logout". Add POST /_ref/device-exporters/{deviceId}/self-revoke, authenticated by the device's own bearer token (requireDeviceExporterCredential), scoped to its own deviceId only — mirrors the existing 403-on-mismatch pattern already used by /heartbeat and /ingest-batches. Contract entry generated via the normal pnpm reference-contract:generate pipeline. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Client-side counterpart to the new self-revoke route: sends a
bearer-authenticated POST to /_ref/device-exporters/{deviceId}/self-revoke
with no body. Used by local-collector logout in the next commit.
Assisted-by: AI
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…g local profile PR #84 red-team P2: logout only ever deleted the local profile file — the device token remained live server-side indefinitely, with no way for a device to revoke itself. runLogout now calls the device's own self-revoke before deleting local state, and only deletes after a confirmed revoke (freshly revoked, or 401/403 meaning already-revoked — idempotent retry). Any other failure (network, timeout, 5xx) fails closed: local credentials stay in place so the operator can retry. --local-only is an explicitly named escape hatch for an unreachable/decommissioned server that skips the server call and deletes unconditionally — it is deliberately not "logout" itself, since it does not close the server-side lane. Also fixes the adjacent architecture-review finding that plain `run` had no real SIGINT handling — Ctrl+C killed the process group by accident, with no flush and no recorded gap. `run` now installs a real SIGINT/ SIGTERM handler for the duration of a pass, reusing the exact AbortController/abortSignal mechanism `--sample <n>` already used internally, so an interrupt flushes already-parsed records to the durable outbox via the existing streamConnectorIntoOutbox path instead of losing them to a process-group accident. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Server side (device-exporter-routes.test.ts): self-revoke succeeds with only the device's own token; owner sessions and missing auth are rejected; a device cannot revoke a different device (auth isolation); retrying after revoke fails closed with 401, not a crash. CLI side (runner.test.ts), table-driven over runLogout: success (revoke-then-delete), idempotent retry (401 treated as already-revoked), ambiguous network/5xx failures fail closed and preserve the local profile, --local-only skips the server unconditionally, and a missing profile is a no-op. Direct orchestration coverage: runCollectorSample's sample-limit discrimination (stops at-or-past the limit vs. completes cleanly under it) against a real spawned fixture connector; runSetup's enroll -> write profile -> sample ordering and its use of freshly enrolled (not caller -supplied) credentials, via an injected deps seam mirroring recoverLocalCollector's existing pattern; installInterruptAbort's SIGINT/SIGTERM registration and cleanup; and an end-to-end test that a real interrupt mid-run flushes the already-emitted record durably instead of losing it. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The runbook's "Stopping a run and interrupt safety" section previously claimed Ctrl+C during plain `run` was already safe via a signal-forwarding/ flush mechanism that did not exist — plain `run` had no SIGINT handler at all until this change. Corrected to describe the real (now-true) behavior: run/sample both install a real handler and abort via the same AbortController path. Also updated logout docs (runbook, package README, reference doc) to describe revoke-then-delete semantics and the --local-only escape hatch, replacing the old "local-only, does not revoke server-side" description. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…ion check A connection delete (deleteConnection) and a queued record write both serialize through withConnectorInstanceWrite for the same connector_instance_id, but the coordinator only provides mutual exclusion, not ordering-aware rejection. When a write's caller resolved the connection before a delete committed, and the write's fence acquisition landed after the delete, the write silently created a live records/record_changes row for a connector_instance_id with no connector_instances row and an existing tombstone -- a zombie record invisible to any owner UI joining through connector_instances. Neither the SQLite nor Postgres schema declares a foreign key from records/record_changes/blobs/blob_bindings to connector_instances, so nothing else would have caught this. ingestRecord/ingestRecords gain an opt-in requireConnectionAdmission option that re-checks connector_instances existence once inside the existing coordinator fence, immediately before the durable write -- closing the race without adding a query per record. The check is opt-in (default off) so ingestRecord/ingestRecords remain usable as a connector-agnostic durable storage primitive by direct callers (internal repair paths, and dozens of existing tests that exercise ingest without enrolling a connection). Only the owner HTTP ingest route (rs-mutation.ts), which resolves a real connection before writing, opts in. persistContentAddressedBlob (blob writes) is HTTP-route-only, so it checks directly and unconditionally inside its own fence. Also repoints a stale no-direct-prepare allowlist entry in records.ts whose line number shifted from this change (7463 -> 7528, same call site, unmigrated). Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…ding The concurrent-capture race in claimProbedStaticSecretIdentity legitimately lets either racing draft win (first-writer-wins on the binding's unique constraint). The oracle hardcoded secondId as the loser, so any run where the second capture won read as draft instead of revoked. Derive winnerId from the converged connection_id and assert the actual loser is revoked while the actual winner stays non-revoked, for both orderings. Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
force-pushed
the
fix/friend-readiness-final-0806
branch
from
August 7, 2026 11:53
c476c8e to
22910f2
Compare
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
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
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 changed
Why
A fresh self-hosted install could advertise actions that were not mounted, lose setup state after navigation, or leave a local collector credential active after logout. The console, runtime, and packaged image now use the same capability and lifecycle contracts.
Validation
Manual review
The two unclaimed checks require real provider credentials: collecting Gmail and one additional supported source. Their setup forms and the interactive browser launch were exercised without claiming provider collection.
Assisted-by: AI