Skip to content

fix(#618): the null-handle gate matched one spelling, and this bridge reaches a handle five ways - #641

Merged
gsdali merged 2 commits into
refactor/381-pass1bfrom
fix/618-guard-checker-cast-sites
Aug 2, 2026
Merged

fix(#618): the null-handle gate matched one spelling, and this bridge reaches a handle five ways#641
gsdali merged 2 commits into
refactor/381-pass1bfrom
fix/618-guard-checker-cast-sites

Conversation

@gsdali

@gsdali gsdali commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #618

Scripts/check-null-handle-guards.py printed "All bridge functions guard the geometry handle as
well as the wrapper pointer" and exited 0. Its use-detector matched param->field and nothing
else, so every site reaching the handle through an indirection was invisible to it, and this bridge
uses four, not the one the issue names.

The five ways this bridge reaches a handle

# form example in the tree old detector
0 direct curve->curve sees it
1 named cast reinterpret_cast<OCCTSurface*>(surfaceRef)->surface, static_cast<...> blind
2 C-style cast + pointer alias auto* s = (OCCTSurface*)surface; ... s->surface blind
3 handle alias auto& surf = ...->surface; and the by-value Handle(Geom_Surface) w = wrapper->surface; copy blind
4 shared bridge helper occtSurfaceToAnalytical(reinterpret_cast<OCCTSurface*>(ref)->surface, ...) blind

Casts are now normalised away (position-preserving, so line numbers are unchanged) before the walk,
and aliases are followed to a fixpoint. Forms 1-3 were hiding unguarded sites.

Form 4 cuts the other way, and is why "teach it the cast spelling" would have been the wrong
fix.
OCCTGeomConvertCurveToAnalytical and occtSurfToAnaSurfResult hand their handle to
occtCurveToAnalytical / occtSurfaceToAnalytical (OCCTBridge_Internal.h:1591, :1620), both of
which open with if (curve.IsNull()) return false;. Checked, one call frame away. A detector taught
1-3 and not 4 reports both as defects, which is exactly #624/#630 one issue earlier: a sibling gate
taught indirection badly was confidently wrong about seven correct entries. The detector now
recognises a bridge helper that IsNull()-checks the parameter it is handed as a guard in its own
right, and the self-test pins that case.

Before / after

Before (origin/refactor/381-pass1b):

$ python3 Scripts/check-null-handle-guards.py; echo "exit=$?"
All bridge functions guard the geometry handle as well as the wrapper pointer.
exit=0
$ python3 Scripts/check-null-handle-guards.py --self-test; echo "exit=$?"
All bridge functions guard the geometry handle as well as the wrapper pointer.   # flag ignored
exit=0

With the corrected detector, before any guard was added: 54 (function, argument) pairs across 39 functions, against the old detector's 1. After measurement and the 21 guards:

$ python3 Scripts/check-null-handle-guards.py; echo "exit=$?"
All bridge functions guard the geometry handle as well as the wrapper pointer.
(23 site(s) exempt by name in ALLOWED, each with its reason.)
exit=0
$ python3 Scripts/check-null-handle-guards.py --self-test; echo "exit=$?"
12/12 cases correct
exit=0

It exits 0 legitimately now: every one of the 54 was adjudicated, not suppressed.

Per-site measurement

Every candidate was treated as a candidate, per #556's method, and measured against the OCCT entry
point it actually reaches. Scripts/repro/556-null-handle-guard-sweep/repro_556.mm grew from 35
probes to 57, the 22 the pre-#618 walk never reached and nobody had measured. Each probe runs the
bridge function's real sequence (Init and Perform), because the crash need not land on the
first call.

Needed a guard: 21 pairs, 16 functions

bridge function OCCT entry point null handle fallback added
OCCTLocalAnalysisCurveContinuity (x2 args) LocalAnalysis_CurveContinuity ctor SIGSEGV false
OCCTLocalAnalysisCurveContinuityFlags (x2) LocalAnalysis_CurveContinuity ctor SIGSEGV 0
OCCTLocalAnalysisSurfaceContinuity (x2) LocalAnalysis_SurfaceContinuity ctor SIGSEGV false
OCCTLocalAnalysisSurfaceContinuityFlags (x2) LocalAnalysis_SurfaceContinuity ctor SIGSEGV 0
OCCTSplitCurve3dContinuity ShapeUpgrade_SplitCurve3dContinuity Init+Perform SIGSEGV 0
OCCTSplitCurve2dContinuity ShapeUpgrade_SplitCurve2dContinuity Init+Perform SIGSEGV 0
OCCTConvertCurve2dToBezier ShapeUpgrade_ConvertCurve2dToBezier Init+Perform SIGSEGV 0
OCCTSplitSurfaceContinuity ShapeUpgrade_SplitSurfaceContinuity Init+Perform SIGSEGV 0
OCCTSplitSurfaceAngle ShapeUpgrade_SplitSurfaceAngle Init+Perform SIGSEGV 0
OCCTSplitSurfaceArea ShapeUpgrade_SplitSurfaceArea Init+Perform SIGSEGV 0
OCCTGeomToolsCurve2dSetWrite GeomTools_Curve2dSet::Add + Write SIGSEGV nullptr
OCCTGeomToolsCurveSetWrite GeomTools_CurveSet::Add + Write returns, guards internally nullptr, and not for crash-safety: see below
OCCTGeomToolsSurfaceSetWrite GeomTools_SurfaceSet::Add + Write SIGSEGV nullptr
OCCTGeomFillNSections GeomFill_NSections + ComputeSurface SIGSEGV nullptr
OCCTGeomFillNSectionsInfo GeomFill_NSections + SectionShape SIGSEGV return;
OCCTProjLibProjectOnSurface (x2) new Geom_TrimmedCurve (the curve arg) SIGSEGV nullptr

Cleared by measurement: 33 pairs, 23 functions, now in ALLOWED with the reason

bridge function(s) OCCT entry point null handle why no guard
OCCTGeomLibToolParameter3D, ...2D GeomLib_Tool::Parameter returns false copes
OCCTGeomLibToolParametersSurface GeomLib_Tool::Parameters returns false copes
OCCTGeomConvertIsCanonical GeomConvert_SurfToAnaSurf::IsCanonical returns copes
OCCTApproxSameParameter (x3) Approx_SameParameter ctor Standard_Failure, all four argument combinations own catch (...)
OCCTExtremaExtCC, ...CCPoint, ...CS, ...CSPoint, ...LocateExtCC (x8) GeomAdaptor_Curve / GeomAdaptor_Surface Standard_Failure own catch (...)
OCCTExtremaExtPS, ...PSPoint, ...ExtSS, ...SSPoint (x6) GeomAdaptor_Surface Standard_Failure own catch (...)
OCCTExtremaLocateExtCC2d (x2) Geom2dAdaptor_Curve Standard_Failure own catch (...)
OCCTGeom2dConvertApproxArcsSegments Geom2dConvert_ApproxArcsSegments Standard_Failure own catch (...)
OCCTGeomLibIsPlanarSurface, OCCTGeomLibPlanarSurfacePlane GeomLib_IsPlanarSurface ctor Standard_Failure own catch (...)
OCCTBRepGraphRepSetSurface, ...Curve3D, ...Curve2D none n/a not an OCCT call: stores into a bridge-owned side registry whose else branch deliberately stores a null handle to clear the slot
OCCTBRepGraphCoEdgeSetPCurve BRepGraph_EditorView::SetPCurve n/a the header documents null as the contract: "Pass a null handle to clear the stored PCurve binding"
makeQualifiedCurve Geom2dAdaptor_Curve n/a pre-existing entry; returns by value, no null-safe fallback. Re-verified: all 9 callers check both pointer and handle first

Handled by rule rather than by allowlist, since the guard genuinely exists:
OCCTGeomConvertCurveToAnalytical and occtSurfToAnaSurfResult (form 4 above).

Was the issue's premise right? Partly, and the wrong part is the useful part

The issue named seven suspected-unguarded sites. Three of the seven needed nothing:

  • OCCTGeomLibToolParameter3D and OCCTGeomLibToolParameter2D reach GeomLib_Tool::Parameter,
    which returns false on a null handle.
  • OCCTApproxSameParameter reaches Approx_SameParameter, which raises a catchable
    Standard_Failure. The function's own catch (...) { return false; } already produces exactly
    the value a guard would.

The issue was right that these were unmeasured, and right that the detector was blind. It was wrong
that all seven were defects. Measuring first is what caught it: three guards that would have been
noise. The other four were real, and eleven more the issue never named were real too, which is
the other half of the same point. A list produced by a blind scanner is short in both directions.

One upstream inconsistency found on the way

The three GeomTools_*Set writers are the same code three times and do not behave the same way.
GeomTools_CurveSet::Add opens return (C.IsNull()) ? 0 : myMap.Add(C); so a null 3D curve is
dropped. GeomTools_Curve2dSet.cxx and GeomTools_SurfaceSet.cxx contain no IsNull at all, and
both crash. Guarded bridge-side here; not filed upstream (no OCCT change in this PR).

Self-test coverage: does it cover both failure modes? Yes, each proved by injection

--self-test runs 12 fixtures through the real unguarded_sites().

Six that must be reported, one per indirection form. Run against the old detector they score
1/6, catching only the plain form:

MISSED    reinterpret_cast straight into OCCT
MISSED    handle alias bound through a cast
MISSED    pointer alias through a C-style cast
MISSED    static_cast spelling
MISSED    array element through a cast
reported  plain param->field, no indirection

The plain form is deliberately kept in the set so the fix is provably additive, not a
replacement.

Six that must not be reported, which is the failure mode #630 was: plain two-condition opener;
guarded through the handle alias; guarded through a by-value Handle copy; guarded through the
pointer alias; DownCast is not a use; and guarded by the bridge helper it is passed to. Drop the
helper rule and that last one flips to FALSE (verified: removing it surfaces exactly the two
analytical sites and nothing else).

Post-review changes

Review of #641 was independent rather than confirmatory, and two things came back that changed the
diff. Both are recorded here because they are corrections to my own claims.

The Approx_SameParameter clearance was thinner than the conclusion it supported. That function
takes three handles and I cleared it on a single all-three-null probe, which only shows the all-null
case survives. repro_556.mm now probes each argument null with the other two valid. All four
combinations raise the same catchable Standard_Failure, so the verdict stands and the evidence now
covers it. The general rule: a multi-argument site needs per-argument probes.

OCCTGeomToolsCurveSetWrite is now guarded too, and measurement did not demand it.
GeomTools_CurveSet::Add guards its own handle, so a null cannot crash it. But this PR had guarded
its two siblings, leaving a three-way split in one family: two refuse the batch, the third silently
drops the null. Write() then emits fewer curves than the caller passed, and
Curve3D.serializeCurves/deserializeCurves is a round trip, so that is a silent truncation whose
surviving indices no longer match the input array. Guarding it is the safer contract and keeps the
family converged, which is what this audit is for. Counts moved with it: 20 -> 21 guarded pairs,
15 -> 16 functions, 24 -> 23 ALLOWED entries, 34 -> 33 cleared.

Accuracy-of-record fixes, the same defect class this batch keeps producing:

The overclaim is withdrawn. CLAUDE.md and the docstring said the bridge uses four indirections
and the checker understands all of them. Four is a fact about this tree, not a closed set. Both now
say the checker handles the four forms present here, and enumerate what it still cannot see:
(*cast).field; a reference-to-wrapper alias; an IsNull() whose result is discarded or does not
dominate the use; a negated guard; extern "C" on the definition line (the " falls outside
FUNC's return-type class, hiding the whole function); and a helper guarding only some paths. Plus
one false-positive direction: Handle(Geom_Curve) w(wrapper->curve); ctor-init syntax would be
wrongly reported. None occurs in this tree, checked rather than assumed, including confirming by
hand that every clearing guard transfers control.

ALLOWED's own contract is now written down, in the script and in CLAUDE.md: it is keyed
(file, function), with no argument index and no re-validation, so an entry exempts every wrapper
argument of that function and every later change to it. Its only safeguard is living in a tracked
file where an entry has to survive review.

Not taken: OCCTBridge_ProjLib_NLPlate.mm:1077's surface arm is redundant under the new rule
(GeomAdaptor_Surface is catchable), but the !s half and the curve arm are both required, so
splitting the condition to drop one catchable-only clause would cost clarity for nothing.

Verification

  • python3 Scripts/check-null-handle-guards.py exits 0; --self-test reports 12/12, exits 0

  • python3 Scripts/check-bridge-index.py reports 0 stale, 0 misfiled, exits 0 (unchanged here)

  • swift build clean

  • Bridge edits proved to compile from source, not masked by a stale OCCTSWIFT_BRIDGE_PREBUILT
    snapshot, by injecting a deliberate type error into OCCTBridge_Healing.mm and confirming the
    build failed on exactly that line

  • repro_556.mm rebuilt and re-run: 57 probes, 36 uncatchable / 10 catchable / 11 return
    (was 35 probes, 24 / 5 / 6)

Tests: which were run, and which were not

Every domain touched by this diff was run, with the exit status taken from swift test itself
rather than from a pipe. Final state, both branches, same filters:

filter this PR refactor/381-pass1b
the full suite 5273 tests / 1394 suites passed, exit=0 reviewer ran 5221 / 1388, exit=0
OCCTSurfaceTests|OCCTAnalysisTests|OCCTStressTests 1390 / 357 passed, exit=0 not run
OCCTCurveTests|OCCTGeom2dTests|OCCTShapeHealingTests|OCCTIOTests 1454 / 355 passed, exit=0 1443 / 353 passed, exit=0
Issue300|Issue525|RobustImportProgress|Cancellation 10 / 4 passed, exit=0 10 / 4 passed, exit=0

Those domains cover every file this PR changes (OCCTBridge_Curve3D.mm, _Geom2d.mm,
_Healing.mm, _IO.mm, _ProjLib_NLPlate.mm, _Surface.mm).

The full suite gap I flagged in the first round is closed: once the machine quietened it ran to
completion on the rebased head, 5273 tests in 1394 suites, exit=0, built from source with
OCCTSWIFT_BRIDGE_PREBUILT unset. The reviewer independently ran 5221 / 1388 green on the earlier
head. The base side of the Surface/Analysis/Stress row is still not run.

Three earlier runs of this branch did fail, and every failure was in the #525 / #300
cancellation family, so it is worth being explicit that they were chased down rather than waved
away:

run machine load result
1 7 concurrent suites loadSTEP cancelled on the first poll (#525) + loadRobust interrupts repair (#300)
2 7 concurrent suites those two passed; A caller that cancels once is not re-asked (#525) failed
3 moderate those passed; loadRobust cancelled during the transfer (#525) failed
4, 5 3 concurrent suites all pass, exit=0, on both filters

A different test failed each time, and none reproduces. A deterministic regression does not
move between runs, and it does not stop happening when the machine quiets down.

The mechanism is the fixture-flake class CLAUDE.md already records for parallel runs: these tests
use fixed, non-unique filenames in the shared system temp directory
(occt300_robust_repair.step at OCCTIOTests.swift:2344, occt525_oneshot_cancel.step at
:2521), so concurrent suites clobber each other's files. One failure said so literally:
.importFailed("Failed to import: /var/folders/.../occt300_robust_repair.step").

Independently, this diff cannot reach any of them. The only OCCTBridge_IO.mm change is inside
OCCTGeomToolsCurve2dSetWrite and OCCTGeomToolsSurfaceSetWrite, whose only callers anywhere are
Surface.swift:2515 and Curve2D.swift:2431. Nothing on the STEP import or cancellation path
calls either.

  • No public API change. Operation counts untouched (the pre-existing 4301-vs-4302 README and
    API_REFERENCE drift is present on origin/refactor/381-pass1b too, and is not from this PR)

Docs: docs/CHANGELOG.md Unreleased entry; CLAUDE.md's claim corrected (it said 24 of 35 entry
points, and did not say that a guard is only required where the OCCT call actually needs one, nor
that the handle is reachable by four spellings a reader would not grep for);
Scripts/repro/556-null-handle-guard-sweep/README.md documents the 22 new entry points.

🤖 Generated with Claude Code

gsdali added a commit that referenced this pull request Aug 2, 2026
Two review rounds. The census is the headline: the issue named 4 unbounded sites; measurement found 21, and 2 of the issue's 4 were wrong (MedialAxis.drawArc was already bounded by #558/#577, merged the day before the issue was filed; two Document.swift line numbers land on a blank line and a MARK comment). The reason the earlier sweep missed them is structural, not an oversight: #558 scoped to samplers, while these are result-buffer capacities on picking, spatial search, projection, intersection, hatching, text conversion and directory listing. A sampler-shaped census could not see them. Sampling.* call sites went 28 -> 49.

Round 1 verified that census independently rather than sampling it: the reviewer wrote its own scanner (every Array(repeating:count:) whose count names an Int parameter of the enclosing declaration), ran it against the PR base, got the same 21 entry points site-for-site, then cross-checked with a second lens over all 388 capacity-shaped bridge functions. The 20/1 capacity-vs-request split was verified per site by reading all 21 fill loops.

Round 1 blocked on three record defects, all fixed and independently re-swept: eight stale reference locations across four files (three needing opening), a CHANGELOG claim that the math family had been 'filed separately' when no such issue existed, and three math sites named nowhere. The deferral rationale was disproved by measurement - a consistency check alone is insufficient, since MathJacobi.eigenvalues(matrix: [1.0], n: -1) satisfies matrix.count == n*n exactly and still aborts, and MathSolver.leastSquares has no consistency check at all and drives an out-of-bounds read. Filed as #640 at P2, alongside #636 from the same work.

Round 2 blocked only on the PR description, which had been pasted over with PR #641's body ('Closes #618', the null-handle-guard write-up), deleting #622's census record and mis-attributing closure. Restored from this branch's own commit messages; no code change.

Verified before merge: 5245 tests / 1391 suites, exit 0, built from bridge source. The reviewer compared exact test-identifier SETS rather than counts - base lists 5232, PR lists 5245, difference exactly the 13 new Issue622AllocationBounds tests, nothing removed. Both ceiling injections reproduced (signal 5 on the Int32 conversion, which corrects the PR's original narrative crediting the allocation). The author retracted their own earlier '5,220 tests' evidence as taken under a stale-prebuilt configuration.

Merged on base-vs-PR failure parity, not CI green (#585).
gsdali and others added 2 commits August 2, 2026 21:17
… reaches a handle five ways

check-null-handle-guards.py printed "All bridge functions guard the geometry handle as well as
the wrapper pointer" and exited 0. Its use-detector matched `param->field` and nothing else, so
every site reaching the handle through an indirection was invisible: a named cast, a C-style cast
plus pointer alias, a handle alias (including the by-value Handle copy), and a shared bridge
helper. Casts are now normalised away position-preserving, so line numbers are unchanged, and
aliases are followed to a fixpoint.

The helper form cuts the other way, and is why teaching it the cast spelling alone would have been
wrong. OCCTGeomConvertCurveToAnalytical and occtSurfToAnaSurfResult pass their handle to
occtCurveToAnalytical / occtSurfaceToAnalytical, both of which open with
`if (curve.IsNull()) return false;`. Checked, one call frame away. A detector taught the first
three forms and not the fourth reports both as defects, which is #624/#630 one issue earlier. A
bridge helper that IsNull-checks the parameter it is handed now counts as a guard in its own right.

54 (function, argument) pairs across 39 functions, against the old detector's 1. Each was treated
as a candidate and measured against the OCCT entry point it actually reaches, per #556's method.
20 pairs across 15 functions needed a guard; 34 did not and are recorded in ALLOWED with the
measured reason.

The issue's own list of seven suspected-unguarded sites was partly wrong, which measuring first is
what caught. OCCTGeomLibToolParameter3D and OCCTGeomLibToolParameter2D reach
GeomLib_Tool::Parameter, which returns false on a null handle; OCCTApproxSameParameter reaches
Approx_SameParameter, which raises a catchable Standard_Failure the function's own catch (...)
already turns into the same false a guard would return. Three of the seven needed nothing. Eleven
sites the issue never named did.

repro_556.mm grew from 35 entry points to 57, the ones the pre-#618 walk never reached: 36 of 57
are uncatchable signals, was 24 of 35. Found on the way: GeomTools_CurveSet::Add guards with
`return (C.IsNull()) ? 0 : myMap.Add(C)` while GeomTools_Curve2dSet and GeomTools_SurfaceSet
contain no IsNull anywhere and both crash.

The script gains --self-test, matching check-bridge-index.py, proving both failure modes by
injection: six fixtures that must be reported, one per indirection form and including the plain
form so the fix is provably additive, and six that must not, including a guard reached only
through a by-value Handle copy and one only through the shared helper.

No public API change. Behaviour changes only for inputs no bridge call can currently produce.

Closes #618

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…amily converged, counts corrected

Addresses the review on PR #641.

- repro README: the #618 section summed to 23 for 22 probes. It is 12 crash, 5 catchable,
  5 return; new Geom_TrimmedCurve was double-counted, having already been probed under #556
  as Curve3DTrimmed. Miscounting a census in a PR about miscounted censuses.
- repro README: stale "none of the 24 is recoverable" replaced, it sat two paragraphs under
  the updated 36 of 57.
- ALLOWED: the OCCTGeom2dConvertApproxArcsSegments reason named the wrong callee. The throw is
  the Geom2dAdaptor_Curve on the preceding line (OCCTBridge_Geom2d.mm:2935).
- guarding_helpers docstring said two helpers qualify in this tree. Seven (function, argument)
  pairs across six functions do; only the two analytical ones are load-bearing.
- the printed want: hint was a type error for the 4 array-parameter sites, since the parameter
  is a pointer to wrappers. It now names an element.
- overclaim removed. Four indirection forms is a fact about this tree, not a closed set. The
  shapes the detector still cannot see are enumerated in the module docstring and CLAUDE.md:
  (*cast).field, a reference-to-wrapper alias, a discarded or non-dominating IsNull, a negated
  guard, extern "C" on the definition line, and a partially-guarding helper; plus the one
  false-positive direction, Handle(Geom_Curve) w(wrapper->curve) ctor-init. None occurs today.
- ALLOWED's own contract recorded: keyed (file, function), no argument index, no re-validation,
  so an entry exempts every argument and every later change to that function.
- Approx_SameParameter was cleared on a single all-three-null probe while taking three handles.
  repro_556.mm now probes each argument null with the other two valid. All four combinations
  raise the same catchable Standard_Failure, so the verdict stands and the evidence now covers
  it. 57 entry points, 60 probes; the two counts are not the same number.
- OCCTGeomToolsCurveSetWrite guarded, removing the three-way split this PR had created in one
  family. Not for crash-safety: GeomTools_CurveSet::Add guards its own handle. It drops the
  null instead, so Write emits fewer curves than were passed and serializeCurves is a round
  trip, making that a silent truncation with shifted indices.

Guarded sites 20 -> 21 pairs, 15 -> 16 functions; ALLOWED 24 -> 23 entries; cleared 34 -> 33.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsdali
gsdali force-pushed the fix/618-guard-checker-cast-sites branch from 3bf2e46 to d7ff196 Compare August 2, 2026 11:21
@gsdali
gsdali merged commit 008d2da into refactor/381-pass1b Aug 2, 2026
1 of 2 checks passed
gsdali added a commit that referenced this pull request Aug 2, 2026
…s-ci

Closes the loop on this batch's most persistent theme: four gate scripts now run on every PR, and three of the four either did not exist or did not work when the batch started - one was blind to four indirection forms and reported clean (#618/#641), one emitted seven FALSE reports from a template-head parse (#624/#630), one was written from scratch and then failed adversarial review twice before it caught anything real (#626/#635).

Seven invocations in a separate gate-scripts job: four gates plus the three self-tests. Separate rather than folded into build-and-test for the status check, not speed - that job is red branch-wide from #585's pinned-kernel mismatch, so a gate inside it would be red for an unrelated reason, which equals no gate. Demonstrated rather than asserted: on this PR build-and-test failed (5273 tests, 56 issues) while gate-scripts went green in 11-12s, and the diff touches no Swift or C++.

Review verified the wiring is genuinely load-bearing: live run 30746091057, all seven steps success, none skipped; no continue-on-error, no job-level if:, no needs:, no path filters, so a non-zero step fails the job - shown on the same run by build-and-test exiting 1 and concluding failure. All four break-it proofs reproduced site-for-site. It also ran a check the PR had not: blinding the detector (unguarded_sites -> return []) leaves the real run GREEN at exit 0, reproducing #618's exact failure mode, while --self-test drops to 6/12. Running the batteries in CI is not ceremony.

The most instructive defect was in the verification, not the code. The hook ran its three real gates with --quiet, so it named which gate failed but never where - and the PR's hook proof used count-operations.py, the ONE invocation without --quiet, i.e. the single case that could not exhibit the defect. Same vacuous-proof shape this batch kept finding in tests, occurring one level up. Fixed, and re-proved by breaking each of the three separately.

Also fixed: count-operations.py silently accepted unknown options, so a future --self-test would have run the ordinary report and exited 0 - a passing self-test that does not exist, with the trap armed by this PR's own house pattern of pairing each gate with its battery. Now exits 2. The Node 20 rationale was factually wrong (annotations are per-job, not per-workflow), so the pins were bumped and the annotation confirmed absent from the runner. And the hook install line used ln -s into .git/hooks, which fails in a linked worktree where .git is a file - this repo runs almost entirely out of .claude/worktrees/*, 35 live at the time.

Hook is opt-in by symlink or core.hooksPath, with its real divergences from CI now listed in its header rather than hidden behind an 'identical' claim: local python3 versus CI's pinned 3.12, exit 0 when python3 is absent, and working-tree rather than staged-snapshot checking.

Follow-ups filed: #648 (repo-wide action pin bump), #649 (make gate-scripts a required check, now safe because it is fast and green on every branch unlike build-and-test while #585 stands). Recorded but not fixed: check-docs-defaults.py exits 0 when it compares zero defaults, so a glob-path regression would pass silently.

Verified before merge: seven invocations present, unknown args exit 2, the sole remaining --quiet is a comment explaining why it is deliberately absent, gate job on @v7 pins with the siblings' @v4 inconsistency filed as #648.

Merged on base-vs-PR failure parity, not CI green (#585).
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