Skip to content

fixed gates for core builds - #6030

Merged
akshaydeo merged 1 commit into
mainfrom
08-10-fixed_gates_for_core_builds
Aug 10, 2026
Merged

akshaydeo merged 1 commit into
mainfrom
08-10-fixed_gates_for_core_builds

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

The harness monitor previously spawned a new process per newman invocation, so a full sweep (main pass + deferred cache-parity pass) produced two unrelated tables with zeroed counters. This PR replaces that with a single long-lived monitor process that follows a pass manifest file (tmp/harness-monitor-passes.jsonl) for the entire lifetime of make run-provider-harness-test, accumulating counters across all passes into one table.

A separate bug is also fixed: the RE_REQUEST_DONE regex previously matched only a single word after the HTTP status code, so 400 Bad Request, 500 Internal Server Error, and similar multi-word statuses fell through every rule and were counted as neither pass nor fail. On a real sweep this silently dropped 1,362 of 1,937 completed requests from the table.

Changes

  • Pass manifest protocol: The Makefile appends one JSON line per newman invocation to tmp/harness-monitor-passes.jsonl (add_pass, end_pass, monitor_note shell helpers). The monitor polls this file and registers/closes passes dynamically rather than being told about a single run at startup.
  • Single monitor teardown: stop_monitor is now called once at the very end of the target instead of after each newman invocation. This means one alt-screen exit, one persistent table snapshot, and one $GITHUB_STEP_SUMMARY block per make run.
  • Tail lifecycle tied to pass boundaries: Closing a tail on end_pass prevents the cache-parity log from being replayed into the main pass's counters when the Makefile appends it to tmp/newman-cli.log.
  • CI heartbeat instead of table reprint: In CI mode the monitor now emits a single greppable line every interval instead of reprinting the full table, reducing ~6,700 lines of stale output in an append-only Actions log to one. The full table is printed exactly once at teardown. --ci-reprint-table restores the old behaviour for debugging.
  • RE_REQUEST_DONE regex fix: Changed (?:\s+[A-Za-z]+)? to (?:\s+[A-Za-z]+)* so multi-word HTTP status texts are matched.
  • Sequential attribution moved to tail handles: seqProvider, seqPendingName, and seqFolder are now per-tail rather than module-level globals, so two concurrent sequential tails (PARALLEL=0 main + cache-parity) cannot cross-contaminate each other's attribution state.
  • test-core-integrations and test-core-unit added as release gate dependencies: Both jobs were missing from the needs and if conditions of every release job (core-release, framework-release, plugins-release, bifrost-http-release, and all Docker release jobs). They were required to succeed but not waited on, so a release could proceed while they were still running or had failed. test-api-integrations was similarly missing from the Docker release jobs.
  • HARNESS_PROVIDERS used consistently: The hardcoded provider list in the parallel branch is replaced with the $(HARNESS_PROVIDERS) variable already used everywhere else.
  • Extracted library modules: lib/monitor-passes.mjs (manifest parsing, collection walking, denominator helpers) and lib/newman-log-lines.mjs (regex patterns and classifyLine) are split out of harness-monitor.mjs so they can be imported by tests without triggering the monitor's top-level side effects.
  • Unit tests added: monitor-passes.test.mjs, newman-log-lines.test.mjs, and monitor-lifecycle.test.mjs cover the manifest parser, collection walkers, and log line classifier. A test-harness-runner-lib Makefile target runs them with no network, no Bifrost, and no credentials.
  • SQLite paths in Python integration config: Hardcoded absolute paths replaced with relative paths.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

# Run the new harness runner unit tests (no network, no credentials)
make test-harness-runner-lib

# Verify the full harness run produces a single table and a single $GITHUB_STEP_SUMMARY block
make run-provider-harness-test CI=1 PROVIDER=passthrough

# Verify the cache-parity pass is counted in the same table, not a second one
make run-provider-harness-test PARALLEL=0 PROVIDER=openai

# Verify the old CI reprint behaviour is still accessible
make run-provider-harness-test CI=1 MONITOR_TABLE_REPRINT=1 PROVIDER=passthrough

Breaking changes

  • Yes
  • No

