ci: a docs-only PR still bought three OS matrices, and four guards a comment could satisfy (#378, #379)

[rainmanjam]

Closes #378. Closes #379.

Two adjacent issues, both about not doing work twice.

---

#378 — the docs filter reaches the build and test jobs

ci.yml's changes job already computed the predicate and only the acceptance
matrix read it. go, crossplatform, ui and cross now read it too, and
each gains a step that says out loud why a green check did no work.

The trap, and how I satisfied myself this cannot recreate #351

I did not reason about which jobs "look like" required checks — I asked:

$ gh api repos/:owner/:repo/branches/main/protection --jq '.required_status_checks.contexts'

Every job I touched publishes a required context:

job	context(s)	matrix?
go	go build, vet, test	no
crossplatform	test: ubuntu-latest, test: macos-latest, test: windows-latest	yes
ui	ui typecheck, lint, build	no
cross	cross-compile all release targets	no

crossplatform is the one that would have reproduced the outage. Its condition
is on every step; the job carries none.

The other three are gated the same way even though a job-level if: would be
correct for them today. A skipped ordinary job reports skipped and branch
protection accepts it. The reason for one flat rule is that the distance between
the safe spelling and the outage is a single strategy: block, added by someone
thinking about Go versions with no reason to be thinking about branch
protection. The price is four runner allocations that do nothing on a docs-only
PR, against the twenty minutes of compute they replace.

Verified mechanically rather than by eye — every step of every gated job carries
the condition, and no job-level if: reads it:

job              needs      job-if                                    matrix  steps  gated
go               changes    None                                      False   19     19
crossplatform    changes    None                                      True    18     18
ui               changes    None                                      False   9      9
cross            changes    None                                      False   6      6
acceptance       changes    None                                      True    9      9
container        container-suites  needs.container-suites.outputs.suites != '[]'  True  8  0

container is untouched and correctly keeps its job-level if: — it reads a
different job's output and its per-leg contexts are deliberately not required.

It is now enforced, not remembered

internal/testenv/docsgate_test.go fails if needs.changes.outputs.code is ever
read from a job-level if:, and fails if a job that declared needs: changes
leaves any step ungated. Red and green fixtures under
testdata/docsgate/ — including the green one that looks wrong (container's
shape), so the rule cannot grow an allowlist.

It found a pre-existing hole on its first run: acceptance's
Diagnostics (what was it waiting for) step was missed by #357. Harmless only
because a docs-only run has no suite step to fail, so failure() was never true
— a coincidence of this job's shape, not a property. Fixed here.

Residual risk, stated

needs: changes means a failing changes job skips these jobs, and for
crossplatform that means no per-leg contexts. This is recoverable by re-running
the job (unlike #351, which was deterministic and permanent), and it is the same
exposure the acceptance job has carried since #357. Not worked around, because
the alternative — expanding the matrix when the gate job failed — would report
green having examined nothing, which is strictly worse.

Out of scope: semgrep/gitleaks/govulncheck live in security.yml, which has no
changes job of its own. #378 scopes itself to "applying ci.yml's existing
output to more jobs".

---

#379 — readUI and stripJSComments promoted to internal/testenv

testenv.ReadUI and testenv.StripJSComments in
internal/testenv/uisource.go, alongside FreeUDPWindow/ReserveUDP and
documented in the same register — the "four copies of freeUDPPort" note in
ports.go is the same argument.

Every copy, and whether they had drifted

copy	language	drifted?
internal/db/facebook_ui_drift_test.go — readUI, stripJSComments, quotedBefore	Go	the original; the only Go copy
internal/testenv/pagesdeploy_test.go — readRepoFile	Go	yes — see below
ui/src/lib/tour-drift.test.ts — stripJSComments, quotedBefore	TypeScript	no, measured

They had drifted, in one respect, and I kept the stronger behaviour.
internal/db's readUI returned whatever it read, including nothing.
internal/testenv's own readRepoFile additionally fatals on an empty file
with "every assertion that reads it would pass by examining nothing".
ReadUI keeps the empty-file check, because a guard asserting a substring is
absent — if strings.Contains(src, "status?.ingest") in the ingest-header
guard is exactly that shape — passes over an empty file while asserting nothing.
Failing names the file; passing vacuously names nothing.

The TypeScript port has not drifted, and that is measured rather than
eyeballed. Both implementations were run over all 96 .ts/.tsx files under
ui/src and their output hashed per file: byte-identical on every one. It stays
a separate copy because vitest cannot call Go, and it names this one as its
origin.

One documented behaviour change, called out

tsUnion in internal/oauth/ui_drift_test.go said comments were "kept
deliberately" and that stripping them would "make the file harder to read in
exchange for nothing". That is now reversed, and the comment says why:

• the forward check is strings.Contains(union, "\"tags\""), so a member
  deleted into a comment kept the guard green over a union that could no longer
  name the field (demonstrated below);
• the body is bounded by the first ; after the type name, so a semicolon
  inside one of those explanatory comments truncates the union early and hides
  every member after it.

Nobody's reading experience changes — the notes are still in types.ts. They
just stop counting as declarations.

internal/db/ingest_header_drift_test.go's "why this lives in internal/db"
note is updated: it said the move to internal/engine was blocked on this
promotion. The blocker is gone; the move itself is left to its own change
because it changes which package a failure points at.

---

Mutation testing

Each mutation was applied to a committed tree, the named test run with -v
to confirm it actually ran, and the file restored from a backup with
command cp -f. Where a guard was previously comment-defeatable, the same
mutation was also run against the pre-change guard to show it used to pass —
otherwise the fix is unevidenced.

#	mutation	named test	before	after
A	| "tags" deleted from MetaField, text left in a comment inside the union	TestUITypesCanNameEveryMetadataField	PASS	FAIL — MetadataField "tags" is absent from the UI's MetaField union
B	the whole Kick row in capabilities.ts block-commented out	TestTheUICapabilityMatrixAgreesWithGo	PASS	FAIL — preset "kick" is in capabilities.go but absent from the UI matrix
C	tags: tagList, deleted from the push payload, text left in a comment	TestTheComposerCanSendFacebookTags	PASS	FAIL — the composer's push body carries no tags field
D	{withCompliance.length > 0 && ( → {false && (, condition left in a JSX comment	TestTheComposerSaysWhenAPushCarriesStoredCompliance	PASS	FAIL — the composer never mentions stored compliance
E	useIngestLive() replaced by the #367 shape, name left in a comment	TestTheHeaderAsksTheAppsOneQuestionAboutBeingLive	(already stripped)	FAIL — AppLayout no longer calls useIngestLive
F	ui/src/hooks/useLiveData.ts truncated to zero bytes	TestIngestLiveIsDerivedFromArrivingBytes	n/a	FAIL — ... is empty; every assertion that reads it would pass by examining nothing
G	job-level if: added to crossplatform — #351 verbatim	TestTheDocumentationGateIsNeverReadFromAJobLevelIf	n/a	FAIL — ci.yml :: job crossplatform :: reads needs.changes.outputs.code from a job-level if:
H	the gate removed from one step of go	TestTheDocumentationGateIsNeverReadFromAJobLevelIf	n/a	FAIL — ci.yml :: job go :: step #7 (unnamed) :: has no needs.changes.outputs.code in its if:

A/B/C/D "before" columns are the pre-change guard from HEAD, run against the
same mutated UI file: all four were green over source that could no longer do
what they claim to watch.

After every restore, git diff over ui/ was empty.

Honest note on F: on this tree the empty-file check is belt-and-braces — the
guard that would pass vacuously is preceded by a strings.Index that fatals
first. It fires and names the file, which is the point; it is not currently
load-bearing.

Gates

go build ./..., go vet ./..., gofmt -l ./cmd ./internal, go test ./... — all clean.
No files under ui/ are changed, so the UI toolchain gates do not apply.

https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud