Skip to content

fix(ci): isolate and bound external smoke monitoring - #7511

Merged
bokelley merged 1 commit into
mainfrom
conductor/fix-external-smoke-ci-isolation
Sep 24, 2026
Merged

bokelley merged 1 commit into
mainfrom
conductor/fix-external-smoke-ci-isolation

Conversation

@bokelley

@bokelley bokelley commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Required integration shards discovered a test against the already deployed public agent. Transient endpoint latency could fail candidate CI at Vitest's 30s timeout, while SDK requests continued past that timeout. This moves the live check into scripts/external-smoke, invoked only by npm run test:external-smoke in a daily/manual synthetic workflow.

Authenticated discovery runs once per attempt and feeds local resolver assertions. Each probe allows two attempts, each with a fresh AbortSignal/fetch closure, a 10s overall budget and explicit 4s transport timeout. The fetch boundary supplies abort even where SDK rc.35 omits it. Only categorized transient transport errors retry. Failed-step/error evidence determines auth; public success requires passed discovery, tools, version and nonempty storyboard/bundle resolution.

Native fetch requests/bodies abort, and late SDK calls cannot start new fetches. SDK JavaScript and connection-cache entries may still unwind/remain until process exit; a custom fetch that ignores abort cannot be cancelled. These limits are documented.

Historical failure diagnosis only (read before editing; these af1-era jobs are not validation of the current rebased base/head):

Validation on exact head 6022db79f2db9a8a7d7479ce2ece679394074364 / base 36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614:

  • Combined external-smoke, frequency-cap, proposal-negotiation, training-agent, tenant routing/configuration, and account-change-feed transport suites: 7 files / 889 tests passed, including 30 focused smoke regressions.
  • Root unit suite: 71 files / 1,063 tests passed. SDK shim/runtime guards: 12/12 passed; installed SDK remains 14.0.0-rc.35.
  • Default Vitest integration discovery selects 161 files and excludes the live harness with fetch/socket calls prohibited. Focused tests also verify default-script isolation and scheduled/manual workflow structure.
  • Exact npm run test:external-smoke command passed against a loopback MCP fixture. No deployed endpoint was called during local validation.
  • Repository server and fixed-trace fixture typechecks, strict harness/regression typecheck, YAML parse, actionlint 1.7.12, changeset-scope and diff checks passed. No actionlint tool-version gap. The existing Vite native-loader __dirname warning is unrelated.
  • Independent exact-delta review in separate Conductor session d815d6db-c363-4d22-9dd7-e39ae24b0ae4: no blockers/issues/nits; independently verified all blobs and patch identities, assessed the combined base, reran 30/30 smoke regressions, actionlint and scope/diff checks, and reviewed completed local validation logs.

Exact-head GitHub CI is complete: 56 checks passed, one intentionally skipped (Ladon review for this Draft), zero failures or pending checks. Both Build Check run 34869082593 and Training Agent Storyboards run 34869082606 succeeded on attempt 1 at head 6022db79f2db9a8a7d7479ce2ece679394074364; no workflow rerun was requested for this head. All 22 storyboard jobs passed, including the final sales aggregate. All server-unit and database-integration shards passed. Exact integration logs total 2,102 passed / 8 skipped tests across 161 files; the removed live smoke test was not executed. PR review comments were rechecked and contain no CodeQL findings.

The push used an explicit lease expecting old remote head 92bf93058591ecfc0793f7f6ac6319c56b124255. Final verification confirmed main/base 36a86d6, PR head 6022db79, Draft status, auto-merge off, and a clean working tree. Prior 92bf930585 CI (including its targeted C2PA infrastructure rerun) and older af1/ea5498 local results are historical, not evidence for this rebased head. The local advisory storyboard matrix was skipped for the script-only package edit; the exact-head GitHub storyboard run above is authoritative.

Exact delta:

  • Base: 36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614
  • Head: 6022db79f2db9a8a7d7479ce2ece679394074364
  • Tree: e3561399f3150a61554218244dba24a355cfc791
  • SHA256 of git diff --binary BASE...HEAD: 0c50f21455b25e05a57f351500df1f7db5a2207c1affbc4ff5321a5a9591eaf3
  • Stable patch ID: a456783ea30142036b164cf8f989b1b6185e2144

After the coordinator released the hold, the exact reviewed patch was rebased without conflicts onto 36a86d6. The seven-file binary diff is byte-for-byte identical to both the original af1-based patch and the published 98fa207c...92bf9305 delta. Every surviving changed-file blob is identical and the integration-test deletion is preserved. Main's intervening #7516/#7515/#7512 changes have no filename overlap. There are no production-source, dependency, or lockfile changes in this delta.

