ci: give the live-instance integration suite a nightly CI home (#1896)

[dolho]

~93 of the test_*.py files in tests/ root need a running Trinity — the fixtures create and delete real agents against TRINITY_API_URL — and no CI job ran them. They gated nothing.

integration-nightly.yml is that home. It composes what already exists rather than inventing anything: scripts/deploy/start.sh for the stack (as frontend-e2e.yml does), the discover → test → comment split and sticky-comment machinery from backend-unit-nightly.yml, and diff-pytest-failures.py for base-vs-head attribution.

It did not need to wait for #1895

The issue's Technical Notes open with "Sequencing: land #1895 first" — but tests/run-core.sh:22 already defines this suite directory-based:

pytest -m "not slow" --ignore=unit --ignore=process_engine

The workflow uses the same formulation, so every file #1895 relocates leaves this job's scope automatically, with no workflow edit. There is no "~60" target to chase and no file list to drift — which is also the AC that keeps this from rotting.

Why nightly (AC 2)

The suite mutates its target and needs ~3-5 min on top of a ~2-3 min boot, twice (base and head). In the merge path that is ~10 min on every backend PR plus a mutating flake surface in front of every merge. Nightly also gets the base-vs-head attribution the ACs require for free — that tooling exists for exactly this shape.

Three things measured, not assumed — each would have shipped this broken

1. --continue-on-collection-errors is load-bearing. Three root modules currently fail to import (two git-sync files wanting utils.credential_sanitizer, plus tests/integration's conftest). Without the flag pytest aborts with Interrupted: 3 errors during collection and writes no JUnit XML at all — the job would have failed every night, for every PR, reporting nothing. With it, the errors land on both sides, cancel in the diff, and a new collection error still surfaces. I found this by actually running the collection, not by reading the invocation.

2. src/cli has to be installed. test_cli_*.py imports trinity_cli, deliberately kept out of requirements-test.txt (an -e ./src/cli line breaks GitHub's dependency-graph updater — tests/setup-env.sh documents it). Without the on-demand install those modules fail to import and their tests silently never run — precisely the no-coverage this issue exists to end. That accounted for 2 of the original 5 collection errors.

3. A missing JUnit must fail the job loudly. diff-pytest-failures.py is fail-closed on a missing input, so a stack that never booted would have been reported to the author as "this PR introduced regressions" — an infrastructure failure wearing a PR's name. That is bug(ci): nightly unit-suite reports a false merge conflict on every PR — shallow fetch on both sides leaves no common ancestor #1941's exact class, so the job now checks both XMLs exist and errors out instead of posting a false verdict.

Also inherited from #1941, since this is a second workflow that merges: full-depth fetch (two shallow sides share no ancestor and every PR reads as conflicting), and a conflict verdict gated on actual unmerged paths.

AC coverage

• 1 — boots a stack via scripts/deploy/start.sh, runs the live root suite
• 2 — gated: nightly + workflow_dispatch (with a single-PR input), never on pull_request
• 3 — JUnit XML on both sides, fed to the same regression diff tooling the unit job uses
• 4 — docker compose down -v between sides so head never inherits base's agents/volumes/DB; the runner VM is destroyed afterwards regardless, which is what makes the instance disposable
• 5 — 45-min ceiling, expected ~15-20, with the escalation written into the workflow header: shard by marker or cut the PR limit before raising the ceiling
• 6 — tests/run-core.sh untouched; a guard asserts the two keep defining the same suite

Guards

13 tests in test_1896_integration_nightly_workflow.py, in the shape of test_1941_nightly_merge_depth.py — they pin the properties whose absence is invisible until the night it matters, including the two load-bearing flags above and the security split (the PR-code job holds only contents: read; the write-scoped comment job never checks out PR code).

What I could not verify

The workflow has never executed — it is nightly and its first real run is after merge. YAML parses, every run block passes bash -n, the diff script's self-test passes and its CLI contract matches, and the pytest invocation was run locally to the point of collection. The stack boot itself is copied verbatim from frontend-e2e.yml, which runs green today. First-run risk is real; workflow_dispatch with a pr_number input exists so it can be smoke-tested against one PR on demand rather than waiting for 06:30 UTC.

Known caveat, not introduced here: the 3 pre-existing collection errors above are real breakage in the root suite. They are exactly what "no CI job runs these" produces, and they belong to the #1895/#1896 cleanup rather than to whichever PR this job runs against first.

Related to #1896

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

Rebased onto current dev — the PR was conflicting, so there was no refs/pull/1963/merge for CI to build and no checks were reporting at all. They should run now.

Conflicts were in tests/registry.json (both sides appended entries; unioned, ours last). Resolved by re-serializing from parsed JSON rather than splicing lines, so the separating comma can't be lost.

Local verification after the rebase is in the individual runs; no source conflicts, only the registry.

Ready for review.

[obasilakis]

This is a well-built workflow and the AC coverage is complete — stack via start.sh rather than a bespoke compose path, nightly-gated, base-vs-head JUnit attribution, down -v between sides, a stated budget with a written escalation ladder, run-core.sh untouched. 13 guard tests, all passing. The #1941 lessons are applied pre-emptively (fetch-depth: 0, no --depth on the PR-head fetch, the unmerged-path gate), which is good to see land in a second workflow before the first one has even merged. The #1891 Python pin catching this file at '3.11' is that guard doing exactly its job.

Two blocking items, both small.

---

Blocking 1 — the false ✅ "clean", filed as #2029

Filed separately because it spans three workflows rather than just this PR: #2029.

Two paths reach it here. The merge infra branch (:177) exits without writing merge_conflict; the missing-XML branch (:278) exits after merge_conflict=false was written but before steps.diff runs. Either way Write status JSON is if: always() (:303), regression defaults to false (:307), and the script's final else (:376) renders:

✅ Live-instance integration suite clean when this PR is merged into dev

:271-273 states the opposite:

Fail the job loudly instead; the comment step then posts nothing for this PR rather than something false.

It posts something false — the comment job is if: always() and never inspects needs.test.result.

This one is the worst of the three instances. backend-unit-nightly.yml:301 guards creation with } else if (status.regression || status.merge_conflict), so a false clean there can only overwrite an existing sticky. Here :396 is a bare } else { → createComment unconditionally, and this is a new workflow where no PR has a sticky yet. The first bad night posts a fresh green tick to up to 20 PRs.

Fix in #2029 — refuse to write the status file when the verdict is unknown, so the comment job skips that PR.

---

Blocking 2 — a secret in the job that runs untrusted fork code

:196

ANTHROPIC_API_KEY: ${{ secrets.E2E_ANTHROPIC_API_KEY || 'placeholder' }}

That env is on the step that runs ./scripts/deploy/start.sh and the pytest suite from the merged PR tree — :234 switches to head-side before boot_stack, so it is arbitrary shell from the PR, not just test files. schedule exposes secrets to fork PRs (unlike pull_request), and discover enumerates them: #2009 is an open fork PR targeting dev right now.

This breaks the invariant the header says is inherited verbatim. backend-unit-nightly.yml's only secret is GITHUB_TOKEN in discover — its test job is genuinely credential-free. This one isn't.

The guard reads as if it covers this but doesn't. test_the_job_running_pr_code_has_no_write_token (:158-161) asserts only on the permissions: block. Token scope is correct; a secrets.* reference in the same job is unguarded — which is precisely how this would survive review. Extending that test to assert no secrets. other than GITHUB_TOKEN appears in the test job would close it.

To be straight about severity: E2E_ANTHROPIC_API_KEY does not exist on the repo today (gh secret list shows seven secrets, not that one), so the expression resolves to 'placeholder' and nothing is currently exposed. The problem is that the workflow is designed around that secret existing, and whoever adds it will not re-review this file. Either make the design fork-safe (have discover skip isCrossRepository PRs) or commit to the placeholder and skip-mark the tests that need a real key — which is the AC below anyway.

---

Non-blocking

1. The "pre-merge" stash comment names the wrong branch. :122-128 says "Workspace is unmodified dev at this point", but the checkout at :117 has no ref: and this repo's default branch is main, so a scheduled run copies the trusted diff script from main. The security property holds (still trusted, still pre-merge), but the script can be a release behind dev's. backend-unit-nightly.yml pins ref: dev explicitly — worth matching, and it would make the comment true.

2. || 'placeholder' silently degrades coverage. With no key, agent-dependent tests fail on both sides and cancel out in the diff, so the job reports clean while a chunk of the suite is inert. #1896's own AC asks to "confirm nothing requires a real external key … or the affected tests must be skip-marked rather than given credentials in CI" — the skip-marking isn't there. Given this issue is specifically about tests that gate nothing, inheriting a silently-inert slice seems worth closing here rather than later. It also interacts with Blocking 2: skip-marking is the fork-safe answer.

3. Fleet runtime. 20 PRs at max-parallel: 3 and ~15-20 min each is roughly 2 hours wall-clock. Fine for a nightly — noting it only because the escalation ladder at :34-39 is written per-PR and the fleet total is the number that will actually bite.

---

The directory-based membership (--ignore=unit --ignore=process_engine, matching run-core.sh:22) is the detail that makes this survive #1895, and the note explaining why it therefore didn't need to wait for #1895 is right. --continue-on-collection-errors being justified with the actual count of currently-broken imports, rather than added defensively, is the kind of thing that makes this reviewable.