Skip to content

fix(deps): pin fast-uri to 3.1.6 for four new high advisories - #2249

Merged
chapati23 merged 3 commits into
mainfrom
fix/2248-fast-uri-3-1-6
Sep 3, 2026
Merged

fix(deps): pin fast-uri to 3.1.6 for four new high advisories#2249
chapati23 merged 3 commits into
mainfrom
fix/2248-fast-uri-3-1-6

Conversation

@chapati23

@chapati23 chapati23 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

The Problem

  • The required Code Quality check was red on main and on every open PR, blocking merges repo-wide for reasons unrelated to any PR's diff. Two independent defects were stacked behind it, and the second was invisible: the job runs its pnpm audit steps before Run Trunk, so it died at the first audit step and never reached Trunk.
  • Four high advisories published 2026-09-02 land on fast-uri 3.1.5: GHSA-5jgf-p345-68v8 (host confusion via skipped IDN canonicalization), GHSA-f65p-4m7j-42xc (SSRF via malformed IPv6 normalization), GHSA-fph4-wmhf-6fwf (SSRF via repeated hostname percent-decoding), and GHSA-jqff-g426-hqxp (host confusion via percent-encoded scheme normalization). ajv pulls fast-uri into all four dependency roots.
  • Every root already carried "fast-uri@<3.1.5": 3.1.5, added to escape GHSA-7p8r-x3mc-p8w7. That clause is satisfied by 3.1.5 itself, so the override actively held the graph on the version the four new advisories hit. This was a stale override, not a missing one.

The Solution

Raise the existing clause to "fast-uri@<3.1.6": 3.1.6 in all four dependency roots and refresh all four lockfiles, so every root resolves fast-uri@3.1.6 — the first patched release on the 3.x line for all four advisories. The repo stops shipping a URI parser with two SSRF and two host-confusion holes into the Cloud Functions that parse webhook payloads.

The clause is replaced, not supplemented: a second <3.1.6 override alongside the old <3.1.5 one would leave the two fighting over the same package. The floor stays on the 3.x line because ajv 8, the only consumer here, depends on fast-uri ^3; npm latest is 4.1.4 and moving there would be a major bump of a transitive dependency with no advisory benefit.

The second defect, cleared in the same PR. Fixing the audit steps let Code Quality reach Run Trunk for the first time since #2228 merged, and it failed there with 6 shellcheck findings in scripts/agent-quality-gate.test.sh (4x SC2329, 1x SC2030). Because the audit runs first, neither fix could turn the check green alone. This PR carries both, which is why it also closes #2251. The shellcheck half is narrow and per-block, not a rule-wide exemption: each # shellcheck disable names the indirect invocation that justifies it, and three flagged stubs were genuinely dead and got deleted rather than suppressed, with a comment recording when a future fixture would need them back.

Material limit: this pins a floor, not a ceiling. If a fifth advisory lands on 3.1.6 the same trap recurs, because the override that escapes an advisory is the thing that later holds the graph on the vulnerable version. Nothing here detects that automatically; the audit gate catching it on the next PR is the mechanism, as it was here.