#7520's C2PA installer remains Draft/REVIEW_REQUIRED and unmerged at final verification; it has not been copied into this PR. The new root npm ci site in .github/workflows/external-smoke.yml will be registered with that wrapper only after #7520 is separately approved/landed and the combined delta is validated. #7513's SDK mismatch and #7523's creative-isolation work are separate; this PR makes no SDK or creative-isolation change.

Human maintainers: renewed review is requested for this exact delta, especially integration isolation, transient-only retries, auth classification, and the documented SDK cancellation limits. Keep this PR Draft with auto-merge off; promotion and merge require a human decision. This is synthetic monitoring, not a release gate; no release invocation is added. The diff is separate from #7510 publication recovery, #7502 immutable-artifact protection, and #7404 grader persistence/completeness/publication; it makes no #7404 closure claim. No changeset: operational harness/tests only. The SDK modern getAgentInfo computed-default-versus-raw timeout propagation gap is reported separately, with no SDK change in this PR.

@bokelley
bokelley force-pushed the conductor/fix-external-smoke-ci-isolation branch from 92bf930 to 6022db7 Compare September 14, 2026 16:30

Copy link
Copy Markdown
Contributor Author

Human maintainers: please review the refreshed external-smoke isolation patch at 6022db79f2db9a8a7d7479ce2ece679394074364 on exact base 36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614.

The seven-file patch and all surviving changed-file blobs are identical to the previously reviewed patch. Separate exact-delta review is clean. Current-head CI completed with 56 passed / 1 intentionally skipped / 0 pending or failed, including Build Check and all 22 storyboard jobs, both on attempt 1. Exact tree/diff identities and local validation are in the updated body.

Please focus on integration isolation, transient-only retries, auth evidence, and the documented cancellation limits. #7520 remains unapproved/unmerged, so registration of this workflow's root install with its wrapper is deferred. #7513 and #7523 remain separate. Keep this PR Draft, auto-merge off; no merge or deployment has been performed.

@aao-secretariat aao-secretariat 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.

Ladon verdict: Escalate to human review

Escalate — gated path requires human/CODEOWNERS approval.

This PR adds .github/workflows/external-smoke.yml, which matches the deterministic gated glob .github/workflows/**. Per decision rule row 2, gated_paths: true AND review_decision is REVIEW_REQUIRED (not APPROVED), so the hard approval gate is not satisfied and the outcome must be escalate.

The change itself is clean: it moves a live external-agent check out of the required integration shards into a dedicated scheduled-only harness, with per-attempt AbortSignal + 4s SDK timeout + 10s budget, transient-only retries, fail-closed auth, and non-zero-exit failure surfacing. The reviewer found no Critical/High/Medium findings (only a non-blocking follow-up on alert fan-out) and correct changeset omission (script/test/workflow only).

Escalation reasons

  • .github/workflows/external-smoke.yml (added) is under the hard gated-paths approval gate (.github/workflows/**); review_decision is REVIEW_REQUIRED. A human/CODEOWNERS approval is required before merge.

Once a real approval lands, this PR would fall through to a clean approve on its findings.

Why human review

  • Touches gated path .github/workflows/external-smoke.yml (matches .github/workflows/**) and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/external-smoke.yml (added) matches .github/workflows/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Sep 23, 2026
@bokelley
bokelley merged commit 2d8bb46 into main Sep 24, 2026
59 checks passed
@bokelley
bokelley deleted the conductor/fix-external-smoke-ci-isolation branch September 24, 2026 12:47
bokelley added a commit that referenced this pull request Oct 15, 2026
…ase (#8208)

The daily external smoke job has failed every run since #7511. It pins the
default hosted line (3.0). A 3.0 pin cannot carry adcp_version, so
@adcp/sdk sends get_adcp_capabilities with only adcp_major_version: 3.
Per docs/reference/versioning.mdx the agent serves its highest 3.x release
and echoes adcp_version "3.2", but testCapabilityDiscovery validates the
response against the 3.0 pin, so 3.2-only shapes (features.bidding_policy,
newer compliance_testing scenarios) are reported as schema errors. The spec
says buyers validate against the echoed release, not their pin.

When the echo differs from the pin, the check now requires the echo to be in
adcp.supported_versions, discovers again pinned to that release (the SDK then
validates strictly against it), requires the agent to keep serving it, and
resolves storyboards from that release's hosted bundle.

Two more failures were hidden behind the schema error:
- SDK 15.8.1 keys comply_test_controller as protocol "compliance" in its tool
  cross-check but parses the compliance_testing block as
  "compliance_testing", so every compliance-capable agent fails that step.
  Waive only that exact message, and only when the response carries the
  block.
- The anonymous probe now gets a 401 Bearer challenge. Classify the SDK's
  "requires OAuth authorization" message as auth evidence.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant