chore: CI/CD optimization (rust-optimizer audit — A1, C2, A3, A4, A6, C1, D2, F1) - #102
Merged
Conversation
Decomposes OPTIMIZATION_SPEC.md's findings into an autopilot pipeline: A3, A6, D2, C1, A4, C2, A1, F1 (spec's own suggested_sequence), with A1 depending on C2 (both touch the same ci.yml job block) and A1/F1 flagged as risk_phases. Autonomy: reviewed. Mirrored into beads as epic finima-w2q with 8 child issues for a queryable work-graph view.
- ci.yml: add CARGO_INCREMENTAL=0 / CARGO_PROFILE_TEST_DEBUG=0 (finding A3). - Makefile: prune .agents/ from format-check-md/lint-md globs — an untracked vendor dump under .agents/ was crashing prettier entirely via an unrelated package.json declaring prettier-plugin-svelte. - pipeline.yml/profile.yml: fix yamllint violations introduced when authoring them (colon/brace spacing, line length). - OPTIMIZATION_SPEC.md: prettier --write (my own artifact's formatting). build/lint/test all green. format_check still red on 9 pre-existing markdown files unrelated to this phase (confirmed via git stash against HEAD) — recorded as blocker blk-fmt0 in .autopilot/discovered/rust-ci-optimization.jsonl. No gate-PASSED marker: phase 0 stops here pending a human decision on how to handle the pre-existing docs-formatting debt.
Promotes blk-fmt0 (pre-existing markdown-formatting debt breaking the shared make format-check Tier-1 gate) into its own phase rather than dismissing or fixing it out-of-band, per user decision. Renumbers the original 8 A-F phases to 1-8 and gives each an explicit depends_on: [0] (not just phase 1) since format_check gates every phase's gate, not only the first one. Mirrored in beads: new issue finima-w2q.9 blocks finima-w2q.1..8.
…ngth Missed in ba96f48 — the deliverable line for the new phase 0 exceeded the 200-char yamllint limit.
Resolves blocker blk-fmt0: prettier --write (formatting only, no wording changes — verified via diff) on the 9 pre-existing markdown files that were failing make format-check, a Tier-1 check on every phase's gate. make format-check/lint/build/test all green. DoD: cmd: make format-check ✓
…se 0's ledger line
A3: CARGO_INCREMENTAL="0" / CARGO_PROFILE_TEST_DEBUG="0" added to .github/workflows/ci.yml's top-level env block. Deliverable already existed from earlier session work; this phase verified it against the gate rather than re-implementing. DoD: grep CARGO_INCREMENTAL:.*0 ✓, grep CARGO_PROFILE_TEST_DEBUG:.*0 ✓, cmd: make lint ✓, cmd: make test ✓
…se 1's ledger line
A6: backend-audit job installs cargo-audit via taiki-e/install-action (prebuilt binary) instead of `cargo install cargo-audit --locked` (compiling from source every run). .github/scripts/cargo-audit.sh and audit-ignore untouched. DoD: grep taiki-e/install-action ✓, grep:absent cargo install cargo-audit ✓, cmd: make audit ✓
…se 2's ledger line
D2: if-no-files-found: ignore added to the playwright-report and playwright-traces upload-artifact steps so a run producing neither (e.g. failing before the browser starts) doesn't fail the upload step itself. DoD: grep if-no-files-found: ignore ✓
…se 3's ledger line
C1: concurrency group added to ci.yml (scoped to workflow+ref) with cancel-in-progress: true, so superseded PR pushes cancel in-flight runs instead of piling up. DoD: grep cancel-in-progress:.*true ✓
…se 4's ledger line
A4: .cargo/config.toml pins -Clink-arg=-fuse-ld=lld for x86_64-unknown-linux-gnu (the ubuntu-latest CI target; no effect on local macOS builds). lld installed via apt-get in the four CI jobs that compile Rust: backend-lint, backend-test, llm-test, e2e-test. DoD: grep fuse-ld=lld in .cargo/config.toml ✓, cmd: make build ✓, cmd: make test ✓
…se 5's ledger line
C2: removed `needs: backend-lint` from backend-test and llm-test so they run in parallel with backend-lint instead of waiting on it. Added a ci-success aggregation job that needs every always-run required job (validate-markdown, validate-yaml, backend-lint, backend-test, backend-audit, frontend-lint, frontend-test) so branch protection can point at one check. llm-test and e2e-test stay excluded since they're conditional on vars.LLM_TESTS_ENABLED/vars.E2E_ENABLED. DoD: grep:absent needs: backend-lint ✓, grep ci-success ✓, cmd: make lint ✓, cmd: make test ✓
…se 6's ledger line
A1: new backend-test-archive job builds the test-profile workspace once via `cargo nextest archive`, uploads it as an artifact; backend-test downloads and runs it via `cargo nextest run --archive-file --workspace-remap .` instead of recompiling with `cargo test --workspace`. postgres service stays on backend-test (the consumer), matching pipeline.yml's own convention note. llm-test is deliberately left untouched (different feature flags/ignored-test semantics can't safely share this archive). Two real bugs found and fixed via adversarial review before this commit, not after: - cargo-nextest doesn't run doctests — added a separate `cargo test --workspace --doc` step (verified against a real, previously-passing doctest in finima-auth::magic_link). - cargo-nextest was installed unpinned in both jobs via taiki-e/install-action, risking an archive-format mismatch if the two installs resolved different versions — pinned cargo-nextest@0.9.140 in both. Also dropped an overly-aggressive retention-days: 1 on the archive artifact (would break re-running a failed job after a day). Empirically validated twice end-to-end (not just YAML review): full 403-test nextest run + doctests, against a real Postgres test DB. DoD: grep nextest archive ✓, grep archive-file ✓, cmd: make test ✓, cmd: make test-integration ✓
…se 7's ledger line
F1: resolved all 13 cargo-machete unused-dependency hits. Removed (confirmed zero source-code references via grep, then proven by cargo build --workspace + full cargo test --workspace against a real Postgres test DB): - finima-feed: serde_json, uuid - finima-api: axum-extra, base64, aws-config, bytes (storage.rs builds its S3 client manually via aws-sdk-s3 + aws-credential-types; never touches aws_config::load_from_env()/SdkConfig) - finima-ingest: serde_json, tracing - finima-db: tokio, tracing (from [dependencies] only — the [dev-dependencies] tokio used by #[tokio::test] is untouched) - finima-auth: serde_json - finima-llm: finima-core (zero references anywhere in the crate, including the candle-gated candle_backend.rs module; the crate's own README claimed a dependency "for AppError conversion" that no longer exists — LlmError is fully self-contained via thiserror — fixed) Documented-ignored rather than removed: - finima-llm: tokenizers, via [package.metadata.cargo-machete]. It's gated behind the optional `candle` feature (heavy ML deps: mistralrs/ hf-hub) not exercised by this repo's default build path, so its removal can't be cheaply/safely verified here — the compiler is the arbiter, and this one requires a build I can't run in this environment. Left as a rationale-documented false-positive-risk ignore, not a blind removal. Adversarial review (qe-security-reviewer subagent) confirmed no transitive-dependency breakage across the workspace and flagged one real defect (fixed: the stale README line above) plus one genuine out-of-scope finding, recorded as parking-lot item pl-orphan-wsdeps: 3 root [workspace.dependencies] entries (axum-extra, aws-config, bytes) now have zero consumers workspace-wide — cargo machete doesn't check the shared manifest itself, only per-crate declarations. Left for a dedicated follow-up since it touches the shared root Cargo.toml. DoD: cmd: cargo machete ✓ (0 hits), cmd: cargo build --workspace ✓, cmd: make test ✓ (+ full cargo test --workspace verified with a real test DB: every crate "ok", 0 failed)
…se 8's ledger line
…findings shipped)
…e PASSED All 9 phases (0-8, findings A1/C1/C2/A3/A4/A6/D2/F1 + the docs-format blocker fix) are gate-PASSED. Reviewed the full accumulated diff across .github/workflows/ci.yml, .cargo/config.toml, Makefile, 6 crate Cargo.toml files, and finima-llm/README.md for duplication or dead seams — none found. The repeated "Install lld"/taiki-e/install-action steps across 3-4 jobs are inherent to GitHub Actions' per-job runner isolation (no shared state between jobs without a reusable/composite action, which is a bigger architectural change out of scope here), not refactor-worthy duplication. Full gate re-verified green end-to-end: format_check, lint, build, test, test_integration, audit all pass. Open parking-lot item for a future pass: pl-orphan-wsdeps (3 orphaned root [workspace.dependencies] entries — axum-extra, aws-config, bytes — now unused workspace-wide after phase 8's per-crate removals). Pipeline complete.
… optimization pass ledger line
…s dependency removals Missed staging this alongside c228fab — Cargo.lock now reflects the 12 removed dependencies (aws-config and its transitive tree, plus the others), verified via the same cargo build --workspace / cargo test --workspace runs from phase 8's gate.
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…arkdown / Validate YAML) Local `make format-check`/`make lint` masked these because their markdown/yaml sub-targets are wrapped in `|| true` — the actual CI jobs (DavidAnson/markdownlint-cli2-action, direct yamllint) hard-fail on the same checks with no such fallback. Verified against the exact tracked-file sets and CI-equivalent invocations before pushing, not just against the soft-failing local wrapper. - AGENTS.md / CLAUDE.md: added blank lines around the PUSH TO REMOTE fenced code block (MD031), added a language tag to the commit-message fence (MD040, AGENTS.md only). - OPTIMIZATION_SPEC.md: promoted the 8 finding headers from h3 to h2 (MD001 heading-increment) to match "## Not a finding" and the top-level h1 — no h2 existed between them. - .autopilot/profile.yml: reverted brace spacing to yamllint's default (no inner spaces) — it had been reformatted to prettier's style (which wants spaces) during an earlier phase, which is what CI's Validate YAML job actually flagged. - .prettierignore: excluded .autopilot/ (matches the existing precedent for .claude/, .claude-flow/, .swarm/) so prettier and yamllint stop fighting over this directory's brace-spacing convention going forward. Verified via the CI-equivalent commands directly (not the soft-failing local wrapper): `git ls-files '*.md' | xargs markdownlint-cli2` → 0 issues in 73 files; `git ls-files '*.yaml' '*.yml' | xargs yamllint -c .yamllint.yaml` → clean.
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.
Summary
Applies all findings from the rust-optimizer CI/CD audit (
OPTIMIZATION_SPEC.md), delivered as 9 gated autopilot phases:make format-checkgates cleanlyCARGO_INCREMENTAL=0/CARGO_PROFILE_TEST_DEBUG=0cargo-auditviataiki-e/install-action(prebuilt binary, not compiled from source)if-no-files-found: ignoreon Playwright artifact uploadsconcurrencycancel-in-progress grouplldfast linker (.cargo/config.toml+ CI install step)backend-lint/backend-testrun in parallel; newci-successaggregation jobcargo nextest archive— backend-test consumes a prebuilt archive instead of recompilingcargo-macheteunused-dependency hits (12 removed, 1 documented ignore)Every phase has a machine-checked Definition of Done and a real green gate (
make format-check/lint/build/test/test-integration/audit), not just a code review. Full history:.autopilot/runs/rust-ci-optimization.jsonl.Notable findings caught during review (not just applied blindly)
cargo-nextestdoesn't run doctests — a real, previously-passing doctest infinima-authwould have silently stopped running in CI. Fixed with a separatecargo test --workspace --docstep, verified against the actual doctest. Also pinnedcargo-nextest@0.9.140in both the archive-build and archive-consume jobs to avoid an archive-format mismatch from independent unpinned installs.cargo build --workspace+ fullcargo test --workspaceagainst a real Postgres test DB (403/403 tests green).tokenizersinfinima-llmwas left as a documentedcargo-macheteignore rather than blindly removed, since it's gated behind the optionalcandlefeature (heavy ML deps) that this environment can't cheaply build to verify.Follow-ups intentionally left open (not in this PR's scope)
pl-orphan-wsdeps: 3 root[workspace.dependencies]entries (axum-extra,aws-config,bytes) now have zero consumers workspace-wide after the per-crate removals above —cargo machetedoesn't check the shared manifest itself. Separate cleanup.Test plan
make format-check/make lint/make build/make test/make test-integration/make auditall green locallycargo test --workspace(403 tests) against a real Postgres test DB, twice (before and after the nextest-archive restructuring)