Skip to content

ci: add isolated non-blocking timing telemetry - #631

Open
tianxiaochannel-oss88 wants to merge 5 commits into
mainfrom
codex/ci-observability
Open

tianxiaochannel-oss88 wants to merge 5 commits into
mainfrom
codex/ci-observability

Conversation

@tianxiaochannel-oss88

@tianxiaochannel-oss88 tianxiaochannel-oss88 commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add read-only CI timing telemetry in an independent best-effort job after the existing quality gate, with a 14-day ci-telemetry artifact and GitHub Step Summary.
  • Capture the current workflow attempt's job/step observations and available shard reports, including failed reports and per-file results.
  • Keep measurement labels explicit: job start delay includes dependencies, shard file sums are not parallel wall-clock time, and elapsed completed-job time is not a DAG critical path.
  • Preserve all existing test selections, platform lanes, exact-once coverage checks, and required quality-gate semantics. No release, version, lockfile, or product-runtime changes.
  • Add failure-only, test-scoped Node command lifecycle diagnostics while preserving every original assertion and the 4000 ms command deadline. This is evidence collection, not a fix for the hosted timeout.

Validation

  • Original telemetry head validation: 292 relevant local CI, scanner, runner, native-verifier, and relay-cleanup tests, 291 passed, 0 failed, 1 pre-existing environment-gated skip (Node 22.22.1, serial file execution).
  • Local workflow contracts and real extracted Bash commands cover successful paginated collection, API failures, invalid JSON/page shapes, and unavailable render fallback.
  • Existing lane-policy regression tests still reject failed, cancelled, skipped, or missing required results.
  • Markdown links, public-repository safety, and git diff --check pass.
  • YAML parsing passes for all 11 CI jobs. actionlint is not installed locally; hosted Actions validation remains necessary.
  • Hosted run 41343476793, attempt 1, on head dfc6a458ed02d93c42e18ea4544f504f1a525e21 failed. The PR is not ready to merge; see the investigation below.
  • The new telemetry job succeeded on that failed run. Its downloaded artifact contains eight shard reports, including the failed daemon file. This validates failure-report collection, not overall CI success.

Hosted CI Investigation (Merge Blocked)

  • The originating failed check is Daemon tests (shard 0), job 124958879262. The required aggregate quality gate also failed. Other daemon shards, package/core lanes, builds, typechecks, platform smoke checks, and CodeQL passed on this head.
  • packages/daemon/test/channel-node-invocation-binding.test.mjs:113 expected the first real ESM command to return succeeded, but it returned timed_out with the existing timeoutMs: 4000 request. The subtest took 4896.695 ms; that file reported four passing tests and one failure, then exited normally. This is not a reproduced HTTP-server leak or a file-runner deadline failure.
  • At the originally failing head dfc6a458, the workflow diff only appended the telemetry job. The failing test, product sources, dependencies/lockfile, and shard/process runner inputs were unchanged from base c472c62fe80ea52a579593c289e6bca42c450205. Telemetry depends on the aggregate quality job and started after it ended, so telemetry execution did not overlap this failure. The later diagnostic-only test changes are described separately below.
  • Local source-revision comparison: 20 trials each on the base and PR head, alternating order, Node 22.22.1, two real shard workers, with acp-approval-journal.test.mjs alongside the victim. The original four-second command limit was unchanged. Package sources and runner inputs were identical; the baseline reused the head's built package outputs and dependencies via symlinks, so these are not independently built revisions or hosted-runner reproductions.
Revision Failed / Trials Subtest ms (min / p50 / p95 / max) First command close ms (min / p50 / p95 / max)
Base c472c62f 0 / 20 477.015 / 489.344 / 526.345 / 551.252 29.037 / 29.958 / 32.801 / 37.423
Head dfc6a458 0 / 20 477.733 / 484.266 / 499.322 / 500.341 28.801 / 29.399 / 30.740 / 31.226
  • Historical hosted samples: this subtest passed at heads a7594c74aab4 (1581.069 ms), 7e752ce119b1 (2679.953 ms), 63bc33230b44 (2309.817 ms), and PR v0.2.0 perf: speed up CI with bounded test shards and cleanup #630 head dd1212c2bb21 (1656.897 ms). The first failing head in this limited sample is dfc6a458; that observation does not establish a causal commit. Historical logs do not contain individual command startup/exit timing.
  • Root cause remains unconfirmed. The local trials did not reproduce the failure; runner load, login-shell startup, process-tracking overhead, and cross-test pollution are not established causes. No timeout increase, skip, retry, hosted rerun, or merge was used to obtain a green result.
  • Next evidence needed: a hosted recurrence with command lifecycle diagnostics. Process-tracker timing and the original stall's cause remain unmeasured. A delay-injection probe demonstrates the deadline and diagnostic mechanisms, not the original delay's cause.

Diagnostic Follow-Up (Not A Flake Fix)

  • Only three daemon test files change. The test observes actual child-process construction, spawn, first stdout/stderr, exit and close with monotonic timings. It does not replace the production spawn function, add polling, or modify product code, dependencies, CI workers or deadlines.
  • On assertion/execution failure, node_goal_command_diagnostic_v1 records command sequence, the unchanged 4000 ms limit, runtime version/platform, lifecycle timings, status, duration, exit code/signal, process cleanup observation and stdout/stderr byte counts. Commands, paths, environment values, output bodies, artifact payloads and error messages are excluded. Listeners and the diagnostic subscription are detached on returned and thrown paths.
  • Fresh Node 22.22.1 validation: 9 binding/diagnostic tests pass; 57 telemetry/workflow/shard/process tests pass. A real two-worker run of the binding, ACP approval journal and diagnostic files passes all 36 tests, with no file timeout or leaked-process classification. Existing unchanged compiled package outputs are reused, not independently rebuilt. After a small assertion-safety adjustment, all four diagnostic tests pass again.
  • Two local five-second startup-delay probes were deliberately run, one directly and one through the real shard runner. Both retain the original failed success assertion and four-second deadline. The latter records a spawned process with no output, SIGTERM at about 4006 ms, process close, status=timed_out, stopReason=timeout, and settledBy=close. The file remains failed, exit 1, and its diagnostic is retained in the normal per-file report log. These are diagnostic validation probes, not reproductions or explanations of the hosted fault.
  • Public-repository safety and staged diff checks pass. A bounded independent read-only audit found no actionable issue; it does not replace hosted evidence.
  • The next push is a diagnostic collection run, not a blind rerun to obtain a passing gate. The original failed run remains recorded above. Do not treat a diagnostic green run as proof of root cause or merge readiness. No timeout increase, retry, skip, production fix, admin merge or deployment is included.

Observation Limits

  • Telemetry failures are isolated from the mandatory quality gate; they do not certify test success.
  • Failure counts apply only to available reports. Missing or partial reports do not establish zero faults for the full run.
  • The workflow was still in progress at telemetry capture time, so that snapshot's run duration is not its final completed duration. File-level timeout counts do not include an internal command timeout that surfaces as a failed test file. Observe 5-10 complete hosted runs before changing shard counts or worker concurrency; this PR claims no fixed speedup or billing reduction.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-03T08:44:05.320680Z
We couldn’t complete this review. Comment @codex review to try again.
d3a03a4 New commits
🔒 Security Review ⚠️ Failed 2026-11-03T08:41:54.909573Z
We couldn’t complete this review. Comment @codex security review to try again.
d3a03a4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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