Skip to content

ci(test): stop the v1 monolith re-running reborn composition/memory/secrets (1a)#5159

Open
serrrfirat wants to merge 1 commit into
mainfrom
firat/trim-legacy-pr-tax
Open

ci(test): stop the v1 monolith re-running reborn composition/memory/secrets (1a)#5159
serrrfirat wants to merge 1 commit into
mainfrom
firat/trim-legacy-pr-tax

Conversation

@serrrfirat

Copy link
Copy Markdown
Collaborator

What (item 1a — pure waste removal)

The legacy tests job re-ran three things already covered by reborn-side CI:

  • cargo test -p ironclaw_reborn_composition --features libsql,postgres
  • cargo test -p ironclaw_memory
  • the standalone secret-store-contract-tests job (-p ironclaw_secrets --test secret_store_contract)

All three already run per-crate in reborn-tests.yml (libsql) and in coverage.yml (cargo llvm-cov --all-features --workspace with a real Postgres service). So the v1 monolith was duplicating them on every legacy run. Removed the two in-job steps + the standalone job (which wasn't gated by the run-tests aggregator).

No coverage lost: libsql via the closure, postgres via coverage.yml + the hooks-parity Postgres job.

Why this PR is 1a only (1b deferred — flagging honestly)

You also asked to stop the v1 cargo test re-running the 31 reborn_*.rs root tests. The obvious one-liner — cargo test ... -- --skip reborn_ — is unsafe: the v1 suite has its own reborn_-named tests that are not dups and must keep running:

  • tests/dockerfile_runtime_home.rs: reborn_dockerfile_*, reborn_entrypoint_*, reborn_hosted_single_tenant_seed_config_contains_postgres_storage, reborn_dockerfile_build_is_covered_by_ci (the CI-coverage guard)
  • src/workspace/mod.rs: the reborn_identity_context module

A blanket skip would silently drop those. Clean binary-level exclusion needs either a nextest -E 'not binary(/^reborn_/)' switch (risky for the 100+-test v1 monolith) or feature-gating the 32 files (ripples into the reborn root-test partitions + coverage). Both are bigger than "waste removal," and the natural home for it is the v1 → build/check + smoke restructure (item 3), where the reborn root tests drop out anyway. So I'm deferring 1b rather than shipping a silent-skip.

Item 2 (FYI)

Already done — the PR gate already runs only the all-features config; default+libsql-only only run on push/nightly.

Automated agent-authored.

…ecrets

The legacy `tests` job re-ran three things already covered by reborn-side
CI:
- `cargo test -p ironclaw_reborn_composition --features libsql,postgres`
- `cargo test -p ironclaw_memory`
- the `secret-store-contract-tests` job (`-p ironclaw_secrets --test
  secret_store_contract`)

All three run per-crate in reborn-tests.yml (libsql) and in coverage.yml
(`cargo llvm-cov --all-features --workspace` with a Postgres service), so
the v1 monolith was duplicating them on every legacy run. Remove the two
in-job steps and the standalone secret-store job (it was not gated by the
run-tests aggregator). Pure waste removal — no coverage lost: libsql via
the closure, postgres via coverage.yml + the hooks-parity Postgres job.

Scope note: this does NOT touch the 31 reborn_*.rs root tests the v1
`cargo test` also re-runs. Excluding those cleanly needs binary-level
exclusion (the obvious `--skip reborn_` is unsafe — the v1 suite has its
own reborn_*-named tests in dockerfile_runtime_home.rs / workspace), so
that is deferred to the v1 build-smoke restructure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a88d8eb0-fab1-4081-a3a0-833942d885bf

📥 Commits

Reviewing files that changed from the base of the PR and between e849e83 and a833dbc.

📒 Files selected for processing (1)
  • .github/workflows/test.yml

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Consolidated test execution in CI/CD workflows by removing redundant test job steps and centralizing test coverage across per-crate workflows.

Walkthrough

The legacy tests job in .github/workflows/test.yml drops its explicit cargo test steps for ironclaw_reborn_composition, ironclaw_memory, and ironclaw_secrets. A comment is added stating those crates are covered by separate workflows (e.g., reborn-tests/coverage).

Changes

CI Legacy Test Job Cleanup

Layer / File(s) Summary
Remove redundant per-crate test steps
.github/workflows/test.yml
Deletes dedicated cargo test invocations for ironclaw_reborn_composition, ironclaw_memory, and ironclaw_secrets; replaces the prior rationale comment with a note that those crates are covered elsewhere.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

Three crates walked free from the monolith's gate,
Their tests now live elsewhere — no need to duplicate.
A comment takes the place of lines forty-two,
The legacy job grows leaner, tidy, and true.
Less noise, same signal. 🦀

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive PR description provides detailed context on what was removed, why, and explicitly defers a related optimization with reasoned justification. However, it lacks the structured template sections (Summary bullets, Change Type checkbox, Validation checklist). Use the repository's PR template with bullet-point summary, Change Type checkboxes, and Validation checklist sections for consistency, even if the freeform narrative is thorough.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits style with type(scope): summary format and accurately describes the main change: removing redundant CI test steps from the v1 monolith.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size: S 10-49 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Jun 23, 2026
@railway-app

railway-app Bot commented Jun 23, 2026

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5159 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jun 23, 2026 at 3:12 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: ci CI/CD workflows size: S 10-49 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant