Skip to content

fix(ci): spec-conformance Mode B gate fails closed on unresolvable merge-base#1801

Merged
danielmeppiel merged 1 commit into
mainfrom
danielmeppiel/fix-spec-conformance-fail-closed
Jun 16, 2026
Merged

fix(ci): spec-conformance Mode B gate fails closed on unresolvable merge-base#1801
danielmeppiel merged 1 commit into
mainfrom
danielmeppiel/fix-spec-conformance-fail-closed

Conversation

@danielmeppiel

Copy link
Copy Markdown
Collaborator

TL;DR

The OpenAPM Spec conformance gate (Mode B silent-extension detector) failed open. This PR closes that hole: full-history checkout in CI plus a fail-closed branch in the detector, locked by a regression test.

Problem (WHY)

.github/workflows/spec-conformance.yml checked out shallow (no fetch-depth: 0), and tests/spec_conformance/mode_b_detector.sh did exit 0 (skip == pass) whenever it could not resolve the merge-base against origin/main. So a PR adding uncited substantive code under a normative critical path could pass the gate by luck of the checkout -- a fail-open hole in a governance gate. Full diagnosis in #1800.

Approach (WHAT)

  • [*] Root-cause fix -- actions/checkout@v4 now uses fetch-depth: 0, so the merge-base is deterministically resolvable in CI and the skip branch is unreachable there.
  • [*] Defense in depth -- under GITHUB_ACTIONS=true, an unresolvable merge-base now emits an ::error:: diagnostic and exit 1 (fail closed). Local dev (shallow clones) keeps the ergonomic skip (exit 0).
  • [*] Regression lock -- new tests pin both invariants (CI fail-closed + local skip) and assert the workflow sets fetch-depth: 0.

Implementation (HOW)

.github/workflows/spec-conformance.yml   + fetch-depth: 0 on checkout
tests/spec_conformance/mode_b_detector.sh   CI -> ::error:: + exit 1; local -> [!] skip + exit 0
tests/spec_conformance/test_mode_b_detector.py   +2 regression tests, +1 helper

Validation evidence (HOW-tested)

  • [+] uv run --extra dev pytest tests/spec_conformance/test_mode_b_detector.py -p no:randomly -q -- 11 passed
  • [+] uv run --extra dev pytest tests/spec_conformance -p no:randomly -q -- 120 passed, 2 skipped
  • [+] Mutation-break: reverting the new exit 1 back to exit 0 makes test_detector_fails_closed_in_ci_on_unresolvable_merge_base fail; restored.
  • [+] Sanity: BASE_REF=origin/main bash tests/spec_conformance/mode_b_detector.sh on this branch -> no critical-path diff; OK (does not self-block; this PR touches only .github/ + tests/spec_conformance/, which are not in critical_paths.txt).
  • [+] CI-mirror lint: ruff check, ruff format --check, pylint R0801, scripts/lint-auth-signals.sh -- all clean.

ASCII-only output throughout.

Closes #1800
Part of #1774

…rge-base

The OpenAPM Spec conformance gate (Mode B silent-extension detector)
failed OPEN: the workflow checked out shallow and the detector did
exit 0 (skip == pass) whenever it could not resolve the merge-base
against origin/main. A PR adding uncited substantive code under a
normative critical path could pass by luck of the checkout.

- spec-conformance.yml: actions/checkout now uses fetch-depth: 0 so the
  merge-base is deterministically resolvable in CI (root-cause fix that
  makes the skip branch unreachable in CI).
- mode_b_detector.sh: under GITHUB_ACTIONS=true an unresolvable
  merge-base now prints an ::error:: diagnostic and exits 1 (fail
  closed). Local dev keeps the ergonomic skip (exit 0).
- test_mode_b_detector.py: regression tests lock both invariants
  (CI fail-closed + local skip; workflow fetch-depth: 0).

Closes #1800
Part of #1774

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 09:58

Copilot AI 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.

Pull request overview

This PR hardens the OpenAPM spec-conformance “Mode B” silent-extension gate so it cannot silently pass when it fails to evaluate (unresolvable merge-base), by ensuring CI has full git history and by failing closed under GitHub Actions.

Changes:

  • Update the spec-conformance workflow checkout to fetch-depth: 0 so origin/main merge-base is deterministically resolvable in CI.
  • Change the Mode B detector to fail closed (exit 1) under GITHUB_ACTIONS=true when merge-base resolution fails, while keeping the local “skip” behavior.
  • Add regression tests to lock the CI fail-closed behavior and to assert the workflow uses full-history checkout.
Show a summary per file
File Description
.github/workflows/spec-conformance.yml Switches CI checkout to full history (fetch-depth: 0) to prevent merge-base resolution from failing due to shallow clones.
tests/spec_conformance/mode_b_detector.sh Makes merge-base resolution failure fail closed in CI, preventing non-evaluation from being treated as a pass.
tests/spec_conformance/test_mode_b_detector.py Adds regression coverage for CI fail-closed behavior and for the workflow’s checkout depth setting.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

Comment on lines +62 to +67
if [ "${GITHUB_ACTIONS:-}" = "true" ]; then
echo "::error::mode_b: cannot resolve merge-base against $BASE in CI;" >&2
echo "::error::refusing to pass without evaluating (fail closed). Ensure" >&2
echo "::error::actions/checkout uses fetch-depth: 0 so origin/main is reachable." >&2
exit 1
fi
Comment on lines +301 to +316
def _run_detector_with_env(repo: Path, **overrides: str) -> subprocess.CompletedProcess:
"""Run the detector with explicit env overrides. GH_PR_BODY is
cleared so only the supplied vars drive behaviour."""
env = {**os.environ}
env.pop("GH_PR_BODY", None)
# Strip any ambient GITHUB_ACTIONS so callers control it explicitly.
env.pop("GITHUB_ACTIONS", None)
env.update(overrides)
return subprocess.run(
["bash", "tests/spec_conformance/mode_b_detector.sh"],
cwd=repo,
env=env,
capture_output=True,
text=True,
check=False,
)
@danielmeppiel danielmeppiel merged commit 56f7944 into main Jun 16, 2026
22 checks passed
@danielmeppiel danielmeppiel deleted the danielmeppiel/fix-spec-conformance-fail-closed branch June 16, 2026 18:30
danielmeppiel added a commit that referenced this pull request Jun 16, 2026
Reconcile cross-PR normative-count collision after #1794 (req-pl-013/014)
and #1801 (Mode B fail-closed CI) merged to main. Union all shared count
sites to cumulative 90 (req-pl-013, req-pl-014, req-pl-015 all present;
85 MUST, 5 SHOULD). Bump req-pl-015 revision-history row to 0.1.4 to keep
Appendix D monotonic. Regenerated CONFORMANCE.{json,md}.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam pushed a commit that referenced this pull request Jun 17, 2026
Sync the 800-line/complexity tightening branch with origin/main tip
788a09a (8 commits ahead of merge-base 45843c3): SBOM export +
declared-license (#1820), dompurify bump (#1789), audit-unmanaged
(#1793), ADO sourceBase (#1810), Antigravity target (#1770),
marketplace token (#1763), spec-conformance (#1801), declared-license
and integrity keys (#1794/#1777).

Conflict resolution preserves the strangler-fig extraction: HEAD's
relocations into sibling _*.py modules win, with main's feature
additions folded into the new homes. Notable folds:
- hook_merge.py: thread container key + antigravity dispatch.
- audit: route fail_on_drift + LockFile through the audit module so
  test monkeypatches on apm_cli.commands.audit.* still take effect.

Resolve merge-introduced CI regressions under the tightened gates:
- ruff complexity: _classify_primitive_type (PLR0911), validate_policy
  (C901/PLR0912 via _validate_security), _audit_content_scan (PLR0912
  via _run_drift_detection).
- file-length <=800: split spdx_data.py (_spdx_exception_ids.py),
  policy_checks.py (_policy_checks_unmanaged.py), pack.py render
  helpers (into _pack_ops.py); all re-exported for the patch contract.

Local CI mirror green: ruff check/format, pylint R0801 10/10,
auth-signals, file-length<=800, full unit suite 17225 passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam pushed a commit that referenced this pull request Jun 17, 2026
PR #1681 (issue #1078) is a pure structural refactor: it relocates code
out of oversized functions/files into sibling _*.py helper modules and
tightens ruff complexity + an 800-line file-length CI gate. No normative
critical-path behaviour changes -- the Mode-B silent-extension detector
(new from #1801) fires only on the large refactor diff, so apply the
documented waiver for a true refactor with no observable behaviour delta.

apm-spec-waiver: pure refactor (strangler-fig module split for #1078), no observable behaviour delta

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

fix(ci): spec-conformance Mode B gate fails open on unresolvable merge-base

2 participants