Uh oh!
There was an error while loading. Please reload this page.
Conversation
…rity test(parity): value-level label parity — catch accept/accept-with-divergent-data (#1009 P0)
…0.3) (#188) * ci: pin the data-ingestors schema ref + add a coverage floor (#1009 P0.3) Two of the three CI drift tripwires from #1009. (The third — a scheduled `sync-validator-goldens.sh --check` against the pinned ingestor — waits for #340 to land on a stable ref, since the value-aware check regenerates against the ingestor read path and needs the #340 fix; the Go-side parity test already enforces the committed goldens on every PR.) (b) Pin the schema-drift check to a data-ingestors SHA, not floating master. sync-schema.sh now builds the fetch URL from scripts/.data-ingestors-ref (a pinned commit SHA), overridable via DATA_INGESTORS_REF. Before, any upstream commit touching the schema reddened every open CLI PR until someone synced; now adopting upstream is a deliberate SHA bump + re-sync in one PR. Pinned to 0de1f148 (current master; the embedded schema matches it — check stays green). (c) Add a per-package coverage floor. `go test -cover` printed numbers and asserted nothing, so the two load-bearing, historically thin-tested packages could silently rot. scripts/coverage-floor.sh fails the build if internal/cli or internal/submit drops below its floor; wired into the Test job after `go test`. Floors are a RATCHET (bump UP only, lowering is a reviewed edit), set just under current develop: internal/cli 68% (now 70.1%), internal/submit 72% (now 74.8%). Bump these up once #186/#187 land (they lift internal/cli to ~72%). The script is bash-3.2-portable (no associative arrays — macOS default). Verified locally: sync-schema.sh --check matches at the pinned SHA; coverage-floor.sh passes and bites (a 99% floor fails, a below-current floor passes); shellcheck clean; build.yml parses; full suite green. Part of backend#1009 (P0 CI drift tripwires). Remaining #1009: the goldens drift job (post-#340), the cross-repo taxonomy contract test, one content-compared ingest e2e. Part of the data-ingest epic backend#1008. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: fail loudly on a malformed coverage-floor entry (#1009 P0.3) A dropped ":floor" in FLOORS left min="$entry" (the whole token); the awk comparison then errored on that as bare source and exited non-zero, which `if awk` read as "not below floor" and printed a bogus "ok" — the ratchet became a silent no-op for that package. Validate each entry has a "package:INT" shape before the awk call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: validate the data-ingestors ref before it enters the schema URL (#1009 P0.3) The ref (pinned file value, or the DATA_INGESTORS_REF override) is interpolated into a raw.githubusercontent.com URL. An unvalidated ref could inject path traversal ("../..") or extra path segments — the same class scripts/install.sh already guards for its release tag. Restrict the ref to a SHA/branch/tag shape (alnum start; alnum . _ - / ; no "..") and fail with exit 2 otherwise. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Asad Iqbal <asad.dsoft@gmail.com>
…x (#1009 P0) (#187) runDataIngest's submit → port-forward → watch → classify → JSON → reclaim tail had no injection seam, so the path a customer actually lives in was covered by nothing (36.6%). A regression where the CLI previews success but the ingestor writes null/fewer, or reclaims staging on a partial failure, would ship silently. Extract the tail into runIngestionRun and route its four cluster-touching steps through package-level seams (mintIngestorTokenFn / portForwardJobsManagerFn / submitRunFn / cleanStagingFn), mirroring the existing listDatasetsFn seam. The extraction is behavior-identical — runDataIngest just calls it and threads jsonEmitted back for the --output-json error defer. Name the reclaim gate: shouldReclaimStaging(status) — the "reclaim ONLY on a clean success" invariant, so a detached / partial / failed / errored run keeps the staged source (for the still-reading Job, or for retry/inspection). This is the "must NOT reclaim on partial failure" gate #1009 calls out. Tests (no cluster needed): - TestRunIngestionRun_Matrix drives the whole tail through the seams and asserts, per outcome, the exit code (5 auth / 8 submit / 9 watch+ingest / 0 success+detached), whether the staging reclaim ran (only on succeeded), and the emitted --output-json status — all in lockstep. Covers the mint-fail (5) and port-forward-fail (8) pre-submit returns too. - TestShouldReclaimStaging pins the gate in isolation. - Extends the existing TestClassifyPushOutcome to cover the exit-5 (auth) and exit-8 (submit) buckets it was missing, plus unknown / nil-result. - TestSeamsWiredToRealFns guards that no seam is left nil. Also hardens submit.ForwardedConnection.Close to no-op on a zero-value connection (nil stopCh) — it panicked on close(nil) before, which blocked using a fake in the seam test; a never-started connection now closes safely. Coverage: internal/cli 70.1% → 72.2%; runIngestionRun 94.1%, shouldReclaimStaging 100%, classifyPushOutcome 92.9%. Part of backend#1009 (P0 seam the money path). Remaining #1009: CI drift wiring (pin the goldens/schema SHA + a per-package coverage floor so this can't rot), the cross-repo taxonomy contract test, and one content-compared ingest e2e. Part of the data-ingest epic backend#1008. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ad instance_segmentation (#1005) (#189) The task taxonomy lives in 5 hand-synced copies across 3 repos; the only divergence today is instance_segmentation — present in the CLI registry (16th entry) but deliberately absent from the ingest.v1 schema + the ingestor registry. #1005's "decide first" question (planned vs dead) is settled by the ingestor: instance_segmentation is DEAD — it "briefly shipped with no validators and no file transfer, so configs half-ingested" and was removed (data-ingestors constants.py, #240/#99). So this drops it from the CLI, rather than keeping a misleading "not implemented" placeholder. - Remove instance_segmentation from categoryRegistry (registry now == the schema enum, 15 == 15) and from the two tests + code comments that named it as a known/pending category. It now falls to the "unrecognized category" gate like any other non-category (still exit 2; test unchanged in outcome). - Add TestRegistryWithinSchema — the reverse of the existing TestRegistryCoversSchemaCategories (schema ⊆ registry). Together they pin registry == schema BOTH ways, so the registry can neither fall behind the schema (a valid --category rejected as "unrecognized" — the token_classification RC drift) nor carry an extra the ingestor won't accept (the instance_segmentation half-ingest class). A future known-but-unschema'd placeholder must be DECLARED in the new registryAliases allow-list (empty today), so an intentional superset is explicit, never silent — exactly the #1005 proposal. Factored the schema-enum parse into a shared helper. - Mutation-verified the new gate bites: re-adding instance_segmentation to the registry fails TestRegistryWithinSchema with an actionable message. This is the CLI third of #1005. data-ingestors already pins registry == schema (tests/test_category_congruence.py + test_schema_validation). Remaining: the backend copies (UserDataSet.CATEGORY_CHOICES + global_meta/constants.py) still carry instance_segmentation and aren't tied to the schema — a separate PR (it needs a choices migration, and global_meta is slated for deletion). Part of backend#1005 (ties RFC-0002 §9 / cli#174). Part of the data-ingest epic backend#1008. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nreadable discovery (#190) `tracebloc client create` could mint a NEW backend client and stamp the cluster's cluster_id anchor onto it, even when a healthy DIFFERENT client was already running on the cluster. The installer then refused to deploy the new client (one-client-per-machine), leaving an orphaned "phantom" that owns the anchor — so every later re-provision 409s ("cluster_conflict", mislabeled as cross-account) and the real client can never reclaim the anchor. Confirmed in the field: edge_device 1060 minted + anchored, never deployed, wedging a cluster that actually runs 1044. Root of the reproducible class: DiscoverInClusterClientID swallowed List/RBAC errors into (nil, nil) — "nothing installed" — which is indistinguishable from a genuinely fresh cluster, so runClientCreate fell through to a mint. Fix (two edits): - cluster/discover.go: DiscoverInClusterClientID is now three-valued. It returns (nil, err) when it CANNOT determine — a deployments List error, a secrets List error, or a release present whose CLIENT_ID is unreadable. (nil, nil) now means only "reachable and genuinely no client". An empty cluster still reports emptiness via an empty list, not an error, so fresh installs are unaffected. - cli/client.go: adoptLiveInClusterClient fails closed on a discovery error when the cluster is REACHABLE (clusterID != "", i.e. the kube-system UID read succeeded over the same kubeconfig) — refusing to mint a duplicate that could strand the anchor. Only a genuinely unreachable cluster (clusterID == "", where the UID read failed too) keeps the old fall-through to a non-anchored mint (which stamps no anchor, so it can't orphan one) — the deliberate headless/no-cluster path. Tests (verified to FAIL against the pre-fix code): - discover_test.go: DeploymentsListError / SecretsListError / ReleaseButNoSecret now expect (nil, error). - client_test.go: DiscoveryErrorReachableFailsClosed (reachable + discovery error -> exitError, no mint) and DiscoveryErrorUnreachableMintsNonAnchored (unreachable -> still mints, cluster_id empty) — the latter guards against over-tightening into the legitimate headless path. Full repo suite green; adversarially reviewed. This is fix#1 of the phantom-client investigation (never mint over a live cluster). Follow-ups (separate): backend same-account re-anchor + honest 409 message; orphan reaper. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cluster_in_use (#191) The CLI mapped every provisioning 409 to a static "registered to a different tracebloc account — sign in to that account, or ask your admin" — which was often FALSE (the same-account phantom case) and a dead end. With the backend's fix#2 (backend#1021) the 409 body now distinguishes: • cluster_conflict — genuinely another account; body carries owner_email; • cluster_in_use — a same-account client is live on this cluster. New conflictMessage() parses the 409 body and picks the right guidance: • cross-account → "registered to another tracebloc account (<owner_email>) — ask them to release it, or sign in as that account" (contact-the-owner, never "delete the cluster" — it isn't ours to wipe; names the owner when supplied); • cluster_in_use → "another tracebloc client (<name>) in your account is already live on this cluster — offboard it first with `tracebloc delete`, or provision on a separate machine". Degrades gracefully against a backend without fix#2 (empty/unparseable body → the generic cross-account text). The client-side not-owned refusal (no HTTP body) keeps the generic message. Reworded crossAccountConflictMsg to match. Companion to backend#1021 (fix#2) and cli#190 (fix#1) of the phantom-client migration. Tests: owner_email surfaced; cluster_in_use names the live client and does NOT read as cross-account; existing cross-account + client-side-refusal messages updated to the new wording. Full suite green; gofmt -s / errcheck / ineffassign / misspell clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ternal) (#192) `tracebloc client` offered users `create` — but provisioning is the installer's job (provision.sh calls `client create` with zero flags, cli#137), not a human command. Exposing it is inconsistent (its sibling `list` is already Hidden as "installer-internal, off the user-facing surface") and it's the front door to phantom-minting: a human running `tracebloc client create` STANDALONE mints a client the installer never deploys — an orphaned phantom (backend#970, the root of the cluster_conflict saga). Mark `create` Hidden, mirroring `list`. It stays fully callable — including `create --help`, so provision.sh's `_cli_supports_provisioning` probe is unaffected — but is no longer advertised. `tracebloc client` now shows only the user-useful `status`. Test: TestClientSubcommandVisibility pins create+list Hidden, status visible, and create still runnable (hidden != disabled). Full suite green; gofmt -s / errcheck / ineffassign / misspell clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…based rate + full has_failures (#193) * fix(summary): align CLI success/failure with the ingestor (silent-success + wrong staging-reclaim) Found in the 2026-07-08 prod audit. The CLI's Summary methods diverged from the ingestor's own success/failure determination, so a partial run could report success AND reclaim (delete) the staged source: - HasFailures() was `FailedRecords>0 || FileTransferFailures>0` — it IGNORED skipped rows, inserted<total, and api_sent<inserted. The ingestor's IngestionSummary.has_failures (data-ingestors ingestors/base.py) counts all five. So a run that silently SKIPPED rows or inserted fewer than total was classified "succeeded" → exit 0 AND the staging-reclaim gate deleted the user's source copy. Now mirrors the ingestor exactly. Every counter it reads is emitted unconditionally by the ingestor banner (reporting.py) and parsed, so the new inserted<total / api_sent<inserted clauses can't false-positive on a clean run (all counters equal). - SuccessRate() was ProcessedRecords/TotalRecords; the ingestor's banner uses inserted/total. processed (passed validation) is a superset of inserted (rows actually in MySQL), so the CLI OVERSTATED success (e.g. 100 processed / 70 inserted showed 100%, not 70%) in the panel header + --output-json `success_rate`. Now inserted/total, matching the banner. - RenderSummary headline: regraded to hard-failures (DB/transfer errors) → red, softer partial (skips / short insert-or-sync) → yellow, clean → green — so the broader HasFailures drives exit-code + reclaim while the headline still distinguishes a hard failure from a partial. Behavior change (intended): a run with skipped rows, inserted<total, or api_sent<inserted now exits 9 (was 0) and its staged source is KEPT (not reclaimed) — matching what the ingestor already reports in its own banner. Tests: HasFailures + SuccessRate matrices rewritten to the ingestor-aligned semantics (incl. the processed-overstates case + one case per new clause); RenderSummary headline cases + one classifyPushOutcome fixture updated to realistic full-counter summaries. Full cli suite green. Standalone CLI correctness fix (files untouched by the in-flight CLI branches). Relates to the data-ingest epic backend#1008 / test-hardening backend#1009. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(summary): label soft partials without skips as "partially", not "skips" RenderSummary routed every non-hard-failure through the yellow "completed with skips" headline, including runs where SkippedRecords==0 and the only shortfall was inserted<total or api_sent<inserted. Exit code and staging-reclaim were correct, but operators could misread an insert/API shortfall as a validator skip. Word the yellow headline by the actual shortfall: "skips" only when rows were skipped, else "partially". Adds a RenderSummary headline case for the zero-skip partial. (Bugbot #193.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ingestion): make the "succeeded" fixture a genuinely clean run Merging develop brought TestRunIngestionRun_Matrix (#1009), whose succeededResult() fixture set only TotalRecords+InsertedRecords. Under this PR's ingestor-aligned HasFailures(), api_sent(0) < inserted(2) classifies that as completed_with_failures → exit 9, no reclaim, breaking the "succeeded" row. Set APISentRecords so every stage counter is equal — a clean run — matching the coverage_test.go "clean" fixture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style: gofmt -s the succeeded fixture (comment broke field alignment) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Asad Iqbal <asad.dsoft@gmail.com>
LukasWodka
approved these changes
Jul 9, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Note
High Risk
Changes provisioning fail-closed behavior and ingest exit codes/staging reclaim on the customer money path; mistakes could block installs or mis-handle partial ingestions.
Overview
Hardens client provisioning and data ingest correctness, tightens CI, and aligns CLI behavior with the ingestor.
Provisioning (
client create) hidescreatefrom the user surface (installer-only), maps backend 409 bodies tocluster_conflictvscluster_in_use(including owner email when present), and fails closed when in-cluster client discovery errors on a reachable cluster—only unreachable clusters still allow a non-anchored mint.DiscoverInClusterClientIDnow distinguishes empty cluster vs read failures instead of treating RBAC errors as “nothing installed.”Ingest path pulls submit → classify → JSON → reclaim into
runIngestionRunwith test seams and matrix tests;shouldReclaimStagingandSummary.HasFailures()match ingestor rules (skipped rows, insert/API shortfalls) so partial runs don’t exit success or delete staged data. Preflight gains value-level label parity vs ingestor goldens;instance_segmentationis dropped from the category registry with bidirectional schema parity tests.CI / tooling: schema sync pins
scripts/.data-ingestors-ref;scripts/coverage-floor.shenforces minimum coverage oninternal/cliandinternal/submit; validator golden checks include value-level fields.Reviewed by Cursor Bugbot for commit 518c85a. Bugbot is set up for automated code reviews on this repo. Configure here.