Skip to content

ci(reborn): add cargo-llvm-cov integration-tier coverage job (T0-COV) - #5430

Merged
henrypark133 merged 10 commits into
mainfrom
reborn-cov-T0-COV
Jul 1, 2026
Merged

henrypark133 merged 10 commits into
mainfrom
reborn-cov-T0-COV

Conversation

@henrypark133

@henrypark133 henrypark133 commented Jun 30, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adds a per-PR, informational coverage signal for the Reborn backend (roadmap task T0-COV, docs/reborn/reborn-backend-coverage-roadmap.md). A new Reborn Coverage workflow runs cargo-llvm-cov over the in-process integration-tier test binaries and surfaces a Reborn-crate-scoped line-coverage % plus a per-crate hole list in the PR job summary.

This is what makes the roadmap's "100% int-tier coverage" goal measurable and yields the real hole list to drive the rest of the coverage workstreams.

Measured baseline

Running the job's exact scope locally produced:

Line coverage (Reborn crates): 13.99% — 8351 / 59713 lines

Lowest-covered crates (the hole list): ironclaw_reborn_config, ironclaw_reborn_identity, ironclaw_reborn_traces, ironclaw_webui_v2(_static) at 0%; ironclaw_reborn_composition 18.9%; ironclaw_reborn 25.8%.

How the % surfaces per-PR

The Render Reborn coverage summary step writes the markdown table to $GITHUB_STEP_SUMMARY, so the % and per-crate breakdown appear on the workflow run's summary page for every PR (the repo's existing job-summary convention; the full per-file JSON export is uploaded as the reborn-coverage artifact for offline inspection).

Scope decisions

  • Test scope = the in-process int-tier suites per the roadmap's definition: tests/reborn_integration_*.rs + tests/reborn_group_*/. Discovered dynamically (scripts/ci/reborn-coverage-int-tier-tests.sh) so new suites auto-enroll.
  • Features = default (postgres+libsql+html-to-markdown+tui), matching how scripts/ci/run-reborn-root-partition.sh runs these same suites. No live Postgres needed (suites skip Postgres backends when DATABASE_URL is unset; backend_matrix stays libsql-only).
  • Combined cargo llvm-cov --workspace --test … --json one-shot is deliberate: the standalone report subcommand cannot take --workspace, so a split --no-report/report would scope the report to only the root package's src/ and report 0% for the linked Reborn crates. The --workspace combined form surfaces the member-crate files (verified: 0% → 13.99%).
  • Non-gating: no coverage threshold, no PR fail — matches "do not gate on coverage yet".
  • Reuses coverage.yml's pinned action SHAs / install action; LLM-neutralizing env mirrors reborn-tests.yml; crate-scope regex mirrors the reborn-tests.yml package allowlist (prefix-match for reborn_*/product_*/webui_v2_*, exact-match for the four single crates).

Verification

  • shellcheck + actionlint clean; YAML validated.
  • Local run: all 13 int-tier binaries pass; baseline % reproduced via scripts/ci/reborn-coverage-summary.sh.
  • No .rs / Cargo.toml / Cargo.lock changes — cargo build/clippy state is identical to the green main base.
  • Reviewed via thermo-nuclear + code-review (approach/local-patterns/maintainability) loops; findings addressed.

Added since open

  • Sticky PR comment (scripts/ci/reborn-coverage-comment.sh): upserts one marker'd comment so the %/hole-list lives in the conversation, not just the job summary. PR-only, continue-on-error: true — a fork's read-only token 403 can't red the check. Per-crate table folded into <details> to keep it compact.
  • Cache hygiene (save-if gated to main): PRs restore a warm main-seeded instrumented cache instead of each PR writing a branch-scoped copy that churns the ~10 GB LRU.
  • Review fixes: iterate all llvm-cov data[] datasets; GH_TOKEN fast-fail guard; timeout-minutes: 180 runaway backstop.
  • Regression tests (scripts/ci/test-reborn-coverage.sh, 43 cases): summary report/--zero-crates, comment POST/PATCH upsert via fake gh, int-tier discovery — mirrors the test-classify-test-scope.sh precedent.

Change Type

CI / tooling (new workflow + shell helpers + tests). No production .rs, Cargo.toml, or Cargo.lock changes.

Linked Issue

Roadmap task T0-COV (docs/reborn/reborn-backend-coverage-roadmap.md). No separate tracking issue.

Validation

  • shellcheck + actionlint clean; YAML validated.
  • scripts/ci/test-reborn-coverage.sh: 43/43 pass (mutation-checked non-vacuous).
  • Live: the Reborn Coverage job ran on this PR and posted the sticky comment (real ~13.9% run) — confirms the workflow, permissions, and upsert path end-to-end.
  • No .rs/Cargo.* changes — cargo build/clippy state identical to the green main base.

Security / Blast Radius

  • Trigger is pull_request (not pull_request_target) — no privileged-context checkout of untrusted code. Minimal token scope: contents: read + pull-requests: write (issues-comment upsert).
  • Fork PRs get a read-only token; the comment step is continue-on-error so its 403 is non-fatal. No secrets consumed (LLM env is neutralized).
  • Purely informational: no gate, cannot block or alter any other check.

Rollback Plan

Self-contained and additive. Revert the workflow + four scripts/ci/reborn-coverage-*.sh files (or disable via .github/workflows/reborn-coverage.yml) — nothing else depends on them, no state/migrations, no effect on other jobs. If the sticky comment misbehaves, deleting the comment step alone leaves the job-summary signal intact.

Review track

Highest review-risk track (CI workflow change). Reviewed via thermo-nuclear + multi-agent code-review loops; valid findings fixed, non-issues documented in the triage comment below.

🤖 Generated with Claude Code

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 June 30, 2026 04:38 Destroyed
@github-actions github-actions Bot added scope: ci CI/CD workflows size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules labels Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new coverage workflow runs Reborn integration-tier tests with cargo llvm-cov, exports JSON, renders a coverage summary, and posts a sticky PR comment. Two Bash scripts discover test targets and format Reborn crate coverage output, with regression tests for the helper scripts.

Changes

Reborn Coverage CI Workflow

Layer / File(s) Summary
Workflow wiring and execution
.github/workflows/reborn-coverage.yml
Adds the workflow triggers, permissions, concurrency, runner setup, test execution, summary rendering, PR comment step, and unconditional JSON artifact upload.
Integration-tier test discovery
scripts/ci/reborn-coverage-int-tier-tests.sh
Discovers reborn_integration_*.rs files and reborn_group_* directories under tests/, deduplicates suite names, and emits --test <name> arguments.
Coverage summary and sticky comment
scripts/ci/reborn-coverage-summary.sh, scripts/ci/reborn-coverage-comment.sh
Computes overall and per-crate Reborn coverage from llvm-cov JSON, renders Markdown output, and upserts a sticky PR comment with the summary plus zero-coverage crate callout.
Coverage helper regression tests
scripts/ci/test-reborn-coverage.sh
Exercises summary, zero-crate, sticky-comment, and discovery helpers against fixture data and temporary test layouts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Poem

🐰 I hop through tests with coverage bright,
JSON blooms in morning light.
A sticky note on the PR wall,
Keeps little crate gaps in view for all.
The carrot of CI gleams so neat,
And bash drums softly: thump-thump-beat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the new Reborn cargo-llvm-cov integration-tier coverage workflow and matches the main change.
Description check ✅ Passed The description covers the workflow, scope, validation, security, rollback, and linked issue, though it does not mirror every template heading exactly.
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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the contributor: core 20+ merged PRs label Jun 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces two new CI helper scripts: reborn-coverage-int-tier-tests.sh to dynamically discover and list Reborn integration-tier test binaries, and reborn-coverage-summary.sh to filter and format coverage data from a cargo-llvm-cov JSON export into a Markdown summary. Feedback was provided regarding a potential crash in the jq parsing logic of reborn-coverage-summary.sh when the data array is empty, suggesting a more robust and idiomatic indexing pattern.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread scripts/ci/reborn-coverage-summary.sh Outdated
@railway-app