Related issues

Security considerations

None. No auth, secrets, PII, or sandboxing changes.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f4cd2ab-96a5-43d5-bb3c-1d25ca2146e4

📥 Commits

Reviewing files that changed from the base of the PR and between b0b2cf4 and 06911ff.

📒 Files selected for processing (9)
  • .github/workflows/release-pipeline.yml
  • Makefile
  • tests/e2e/api/runners/harness-monitor.mjs
  • tests/e2e/api/runners/lib/monitor-lifecycle.test.mjs
  • tests/e2e/api/runners/lib/monitor-passes.mjs
  • tests/e2e/api/runners/lib/monitor-passes.test.mjs
  • tests/e2e/api/runners/lib/newman-log-lines.mjs
  • tests/e2e/api/runners/lib/newman-log-lines.test.mjs
  • tests/integrations/python/config.json

📝 Walkthrough

Summary by CodeRabbit

  • CI/CD Improvements

    • Release and Docker builds now verify applicable unit, integration, and component checks before proceeding.
    • Build workflows better handle intentionally skipped checks while preserving required safeguards.
  • Testing Improvements

    • Added provider-harness unit test coverage and improved monitoring for parallel, sequential, and repeated test runs.
    • Test monitoring now provides clearer progress reporting, consolidated summaries, and more reliable failure detection.
  • Configuration

    • Updated integration test database paths to work consistently across development environments.

Walkthrough

The PR adds shared multi-pass monitoring for provider harness runs, standalone runner-library tests, release workflow test gates, and repository-relative SQLite paths for Python integration tests.

Changes

Provider harness monitoring

Layer / File(s) Summary
Monitoring contracts and parser tests
tests/e2e/api/runners/lib/monitor-passes.mjs, tests/e2e/api/runners/lib/monitor-passes.test.mjs, tests/e2e/api/runners/lib/newman-log-lines.mjs, tests/e2e/api/runners/lib/newman-log-lines.test.mjs
Added pass-manifest, collection-counting, pass-total, and Newman log-line helpers with direct Node.js test coverage.
Multi-pass monitor runtime
tests/e2e/api/runners/harness-monitor.mjs
The monitor now processes multiple passes, maintains per-pass state, handles log-tail attribution, aggregates statuses, and emits compact CI heartbeats by default.
Harness execution and test wiring
Makefile
Added the runner-library test target and changed parallel, sequential, and cache-parity runs to use shared pass registration and one final teardown.

Release pipeline gating

Layer / File(s) Summary
Release and artifact test gates
.github/workflows/release-pipeline.yml
Release, Docker build, and changelog jobs now wait for core and API tests and accept successful or skipped applicable prerequisites.

Integration test path portability

Layer / File(s) Summary
Repository-relative SQLite paths
tests/integrations/python/config.json
Changed the configuration and logs store paths from a developer-specific absolute path to repository-relative paths.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Makefile
  participant harness-monitor.mjs
  participant Newman
  participant PassManifest
  participant StatusFiles
  Makefile->>PassManifest: Register main and cache-parity passes
  Makefile->>Newman: Run provider collections
  Newman->>StatusFiles: Write request and provider statuses
  harness-monitor.mjs->>PassManifest: Poll pass entries
  harness-monitor.mjs->>StatusFiles: Read per-pass statuses
  harness-monitor.mjs->>harness-monitor.mjs: Aggregate totals and final statuses
  Makefile->>harness-monitor.mjs: Close passes and perform one teardown
Loading

Possibly related PRs

Suggested reviewers: pratham-mishra04

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 08-10-fixed_gates_for_core_builds

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

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@akshaydeo
akshaydeo marked this pull request as ready for review August 10, 2026 15:11
@akshaydeo
akshaydeo requested a review from a team as a code owner August 10, 2026 15:11
@akshaydeo
akshaydeo merged commit ae1d1d9 into main Aug 10, 2026
15 of 16 checks passed
@akshaydeo
akshaydeo deleted the 08-10-fixed_gates_for_core_builds branch August 10, 2026 15:12
atharvamhaske pushed a commit to atharvamhaske/bifrost that referenced this pull request Aug 13, 2026
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant