Repository navigation
fix(ci): isolate and bound external smoke monitoring - #7511
Conversation
92bf930 to
6022db7
Compare
|
Human maintainers: please review the refreshed external-smoke isolation patch at 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. |
There was a problem hiding this comment.
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_decisionisREVIEW_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.
…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>
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 bynpm run test:external-smokein 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):
af1ea2666d77a56e287567567daa72a301562e52: discovery and duplicate resolver discovery timed out at 30s.2688c7a1cfb9e3ce696d5554ee6117fd3d64db40: all three passed in 1.476s. This supports a harness defect under latency, not a fix(release): fail closed on beta artifact deletions #7502 product regression.Validation on exact head
6022db79f2db9a8a7d7479ce2ece679394074364/ base36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614:14.0.0-rc.35.npm run test:external-smokecommand passed against a loopback MCP fixture. No deployed endpoint was called during local validation.__dirnamewarning is unrelated.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/base36a86d6, PR head6022db79, Draft status, auto-merge off, and a clean working tree. Prior92bf930585CI (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:
36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f6146022db79f2db9a8a7d7479ce2ece679394074364e3561399f3150a61554218244dba24a355cfc791git diff --binary BASE...HEAD:0c50f21455b25e05a57f351500df1f7db5a2207c1affbc4ff5321a5a9591eaf3a456783ea30142036b164cf8f989b1b6185e2144After 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 published98fa207c...92bf9305delta. 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 cisite in.github/workflows/external-smoke.ymlwill 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.