railway-app Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

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

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jul 1, 2026 at 6:37 am

serrrfirat
serrrfirat previously approved these changes Jun 30, 2026
henrypark133 and others added 2 commits June 30, 2026 17:10
Adds a per-PR coverage signal for the Reborn backend. A new `Reborn
Coverage` workflow runs cargo-llvm-cov over the in-process
integration-tier test binaries (tests/reborn_integration_*.rs +
tests/reborn_group_*/) and surfaces a Reborn-crate-scoped line-coverage
percentage plus a per-crate hole list in the PR job summary.

This makes the roadmap's "100% int-tier coverage" goal measurable
(docs/reborn/reborn-backend-coverage-roadmap.md, T0-COV) and yields the
real hole list. It is informational only and does not gate PRs.

- .github/workflows/reborn-coverage.yml: the job. Combined
  `cargo llvm-cov --workspace --test <int-tier>` invocation so the report
  is scoped to every linked member crate (the standalone `report`
  subcommand cannot take --workspace and would scope to the root package
  only). Default features, matching run-reborn-root-partition.sh; no live
  Postgres needed. Reuses coverage.yml's pinned action SHAs / install
  action. Surfaces via $GITHUB_STEP_SUMMARY per repo norm.
- scripts/ci/reborn-coverage-int-tier-tests.sh: dynamically discovers the
  int-tier `--test` targets (auto-expands as new suites land).
- scripts/ci/reborn-coverage-summary.sh: filters the llvm-cov JSON export
  to the Reborn crate families and renders the % + per-crate table.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface the Reborn coverage %/hole-list as an upserted sticky PR comment so
it lives in the conversation, not just the Actions job summary. Visibility
only — never gates: the step is `continue-on-error: true` and PR-only, so a
fork PR's read-only GITHUB_TOKEN (comment API 403s) can't red the check, and
nothing in the job exits non-zero on a coverage number.

- reborn-coverage.yml: grant `pull-requests: write`; add a "Post sticky
  coverage comment" step after the existing $GITHUB_STEP_SUMMARY render
  (kept as-is), before the artifact upload.
- reborn-coverage-comment.sh (new): upsert one marker'd comment via pure
  `gh api` (no new action). Reuses reborn-coverage-summary.sh for both the
  body and the breadth holes (--zero-crates) — no duplicated aggregation jq.
  Prepends a 0-coverage breadth callout (informational; "target: 0" is the
  roadmap goal, not a check). Lookup uses `--paginate` so a busy PR's aged
  sticky isn't missed (which would post duplicates); marker passed via env
  to jq (no string interpolation); ids captured before `head` to avoid a
  pipefail SIGPIPE.
- reborn-coverage-summary.sh: add `--zero-crates` mode (single owner of the
  crate filter + aggregation); fold the per-crate table into <details> so
  the comment/summary stays compact (headline % + callout always visible);
  reword the informational note to cover the callout too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 1, 2026 00:27
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 00:27 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/reborn-coverage.yml:
- Around line 21-46: The workflow block is missing an explicit job timeout in
the Reborn Coverage CI, so add a bounded timeout to the job that runs this
workflow to prevent runaway or hung runs from consuming a runner for the default
360 minutes. Update the job definition associated with the Reborn Coverage
workflow so the coverage job has a generous but finite limit (for example,
90–120 minutes) while keeping the existing trigger, permissions, and concurrency
settings unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: db2f1147-4a1f-4c48-9282-c4830de53e51

📥 Commits

Reviewing files that changed from the base of the PR and between 0abb631 and ca39cda.

📒 Files selected for processing (4)
  • .github/workflows/reborn-coverage.yml
  • scripts/ci/reborn-coverage-comment.sh
  • scripts/ci/reborn-coverage-int-tier-tests.sh
  • scripts/ci/reborn-coverage-summary.sh
💤 Files with no reviewable changes (3)
  • scripts/ci/reborn-coverage-comment.sh
  • scripts/ci/reborn-coverage-int-tier-tests.sh
  • scripts/ci/reborn-coverage-summary.sh

Comment thread .github/workflows/reborn-coverage.yml
The reborn-coverage job had no `save-if`, so it wrote a multi-GB instrumented
rust-cache on every PR push. GitHub scopes caches per-branch, so those PR-branch
saves are invisible to other PRs and only churn the ~10 GB repo cache LRU —
evicting the one main-seeded cache that PRs actually restore from.

Gate saves to `push` on `main` (the workflow already triggers there), matching
the convention in test.yml/code_style.yml/replay-gate.yml. PRs now restore the
warm main-seeded instrumented cache and skip the wasteful save. Instrumented
caches stay separate from the non-instrumented reborn-tests caches by design
(coverage RUSTFLAGS change every crate's fingerprint — no cross-reuse possible).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 00:32 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new CI workflow that runs cargo llvm-cov against the Reborn integration-tier (in-process) test binaries and publishes a Reborn-crate-scoped line coverage percentage plus a per-crate “hole list” (job summary + optional sticky PR comment) to make the backend coverage roadmap measurable.

Changes:

  • Introduces Reborn Coverage GitHub Actions workflow to generate and upload an llvm-cov JSON export and render a PR-visible markdown summary.
  • Adds scripts to (a) dynamically discover integration-tier test binaries and (b) compute Reborn-crate-scoped aggregate + per-crate coverage from the JSON export.
  • Adds a script to upsert a sticky PR comment containing the same coverage summary for better visibility.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
scripts/ci/reborn-coverage-summary.sh Renders a Reborn-scoped markdown summary + per-crate breakdown from cargo llvm-cov --json output.
scripts/ci/reborn-coverage-int-tier-tests.sh Discovers integration-tier test binaries and emits --test <name> args for coverage execution.
scripts/ci/reborn-coverage-comment.sh Upserts a sticky PR comment with the rendered summary (visibility-only).
.github/workflows/reborn-coverage.yml New workflow wiring toolchain setup, coverage generation, summary rendering, sticky comment posting, and artifact upload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/ci/reborn-coverage-summary.sh Outdated
Comment thread .github/workflows/reborn-coverage.yml
Comment thread scripts/ci/reborn-coverage-comment.sh
Copilot AI review requested due to automatic review settings July 1, 2026 00:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread scripts/ci/reborn-coverage-summary.sh
Comment thread .github/workflows/reborn-coverage.yml
@github-actions

github-actions Bot commented Jul 1, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ 5 Reborn crate(s) have 0 int-tier coverage (target: 0) — ironclaw_reborn_config, ironclaw_reborn_identity, ironclaw_reborn_traces, ironclaw_webui_v2, ironclaw_webui_v2_static

Reborn integration-tier coverage

Line coverage (Reborn crates): 14.2% — 8649 / 60891 lines

Per-crate breakdown (12 crates, lowest-covered first)
Crate Line % Covered / Total
ironclaw_reborn_config 0% 0 / 1142
ironclaw_reborn_identity 0% 0 / 237
ironclaw_reborn_traces 0% 0 / 6662
ironclaw_webui_v2 0% 0 / 2653
ironclaw_webui_v2_static 0% 0 / 110
ironclaw_reborn_event_store 0.73% 6 / 825
ironclaw_product_adapter_registry 5.62% 25 / 445
ironclaw_product_workflow 6.22% 562 / 9036
ironclaw_product_adapters 12.74% 283 / 2221
ironclaw_reborn_composition 19.59% 5954 / 30395
ironclaw_reborn 25.3% 1809 / 7151
ironclaw_product_context 71.43% 10 / 14

This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout.

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Add an informational per-PR Reborn integration-tier coverage workflow that reports cargo-llvm-cov line coverage and per-crate coverage holes.

Stats: 7 findings (from 9 raw, 7 after duplicate/live-thread suppression) across 4 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 1.

Existing unresolved threads already cover the issues: write permission, jq .data[0]/grouping correctness, GH token validation, and job timeout surfaces, so this review does not repost those.

performance

  1. Medium Coverage clean deletes the restored instrumented cache (.github/workflows/reborn-coverage.yml:103-109, confidence 88) — anchor: .github/workflows/reborn-coverage.yml:103
    The job restores a main-seeded instrumented cache, but then runs cargo llvm-cov clean --workspace immediately before coverage. cargo llvm-cov clean removes prior coverage build artifacts; for this workflow that means PRs can discard the expensive restored instrumented target and pay the cold rebuild cost the cache comment says it is avoiding.

tests

  1. Medium Coverage aggregation has no committed edge-case test (scripts/ci/reborn-coverage-summary.sh:49-103, confidence 75) — anchor: scripts/ci/reborn-coverage-summary.sh:49
    The new summary CLI owns the Reborn crate filter, aggregate percentage, per-crate sorting, no-data fallback, and --zero-crates output, but nothing exercises mixed Reborn/non-Reborn llvm-cov JSON, empty/no-match JSON, or zero-covered crates. A jq or regex change can silently make the PR-visible coverage signal report the wrong scope or omit breadth holes.
  2. Medium Sticky comment upsert path is untested (scripts/ci/reborn-coverage-comment.sh:69-78, confidence 75) — anchor: scripts/ci/reborn-coverage-comment.sh:69
    The new PR-visible comment path branches between PATCHing an existing marker comment and POSTing a new one via gh api --paginate, but no test stubs gh to verify either branch. A marker, pagination, or endpoint regression would duplicate comments or stop updating the sticky coverage summary without a local signal.
  3. Low Dynamic test discovery lacks empty and multi-suite coverage (scripts/ci/reborn-coverage-int-tier-tests.sh:19-35, confidence 75) — anchor: scripts/ci/reborn-coverage-int-tier-tests.sh:19
    The workflow relies on this script to discover every Reborn integration-tier binary and to fail when none exist, but there is no test covering an empty tests tree, single-file suites, group-directory suites, and sorted/deduplicated output. A pattern or maxdepth regression could silently drop test binaries from coverage.

maintainability

  1. Medium Coverage scope duplicates the Reborn crate family allowlist (scripts/ci/reborn-coverage-summary.sh:47-47, confidence 80) — anchor: scripts/ci/reborn-coverage-summary.sh:47
    The summary script hardcodes a Reborn crate-family regex that is intended to mirror the package allowlist in reborn-tests.yml. Keeping those scopes as separate hand-maintained filters can let coverage percentage and hole-list reporting drift away from the test surface it claims to measure.
  2. Low Zero-coverage callout is split across two scripts (scripts/ci/reborn-coverage-summary.sh:27-31, confidence 75) — anchor: scripts/ci/reborn-coverage-summary.sh:27
    reborn-coverage-summary.sh has a second --zero-crates mode only so the comment script can call it again and stitch one extra callout around the normal report. That makes the PR comment and job summary shapes differ and spreads report composition across two files.

conventions

  1. Low CI workflow change is missing a documented rollback plan (.github/workflows/reborn-coverage.yml:1-1, confidence 75) — anchor: CONTRIBUTING.md:121 (no diff position — body only)
    The PR adds a new CI workflow, and the repo contribution rules classify CI workflow changes as needing a documented rollback plan. The PR body documents scope and verification, but not what to do if the workflow overloads CI or emits bad reports.

Comment thread .github/workflows/reborn-coverage.yml
Comment thread scripts/ci/reborn-coverage-summary.sh
Comment thread scripts/ci/reborn-coverage-comment.sh
Comment thread scripts/ci/reborn-coverage-int-tier-tests.sh
Comment thread scripts/ci/reborn-coverage-summary.sh
Comment thread scripts/ci/reborn-coverage-summary.sh
henrypark133 and others added 2 commits June 30, 2026 18:33
…meout (T0-COV)

Fixes from PR #5430 review (valid findings only):

- summary.sh: iterate all llvm-cov `data[]` datasets (`.data[]?.files[]?`)
  instead of `.data[0]` — the export format permits multiple datasets and
  index-0-only would undercount. The trailing `?`s keep the empty/missing-data
  path crash-free (unchanged behavior; `{"data":[]}` / `{}` still fall through
  to the no-data message).
- comment.sh: fast-fail if GH_TOKEN is unset, matching the existing
  GITHUB_REPOSITORY/PR_NUMBER guards (clearer failure for local runs; gh needs
  it for the API calls).
- reborn-coverage.yml: add `timeout-minutes: 180` as a runaway-hang backstop
  (well above any real build; halves GitHub's 360-minute default). Corrects the
  prior comment that claimed coverage.yml runs its instrumented job without a
  timeout — its matrix job caps at 60.

Rejected as non-issues: jq null-index "crash" (jq indexes null as null, not an
error — verified); group_by requiring pre-sorted input (jq's group_by sorts
internally); needing `issues: write` (the sticky comment already posts on a
same-repo PR with pull-requests: write). `clean --workspace` preserving the
cache is intentional (it scopes to workspace members, keeping instrumented
deps).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add scripts/ci/test-reborn-coverage.sh (mirrors the test-classify-test-scope.sh
precedent) covering the three new coverage helpers — closes the "no committed
edge-case test" review findings on PR #5430:

- summary.sh report mode: mixed Reborn/non-Reborn filtering, no-match no-data
  message, empty/absent `data` no-crash, multi-dataset `.data[]` aggregation,
  zero-covered crate sorted to top.
- summary.sh --zero-crates: exact zero-covered name list, all-covered/empty →
  empty output.
- comment.sh upsert (fake `gh` on PATH): no-sticky → POST with marker body,
  existing sticky at a non-first list position → PATCH (not a duplicate POST),
  zero-covered callout prepended.
- int-tier discovery: empty tree → exit 1, single file/dir suites, mixed
  suites sorted+deduped.

43 cases, self-contained (mktemp + trap cleanup), shellcheck-clean. Not wired
into a workflow — matches the unwired test-classify-test-scope.sh precedent;
run manually as a local regression signal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 01:41 Destroyed
@github-actions github-actions Bot added size: XL 500+ changed lines and removed size: L 200-499 changed lines labels Jul 1, 2026
…-COV)

`assert_line_before`'s `grep -n | head | cut` pipelines run under
`set -euo pipefail`; a missing needle made grep exit 1, propagated by pipefail
to the assignment, aborting the whole suite before the function's own empty
checks could report a normal FAIL — contradicting the "run every case" harness
design. Append `|| true` so a missing needle yields an empty line number and
falls through to the FAIL path.

Addresses the Copilot review finding on PR #5430.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 05:18 Destroyed

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Add non-gating Reborn integration-tier coverage reporting to CI with workflow, shell helpers, and PR comment summaries.

Stats: 2 findings (from 10 raw, 2 after dedup/filter) across 2 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0

Performance / Concurrency

  1. Medium Concurrency key can cancel unrelated fork PR coverage runs (.github/workflows/reborn-coverage.yml:36, confidence 88) — anchor: .github/workflows/pr-label-classify.yml:13
    For pull_request events, github.head_ref is only the source branch name, so two different fork PRs with the same branch name share one coverage concurrency group.

Conventions

  1. Low Do not cite a missing coverage roadmap as the scope source (scripts/ci/reborn-coverage-int-tier-tests.sh:6, confidence 75) — anchor: AGENTS.md:78
    The helper and workflow cite docs/reborn/reborn-backend-coverage-roadmap.md, but that file is absent from this PR head.

Suppressed as already addressed or not fresh: the issues: write concern is already resolved in live review threads and contradicted by the posted sticky comment; the allowlist duplication is explicitly deferred in the PR body; the fork-token concern is limited by pull_request read-only fork tokens and first-time-contributor approval; low-value missing-test suggestions were not posted.

Comment thread .github/workflows/reborn-coverage.yml Outdated
Comment thread scripts/ci/reborn-coverage-int-tier-tests.sh Outdated
…0-COV)

Two PR review fixes:

- reborn-coverage.yml: key the pull_request concurrency group on
  `github.event.pull_request.number` instead of `github.head_ref`. head_ref is
  just the source branch name, so two fork PRs sharing a common branch name
  (main/fix/…) collided on the same group and, with cancel-in-progress, one
  push could cancel the other PR's coverage run. number is null off-PR, so it
  falls back to github.ref for push/dispatch (pattern from pr-label-classify.yml).
- Drop the citation of docs/reborn/reborn-backend-coverage-roadmap.md from the
  workflow header and the int-tier discovery script — that file exists on
  neither this branch nor main, so it was a dangling source-of-truth pointer.
  The int-tier scope is already stated inline (reborn_integration_*.rs +
  reborn_group_*/), so nothing is lost.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 1, 2026 05:32
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 05:32 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Add an informational Reborn integration-tier coverage workflow using cargo-llvm-cov with summaries, sticky PR comments, helper scripts, and tests.
Stats: 5 findings (from 7 raw, 5 after dedup/manual validation) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0

Security

  1. High PR code receives a write-scoped GitHub token (.github/workflows/reborn-coverage.yml:125-131, confidence 75) — anchor: .github/workflows/reborn-coverage.yml:129
    This pull_request workflow checks out and executes the PR-controlled version of scripts/ci/reborn-coverage-comment.sh, then passes it github.token while the job grants pull-requests: write. GitHub downgrades fork PR tokens to read-only, but same-repo PRs use the configured write scope, so a modified PR script can use the bot token to mutate PR conversation state instead of only publishing the coverage summary.

