Skip to content

[Infra] Preserve benchmark failures through the metrics tee - #37

Merged
DongyuZhao merged 1 commit into
mainfrom
fix-benchmark-tee-pipefail
Jul 22, 2026
Merged

[Infra] Preserve benchmark failures through the metrics tee#37
DongyuZhao merged 1 commit into
mainfrom
fix-benchmark-tee-pipefail

Conversation

@DongyuZhao

@DongyuZhao DongyuZhao commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

GitHub's default run shell for Linux/macOS steps is bash -e {0} — without -o pipefail; only the explicit shell: bash form adds it. The four benchmark steps that tee their output into build/pr-metrics-logs/ therefore let tee's zero exit mask a failing benchmark suite, so a broken benchmark could still satisfy Benchmarks - Ready.

All four teeing steps (C, Kotlin, ES, Swift) now declare shell: bash. No behavior change on the green path.

Found by review on the ported tex-core copy (nouprax/tex-core#11, same fix landed there).

🤖 Generated with Claude Code

@DongyuZhao @claude
[Infra] Preserve benchmark failures through the metrics tee.
c792e7c 
The default run shell is bash -e without pipefail, so piping a
benchmark suite into tee masks its failure with tee's zero exit and a
broken benchmark still satisfies Benchmarks - Ready. The four teeing
benchmark steps now use the explicit bash shell, which adds
-o pipefail. Found by review on the ported tex-core copy
(nouprax/tex-core#11).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DongyuZhao
DongyuZhao requested a review from a team July 22, 2026 18:31
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

PR performance and binary-size diff

Benchmark execution is required; hosted-runner timing, memory, and size deltas are informational only.

Baseline: exact PR base b78fd1ec1354 (successful main CI).

Performance

Runtime Workload Median Perf diff Memory Memory diff
c representative_large 7.065 ms ⚠️ +15.0% 8,692 KiB ✅ +1.5%
es deep_nesting 0.358 ms ⚠️ +21.0% 142,004 KiB ⚠️ +7.5%
es large_document 29.498 ms ✅ -2.3% 142,004 KiB ⚠️ +7.5%
kotlin deep_nesting 0.240 ms ⚠️ +4.0% 110,388 KiB ✅ -1.4%
kotlin large_document 28.259 ms ⚠️ +2.8% 105,404 KiB ✅ -1.7%
swift deep_nesting 0.082 ms ⚠️ +18.4% 25,968 KiB ✅ -1.0%
swift large_document 12.941 ms ⚠️ +17.5% 25,872 KiB ✅ -0.9%

Binary size

Artifact Bytes KiB Size diff
c-shared-library 501,888 490.1 ✅ 0 B (0.0%)
es-wasm 288,573 281.8 ✅ 0 B (0.0%)
kotlin-jvm-jar 312,972 305.6 ✅ 0 B (0.0%)

Head CI run

Size deltas are deterministic for identical build inputs. Performance deltas compare separate hosted runners and should be treated as directional evidence, not a pass/fail threshold.

@DongyuZhao
DongyuZhao merged commit b6b2c00 into main Jul 22, 2026
87 checks passed
@DongyuZhao DongyuZhao mentioned this pull request Jul 27, 2026
Merged
DongyuZhao added a commit that referenced this pull request Jul 27, 2026
@DongyuZhao @claude
[Infra] Pin squash-only merges with PR-title messages in bootstrap (#39)
384e8a7 
## Summary

- The template's embedded `bootstrap-repository.sh` now pins the merge
policy: squash-only (`allow_merge_commit=false`,
`allow_rebase_merge=false`) with `squash_merge_commit_title=PR_TITLE` +
`squash_merge_commit_message=PR_BODY`, so both direct merges and
merge-queue commits land as `title (#N)` + PR description. The `PATCH`
sits between the default-branch validation and the ruleset upserts,
matching the tex-core placement, and the re-run stays idempotent.
- Ported from tex-core (nouprax/tex-core#20), the same direction as
#36/#37. tex-core changed both its checked-in
`scripts/bootstrap-repository.sh` and its template's embedded copy;
markdown-core carries the script only as the embedded copy in
`docs/repository-setup-template.md`, so this PR touches that one spot.
- No CHANGELOG entry: markdown-core's changelog records release changes
only (no Unreleased section), matching #36/#37.

## Verification

`shellcheck` clean on the extracted embedded script; `audit:repository`
and `audit:ci` green.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 28, 2026
Merged
Merged
Merged
DongyuZhao added a commit that referenced this pull request Jul 28, 2026
@DongyuZhao @claude
[Infra] Default bootstrap to active enforcement and fail closed on ru…
c4249db 
…leset downgrades (#65)

## Summary

- The template's embedded `bootstrap-repository.sh` now defaults to the
mature-repository shape: `RULESET_ENFORCEMENT=active` and
`MAIN_ADMIN_BYPASS=false`, so a maintenance re-run with only the
required inputs converges on full protection instead of silently
re-weakening it. Initial bootstrap of a fresh repository can still opt
into `RULESET_ENFORCEMENT=evaluate` explicitly while CI stabilizes,
exactly as the migration runbook already does.
- `upsert_ruleset` now fails closed when updating an existing ruleset:
downgrading enforcement from `active` to `evaluate`/`disabled`, or
adding bypass actors beyond the current set, aborts with an explanation
unless `ALLOW_PROTECTION_DOWNGRADE=true` is passed deliberately.
- A short note after the migration examples documents the new defaults
and the override variable.
- Ported from tex-core (nouprax/tex-core#70), the same direction as
#36/#37/#39. markdown-core carries the bootstrap script only as the
embedded copy in `docs/repository-setup-template.md`, so this PR touches
that one spot (same as #39).
- No CHANGELOG entry: markdown-core's changelog records release changes
only, matching #36/#37/#39.

## Verification

`shellcheck` clean and `bash -n` on the extracted embedded script;
`audit:repository` and `audit:ci` green.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
DongyuZhao added a commit that referenced this pull request Jul 28, 2026
@DongyuZhao @claude
[Infra] Pin workflow actions to commit SHAs and audit movable refs (#66)
fde3f72 
## Summary

- Every `uses:` reference across the five workflows (`ci`, `codeql`,
`release`, `release-dry-run`, `pr-metrics-comment`) is now pinned to a
full commit SHA with the release version recorded in a trailing comment.
A movable major tag lets a tag replacement change the code that CI and
release jobs execute without a reviewed diff; the SHA makes the executed
bytes part of the reviewed history.
- `audit-ci-policy.sh` (run as `audit:ci` in the repository health
check) now rejects any workflow action reference that is not a 40-hex
commit SHA, so a future workflow edit cannot quietly reintroduce a
movable ref. Local `uses: ./…` references stay exempt.
- Every SHA↔version pair was verified against the GitHub API
(`repos/<action>/commits/<tag>`) before landing; the pins match the ones
tex-core reviewed.
- Ported from tex-core (nouprax/tex-core#70), the same direction as
#36/#37/#39/#65.
- No CHANGELOG entry: markdown-core's changelog records release changes
only, matching the previous mirrors.

## Verification

`audit:ci` green on the pinned tree; negative test confirmed the new
check fails with a pointed message when a single ref is reverted to
`@v7`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
DongyuZhao added a commit that referenced this pull request Jul 28, 2026
@DongyuZhao @claude
[Infra] Hash-lock Python tool installs and pin the emsdk commit (#67)
2f13f53 
## Summary

- The repo-managed Python tool venvs now install from hash-locked
requirements files (`pip --require-hashes`):
`scripts/requirements/clang-format.txt` for the clang-format venv in
`init-environment.sh`, and `scripts/requirements/cmakelang.txt`
(cmakelang + PyYAML + the transitive six) for the cmake-format venv in
`format-cmake.sh`, which is where markdown-core builds that venv. Every
distribution PyPI publishes for the pinned versions is enumerated, so an
index-side replacement cannot change the bytes the tools run.
- `install_emscripten` now pins the emsdk manager itself to
`EMSCRIPTEN_COMMIT` — the immutable commit of the `4.0.23` release tag,
kept in lockstep with `EMSCRIPTEN_VERSION` — and verifies the checkout,
so a moved tag or new default-branch commit can never change the
installer bytes it executes.
- `audit-ci-policy.sh` (`audit:ci`) now enforces all of this: it fails
if `init-environment.sh` loses the emsdk commit pin or
`--require-hashes`, or if `format-cmake.sh` loses `--require-hashes`.
- Ported from tex-core (nouprax/tex-core#70), the same direction as
#36/#37/#39/#65/#66. Adapted where the repos differ: markdown-core
installs cmakelang via `format-cmake.sh` rather than inline in
`init-environment.sh`, so the requirements file is wired (and audited)
there. The emsdk commit was verified against the GitHub API to be the
`4.0.23` tag commit.
- No CHANGELOG entry: markdown-core's changelog records release changes
only, matching the previous mirrors.

## Verification

`audit:ci` green with the new checks active; both requirements files
installed for real with `--require-hashes` into fresh venvs on macOS
arm64 (`cmake-format 0.6.13`, `clang-format 22.1.8`); the modified
`format-cmake.sh --check` ran end-to-end against the repo with a fresh
tool dir; `shellcheck` reports nothing new on either script.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@DongyuZhao