fix(scripts): three comments and one guard that cite mechanisms not on main - #4
Merged
Merged
Conversation
…n main Sweep for gcy-ajv: comments that justify omitting or shaping something by citing a mechanism elsewhere, where the cited mechanism does not exist on origin/main. Method was mechanical — extract every path cited in .githooks/, scripts/ and the Makefile (104 distinct), then `git cat-file -e origin/main:<p>` each one. Three real hits after discarding Go import paths, symbol references and test fixtures. 1. scripts/check-core-boundary.sh — COMMERCIAL_SURFACE lists internal/serviceproto, which does not exist on main, and the loop did `[ -e "$p" ] || continue`. So check (f) scanned two of its three surface members and the script still printed "OK (no commercial coupling in the OSS module)". This contradicts the file's own contract, stated ~100 lines above at the header: "FAILS CLOSED: if a check cannot evaluate ... that is a violation, not a pass. A guard that silently passes when it cannot evaluate manufactures false confidence." The absence is currently legitimate — serviceproto is planned for this module, not the hosted one (engdocs/plans/hosted-onboarding/DESIGN.md:154), so this is latent rather than an active hole: check (f) is wired ahead of the package, the same way check (e) is wired ahead of OpenFeature. The difference is that (e)'s inertness is documented and (f)'s was not. Made the skip announce itself via note() rather than fatal, so behaviour and exit status are unchanged (CI runs this at .github/workflows/ci.yml:189) but the guard cannot stay silently switched off once serviceproto lands, or if a path here is ever misspelled. Verified: prints the notice, still exits 0. 2. scripts/gc-session-docker:45 — "Matches the K8s provider's skip list in internal/session/k8s/pod.go buildPodEnv()." No internal/session/k8s exists on main, and the one sibling that does exist, contrib/session-scripts/gc-session-k8s, carries no equivalent list. The claim was load-bearing: it is the stated reason the del(...) list has the contents it has, telling a maintainer their edit needs to match a canonical copy that is not there. Replaced with what is actually true — the list is maintained here and derived from nothing else. 3. scripts/gc-session-docker:8 — pointed at internal/session/exec/exec.go for the exec provider protocol; that file does not exist. The protocol is documented in docs/reference/exec-session-provider.md, which does. Checked and deliberately not changed: scripts/lib/common.sh and scripts/lib/test-slice.sh (cited relatively, both present); install-bd-archive.sh / install-dolt-archive.sh / mol-dog-backup.sh (cited with .github/ and examples/ prefixes the extraction stripped, all present, and guarded by their own pin tests); internal/example (a test fixture package name, not a path). The other two instances gcy-ajv catalogued are fixed separately: both .githooks/pre-push comments in the gcy-edv change, and AGENTS.md's `go` shim in the gcy-2o1 change. Refs: gcy-ajv, gcy-edv, gcy-2o1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs
gcy-ajv. Completes the sweep that PRs #1 (gcy-edv) and #3 (gcy-2o1) each fixed one instance of.The pattern
A comment asserts a protection or a canonical source that is not present, and that comment is the stated reason the code does what it does. It isn't merely stale — it's load-bearing, because a reader uses it to conclude an omission is intentional and covered.
Method
Mechanical and reproducible: extract every path cited in
.githooks/,scripts/, and theMakefile(104 distinct), thengit cat-file -e origin/main:<path>each one. Branch-locality is the trap — checking the working tree finds nothing, because the repo root sits on a deploy branch. Three real hits after discarding Go import paths, symbol references, and test fixtures.1.
scripts/check-core-boundary.sh— a guard that silently checks nothingCOMMERCIAL_SURFACElistsinternal/serviceproto, which does not exist onmain, and the loop did[ -e "$p" ] || continue. Check (f) therefore scanned two of its three surface members and the script still printedOK (no commercial coupling in the OSS module).That contradicts the file's own contract, stated ~100 lines above in its header:
This is latent, not an active hole. The absence is legitimate today —
serviceprotois planned for this module, not the hosted one (engdocs/plans/hosted-onboarding/DESIGN.md:154), so check (f) is wired ahead of its package exactly as check (e) is wired ahead of OpenFeature. The difference is that (e)'s inertness is documented and (f)'s was not.So the skip now announces itself via
note()rather than becoming fatal. Behaviour and exit status are unchanged — this runs in CI at.github/workflows/ci.yml:189— but the guard can no longer stay silently switched off onceserviceprotolands, or if a path here is ever misspelled.2.
scripts/gc-session-docker:45— a canonical copy that isn't thereNo
internal/session/k8sexists onmain, and the one sibling that does exist —contrib/session-scripts/gc-session-k8s— carries no equivalent list. The claim is load-bearing: it is the stated reason thedel(...)list has the contents it has, telling a maintainer their edit must match a canonical copy that does not exist. Replaced with what is true — the list is maintained here and derived from nothing else.3.
scripts/gc-session-docker:8— wrong pointerPointed at
internal/session/exec/exec.gofor the exec provider protocol; that file does not exist. The protocol is documented indocs/reference/exec-session-provider.md, which does.Checked, deliberately not changed
scripts/lib/common.sh,scripts/lib/test-slice.sh(cited relatively, both present);install-bd-archive.sh/install-dolt-archive.sh/mol-dog-backup.sh(cited with.github/andexamples/prefixes the extraction stripped — all present, and guarded by their own pin tests);internal/example(a test-fixture package name, not a path).bash -nclean on both files. No.go, web, or docs paths staged, so pre-commit is a no-op.