Details

  • Overrides changed in pnpm-workspace.yaml, governance-watchdog/pnpm-workspace.yaml, alerts/infra/oncall-announcer/pnpm-workspace.yaml, and alerts/infra/onchain-event-handler/pnpm-workspace.yaml. Each comment names the four new GHSAs, the superseded GHSA-7p8r-x3mc-p8w7 floor, and the ajv constraint that keeps the pin on 3.x.
  • The override value is an exact version, never a range. Range values forward-resolve on fresh lockfiles, which is the undici failure recorded at the top of pnpm-workspace.yaml (fix: pin undici to 7.24.2 to restore governance-watchdog Discord delivery #831/Upgrade discord.js to restore undici 8.x compatibility in governance-watchdog #833/Cap or scope the root undici override (>=6.24.0) to prevent another forward-resolution to 8.x #837).
  • fast-uri was already in minimumReleaseAgeExclude in all four roots, so the 4320-minute release-age gate did not block a same-day security release. No new exclusion was needed.
  • Dependency paths cleared, all through ajv: .>@google-cloud/functions-framework>cloudevents>ajv>fast-uri, .>@google-cloud/functions-framework>cloudevents>ajv-formats>ajv>fast-uri, .>env-schema>ajv>fast-uri.
  • Lockfile diff scope: the only non-fast-uri change is pnpm recording a deprecated: metadata field on eslint@9.39.4 in three lockfiles. The version and integrity hash are unchanged; the registry added that notice upstream and re-resolution picked it up. No package version moved.
  • Also corrects one stale bullet in docs/pr-checklists/recurring-review-patterns.md. Its supply-chain section told reviewers that alerts/infra/* carry a package.json overrides block pinning an older fast-uri; fix(alerts-infra): bound standalone security overrides #1849 removed those blocks, and the files have no overrides key today. The surviving point — pnpm 11 ignores a package.json overrides block — is kept.

Validation

  • All four pnpm-audit-high-gate.mjs gates (., governance-watchdog, alerts/infra/oncall-announcer, alerts/infra/onchain-event-handler) report no high/critical pnpm advisories, both locally and inside the required Code Quality job. This proves the advisory database returns nothing high or critical for these lockfiles; it does not prove they stay clear as new advisories publish.
  • Supply-chain suites: pnpm-audit-high-gate.test.mjs 9 passed, alerts-uuid-overrides.test.mjs 2 passed, lockfile-lint.test.mjs 64 passed, version-skew-check.test.mjs 5 passed, override-prune-report.test.mjs 26 passed. lockfile-lint.mjs at the root and each standalone root: integrity hashes valid, no custom registry, no unbounded override floors.
  • pnpm install --frozen-lockfile at the root and --frozen-lockfile --lockfile-only in each standalone root: each lockfile matches its workspace file. grep -rn fast-uri across every lockfile and workspace file: no 3.1.5 residue.
  • Unit tests on the three packages resolving fast-uri: governance-watchdog 108 passed / 17 files, alerts/infra/onchain-event-handler 125 passed / 18 files, alerts/infra/oncall-announcer passed. turbo run lint typecheck across those three: 6 tasks successful. These prove the services build, lint and pass their own suites on 3.1.6; none exercises the changed URI-parsing paths directly, so they do not prove behavioural equivalence between 3.1.5 and 3.1.6 for adversarial URIs.
  • ./tools/trunk check --ci --all clean, and GATE_TEST_FOCUS=coordinator bash scripts/agent-quality-gate.test.sh exit 0 — the family that defines and invokes run_teardown_drain_command_identity_regression, the fixture the shellcheck half edits. That fixture's assertions hold; no negative control was run against them, so this does not prove they would still catch a regression. CI's Lint + test root scripts job runs the whole suite.
  • Independent review: codex-review.sh against origin/main returned PASS with zero findings in all three tiers. Claude's auto-review returned LGTM with an empty findings roll-up. CodeRabbit did not review this PR — it hit the org's usage spending cap and reported Review limit reached, so this PR has no CodeRabbit coverage.
  • Not run: the local pnpm agent:quality-gate --run never executed. The machine-wide gate coordinator was serving from a deleted worktree path, so its authority probe returned ENOENT, no client could join, and it held ~/.cache/agent-quality-gate/run.lock permanently — every gate run fell back to the legacy lock and timed out. The --no-lock escape hatch would have unblocked this PR's own work by weakening a control, which docs/notes/pr-operating-card.md forbids without recorded operator consent, so the mapped checks above were run individually instead. The dashboard, indexer, aegis, metrics-bridge, integration-probes and Terraform surfaces the gate would also have mapped were covered by CI on this PR, not locally.

Deferrals

  • None.

Closes #2248
Closes #2251


Correction: after this PR merged, an automated session briefly edited this description to describe folding in fix/2252-gate-test-shellcheck, acting on stale state. That fold never happened here — the shellcheck fix was already included before merge. This description has been restored to describe what actually merged. PR #2254 remains open and is not superseded; see the comment thread.

🤖 Generated with Claude Code

https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4

Four high advisories published 2026-09-02 land on fast-uri 3.1.5:

- GHSA-5jgf-p345-68v8 (host confusion via skipped IDN canonicalization)
- GHSA-f65p-4m7j-42xc (SSRF via malformed IPv6 normalization)
- GHSA-fph4-wmhf-6fwf (SSRF via repeated hostname percent-decoding)
- GHSA-jqff-g426-hqxp (host confusion via percent-encoded scheme normalization)

Every dependency root already carried "fast-uri@<3.1.5": 3.1.5, added to
escape GHSA-7p8r-x3mc-p8w7. That clause is satisfied by 3.1.5 itself, so it
held the graph on the version the four new advisories hit; the audit gate
could not resolve past it. Raise the same clause to "fast-uri@<3.1.6": 3.1.6
in all four roots rather than adding a second clause that would fight it.

3.1.6 is the first patched release on the 3.x line for all four advisories.
Stay on 3.x because ajv 8, the only consumer here, depends on fast-uri ^3.
Pin the exact version, never a range: range override values forward-resolve
on fresh lockfiles (#831/#833/#837).

Also corrects a stale claim in the supply-chain section of
docs/pr-checklists/recurring-review-patterns.md: the alerts/infra package.json
overrides blocks it describes were removed in #1849.

Closes #2248

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
monitoring-dashboard Ready Ready Preview Sep 3, 2026 6:31am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available. Your 89 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 5b4a3c9e-d8ea-4652-ad14-59970d748ea7

📥 Commits

Reviewing files that changed from the base of the PR and between 5b3c21a and 3f2d3f5.

⛔ Files ignored due to path filters (4)
  • alerts/infra/oncall-announcer/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
  • alerts/infra/onchain-event-handler/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
  • governance-watchdog/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !pnpm-lock.yaml, !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • alerts/infra/oncall-announcer/pnpm-workspace.yaml
  • alerts/infra/onchain-event-handler/pnpm-workspace.yaml
  • docs/pr-checklists/recurring-review-patterns.md
  • governance-watchdog/pnpm-workspace.yaml
  • pnpm-workspace.yaml
  • scripts/agent-quality-gate.test.sh

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-03T06:32:56.678010Z 3f2d3f5 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @chapati23's task in 46s —— View job


Claude's Review

Verdict: LGTM

Verified the change end to end against origin/main:

  1. Override semantics are correct. All four pnpm-workspace.yaml files replace (not supplement) the stale fast-uri@<3.1.5: 3.1.5 clause with fast-uri@<3.1.6: 3.1.6. Grepped every pnpm-workspace.yaml for fast-uri — no duplicate/competing clause exists, confirming this is a clean replace as the PR body claims.
  2. Lockfiles are consistent with the override. All four pnpm-lock.yaml diffs resolve fast-uri@3.1.6 (new integrity hash) everywhere ajv@8.x depends on it, and the only unrelated hunk is the eslint@9.39.4 deprecated: metadata line pnpm picked up from the registry — version/integrity unchanged, matching the PR's stated lockfile-diff scope.
  3. minimumReleaseAgeExclude already lists fast-uri in all four roots (confirmed via grep), so the release-age gate claim is accurate — no new exclusion needed.
  4. Audit gate mechanics check out. Read scripts/supply-chain/pnpm-audit-high-gate.mjs: it's a fail-closed pnpm audit wrapper with a narrow path-scoped exception list that has no entry for fast-uri. That confirms the four new GHSAs would genuinely fail Code Quality on main until the override moved off 3.1.5, matching the PR's problem statement.
  5. Docs correction is accurate. docs/pr-checklists/recurring-review-patterns.md no longer claims alerts/infra/* carry a stale package.json overrides block — checked, those files have no overrides key today, so the correction is a legitimate stale-doc fix, not scope creep.

Cross-cutting notes

  • The PR body's own "Material limit" section is the right level of honesty here: this pins a floor, not a ceiling, and a fifth advisory on 3.1.6 would recur. No action needed — the gate is the intended detection mechanism.
  • The unresolved local agent:quality-gate lock (stale worktree path holding run.lock) is well-documented as a known infra issue, not something this PR should paper over by using --no-lock. Correct call per the operating card's "never weaken a control that blocks your own work" rule.

Findings roll-up

None. No inline comments — the diff is a minimal, internally consistent security pin with no correctness, security, or style issues found.

@chapati23

Copy link
Copy Markdown
Contributor Author

Your Code Quality failure is not from this diff. The audit steps passed — this
PR does fix them. The job then reached Run Trunk, which failed on six
shellcheck findings in scripts/agent-quality-gate.test.sh
(run 33684909673, head 13c6a573): five SC2329 and one SC2030, at lines 6600,
6604, 6669, 6688, 6694 and 6695.

Those are invisible on main because main still resolves fast-uri@3.1.5, so
its Code Quality dies at the first audit step and never reaches Trunk. The
last green run on main was 33650631209 at 15:45:36 UTC on 5a37407a; both
commits that touched that file since then merged while the audit step was
already masking Trunk. Tracked in issue 2251.

So this PR needs either that fix stacked on top, or issue 2251 landed
alongside, before Code Quality can go green.

Separately: I built the same fix independently on
fix/2245-governance-watchdog-fast-uri-audit under issue 2245 (a duplicate of
2248) before finding this PR, and opened no second PR. Diffing the two heads,
they are byte-identical in all four pnpm-workspace.yaml override entries and
all four pnpm-lock.yaml files; yours additionally corrects the stale
package.json overrides sentence in recurring-review-patterns.md, so yours is
a strict superset. Independent verification against that identical tree, in case
it is useful evidence here: the audit gate exits 0 on all four roots,
pnpm install --frozen-lockfile exits 0 in all four, pnpm lockfile:lint and
pnpm skew:check pass, and governance-watchdog typecheck, build and 108 unit
tests pass. I also probed the override binding rather than assuming it: a
scratch package on ajv@8.17.1 resolves fast-uri@3.1.6 with this override and
3.1.7 without, so the selector really does match ajv's ^3.0.1.

@chapati23

Copy link
Copy Markdown
Contributor Author

Review status on head 13c6a573, all surfaces enumerated:

  • Claude auto-review — LGTM, findings roll-up empty, no inline comments. Nothing to fix or decline.
  • Codex — Code Review completed on 13c6a57, no comment posted and 👍 on the description, which is its no-findings signal.
  • CodeRabbit — did not review. It hit the org's usage spending cap and reported Review limit reached, so this PR has no CodeRabbit coverage. Recording the gap rather than claiming the check passed.
  • codex-review.sh (local, independent model family, pre-push) — PASS, zero findings across all three severity tiers.

One thing changed after those reviews ran, so it is worth flagging rather than leaving in the diff of the description: Code Quality is still red, and it is not this PR. Clearing the fast-uri audit steps let the job reach its Run Trunk step for the first time since #2228 merged, and that step fails with 6 shellcheck findings in scripts/agent-quality-gate.test.sh. A clean detached worktree at origin/main 5b3c21a2, carrying none of these changes, reproduces all 6 verbatim. Two independent defects were stacked behind one red required check; the second is tracked in #2252 and is deliberately not fixed here, because the fix means adding shellcheck disables to the quality gate's own test file in order to unblock this PR.

@chapati23

Copy link
Copy Markdown
Contributor Author

Code Quality is now blocked by issue #2251 after this PR fixes fast-uri. PR #2237 already contains the exact ShellCheck repair on current main: commits 041f41dc1d1bcf40e236f2150b4e3230849fd3da and a0a5b6d66e9a145ce81f720593969efafaeb4614. Together they only extend the two existing eval-scope disable groups in scripts/agent-quality-gate.test.sh with SC2329; ./tools/trunk check --ci --all passes. Please apply or cherry-pick both changes so #2249 can make protected Code Quality green. I am leaving this branch with its current owner.

@chapati23

Copy link
Copy Markdown
Contributor Author

Correction: the existing owner thread tracks the same six ShellCheck findings in #2252. #2251 is a duplicate report of that defect.

@chapati23

Copy link
Copy Markdown
Contributor Author

Apply order: update #2249 to current main first, then cherry-pick 041f41dc1 and a0a5b6d66. Those commits modify the drain regression added on current main.

* origin/main:
  fix(gate): arm the drain refresh barrier on the named drain only (#2228)
  Remove obsolete review metrics collector (#2243)
`Code Quality` runs three `pnpm audit` steps before `Run Trunk`. While the repo
resolved `fast-uri@3.1.5` the job died at the first audit step, so Trunk never
ran and six findings that #2228 introduced in
`scripts/agent-quality-gate.test.sh` stayed invisible. The audit fix in this
branch reaches `Run Trunk`, which then fails on them.

All six sit in `run_teardown_drain_command_identity_regression`, which lifts
`teardown_active_timeouts` out of the gate and `eval`s it into a fixture
subshell:

- `drain_completed_parallel_command` and `drain_completed_darwin_command_cohort`
  (SC2329) are called by the eval'd teardown, which shellcheck cannot see
  through. Instrumenting them records two calls each, and the existing
  assertions read what those calls wrote. Both keep a narrow per-line disable
  naming the caller.
- `collect_process_tree` (twice) and `gate_darwin_exact_identity_terminate`
  (SC2329) are never called: the teardown reaches them only for a registered
  timeout record and this fixture registers none. They are removed, with a
  comment at each site saying when to restore them. Nothing falls through to a
  real implementation, because the fixture subshell defines only the two lifted
  functions.
- `gate_run_id` (SC2030) is a fixture input the eval'd teardown reads inside the
  same subshell. Shellcheck pairs the write with reads it inlines from
  `scripts/gate/run-handles.sh` under `-x`; dropping `-x` drops the finding.
  The write carries a per-line disable stating that.

No assertion, no fixture expectation, and no gate runtime changed. The diff is
byte-identical to PR 2254, which fixes the same defect on a branch off `main`,
so whichever lands first the other merges without a conflict.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9qMnHNftC9epVzng7JTwN
@chapati23

Copy link
Copy Markdown
Contributor Author

Stacked the second Code Quality fix onto this branch at 3f2d3f5e3fd7dd5f3d8354402a9905b9c7a99b32, with the operator's approval, so one head carries both halves. Code Quality now passes on this PR in 1m41s (run 33723512943): the three pnpm audit steps and Run Trunk are green in the same job for the first time.

What the commit does, in scripts/agent-quality-gate.test.sh:

  • drain_completed_parallel_command and drain_completed_darwin_command_cohort (SC2329) are really called, by the teardown_active_timeouts body this fixture evals. Both keep a narrow per-line disable naming the caller. Instrumenting the two overrides records two calls each, and the assertions below them read exactly what those calls wrote.
  • collect_process_tree (two sites) and gate_darwin_exact_identity_terminate (SC2329) are called by no one. The teardown reaches them only for a registered timeout record and this fixture registers none, which the same instrumented run confirms with zero calls. They are removed, each replaced by a comment saying to restore the stub if a future fixture registers records. Nothing falls through to a real implementation, because the fixture subshell defines only the two lifted functions.
  • gate_run_id (SC2030) is a fixture input the eval'd teardown reads inside the same subshell; nothing outside reads it. Under -x shellcheck inlines scripts/gate/run-handles.sh, which reads gate_run_id, and pairs those reads with this write. Checking the same file without -x reports no SC2030. The write keeps a per-line disable saying so.

No assertion, no fixture expectation, and no gate runtime changed.

origin/main is merged in at 16092c16 because the six findings live in the version of the file that PRs 2228 and 2243 landed after this branch was cut. Without the merge the fix would edit lines main no longer has.

Verification at the new head: ./tools/trunk check --ci --all reports no issues over 2486 files, which is the command CI's Run Trunk runs; bash -n on the file exits 0; pnpm gate:routing-table:test passes 71; pnpm docs:navigation-eval -- --check-fixtures returns valid; and the affected regression, lifted into a standalone runner, passes. pnpm agent:quality-gate --run exits 2 without running a mapped command, waiting on run.lock held by pid 73768, a coordinator serving a deleted worktree path — issues 2231 and 2224, not this diff — so the push used --no-verify once under the operator's standing authorization for host-state failures. Details and limits are in the updated description.

One thing for the operator to decide: PR 2254 fixes the same six findings on a branch off main under issue 2252, which duplicates issue 2251. The hunks here are byte-identical to that PR, so whichever lands first the other merges without a conflict, but only this head satisfies issue 2251's Done means of a green Code Quality on a head that also carries the fast-uri fix. One of the two should be closed unmerged.

@chapati23
chapati23 merged commit a0f2cde into main Sep 3, 2026
43 checks passed
@chapati23
chapati23 deleted the fix/2248-fast-uri-3-1-6 branch September 3, 2026 06:36
chapati23 added a commit that referenced this pull request Sep 3, 2026
…-no-incremental

* origin/main:
  fix(deps): pin fast-uri to 3.1.6 for four new high advisories (#2249)
  fix(gate): arm the drain refresh barrier on the named drain only (#2228)
  Remove obsolete review metrics collector (#2243)
chapati23 added a commit that referenced this pull request Sep 3, 2026
…ellcheck

* origin/main:
  fix(deps): pin fast-uri to 3.1.6 for four new high advisories (#2249)

# Conflicts:
#	scripts/agent-quality-gate.test.sh
chapati23 added a commit that referenced this pull request Sep 3, 2026
…t two teardown comments (#2254)

* fix(gate): clear the six shellcheck findings in the gate test suite

`Code Quality` has been red on `main` since PR #2228 merged, and stayed
invisible until PR #2249 cleared the `fast-uri` advisories that were aborting
the job before its `Run Trunk` step. `./tools/trunk check --ci --all` reports
six shellcheck findings in `scripts/agent-quality-gate.test.sh`, all inside
`run_teardown_drain_command_identity_regression`. The check is
ruleset-required, so this blocks `main` and every PR.

Cause. The test lifts `teardown_active_timeouts` out of the gate and runs it
under `eval` inside `( … )` fixture subshells. shellcheck cannot see through
the `eval`, so every function the lifted body calls by name reads as never
invoked (SC2329). The `gate_run_id` fixture write is the same shape from the
other side: `shellcheck -x` follows `scripts/gate/run-handles.sh`, which a
later test sources, and pairs its `gate_run_id` reads with the subshell write
(SC2030).

Two of the five flagged functions are real indirect call sites, and a runtime
probe of the lifted function confirms both run: the portable subshell calls
`drain_completed_parallel_command` twice, the Darwin subshell calls
`drain_completed_darwin_command_cohort` twice, and the assertions read what
those calls wrote. Each keeps a `# shellcheck disable=SC2329` naming the
indirect call site. The `gate_run_id` write keeps a `# shellcheck
disable=SC2030` naming the sourced file whose reads shellcheck pairs it with.

The other three were dead. Both fixtures set `active_timeout_records=()`, so
the lifted teardown returns before its descendant walk and never enters the
Darwin exact-identity terminate loop. The same probe recorded zero calls to
`collect_process_tree` (defined in both subshells) and
`gate_darwin_exact_identity_terminate`. Deleting them removes three of the six
findings outright instead of waiving them; a comment at each site records why
no stub is needed and when one would be.

The regression still binds: with the gate's per-worker
`gate_drain_active_mapped_command` assignment mutated away, the test fails with
"the teardown named './tools/trunk check --ci x' for the first worker".

The operator gave explicit consent on 2026-09-03 to edit this file even though
it is the quality gate's own test suite.

Closes #2252

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4

* fix(gate): correct the deletion-site comments and pin the SC2030 waiver

Review of the previous commit found two defects in what it added. Both are in
`run_teardown_drain_command_identity_regression`; neither changes behaviour.

The Linux deletion-site comment named the wrong mechanism. It said the teardown
"returns before its descendant walk", but on that path the guard at
`scripts/agent-quality-gate.sh:1508-1513` returns only when every registry is
empty, and the fixture sets `active_worker_pgids=(4000001 4000002)`. Execution
reaches the walk. `collect_process_tree` goes uncalled because `roots` is built
only from `active_timeout_records`, which the fixture leaves empty, so the loop
iterates zero times. A maintainer trusting the old wording would conclude that
nothing after the guard runs. The comment now names `roots`.

The SC2030 waiver was positional. shellcheck reports one SC2030 per variable and
currently attributes it to the Darwin `gate_run_id` write, which carries the
disable. The identical write in the Linux fixture had none, and was quiet only
because the Darwin one absorbed the finding. Deleting the Darwin assignment
moves the finding to that unprotected line: verified before this change
(`SC2030` at 6586) and after it (clean). Since the point of this PR is to keep
the required Code Quality check green, the directive is now mirrored onto both
writes so a later edit to either fixture cannot re-red it.

The Darwin restore-guidance also named `collect_process_tree` as a stub to put
back. That path returns at `scripts/agent-quality-gate.sh:1679` before the walk
whether or not records exist, so only the terminate stub could ever be needed
there; restoring the other would re-raise SC2329.

Validation: `shellcheck -x` clean; `./tools/trunk check --ci` clean on the file;
the lifted regression still passes with the same stub-invocation counts
(2/2/0/0); and it still fails under the mutated gate that drops the per-worker
mapped-command assignment.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
chapati23 added a commit that referenced this pull request Sep 3, 2026
…-reconciled

* origin/main:
  fix(deps): pin fast-uri to 3.1.6 for four new high advisories (#2249)
chapati23 added a commit that referenced this pull request Sep 3, 2026
…t two teardown comments (#2254)

* fix(gate): clear the six shellcheck findings in the gate test suite

`Code Quality` has been red on `main` since PR #2228 merged, and stayed
invisible until PR #2249 cleared the `fast-uri` advisories that were aborting
the job before its `Run Trunk` step. `./tools/trunk check --ci --all` reports
six shellcheck findings in `scripts/agent-quality-gate.test.sh`, all inside
`run_teardown_drain_command_identity_regression`. The check is
ruleset-required, so this blocks `main` and every PR.

Cause. The test lifts `teardown_active_timeouts` out of the gate and runs it
under `eval` inside `( … )` fixture subshells. shellcheck cannot see through
the `eval`, so every function the lifted body calls by name reads as never
invoked (SC2329). The `gate_run_id` fixture write is the same shape from the
other side: `shellcheck -x` follows `scripts/gate/run-handles.sh`, which a
later test sources, and pairs its `gate_run_id` reads with the subshell write
(SC2030).

Two of the five flagged functions are real indirect call sites, and a runtime
probe of the lifted function confirms both run: the portable subshell calls
`drain_completed_parallel_command` twice, the Darwin subshell calls
`drain_completed_darwin_command_cohort` twice, and the assertions read what
those calls wrote. Each keeps a `# shellcheck disable=SC2329` naming the
indirect call site. The `gate_run_id` write keeps a `# shellcheck
disable=SC2030` naming the sourced file whose reads shellcheck pairs it with.

The other three were dead. Both fixtures set `active_timeout_records=()`, so
the lifted teardown returns before its descendant walk and never enters the
Darwin exact-identity terminate loop. The same probe recorded zero calls to
`collect_process_tree` (defined in both subshells) and
`gate_darwin_exact_identity_terminate`. Deleting them removes three of the six
findings outright instead of waiving them; a comment at each site records why
no stub is needed and when one would be.

The regression still binds: with the gate's per-worker
`gate_drain_active_mapped_command` assignment mutated away, the test fails with
"the teardown named './tools/trunk check --ci x' for the first worker".

The operator gave explicit consent on 2026-09-03 to edit this file even though
it is the quality gate's own test suite.

Closes #2252

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4

* fix(gate): correct the deletion-site comments and pin the SC2030 waiver

Review of the previous commit found two defects in what it added. Both are in
`run_teardown_drain_command_identity_regression`; neither changes behaviour.

The Linux deletion-site comment named the wrong mechanism. It said the teardown
"returns before its descendant walk", but on that path the guard at
`scripts/agent-quality-gate.sh:1508-1513` returns only when every registry is
empty, and the fixture sets `active_worker_pgids=(4000001 4000002)`. Execution
reaches the walk. `collect_process_tree` goes uncalled because `roots` is built
only from `active_timeout_records`, which the fixture leaves empty, so the loop
iterates zero times. A maintainer trusting the old wording would conclude that
nothing after the guard runs. The comment now names `roots`.

The SC2030 waiver was positional. shellcheck reports one SC2030 per variable and
currently attributes it to the Darwin `gate_run_id` write, which carries the
disable. The identical write in the Linux fixture had none, and was quiet only
because the Darwin one absorbed the finding. Deleting the Darwin assignment
moves the finding to that unprotected line: verified before this change
(`SC2030` at 6586) and after it (clean). Since the point of this PR is to keep
the required Code Quality check green, the directive is now mirrored onto both
writes so a later edit to either fixture cannot re-red it.

The Darwin restore-guidance also named `collect_process_tree` as a stub to put
back. That path returns at `scripts/agent-quality-gate.sh:1679` before the walk
whether or not records exist, so only the terminate stub could ever be needed
there; restoring the other would re-raise SC2329.

Validation: `shellcheck -x` clean; `./tools/trunk check --ci` clean on the file;
the lifted regression still passes with the same stub-invocation counts
(2/2/0/0); and it still fails under the mutated gate that drops the per-worker
mapped-command assignment.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01McsUkrwYBL6oC7CdZCCsc4

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
chapati23 added a commit that referenced this pull request Sep 3, 2026
* origin/main:
  fix(gate): mirror the SC2030 waiver onto the Linux fixture and correct two teardown comments (#2254)
  docs(sweep): groom the backlog queue and refine pkg:tooling independence (#2233)
  chore(coderabbit): stop incremental reviews to cut add-on spend (#2236)
  fix(deps): pin fast-uri to 3.1.6 for four new high advisories (#2249)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant