Commit Graph

13 Commits

Author SHA1 Message Date
ethernet 9b3c42329c fix(ci): place the resource overlay over the profiled window, not the job
the sparklines were stretched across the whole gantt bar, but the profiler
wraps a SINGLE step (.github/actions/profile runs it between start/stop), so
on a job dominated by checkout + uv sync + post-job cleanup the samples only
describe a slice in the middle. reproduced: a 30s profile inside a 100s job
whose profiled step ran t=60..90 drew at left 0% width 100% instead of
left 60% width 30% — putting a cpu spike visually under a step that never ran.

the profile json had no wall-clock anchor to place it with, only duration_s,
so emit started_at/completed_at as iso-8601 utc in the same shape as github's
job timestamps. monotonic() still drives the sampling loop (immune to clock
steps); the timestamps are purely for placement.

_profile_window_pct() converts that window into bar-relative percentages and
both overlay states now use it — the expanded holder directly, the collapsed
strip via a .res-clip wrapper so its 100%-width is relative to the window
rather than the bar. the two states are asserted to agree on the x-axis.

falls back to the full bar, i.e. exactly today's behaviour, when the profile
predates these fields, when the timestamps don't parse, or when the window
doesn't overlap the job at all (clock skew between the runner writing the
profile and github's timestamps). a profiler that outran the job's
completed_at is clamped to the bar, and a sub-percent window keeps a 0.5%
hairline so it can't collapse to invisible.

7 new tests. verified they discriminate: forcing the old always-stretch
behaviour fails 4 of them, while the 3 fallback tests keep passing since
full-bar is what they want. tests/ci 145/145.
2026-08-05 09:22:26 -04:00
ethernet 979b9bf7b6 feat(ci): overlay cpu/ram/disk sparklines on the timing report gantt
ops/s alone can't tell you whether a device is saturated — it reads low on a
few large IOs that pin the disk at 100% busy, and high on many small cached
ones. sample io_ticks (diskstats field 13) instead: its delta over the
interval is device busy time, i.e. iostat's %util. take the busiest single
device rather than the sum, since summing across devices exceeds 100% on a
multi-disk node and means nothing as a saturation percentage.

divide by the real elapsed gap rather than the nominal interval — a loaded or
throttled runner drifts well past 1.0s and would otherwise report >100%.

emit a downsampled series (cpu/mem/disk, 0-100 ints) alongside the existing
summary, mean-bucketed to 180 points so a 40min job costs the same few KB as
a 40s one. the report inlines every profile into one self-contained html
file, so an unbounded 1Hz series would dominate its size. mean, not every Nth
sample: a spike that survives decimation by luck is misleading.

the report renders the series as svg sparklines over each gantt bar, in two
states off the same markup (3px strip when collapsed, full height when
expanded) via preserveAspectRatio=none. profiles predating the series field
degrade to table-only, no overlay.

tests/ci 62/62.
2026-08-05 07:06:03 -04:00
ethernet 5a9788babe ci: add newci-* shadow workflows on GKE self-hosted runners
Run a duplicate of CI on the new ARC (Actions Runner Controller) runners in
GKE, beside the existing CI. The duplicate does not change production CI.
Every workflow in .github/workflows/ that does not start with newci- is
byte-identical to main. Watch the shadow runs for a few days, then migrate.

The shadow set is 16 files: newci-ci.yml plus the 15 reusable workflows that
ci.yml calls on a pull request. Only pull-request workflows are copied.
js-autofix, deploy-site, and skills-index run on push or on a schedule. A
copy of those would push branches and deploy the site a second time.

Safety properties of the shadow:

- Concurrency groups are newci-prefixed. This is the important one. The
  production groups use cancel-in-progress, so a shared group would let a
  shadow run cancel the production run.
- Cache keys are newci-prefixed. The shadow cannot poison or evict a
  production cache entry.
- Reusable-workflow calls point only at other newci-* files. No shadow job
  calls a production workflow.
- The PR review comment runs with --dry-run. It prints the comment body to
  the job log. Two pollers cannot fight over the hermes-ci-review-bot
  comment.
- The gate job is renamed to "[newci] All checks pass (informational)". The
  production check "All required checks pass" stays the only merge gate.
- The shadow runs on pull_request only. The push trigger is removed.
- docker publish and merge jobs are unreachable. Their conditions require a
  push to main or a release.

Runner infrastructure, in the shadow copies only:

- Jobs go to three scale sets: arc-runner-small for short gate jobs,
  arc-runner-set for general work, arc-runner-docker and arc-runner-arm64
  for image builds. dind is only on the docker sets, so the other jobs stop
  paying for a privileged sidecar.
- The runner image supplies node 26, npm 12, uv, Python, and ripgrep. The
  setup-node, setup-uv, and per-job install steps are gone.
- Checkout uses a node-local git mirror, seeded from the runner pod env.
- buildx layer cache moved to Artifact Registry in us-central1, the same
  region as the runners. Reads are keyless through GKE Workload Identity.
  Writes use GitHub OIDC and happen only on main pushes and releases, so
  pull-request code cannot write a layer that the publish job reads.

Merge-base work, in the shadow copies only:

- A new composite action, .github/actions/merge-base, deepens a shallow
  clone until the two histories connect. fetch-depth: 0 fetches all ~1400
  refs and measured 76-81s, against 3-6s for a shallow checkout.
- The action fails by default when no merge base exists. A three-dot diff
  over a missing merge base scans nothing and reports clean, so the
  supply-chain audit must stop. history-check sets fail-on-missing to false,
  because absence is the result it measures.
- lint diffs against the base commit directly. The job checks out the PR
  merge ref, so base.sha is already the correct comparison point.
- contributor-check uses origin/main..HEAD. The result equals the merge-base
  form, and the extra git call also expanded a SHA without quotes.

Other changes:

- .github/actionlint.yaml declares the four ARC labels. actionlint knows
  only GitHub-hosted labels, so every runs-on in the repo was reported as an
  unknown label: 40 warnings that hid real findings.
- scripts/ci/resource_profile.py records CPU and memory for a job step. The
  timing report shows the data per step.
- run_tests_parallel.py can list test files from the git index. The slice
  generator then needs no blobs.
- Docker test files are split so boot-heavy tests run in parallel.
- Container-environment parity fixes in doctor, gateway, and skill_utils,
  with tests.