Approach

  1. Medium Informational coverage still fails PR checks (.github/workflows/reborn-coverage.yml:94-114, confidence 75) — anchor: .github/workflows/reborn-coverage.yml:96
    The workflow and PR describe this as informational/non-gating, but the pull_request job runs discovery and cargo llvm-cov under set -e in a normal check. Any coverage tooling, discovery, or test failure turns Reborn Coverage red on the PR, duplicating the existing Reborn test gate rather than merely surfacing a coverage signal.

Tests

  1. Medium Crate allowlist boundary cases are not covered (scripts/ci/reborn-coverage-summary.sh:43-47, confidence 75) — anchor: scripts/ci/reborn-coverage-summary.sh:47
    The regex intentionally prefix-matches the reborn/product/webui_v2 families while exact-matching single crates such as ironclaw_architecture and the v2 adapters, but the regression fixtures only exercise family prefixes and non-Reborn exclusion. A future regex change could include lookalikes like ironclaw_architecture_extra or drop an exact-match adapter without failing the suite. Also flagged by: maintainability/Low.
  2. Low Summary missing-file branch is untested (scripts/ci/reborn-coverage-summary.sh:35-37, confidence 100) — anchor: scripts/ci/reborn-coverage-summary.sh:35
    The summary helper has an explicit fast-fail path for a nonexistent coverage JSON file, but the regression suite covers valid, empty, and non-Reborn JSON fixtures only. A regression could drop the clear error or non-zero exit without failing tests.
  3. Low Comment helper missing-file branch is untested (scripts/ci/reborn-coverage-comment.sh:28-30, confidence 100) — anchor: scripts/ci/reborn-coverage-comment.sh:28
    The sticky-comment helper rejects a missing coverage JSON file before reading GitHub environment or calling gh, but the tests only cover env guards and POST/PATCH behavior. That pre-mutation failure path can regress independently.

Dropped During Validation

  • Dropped the performance JSON-size item because the PR intentionally uploads the full JSON artifact for offline inspection.
  • Dropped the maintainability allowlist-duplication item as already covered by a resolved prior thread and tracked separately; retained the narrower test-boundary gap above.

Comment thread .github/workflows/reborn-coverage.yml
Comment thread .github/workflows/reborn-coverage.yml
Comment thread scripts/ci/reborn-coverage-summary.sh
Comment thread scripts/ci/reborn-coverage-summary.sh
Comment thread scripts/ci/reborn-coverage-comment.sh
Add three regression cases to test-reborn-coverage.sh (PR #5430 review):

- A6: crate-filter boundary — fixture with exact single-crate matches
  (ironclaw_architecture, ironclaw_slack_v2_adapter), a family-prefix crate
  (ironclaw_reborn_config), and a lookalike (ironclaw_architecture_extra).
  Asserts the exact + prefix crates appear, the lookalike is excluded, and its
  999 lines are dropped from the aggregate (16/30, not 16/1029) — pins the
  prefix-vs-exact regex semantics against a future edit that widens the match.
- A7: summary.sh on a nonexistent JSON path -> non-zero exit + "coverage JSON
  not found".
- C7: comment.sh on a nonexistent JSON path -> non-zero exit + "coverage JSON
  not found" + NO gh mutation (guard fires before any gh call).

60 cases, shellcheck-clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 06:11 Destroyed

@henrypark133 henrypark133 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review (multi-agent)

Intent: Add a non-gating Reborn coverage workflow that measures integration-tier test coverage and posts per-crate hole lists in PR summaries/comments.
Stats: 1 finding (from 2 raw, 1 after dedup/reply validation) across 1 file. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0

Tests

  1. Medium Malformed coverage JSON lacks caller-level no-mutation coverage (scripts/ci/reborn-coverage-comment.sh:44-44, confidence 75) — anchor: scripts/ci/reborn-coverage-comment.sh:44
    reborn-coverage-comment.sh renders the coverage JSON before it POSTs or PATCHes the sticky PR comment, so malformed existing JSON should fail before any GitHub mutation. The current tests cover missing files and env guards, but not an existing malformed JSON file, so they do not lock in that jq/render failures abort before the gh mutation path.

Prior Reply Notes

  • Accepted the write-scoped token reply as sound for this PR: the workflow uses pull_request, fork tokens remain read-only, and same-repo PR authors already have collaborator push rights. I did not repost that item.
  • Accepted the non-gating reply as sound: API/ruleset checks did not show Reborn Coverage as a required status check, so a red coverage job is visible but not merge-gating. I did not repost that item.
  • Verified the prior test gaps are fixed in 113c5d09: A6 covers allowlist exact-vs-prefix boundaries, A7 covers missing summary JSON, and C7 covers missing comment JSON with no fake-gh mutation.
  • Suppressed the cache-clean finding as already covered by a resolved thread with a sound reply: cargo llvm-cov clean --workspace preserves dependency artifacts while avoiding stale workspace coverage; the suggested --profraw-only/--no-clean path risks stale coverage for changed workspace code.

Comment thread scripts/ci/reborn-coverage-comment.sh
Add case C8 to test-reborn-coverage.sh: an existing-but-malformed coverage
JSON. Distinct from C7 (missing file) — it passes comment.sh's `[ -f ]` guard
and must instead fail at the reborn-coverage-summary.sh render (jq parse error
under `set -e`), before any POST/PATCH. Asserts non-zero exit and no fake-gh
mutation, pinning the render-before-mutate ordering. Exit asserted non-zero
(not an exact code) since jq's parse-error exit varies across versions.

62 cases, shellcheck-clean. Addresses the PR #5430 review finding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 1, 2026 06:36
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5430 July 1, 2026 06:36 Destroyed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/reborn-coverage.yml

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5430 — a8f1b10b Deployed Jul 1, 2026 by railway-app[bot]
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: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants