Repository navigation
Commit d4cb05c
fix(pm): the proxy re-exec guard is the CALLER’s name, and it speaks when it suppresses (#18980)
Fixes #18939
Clause-②: no
`skip-changeset`: the whole diff is `scripts/pm/**` (PM loop tooling).
Nothing in any
package's `files[]` moves, so nothing publishes.
## What was wrong
`proxyRearmPlan` (`scripts/pm/check-half-states.mjs`) read **one
hard-coded** guard
variable — `OS_HALF_STATES_PROXY_REARMED`, this file's own — while every
consumer sets
its **own** name on the child it re-execs. So any sibling instrument's
guard, once
inherited, answered "already re-armed" for a tool that had **never**
re-armed. The
suppressed run then took the bypassed route and answered **401 Bad
credentials** on
every endpoint, `/rate_limit` included — a self-consistent story about a
dead
credential, and the recorded cost of reading it as one is about an hour
spent treating
a live channel as dead.
Two separate halves, and the second one is the expensive one:
1. the guard **name** was shared where it should have been per-caller;
2. the suppression was **silent** — `rearm: false, hint: false` prints
nothing at any
consumer, so the run log carried no line pointing at the cause.
## The fix
```js
proxyRearmPlan({ env, execArgv, flagSupported, guard = PROXY_REARM_GUARD })
// -> { rearm, hint, flag?, guarded?, reason }
```
- `guard` is a parameter defaulting to this file's own name. A caller
passes the
variable it actually sets, so only its own guard can stop its own
re-exec.
- The guard branch returns `hint: true` and `guarded: guard`, and its
`reason` names
the variable, the proxy and the 401 it would otherwise be mistaken for.
`hint` is the
one branch **every** consumer already prints, so the silence is gone
repo-wide —
including for the nine consumers this PR does not edit.
- That branch can only fire on the anomaly: a real re-exec's child
carries
`--use-env-proxy` in its `execArgv` and is answered by the `routed`
branch above it,
never by this one.
- `post-stamped.mjs`, `label-write.mjs` and `board-snapshot.mjs` each
pass
`guard: PROXY_REARM_GUARD` and drop the own-guard `if` that sat
**after**
`if (!plan.rearm) return null;` — it was never reached for this defect
and is now the
plan's own branch. Their child env is unchanged: each still sets its own
name.
⛔ No behaviour change where no proxy is configured. On a GitHub Actions
runner the
`no HTTPS_PROXY` branch returns first, so no child is spawned and no
extra word is
printed; re-pinned in both directions.
## A/B — measured, same box, same minute, `--dry-run`, no write
Exit codes captured redirect-then-`$?`, never across a pipe. No token is
printed by any
of these.
**Before** (`26c73fb4e`), the card's own repro, reproduced here:
```
$ node scripts/pm/label-write.mjs --repo objectstack-ai/objectstack --issue 18932 --add finding --dry-run
ℹ️ re-exec with --use-env-proxy: HTTPS_PROXY is set (http://127.0.0.1:37101) and node's fetch does not read it.
[…] ① 取现集 — #18932 (open) carries 3 label(s) …
CONTROL_EXIT=0
$ OS_HALF_STATES_PROXY_REARMED=1 node scripts/pm/label-write.mjs (same args)
[…] ① read — COULD NOT READ: GET /repos//issues/18932 -> HTTP 401 (Bad credentials)
PREREQUISITE NOT MET … ⛔ NOT MEASURED
PROBE_EXIT=3 ← and not one line said why
```
**After** (`e424b1cf9`), the same two commands plus the own-guard leg:
```
$ node scripts/pm/label-write.mjs (same args)
ℹ️ re-exec with --use-env-proxy: HTTPS_PROXY is set (http://127.0.0.1:37101) …
AFTER_CONTROL_EXIT=0
$ OS_HALF_STATES_PROXY_REARMED=1 node scripts/pm/label-write.mjs (same args)
ℹ️ re-exec with --use-env-proxy: HTTPS_PROXY is set (http://127.0.0.1:37101) …
[…] ① 取现集 — #18932 (open) carries 3 label(s) …
AFTER_SHARED_EXIT=0 ← the sibling's guard no longer suppresses this tool
$ OS_LABEL_WRITE_PROXY_REARMED=1 node scripts/pm/label-write.mjs (same args)
ℹ️ OS_LABEL_WRITE_PROXY_REARMED=1 is set and this process is NOT routed through
http://127.0.0.1:37101, so the one re-exec this run allows was already spent and
every request below bypasses the proxy — if they answer 401 Bad credentials, that
inherited guard is why, and unsetting OS_LABEL_WRITE_PROXY_REARMED is the fix
rather than a new token. A refusal below may be about the route, not this container.
[…] ① read — COULD NOT READ: … -> HTTP 401 (Bad credentials)
AFTER_OWN_EXIT=3 ← still exit 3, but the cause is now the FIRST line on stderr
```
And the half that needed no edit — an **unedited** consumer,
`sweep-stale-finding.mjs`:
```
$ OS_HALF_STATES_PROXY_REARMED=1 node scripts/pm/sweep-stale-finding.mjs --dry-run
ℹ️ OS_HALF_STATES_PROXY_REARMED=1 is set and this process is NOT routed through … 401 …
sweep-stale-finding: PREREQUISITE NOT MET — GET /repos/…/issues?… -> HTTP 401
EXIT=3
```
Its re-exec is still suppressed (it passes no `guard` — see the table),
but it is no
longer suppressed **in silence**, which is what the triage asked the
deliverable to
contain.
⛔ Not measured, and not claimed: whether a real seat's process actually
inherits one of
these variables from a sibling. The card flagged that as unmeasured and
it stays
unmeasured — every leg above sets the variable by hand.
## The 13 consumers, read at `26c73fb4e`
`git grep -l proxyRearmPlan scripts` lists 14 files; one of them defines
the plan, so 13
consume it. Only the three in the first row are edited.
| consumer | shape | after this PR |
|---|---|---|
| `pm/post-stamped.mjs`, `pm/label-write.mjs`, `pm/board-snapshot.mjs` |
own guard name, `process.env` handed to the plan raw | **edited** — each
passes its own `guard`; cross-suppression gone, and a suppressing guard
speaks |
| `check-single-claim-paths.mjs` (`proxyPlanEnv()`),
`pm/check-expected-skips.mjs`, `pm/check-prior-rulings.mjs` | map their
own name **onto** the shared default, then call | unchanged and still
correct — the default is untouched, pinned in this PR from the plan's
side; they gain the loud line for free |
| `pm/changeset-deadline-census.mjs`, `pm/check-clause2-carriers.mjs`,
`pm/sweep-closed-cards.mjs`, `pm/sweep-stale-finding.mjs` | own guard
name, `process.env` raw — **the same live defect** | **not edited** (out
of this card's ruled surface): still cross-suppressible, but no longer
silently. Filed as a finding |
| `check-issue-citations.mjs` | calls the plan for refusal **text**
only; never re-execs | unaffected |
| `pm/check-governed-merges.mjs` (declares a second plan of the same
name), `pm/ci-failure.mjs` (imports it) | a separate implementation
reading `OS_GOVERNED_MERGES_PROXY_REARMED`, shared across those two
files | untouched by this PR; the same one-name-two-scripts shape one
layer over. Filed as a finding |
## Self-test counts, before / after
| file | cases | batteries | floor |
|---|---|---|---|
| `pm/check-half-states.mjs` | 4963 → **4975** | 6 → 6 (new cases are
unbatteried `t()`) | 6 → 6, every battery pin holds |
| `pm/post-stamped.mjs` | 349 → **360** | 13 → **14** | 13 → 13 (held,
see below) |
| `pm/label-write.mjs` | 64 → **75** | 8 → **9** | 8 → **9** |
| `pm/board-snapshot.mjs` | 145 → **156** | 11 → **12** | 10 → **12**
(it had drifted below its own roster) |
`post-stamped`'s `SELF_TEST_BATTERY_FLOOR` line sits inside PR #18934's
open hunk range,
so it is deliberately **not** touched: the roster rises to 14 while the
pinned floor
holds at 13. Both sides stay green whichever lands first, and #18934's
own bump to 14
still applies. Every new battery is declared, registered and floored.
Each of the three callers gains the same battery — *the re-exec guard:
the name this
tool sets, and the patrol name that must not silence it* — pinning: this
tool's guard is
not the patrol's; the patrol literal pinned there **is** the plan's
default (so renaming
the shared constant reds the pin instead of leaving a phantom); a
proxied run with no
guard re-execs; its own guard suppresses; the patrol's does not; the
suppression prints
a line naming the variable and the 401; the runner leg is unchanged; and
two structural
properties — the dispatch really hands the plan this file's guard, and
the plan is
imported rather than restated.
## Reader test
A seat whose parent process leaked `OS_HALF_STATES_PROXY_REARMED=1` runs
`label-write`
and reads its **banner**, not a 401 story. If the leaked variable is
instead that tool's
*own* guard, the seat still gets exit 3 — correctly, the re-exec really
was spent — but
the first line on stderr names the variable and says unsetting it is the
fix, so nobody
goes looking for a new token.
## Acceptance notes
Noted while reading the surface, ⛔ not fixed here (outside this card's
ruled surface),
reported to the seat for filing:
- Four more consumers carry the **identical** defect and were not in the
card's count:
`pm/changeset-deadline-census.mjs`, `pm/check-clause2-carriers.mjs`,
`pm/sweep-closed-cards.mjs`, `pm/sweep-stale-finding.mjs`. Each declares
its own guard
and hands `process.env` to the plan raw. One line each would fix them.
The card
explicitly read **0** own-guard names in `check-clause2-carriers.mjs`;
the name is
there at `:816`.
- `pm/check-governed-merges.mjs` declares a **second** `proxyRearmPlan`
rather than
importing the patrol's, and shares its single guard name with
`pm/ci-failure.mjs` —
the same cross-suppression shape between those two files, in a plan this
PR does not
touch.
- `noted, not filed:` the card's 「装饰性」 reading of the three own-guard
constants is
half right — each **is** read, by its own file and by nothing else. No
carrier: this
is a correction to a card body, not a defect in the tree.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01BTeBejoPUvRHN8WdAJC6oF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7d70612 commit d4cb05c
4 files changed
Lines changed: 172 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1766 | 1766 | | |
1767 | 1767 | | |
1768 | 1768 | | |
| 1769 | + | |
| 1770 | + | |
| 1771 | + | |
| 1772 | + | |
| 1773 | + | |
| 1774 | + | |
| 1775 | + | |
1769 | 1776 | | |
1770 | 1777 | | |
1771 | 1778 | | |
1772 | 1779 | | |
| 1780 | + | |
1773 | 1781 | | |
1774 | 1782 | | |
1775 | 1783 | | |
1776 | 1784 | | |
1777 | 1785 | | |
1778 | 1786 | | |
1779 | | - | |
1780 | 1787 | | |
1781 | 1788 | | |
1782 | 1789 | | |
| |||
1888 | 1895 | | |
1889 | 1896 | | |
1890 | 1897 | | |
| 1898 | + | |
1891 | 1899 | | |
1892 | 1900 | | |
1893 | 1901 | | |
| |||
1900 | 1908 | | |
1901 | 1909 | | |
1902 | 1910 | | |
1903 | | - | |
| 1911 | + | |
1904 | 1912 | | |
1905 | 1913 | | |
1906 | 1914 | | |
| |||
1945 | 1953 | | |
1946 | 1954 | | |
1947 | 1955 | | |
| 1956 | + | |
| 1957 | + | |
| 1958 | + | |
| 1959 | + | |
| 1960 | + | |
| 1961 | + | |
| 1962 | + | |
| 1963 | + | |
| 1964 | + | |
| 1965 | + | |
| 1966 | + | |
| 1967 | + | |
| 1968 | + | |
| 1969 | + | |
| 1970 | + | |
| 1971 | + | |
| 1972 | + | |
| 1973 | + | |
| 1974 | + | |
| 1975 | + | |
| 1976 | + | |
| 1977 | + | |
| 1978 | + | |
| 1979 | + | |
| 1980 | + | |
1948 | 1981 | | |
1949 | 1982 | | |
1950 | 1983 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19653 | 19653 | | |
19654 | 19654 | | |
19655 | 19655 | | |
| 19656 | + | |
| 19657 | + | |
| 19658 | + | |
| 19659 | + | |
| 19660 | + | |
| 19661 | + | |
19656 | 19662 | | |
19657 | 19663 | | |
19658 | 19664 | | |
| |||
19684 | 19690 | | |
19685 | 19691 | | |
19686 | 19692 | | |
19687 | | - | |
19688 | | - | |
19689 | | - | |
19690 | | - | |
| 19693 | + | |
| 19694 | + | |
| 19695 | + | |
| 19696 | + | |
| 19697 | + | |
| 19698 | + | |
| 19699 | + | |
| 19700 | + | |
| 19701 | + | |
| 19702 | + | |
| 19703 | + | |
| 19704 | + | |
| 19705 | + | |
| 19706 | + | |
| 19707 | + | |
| 19708 | + | |
| 19709 | + | |
| 19710 | + | |
| 19711 | + | |
| 19712 | + | |
| 19713 | + | |
| 19714 | + | |
| 19715 | + | |
| 19716 | + | |
19691 | 19717 | | |
19692 | 19718 | | |
19693 | 19719 | | |
19694 | 19720 | | |
19695 | 19721 | | |
19696 | | - | |
| 19722 | + | |
| 19723 | + | |
| 19724 | + | |
| 19725 | + | |
| 19726 | + | |
| 19727 | + | |
| 19728 | + | |
| 19729 | + | |
| 19730 | + | |
| 19731 | + | |
| 19732 | + | |
19697 | 19733 | | |
19698 | 19734 | | |
19699 | 19735 | | |
| |||
35334 | 35370 | | |
35335 | 35371 | | |
35336 | 35372 | | |
| 35373 | + | |
| 35374 | + | |
| 35375 | + | |
| 35376 | + | |
| 35377 | + | |
| 35378 | + | |
| 35379 | + | |
| 35380 | + | |
| 35381 | + | |
| 35382 | + | |
| 35383 | + | |
| 35384 | + | |
| 35385 | + | |
| 35386 | + | |
| 35387 | + | |
| 35388 | + | |
| 35389 | + | |
| 35390 | + | |
| 35391 | + | |
| 35392 | + | |
| 35393 | + | |
| 35394 | + | |
| 35395 | + | |
| 35396 | + | |
| 35397 | + | |
| 35398 | + | |
35337 | 35399 | | |
35338 | 35400 | | |
35339 | 35401 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
| 102 | + | |
102 | 103 | | |
103 | 104 | | |
104 | 105 | | |
| |||
788 | 789 | | |
789 | 790 | | |
790 | 791 | | |
| 792 | + | |
791 | 793 | | |
792 | 794 | | |
793 | 795 | | |
| |||
797 | 799 | | |
798 | 800 | | |
799 | 801 | | |
800 | | - | |
| 802 | + | |
801 | 803 | | |
802 | 804 | | |
803 | 805 | | |
| |||
817 | 819 | | |
818 | 820 | | |
819 | 821 | | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
| 825 | + | |
| 826 | + | |
| 827 | + | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
| 838 | + | |
| 839 | + | |
| 840 | + | |
| 841 | + | |
| 842 | + | |
| 843 | + | |
| 844 | + | |
| 845 | + | |
| 846 | + | |
820 | 847 | | |
821 | 848 | | |
822 | 849 | | |
| |||
1012 | 1039 | | |
1013 | 1040 | | |
1014 | 1041 | | |
| 1042 | + | |
| 1043 | + | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
| 1048 | + | |
1015 | 1049 | | |
1016 | 1050 | | |
1017 | 1051 | | |
1018 | 1052 | | |
| 1053 | + | |
1019 | 1054 | | |
1020 | 1055 | | |
1021 | 1056 | | |
1022 | 1057 | | |
1023 | 1058 | | |
1024 | 1059 | | |
1025 | | - | |
1026 | 1060 | | |
1027 | 1061 | | |
1028 | 1062 | | |
| |||
0 commit comments