To retire the shadow: delete .github/workflows/newci-*.yml.
2026-08-05 00:22:10 -04:00
ethernet b162371f50
fix(ci): retain delayed and composite-action jobs in review (#69768)
Keep the live PR review comment polling for a short grace period after
visible jobs complete. Preserve composite-action jobs in timing and live
status collection, and split the timing HTML from its linked review-status
artifact.
2026-07-22 22:43:21 -04:00
ethernet 433673067e
ci: surface E2E screenshots in review comment (#69631)
* ci: surface E2E screenshots in review comment

* ci: mark completed review commits in past tense

* ci: surface approved sensitive-file reviews

* ci: link sensitive files to reviewed changes

* ci: stage desktop E2E visual evidence

Track screenshots newly introduced against main and package visual diffs for a trusted publisher.

* fix(ci): pass E2E evidence output paths

Supply the manifest and staging-directory arguments required by the screenshot status helper.

* fix(ci): download the OSV SARIF artifact

Match the artifact name and result filename emitted by the pinned upstream reusable workflow.
2026-07-22 23:19:53 +00:00
ethernet b9f82ed39f ci: live-updating PR review comment with structured job statuses
Replace the static comment-pending + comment-results two-job pattern
with a live-updating comment system that polls the GitHub Actions API
every 15s, re-assembles the review comment from whatever results are
available, and upserts it via the <!-- hermes-ci-review-bot --> marker.
The comment updates in real time as each job finishes — no waiting for
the full pipeline.

Every CI job that wants to appear in the review comment emits a
review_status output — a JSON array of objects, each with a source
and a results array:

    [
      {
        "source": "review-label-gate",
        "results": [
          {"kind": "action_required", "title": "...", "summary": "...",
           "how_to_fix": "..."},
          {"kind": "info", "title": "...", "summary": "..."}
        ]
      },
      {
        "source": "ci timing",
        "results": [
          {"kind": "warning", "title": "CI timings", "summary": "...",
           "detail": "...", "link": "..."}
        ]
      }
    ]

One job can emit multiple results of different kinds. The source field
is used to exclude the corresponding job from the synthesized error
list (case-insensitive, hyphen-normalized matching against GitHub
Actions job display names).

| job                        | source                   | kind (on failure)         | section              |
|----------------------------|--------------------------|---------------------------|----------------------|
| review-labels              | review label gate        | action_required / info    | Action required      |
| lockfile-diff              | lockfile-diff            | action_required           | Action required      |
| ci-timings                 | ci timing                | warning / info            | Warnings             |
| supply-chain scan          | supply chain             | error / (none)            | Job failures         |
| supply-chain dep-bounds    | supply chain             | action_required / (none)  | Action required      |
| osv-scanner                | osv scan                 | warning / (none)          | Warnings             |
| uv-lockfile-check          | uv.lock check            | action_required / (none)  | Action required      |
| history-check              | unrelated histories      | action_required           | Action required      |
| contributor-check          | contributor attribution  | action_required           | Action required      |

Jobs that find nothing emit [] (empty array) — no noise info items.

A single comment-live job polls the GitHub Actions API every 15s,
classifies jobs into (completed, pending), assembles the comment, and
upserts it. Merges review_status outputs from all needs jobs via
toJSON(needs.*.outputs.review_status), and downloads the ci-timings
artifact when it becomes available. Shows commit SHA + message below
the header.

The assembler has ZERO job-specific knowledge. It just:
1. collect_from_statuses() — flattens all nested status objects into ReviewItems
2. collect_failed_jobs() — synthesizes errors for failed jobs with no declared status
3. _attach_job_urls() — fills in per-job log links for ALL items
4. render_comment() — groups by severity, renders with group headers

Each item shows links inline next to the title: View report (job-emitted
URL) and View job (auto-attached logs link). Each info item is its own
collapsible <details> block.

    # ૮ >ﻌ< ა ci review

    running on abc1234 — commit message first line

    ##  Job failures
    ### {title} · [View job](url)
    {summary}

    ## ⚠️ Action required
    ### {title} · [View job](url)
    {summary}
    **How to fix:**
    {how_to_fix}

    ## ⚠️ Warnings
    ### {title} · [View report](url) · [View job](url)
    {summary}
    {detail}

    <details><summary>{title}</summary>
    {content}
    </details>

    Still running 3 jobs: ci-timings, docker

- test_assemble_review_comment.py (48 tests): collect_from_statuses,
  collect_failed_jobs with exclude_sources, _attach_job_urls,
  render_comment (group headers, inline links, commit info, per-item
  details, pending footer), assemble integration
- test_live_comment.py (16 tests): classify_jobs pure function
- test_timings_report.py (10 tests): generate_review_status nested format
- test_lockfile_diff.py (6 tests)
- test_classify_changes.py (32 tests, pre-existing)
2026-07-20 16:48:25 -04:00
Brooklyn Nicholson ecd54a001e fix(ci): make timings report fork-safe (missed by #66577)
#66373 swapped GITHUB_TOKEN -> AUTOFIX_BOT_PAT across the workflows and
#66577 restored the `|| github.token` fork fallback for detect-changes and
the label gates -- but it missed the ci-timings "Collect timings and
generate report" step, which still passes a bare AUTOFIX_BOT_PAT. On fork
PRs that PAT is empty, so timings_report.py hard-fails at
expect_env("GITHUB_TOKEN") before it can reach its own "degraded run must
never redden the PR" soft-fail path. Every fork PR gets a red run from this
advisory job (e.g. #66573).

- ci.yml: apply the same `secrets.AUTOFIX_BOT_PAT || github.token` fallback
  to the timings step. github.token has `actions: read`, enough to read the
  run's job/step durations on forks.
- timings_report.py: treat a missing/empty GITHUB_TOKEN as a degraded run
  (TimingsUnavailable) instead of a hard ValueError, so this whole class of
  failure can never redden a PR again even if a future workflow drops the
  token. Still writes no JSON, so no empty baseline is ever cached.
2026-07-18 01:06:36 -04:00
ethernet 29b8cacfab fix(ci): add missing Δ Wait column to skipped job rows 2026-07-13 17:46:11 -04:00
ethernet e117478eb3 fix(ci): align baseline gantt bars to current job start
Baseline bars were positioned at their absolute timeline offset, which
included the baseline run's own wait time — making duration comparisons
hard since the bars were visually offset. Now each baseline bar starts
at the same left position as its corresponding current job, so the two
bars directly overlap for at-a-glance duration comparison.

Also removed the now-unused bl_t0 / bl_max / bl_jobs_timed variables
that tracked the baseline timeline position.
2026-07-13 17:46:11 -04:00
ethernet f0b7cf3836 feat(ci): show per-job wait times in timing report
Add wait time computation: for each job, wait_s = started_at - max(completed_at
of all jobs that finished before it started). This is a timestamp heuristic
(no workflow YAML dependency parse needed) that's accurate for pipeline-shaped
CI where the critical path is linear at each stage.

Shows up in:
- Job table: new Wait + Δ Wait columns
- Gantt chart: hatched bar segment before the run bar
- Step details: '(wait Xs)' annotation in the summary line
- Markdown summary: Total wait row with delta vs baseline
- Stats: total_wait / bl_total_wait in compute_stats

Also completes the skipped-jobs UI:
- Skipped stat card in stats cards
- Skipped row in markdown summary table

Backward compatible: old cached baselines without wait_s annotate on load.
2026-07-13 17:46:11 -04:00
ethernet 7beca22bc0 fix(ci): exclude skipped jobs from timing deltas and stats
Skipped jobs (conclusion == 'skipped') have null/zero-duration timestamps
that polluted every downstream computation: they counted as 'unchanged'
(0 vs 0) in faster/slower tallies, showed meaningless '0.0s (0%)' deltas
in the job table, rendered phantom gantt bars, and inflated wall/compute
totals.

Add is_skipped() helper and apply it consistently:
- compute_stats: exclude from wall/compute + faster/slower/unchanged;
  add 'skipped' and 'bl_skipped' counts
- _gantt_bars: filter from current bars, baseline bars, and axis calc
- _job_table: show 'skipped' label instead of durations/deltas
- _step_details: skip entirely (no meaningful step data)
- _regressions: exclude from both current and baseline sides
2026-07-13 17:46:11 -04:00
Teknium 5e685999af
fix(ci): make the CI timing report unflakeable (#59818)
The 'CI timing report' job is pure observability — it collects per-job/step
durations from the GitHub API after the run and publishes an HTML gantt
report + PR-vs-main timing diff. It gates nothing (all-checks-pass does not
include it), yet it could redden a PR: the script makes dozens of paginated
API calls with the shared repo GITHUB_TOKEN and had zero retry handling, so
a single 403 (rate-limit burst when several PRs run CI concurrently) failed
the job. Observed twice in a row on PR #59805.

- api_get(): retry 403/429/5xx and connection errors with exponential
  backoff, honoring Retry-After / X-RateLimit-Reset (max 5 attempts, 120s
  cap). Non-transient statuses (404 etc.) still fail fast.
- main(): exhausted retries raise TimingsUnavailable, caught to emit a
  degraded summary line + placeholder HTML artifact and exit 0 — a metrics
  collector must never fail the PR's checks. No timings JSON is written on
  the degraded path so an empty baseline can never be cached.
- ci.yml: baseline-save steps on main skip gracefully when no JSON exists.

Verified with a mocked urlopen harness: retry-then-success (3 attempts),
exhausted-retries -> TimingsUnavailable, 404 fails fast without retry,
degraded main() exits 0 with summary + placeholder and no JSON, and the
--from-json happy path is unchanged.
2026-07-06 12:44:07 -07:00
ethernet 808ba82125 feat(ci): add CI timing report 2026-06-29 19:07:00 -07:00