release-train: develop -> staging - #432
Merged
Merged
Conversation
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…v2 (#423) * ci: migrate .golangci.yml to v2 format and enable gosec golangci-lint v1 is EOL and cannot typecheck this module (go.mod says go 1.26.0), and v2 binaries -- including what brew ships -- refuse v1-format configs, so make lint-full was broken for fresh installs. Migrated via `golangci-lint migrate` with the narrative comments preserved; same linter set (still no SSA linters, per #6), plus gosec for insecure-pattern scanning of our own code. Test files are gosec/errcheck-exempt per convention (12 additional findings fire there, all fixed-temp-path/perms test idioms). Part of tracebloc/backend#1305 (epic #930, Layer 1). * ci: run golangci-lint + gosec in CI (advisory, not required) The config existed but nothing in CI loaded it. Official action pinned v9.3.0, golangci-lint pinned v2.12.2 (built with Go 1.26). Advisory by design: not in branch protection, and continue-on-error at the job level so the pre-existing 8-finding gosec backlog does not red-X unrelated PRs -- comes off at the required-flip after cleanup (backend#1303 pattern). Part of tracebloc/backend#1305 (epic #930, Layer 1). * ci: advisory mode via --issues-exit-code=0, not continue-on-error First run on #423 confirmed the quirk: job-level continue-on-error greens the workflow RUN but the job check run still red-Xs in the PR checks list. --issues-exit-code=0 gives the intended semantics: findings -> green job with inline annotations; infrastructure breakage (bad config, typecheck failure) -> still fails. Remove the arg at the required-flip (backend#1303 pattern). * ci: document the typecheck blind spot of advisory mode accurately Bugbot on #423: typecheck findings ride the issues exit path, so --issues-exit-code=0 greens them too -- the previous comment wrongly claimed typecheck failures still fail the job. Verified empirically (broken type: exit 0 with the flag, 1 without). Comment now states the real containment: required Test/Lint/Build jobs red a non-compiling PR, and a broken config still fails this job via the config-verify pre-step. Behavior unchanged.
* docs: add Bugbot resolve-and-reply team norm to .cursor/BUGBOT.md Part of tracebloc/backend#1308 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: point CONTRIBUTING.md at the Bugbot findings norm Part of tracebloc/backend#1308 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…-0003 I6, backend#1209) (#424) * feat(data delete): reap the ingestor's bookkeeping rows with the table (RFC-0003 I6, backend#1209) Dropping a table stranded its run-journal rows (tracebloc_ingest_runs) and pseudonymization-salt row (tracebloc_ingest_meta). Under per-ingestion tables (data-ingestors#408) every dataset is its own table, so every delete would leak one husk row of each kind, unbounded. Teardown now DELETEs both best-effort after the DROP — separately per bookkeeping table (either may be absent on clusters that never ran a journal-aware ingestor), never failing a teardown whose DROP succeeded (TeardownResult.BookkeepingCleaned reports it). plan.Table has passed ValidateTableName, so it cannot escape the quoted literal. Benefits legacy label tables identically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(teardown): feed bookkeeping SQL on stdin — shell quoting ate the string literal (Bugbot, High) The DELETEs embedded a single-quoted SQL literal inside a single-quoted sh -c string: the shell stripped the inner quotes, mysql saw an unquoted identifier, and the best-effort cleanup silently no-opped forever — exactly the leak this PR exists to stop. SQL now rides stdin (the runMySQLQuery pattern), sidestepping shell quoting entirely. The recording executor now captures stdin, and the test asserts the quoted literal arrives intact AND that no DELETE ever appears as a shell argument — pinning the whole bug class, not just this instance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: goimports grouping in teardown_test (CI lint) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(teardown): surface bookkeeping failures + reuse runMySQLQuery + pin the column contract (review) 1. Observability: TeardownResult gains BookkeepingErrs (per-table failure with mysql stderr folded in via runMySQLQuery); data delete prints a warning on incomplete cleanup and --output-json gains bookkeeping_cleaned — schema drift is now diagnosable in the field instead of collapsing into a silent false. 2. Column contract pinned in a comment against data-ingestors database.py: both bookkeeping tables key by table_name (tracebloc_ingest_runs indexed, tracebloc_ingest_meta PK). 3. The inline stdin exec is gone — DELETEs ride runMySQLQuery; its error prose neutralized to 'running mysql query' (the 'querying datasets' wording lives on only in list.go's own exec, whose test asserts it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(overwrite): surface bookkeeping-cleanup failures on the ingest pre-clean too (Bugbot) + JSON cosmetic (review) data ingest --overwrite runs the identical teardown but discarded the result — a bookkeeping failure printed unconditional success, hiding on this path the exact schema-drift signal data delete now surfaces. The overwrite pre-clean warns the same way. Also: dry-run/declined emit bookkeeping_cleaned=false (nothing was attempted — a strict consumer must never read 'cleanup happened' out of a run that deleted nothing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…dings (18 real sites) (#427) Each gosec finding got an individually reviewed inline waiver with a site-specific justification, so the advisory golangci job can go to zero and backend#1305 can later drop --issues-exit-code=0. The advertised backlog of 8 was an artifact of golangci-lint's default issues.max-same-issues=3: the 13 G304s share one message text, so only 3 surfaced per run (which 3 flapped with cache state). The real, uncapped backlog is 18: 4x G204, 13x G304, 1x G115. All 18 are waived here; waiving only the visible 8 would have been whack-a-mole. Hardening was considered per site and deliberately not bolted on: the meaningful control (symlink rejection on the dataset walk, re-checked at stream time) already exists, and filepath.Clean wrappers would silence gosec without confining anything. Comment-only change; no behavior touched. Verified with the CI-pinned golangci-lint v2.12.2 (go1.26.3): 0 issues with --max-same-issues=0 --max-issues-per-linter=0. Part of tracebloc/backend#1305 (epic #930, Layer 1). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* chore: clear house-rules findings Fix every finding the shared org checker (tracebloc/.github scripts/house-rules.sh) reports at develop HEAD: missing curl timeouts/TLS floors, plus (cli) a missing pipefail. Waivers only where the finding is a documented false positive. Part of tracebloc/backend#1303. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(install): stall-based download bounding via a shared dl() helper (review) --max-time 300 made the ~50MB binary fail under ~1.4 Mbps and the ~90MB cosign bootstrap under ~2.6 Mbps -- slow-but-alive links must be allowed to finish. dl() replaces the wall-clock cap with --speed-limit 1024 --speed-time 60 (abort only when under 1 KiB/s for 60s straight = dead connection), keeps the TLS 1.2 floor + --connect-timeout 30, and deduplicates the flag string across all six body fetches. The HEAD tag-resolve keeps its 30s cap (header-only, wall-clock is right there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* ci(golangci): drop the advisory flag + uncap max-same-issues (backend#1305) Backlog is zero after the reviewed #nosec waivers (#427): findings now fail the job. max-same-issues: 0 so repeated findings can never hide behind the default cap of 3 again (the '8 findings were really 18' lesson). Branch-protection required-flip follows once this merges. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(golangci): drop the advisory flag + uncap max-same-issues (backend#1305) Backlog is zero after the reviewed #nosec waivers (#427): findings now fail the job. max-same-issues: 0 so repeated findings can never hide behind the default cap of 3 again (the '8 findings were really 18' lesson). Branch-protection required-flip follows once this merges. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build: make ci runs lint-full — mirror the now-failing golangci gate (Bugbot) golangci-lint fails PRs on findings since this branch; make ci skipping it broke the 'make ci mirrors CI exactly' rule (green local, red PR). lint-full's guard already gives install instructions when the tool is missing, which is correct mirroring rather than a soft skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build: pin lint-full to the CI golangci version via go run (Bugbot) lint-full ran whatever golangci-lint was on PATH while CI pins v2.12.2 -- with ci depending on lint-full, version drift could green a local run that reds the PR gate. Now runs the exact pinned version through the Makefile's own 'go run tool@version' pattern (like errcheck/ staticcheck/govulncheck): no PATH dependency, no brew-version drift. GOLANGCI_LINT var removed (unused); lockstep note added on both sides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Contributor
Author
|
👋 Heads-up — Code review queue is at 36 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f37dad5. Configure here.
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
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.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes touch destructive teardown and JSON contracts (new field, new warnings) plus a new blocking CI gate; bookkeeping failures are intentionally non-fatal but operators must interpret warnings correctly.
Overview
This train bundles dataset teardown hygiene, stricter lint/security CI, and smaller installer/script hardening.
Teardown (RFC-0003 I6): After
DROP TABLE,push.Teardownnow best-effort deletes ingestor run-journal and salt rows viarunMySQLQueryon stdin (avoids shell-quoting no-ops).TeardownResultexposesBookkeepingCleaned/BookkeepingErrs;data deleteand ingest--overwritepre-clean warn when bookkeeping fails, and--output-jsonaddsbookkeeping_cleaned(false for dry-run/declined). New tests pin the DELETE behavior.CI & local parity:
.golangci.ymlmigrates to v2, enables gosec, moves formatters out of linters, and setsmax-same-issues: 0. A blocking.github/workflows/golangci.ymlruns pinned golangci-lint v2.12.2;make ciaddslint-fullvia the same pinnedgo runversion. A reusable code-quality workflow caller is added. Reviewed#nosecannotations clear the gosec backlog across CLI/push paths.Docs & ops: Bugbot reply/resolve norms in
.cursor/BUGBOT.mdandCONTRIBUTING.md.scripts/install.shcentralizes downloads indl()with stall-based limits (replacing a wall-clock cap that broke slow links); fixture sync scripts get curl timeouts; install-verify harness usespipefail.Reviewed by Cursor Bugbot for commit f37dad5. Bugbot is set up for automated code reviews on this repo. Configure here.