From c6abff4d6f3a55fbaadb7d5bf428e182dfe8759a Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 15:44:09 +0900 Subject: [PATCH 1/9] docs: plan isolated port-probe disposal prerequisite --- .../_plan/260905_now_split_train/000_plan.md | 77 +++++++ .../003_parent_decisions.md | 205 ++++++++++++++++++ .../445_server_port_probe_disposal.md | 125 +++++++++++ 3 files changed, 407 insertions(+) create mode 100644 devlog/_plan/260905_now_split_train/000_plan.md create mode 100644 devlog/_plan/260905_now_split_train/003_parent_decisions.md create mode 100644 devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md diff --git a/devlog/_plan/260905_now_split_train/000_plan.md b/devlog/_plan/260905_now_split_train/000_plan.md new file mode 100644 index 0000000000..adfc936873 --- /dev/null +++ b/devlog/_plan/260905_now_split_train/000_plan.md @@ -0,0 +1,77 @@ +# 260905 — RESOLVABLE_NOW split train (stacked PRs) + +Date: 2026-09-05. Worktree a2c0, docs branch `codex/260905-modular-debt-ledger-docs` +at 4cc219549 (source basis 980a9fbed; origin/dev tip at unit open 583d6a91b, +6 commits ahead, only one of which touches a NOW file — see 001). Session +01a06e97-b9d8-7250-8204-bb788338c288, goalplan +`.codexclaw/goalplans/reduce-the-68-resolvable-now-modularization-debt/`. +Input ledger: `devlog/_plan/260905_modular_debt_ledger/021_ledger.md` (68 rows +with `RESOLVABLE_NOW`); lane evidence in that unit's 011–016. + +## Objective + +Bring each of the 68 files under the cxc-dev §1 400-line limit by pure-move +splits (leaf modules + barrel re-exports), published as stacked PRs against +`dev`, each layer independently reviewable and mergeable. Zero behavior +change; every existing export stays importable from its original path. +Per-file success is `RESOLVED` or `RESIDUAL-FN` (003 RESIDUAL-ACCOUNTING-01); +the closeout tallies both and only the first counts as resolved. + +## Constraints (binding on every layer) + +- Pure move only. No renames of exported identifiers, no signature changes, + no deletion of exports, no "while I'm here" fixes. A behavior defect found + during a move is recorded in the decade doc and left alone. +- New leaf files ≤400 lines; the residual original file ≤400 lines or the + layer states why a second layer (`#b`) follows (003 INTERMEDIATE-RESIDUAL-01, + RESIDUAL-FN-01). +- The ≤500-line PR cap is measured on the non-move diff for pure-move layers + (003 PURE-MOVE-SIZE-01); non-move diff ≤150 lines. +- Re-export binds nothing locally (260818 WP1 lesson): internal call sites in + the residual file import from the leaf explicitly. +- Text-oracle tests that read a split file as source (001 column + `textoracle`) are retargeted to the leaf **without weakening**; the + decade doc names each and the C phase drives the retargeted guard red once + when it is a guard. +- `tests/lab/core-lab-boundary.test.ts` PROTECTED roots are never edited; + a new leaf imported from a protected root must not reach `src/lab`. +- Verification from WP400 onward: typecheck, focused tests, privacy scan and + full suite run in an isolated checkout on `ssh lidge`; no local suites. +- Git: layer branches `codex/split-`; bottom layer base `dev`, each + upper layer base = the branch below; push + PR creation pre-authorized by + the user for this loop; **merge never** (DEV-STACK-04 ESCALATE). Cascade + with `git rebase --update-refs` + `--force-with-lease` when a lower layer + changes (DEV-STACK-02). +- Open-stack depth cap: 5 dependent PRs. S04 contains six total layers, + including prerequisite layer 105, but STACK-INDEPENDENCE-01 replaced the initial + six-deep linear proposal: its longest current base chain is 3. Across the + 77-layer map, the longest planned chain is 4. The former S04 depth-six + exception is historical, not permission to create a six-deep stack now. +- From WP400 onward, code and receipts use the existing a2c0 worktree in + place (003 WORKTREE-EVIDENCE-01). Preserve each previous branch before + selecting the next layer branch. Never relocate or recreate a2c0. + +## Work-phase map (dependency-ordered) + +| WP | Deliverable | Depends on | Verifier | +|---|---|---|---| +| wp1 | 000–002 + every layer's decade doc (010…750) at diff level | — | docs checks (numbered only, every layer has a doc, every NOW file appears in exactly one stack); privacy scan | +| wp2… | one layer per work-phase, dependency-ordered by the base edges in 002; independent groups may be interleaved | its declared base layer, if any | the current decade document's Verification and Accept criteria sections | + +Total: 77 implementation layers across 21 stacks (002_layer_map.md; 105 and +625 appended per 003). + +## Out of scope + +The 151 `RESOLVABLE_AFTER` and 19 `ACCEPTED` rows; core.ts / config.ts / +service.ts / auth-api.ts; merges; releases. + +## Terminal outcome expected + +DONE when every layer in 002 has an open PR with a green exact-head CI rollup +recorded in its decade doc. + +## Completion spine + +- WP400 closed through C→D with head `bbf8d3cd25ccf70eb595bc7982f63528d060c1bd`, ready PR #3611 against dev, clean remote receipt, all current logical CI checks passed/configured-skipped, and zero unresolved review threads. The CLI returned to IDLE and immediately entered P for WP450. The 1298-line facade still has its declared WP410 successor; this is layer completion, not completion of all68files. +- Earlier layer records remain in their decade documents. Global criterion c-5 still requires final reconciliation, including the known older verification debts; no whole-goal completion is claimed. diff --git a/devlog/_plan/260905_now_split_train/003_parent_decisions.md b/devlog/_plan/260905_now_split_train/003_parent_decisions.md new file mode 100644 index 0000000000..48a6b4b0cd --- /dev/null +++ b/devlog/_plan/260905_now_split_train/003_parent_decisions.md @@ -0,0 +1,205 @@ +# 003 — Parent decisions on drafter escalations (binding amendments to 000/002) + +Twenty-one drafters (one per stack) returned 75 decade docs. Fourteen of them +escalated the same conflict and six raised stack-specific questions. Each +decision below is an amendment to 000_plan.md constraints and is what the +A-phase audits and every executor obey. + +## PURE-MOVE-SIZE-01 — the ≤500-line changeset cap for pure-move layers + +Conflict: cxc-dev §1 says "PR changeset >500 lines → split" (DEFAULT class: +exceed only with a stated reason). A pure move of a file that must lose +≥1,000 original lines produces ≥2,000 raw diff lines however it is layered; +adding layers only multiplies fully-gated PRs and leaves intermediate residuals +over 400 with no review benefit (S02, S03, S05, S07, S08, S10, S11, S13, S15, +S16, S19, S20, S21 all showed this arithmetic). + +Decision (stated reason for exceeding): for a layer whose decade doc classes +it as pure-move, the 500-line cap is measured on the **non-move diff** — the +lines that are not a verbatim relocation: re-export blocks, import edits in +the residual and in consumers, test retargets, route-registry metadata. That +non-move diff must stay ≤150 lines per layer. Moved lines are reviewed as +moves: the PR body links `git diff --color-moved=dimmed-zebra` guidance and the +executor's C phase records `git diff -M --stat` plus a symbol-inventory check +(every symbol in the doc's inventory appears exactly once in the tree after +the move). + +Permitted transformations of a moved line (still pure-move): + +1. Adding or removing the `export` modifier on a moved declaration (a leaf must + export what the residual re-exports; a symbol that was module-private and + is now consumed only inside its leaf may stay private). +2. Changing the import specifier path of a moved symbol's own imports. +3. Object-literal method → factory-produced function when an adapter's + returned object literal is split across leaves (S03 Anthropic #b, + `createAnthropicAdapter` returns `{ ...methods }` capturing lexical + `provider`/`toolNames`): the method body is moved verbatim into a + leaf function `makeX(captured…)` whose parameters are exactly the + lexical bindings the body captures, called once inside the original + factory so the returned property becomes `x: makeX(provider, toolNames)`. + Capture identity and invocation lifetime are preserved because the + factory is invoked in the same closure scope the literal was built in. + Evidence: the C phase pastes `git diff --color-moved=dimmed-zebra + --color-moved-ws=allow-indentation-change` for each converted method and + shows the body as a move block; the layer's focused tests cover every + converted method (listed in the doc's Tests section). The same rule + covers a class method split by `this`-fields, should one occur. +4. JSX block → sibling component with verbatim props (GUI-SEAM-01). + +Anything else (reordering statements inside a moved body, renaming, changing +a literal, altering control flow) is not pure-move; the layer falls back to +the literal 500-line cap or is re-sliced. + +The layer count in 002 stands as drafted; no stack is re-sliced for size. + +## RESIDUAL-FN-01 — residual >400 caused by a single function + +S07 L1: `parseRequest` is 464 lines by itself, so `src/responses/parser.ts` +cannot reach ≤400 by moving other symbols. Splitting the function is a +behavior-preserving extraction, not a move, and is out of this train's scope. +Decision: the layer moves everything movable, the residual stays over 400, +and the doc records the function as `RESOLVABLE_AFTER(design:L1-parse-request-extraction)` +for the 021 ledger's next revision. Same rule applies to any other layer that +finds a single >350-line function (none other reported). + +## INTERMEDIATE-RESIDUAL-01 — over-400 residuals inside a multi-part file + +S13 (config-export #a), S18 (IntegrationsOverview), S21 (release-notes #a), +S02 (registry #a/#b): an intermediate residual over 400 is acceptable when a +**bounded successor chain inside the same stack** brings it under 400 and +each doc states the number it hands to the next layer (registry: +3250 → 2429 → 1267 → 219 across #a/#b/#c). S18 had no next layer: **layer +625 (IntegrationsOverview #b)** is appended to 002 and drafted by the same +agent. + +## RESIDUAL-ACCOUNTING-01 — what "done" means for a file + +000's objective is amended: the train's success measure is per file, one of +`RESOLVED` (residual ≤400 and all leaves ≤400), or `RESIDUAL-FN` (residual +>400 solely because of one unsplittable function, recorded per +RESIDUAL-FN-01 with the `design:` id for the ledger). The closeout doc +tallies both; a file in the second bucket is *not* counted as resolved. At +draft time exactly one file is expected there: `src/responses/parser.ts` +(561, `parseRequest`). + +## TYPE-CYCLE-01 — pre-existing type-only cycles + +S04 L1 reports `src/types.ts → src/types/provider.ts → native-exec-desktop.ts +→ native-exec-tools.ts → tool-definitions.ts → src/types.ts`; S02 reports an +Antigravity type cycle. Both pre-exist on `dev` and are erased at runtime. +Decision: a layer must not add a **runtime** cycle and must not add a new +type-only cycle; it may leave existing ones untouched. The audit checks the +delta, not the whole graph. + +S04 is the exception: its new leaves would each join the existing type cycle +(`tool-naming → ../../types → provider → native-exec-desktop → +native-exec-tools → tool-definitions → tool-naming`), which is a *new* cycle +through new files. Decision: the prerequisite the S04 drafter named is +approved and becomes **layer 105 (`codex/split-cursor-desktop-executor-contract`, +base `dev`, new bottom of S04)**: move `DesktopExecutorConfig` +(`src/adapters/cursor/native-exec-desktop.ts:28–37`) to a new dependency-free +`src/adapters/cursor/desktop-executor-contract.ts`, keep it exported from +`native-exec-desktop.ts` via `export type { DesktopExecutorConfig } from +"./desktop-executor-contract"` plus a local `import type`, and retarget the +inline `import("../adapters/cursor/native-exec-desktop").DesktopExecutorConfig` +at `src/types/provider.ts:701` to the contract file. Type-only, zero runtime +effect; breaks the provider → desktop-implementation edge for good. 110's +base becomes `codex/split-cursor-desktop-executor-contract`. S04 has six +members including 105. The original linear proposal called that depth 6 and +made an exception; STACK-INDEPENDENCE-01 below superseded that topology. +Current planned parents are 105→dev, 110/120/130→105, 140→130, 150→110. +Thus S04's maximum dependent depth is 3, and the five-layer cap still applies. + +## COMPANION-EDIT-01 — allowed edits outside the split file + +- S09 L2/L3: `src/server/management/route-registry.ts` module-path metadata + for routes whose handler moves to a leaf — allowed (it is the route table's + pointer to the owning file; the registry test enumerates siblings). +- S02 L3: one `import type` path change for FastWire types — allowed + (type-only, no runtime effect). +- Consumer import edits are only allowed when the doc lists them; default is + that consumers keep importing from the original path via re-export. + +## GUI-SEAM-01 — React component extraction as the seam + +S17 (Storage policy panel) and other gui layers: extracting a JSX block into a +sibling component file with its props passed through verbatim counts as a +pure move for this train when the rendered tree is unchanged. Verification for +such layers adds the GUI checks: `bun run lint:gui`, `bun run build:gui`, and +a before/after screenshot of the affected page attached to the PR (the +`enforce-target` gate requires a screenshot for gui PRs anyway). + +## STACK-INDEPENDENCE-01 — stacks whose layers do not depend on each other + +DEV-STACK-01 says independent parts go as parallel PRs off trunk. The +original 002 chained every stack by directory. Decision, applied **per +layer** to every stack: a layer's base is the nearest lower layer in its +stack that it imports from (001's 47 edges) or that is a `#`-part of the +same file; S04 layers additionally base on the 105 type-contract layer; +otherwise the base is `dev`. 002 is regenerated with this rule (29 chained +layers, 48 `dev`-based). The stack id still groups execution order and PR +stack-map navigation; a `dev`-based layer's PR body still shows its stack's +map but states "base: dev — no dependency on the layers below". Each decade +doc's PR section is the authority for its own base and must match 002. + +## S06-ORACLE-01 — correcting 002 + +002's S06 thesis said "47 text oracles retargeted". The drafter showed the +count came from a broad `index.ts` basename match; no test reads +`src/vision/index.ts` as text. 002 is corrected to "no text oracle; three +recursive source-walk guards must include the new leaves". + +## S10-SIZE-01 — resolved by PURE-MOVE-SIZE-01 + +prompt-layers stays two layers (518 + 913 moved lines) under the pure-move +measure. + +## WORKTREE-EVIDENCE-01 — real implementation and receipt identity + +Each active decade document owns its branch and pinned base. The verifier +derives the tested layer head from the clean current branch and matches the +fetched remote branch; a base commit is never substituted for that head. +Scoped CI reruns and repair work are authorized; no local suite or merge is requested. + +Closed WP400 example: branch `codex/split-clients-config-export-a`, PR #3611, +base dev at `be81013fab6d83ff630ca5f38e7881678a303871`, final verified head +`bbf8d3cd25ccf70eb595bc7982f63528d060c1bd`. #3610 had already landed as +`5ab8aa9a2d9d2a3926469f9d8c82387b43c6d0e9`; it is not an open prerequisite. + +Historical only: WP400 temporarily used #3610 at +`afdd38ff43c64696153372fc2e27a38aff208c73` to separate a verification fix +from the split. That older basis and its open-parent workflow are retired. +The historical evidence remains in400; do not execute it as the current plan. + +The original dedicated-worktree execution choice conflicts with the FSM's +checkout-local source identity. Operational audit by Wegener found no +separate supported execution-root binding: `--cwd` selects both state and +source. The main agent amends its own topology choice, not the user's scope. + +From WP400, preserve the docs branch and every completed layer branch, then +create the current layer branch in the same a2c0 directory from its pinned +base. Carry 000, 003 and the current decade doc as tracked layer documentation; +the complete roadmap remains on `codex/260905-modular-debt-ledger-docs` and +can be read by immutable commit/ref. The ignored `.codexclaw` state stays +in a2c0; do not copy, hand-edit or relocate session state. Actual source edits +must occur there during B. Commit the layer before C and preserve that HEAD +through its receipt and C→D. Source changes from another checkout cannot be +represented by a documentation-only delta. + +All tests from WP400 run remotely. Each run uses its own mktemp checkout, +fetches the layer branch, and requires the fetched SHA to equal a2c0 HEAD. +Never switch or reset the shared remote seed checkout. Install root and GUI +dependencies with frozen lockfiles, then typecheck, focused checks, privacy +scan and full suite. Preserve full output and propagate each actual exit +code, including SSH transport failures. Failed or incomplete gates keep the +layer unverified; do not synthesize a passing receipt. Retain temporary +checkouts/evidence until scoped cleanup is authorized. + +Use the active decade document's complete isolated Bash recipe from C. +WP400 supplies the verified pattern; substitute the active layer's own branch, +test list and evidence names instead of reusing WP400's targets. +It checks the clean local layer HEAD, fetched remote HEAD and final remote +state, while preserving output and failures inside the receipt command. +No local Bun test command is allowed. Older shared-checkout recipes must not +be reused; each current plan must supply its isolated verifier. Availability +and success require real execution evidence. diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md new file mode 100644 index 0000000000..056a055de0 --- /dev/null +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -0,0 +1,125 @@ +# 445 — Temporary port-probe peer disposal + +## Loop spec and authority + +C3 bounded behavior-fix prerequisite, separate from pure-move WP450. User +authorization covers scoped verification repairs and stacked PR maintenance; +local suites remain prohibited. Work stays in the bound a2c0 checkout, on +`codex/fix-port-probe-peer-disposal`, base `dev` at +`a687eb735afc7307f902816972c2f8fb522ed2f3`. Main owns Git/FSM/remote checks; +gpt-6-astra high workers have bounded file ownership. No merges, deployment, +live proxy changes, dependency installation outside isolated remote checkouts, +credential changes, or unrelated cleanup work. Time/tokens are user-unbounded; +individual subprocesses and probes remain bounded. + +Stop only when this repair has a reviewed PR, exact-head remote gates and CI +evidence. Then D returns to the suspended WP450 for its own re-plan/restack and +fresh verification. Its existing acceptance criteria are unchanged. Source +and receipt identity remain in the same checkout throughout each cycle. + +## Problem and evidence + +Both temporary TCP servers in `src/server/ports.ts` wait for `server.close()` +before resolving, but neither disposes connections accepted during the brief +bind probe. These listeners are not application servers and have no request +handler. An accepted peer can therefore hold selection open before startup +publishes runtime records. + +Remote experiment on Bun1.4.0, unchanged actual CI merge tree: a concurrent +TCP peer held `isPortAvailable()` for2s; closing only that peer released the +promise. A second experiment used aborting HTTP readiness requests: after5s +all fetches had settled, yet the probe remained pending another2s. The +experiments establish the socket-lifetime defect. They do not alone prove +that every historical CI recovery failure has the same cause. + +Competing hypotheses: H1 pre-bind wait; H2 early runtime failure with retained +handles; H3 probe/environment mismatch. The controlled peer-close toggle +rules out bind contention/permissions and runtime startup code for this +specific reproduction. The HTTP variant demonstrates that client abort is +not sufficient disposal. The prior CI instance has no live stack, so its +precise attribution remains unconfirmed until further evidence. + +Unchanged head and CI merge-tree singleton/batch controls passed. Even the +complete original CI shard4/4 passed remotely with Bun1.4.0, isolate mode, +GUI built, and two-core affinity. This does not erase the failed hosted job. + +## Search, ownership, and rejected alternatives + +Main read the complete163-line ports module, its existing tests, reclaim +caller, and startup selection path. `isPortAvailable` is the primitive used +by availability/reclaim; `allocateEphemeralPort` repeats the same temporary +server lifetime for port0. Existing `setEphemeralPortAllocatorForTests` +bypasses the affected implementation and is unsuitable for regression proof. + +Keep this resource lifecycle in its existing owner; no new module, dependency, +export, global setter, timer, socket registry, or cycle. Do not change retry +deadlines, bind-error interpretation, ephemeral fallback policy, or reserved +port handling. Premature resolve, `unref`, `end`, and a timeout race leave the +resource problem intact and are rejected. Fix both same-owner instances, not +the downstream recovery assertion. Recovery fixture cleanup is separate debt. + +## Exact implementation scope + +- MODIFY `src/server/ports.ts`: add a small private temporary-server factory + (under15lines) whose connection listener is installed before listen. It + registers a narrow socket-error disposal handler and immediately destroys + each accepted socket. Use it at the two existing `createServer()` sites. + Keep success inside the real server-close callback and preserve all existing + signatures, bind-error handlers, and caller behavior. +- MODIFY `tests/server/ports.test.ts`: retain all original assertions. Add + deterministic subprocess-isolated regression coverage of the real + `isPortAvailable(port)` and `findAvailablePort(0)` implementations. Inside + each disposable test subprocess, a `node:net` server-factory double delivers + two accepted peers and only completes close after both are destroyed. Check + socket error disposal, close-completion-before-result, and the independently + specified selected port. Never mock `node:net` in the parent test process. + Resolve source through `tests/helpers/repo-root.ts`; no new test file. +- MODIFY `structure/01_runtime.md`: add one ownership row describing temporary + port-probe socket disposal; no authentication or server-composition changes. +- This plan and carried000/003 are the only other tracked changes. + +Expected source change under25lines; test amendment under120lines; each file +remains below400lines. Main owns docs/SoT; worker owns only source/test files. +Any additional behavior or broader cleanup requires a separate P amendment. + +## Audit and verification + +Independent A review must verify both affected call sites, safe disposal before +listen, no premature close success, regression isolation, unchanged exports, +and exact scoped writes. An operational review checks the scheduling amendment: +new445before450,450suspendedpending with all evidence/criteria preserved. + +Before code change, run the new regression remotely against unchanged source +and require failure in both paths. After correction require green. Revert only +disposal in a disposable remote clone and require the regression to fail again; +restore before final checks. Run the real socket and aborted-fetch experiments +against the corrected source; they must terminate without client cooperation. +Local inspection may use diff/AST/bash syntax only, never local tests/typecheck. + +Final remote recipe follows003 and the already-reviewed WP450 recipe, with +this branch and explicit package Bun1.4.0 on PATH. From clean published head, +`cxc receipt test` must wrap local head/clean checks before and after SSH. +SSH creates a new `mktemp -d` clone; fetch/match the exact branch SHA; frozen +root+GUI install; build GUI; run typecheck; run focused +`tests/server/ports.test.ts`, `tests/server/port-reclaim.test.ts`, +`tests/update/update-stop-first.test.ts`, and +`tests/lab/core-lab-boundary.test.ts`; privacy; full `bun run test`; and final +HEAD/clean checks. Propagate all exits and preserve complete output. The +executable recipe is written and syntax-reviewed before A closes. + +Acceptance: deterministic red/green/revert-red; real peer experiments settle; +all named focused checks, typecheck/privacy/full suite exit0; unchanged public +API and ownership boundary; independent C review; exact-head CI green; PR open +with every template section and actual evidence. Never count this support +repair as resolving another modularization ledger row. + +## Stack map + +| Layer | Branch | Base | Scope | +|---|---|---|---| +| WP450 / PR3633 | codex/split-cli-status | this repair after rebase | original pure-move status extraction only | +| WP445 / PR pending | codex/fix-port-probe-peer-disposal | dev | temporary probe peer disposal only | + +Keep parent open until its child is retargeted appropriately; no merge is +authorized. WP450 needs fresh head-bound evidence after restacking. The old +4a71894 receipt remains historical proof, not the new head's acceptance. From 3f152f8369445ea848fb701a16d9f01b7683144e Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 15:45:55 +0900 Subject: [PATCH 2/9] docs: add sequential readiness-probe reproduction evidence --- .../445_server_port_probe_disposal.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index 056a055de0..ea45112751 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -39,6 +39,12 @@ specific reproduction. The HTTP variant demonstrates that client abort is not sufficient disposal. The prior CI instance has no live stack, so its precise attribution remains unconfirmed until further evidence. +A third control matched the recovery test's sequential polling: each HTTP +attempt aborts after2s, then waits100ms before the next attempt. It also held +the real probe at iteration0 beyond5s, with no active fetch, and remained held +for another2s after polling stopped. This removes aggressive overlapping +polling as a prerequisite for the reproduced defect. + Unchanged head and CI merge-tree singleton/batch controls passed. Even the complete original CI shard4/4 passed remotely with Bun1.4.0, isolate mode, GUI built, and two-core affinity. This does not erase the failed hosted job. From ee7be1a5c4b3c12c4af07c6e8480facb681ce20d Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 15:47:29 +0900 Subject: [PATCH 3/9] docs: close port-probe audit and record serial CI coordination --- .../445_server_port_probe_disposal.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index ea45112751..bf8cceef2e 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -129,3 +129,21 @@ repair as resolving another modularization ledger row. Keep parent open until its child is retargeted appropriately; no merge is authorized. WP450 needs fresh head-bound evidence after restacking. The old 4a71894 receipt remains historical proof, not the new head's acceptance. + +## A closure and CI scheduling + +Hooke passed the two-site disposal design, nine original exports, and isolated +regression plan. Both reviewers found a verifier error-propagation issue: +inline `test -z` around Git-status substitution could conceal Git failure. +All five sites now assign status in a standalone command before checking +emptiness. Hooke and Wegener independently closed that blocker with PASS; +the script passes Bash syntax checking. No runtime result is inferred. + +The user's latest instruction requires cross-task CI coordination: leave the +Windows task alone, message other owners, and schedule non-Windows CI one at +a time. Main has contacted the provider, registration, image, and Reserve +owners and will hold this repair's push/full verification until its slot. +Implementation and static review may proceed while that queue drains. Never +cancel another task's run without confirming ownership and communicating the +chosen order. Existing successful job evidence must be preserved where the +CI platform supports rerunning only failed/cancelled jobs. From 0ea491ea7a202f27bf2041f916bd902056ecc225 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 16:04:16 +0900 Subject: [PATCH 4/9] test: stage isolated port-probe peer lifecycle regressions --- .../445_server_port_probe_disposal.md | 26 ++++- tests/server/ports.test.ts | 98 +++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index bf8cceef2e..3eb65e6bf6 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -75,10 +75,12 @@ the downstream recovery assertion. Recovery fixture cleanup is separate debt. - MODIFY `tests/server/ports.test.ts`: retain all original assertions. Add deterministic subprocess-isolated regression coverage of the real `isPortAvailable(port)` and `findAvailablePort(0)` implementations. Inside - each disposable test subprocess, a `node:net` server-factory double delivers - two accepted peers and only completes close after both are destroyed. Check + each disposable test subprocess, a `node:net` Server-prototype method double + delivers two accepted peers and only completes close after both are destroyed. Check socket error disposal, close-completion-before-result, and the independently - specified selected port. Never mock `node:net` in the parent test process. + specified selected port. Override listen/close/address only in the isolated + child, retaining the real createServer constructor and connection-listener + registration. Never replace network methods in the parent test process. Resolve source through `tests/helpers/repo-root.ts`; no new test file. - MODIFY `structure/01_runtime.md`: add one ownership row describing temporary port-probe socket disposal; no authentication or server-composition changes. @@ -147,3 +149,21 @@ Implementation and static review may proceed while that queue drains. Never cancel another task's run without confirming ownership and communicating the chosen order. Existing successful job evidence must be preserved where the CI platform supports rerunning only failed/cancelled jobs. + +## B regression-harness correction + +The first two-case remote run completed in97ms and failed both cases, but +those failures were not accepted as RED evidence: Bun1.4.0 did not route the +native named createServer import through the child `mock.module` replacement. +The double reported zero factory calls and the real probe completed before +the controlled close flag. This tests the broken double, not peer disposal. +Product source remains unchanged. + +The test-only repair uses child-local Server prototype method overrides so +the real constructor retains connection-listener registration while the +double controls listen events, address and close completion. This changes +only instrumentation, not the intended behavioral assertions or public API. +Remote RED must be repeated at an allocated CI handoff before source changes. +The wrapper also used unavailable remote `rg` after the run; its final marker +check now uses grep. Neither the wrapper exit127 nor the two wrong-reason +failures count as a valid regression result. diff --git a/tests/server/ports.test.ts b/tests/server/ports.test.ts index 6d573ad752..bde29b215a 100644 --- a/tests/server/ports.test.ts +++ b/tests/server/ports.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, expect, test } from "bun:test"; import { createServer, type Server } from "node:net"; +import { pathToFileURL } from "node:url"; import { findAvailablePort, isAddrInUse, isPortAvailable, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../../src/server/ports"; +import { repoPath, repoRoot } from "../helpers/repo-root"; const servers: Server[] = []; @@ -30,6 +32,102 @@ afterEach(async () => { }); describe("port selection", () => { + test.each(["isPortAvailable", "findAvailablePort"] as const)( + "%s disposes accepted peers and waits for probe close completion", + (operation) => { + // Keep Server.prototype overrides out of this process and its real-socket tests. + const portsUrl = pathToFileURL(repoPath("src", "server", "ports.ts")).href; + const childSource = ` + import assert from "node:assert/strict"; + import { EventEmitter } from "node:events"; + import { Server } from "node:net"; + + const operation = ${JSON.stringify(operation)}; + const peers = Array.from({ length: 2 }, () => { + const peer = new EventEmitter(); + peer.destroyed = false; + peer.destroyCalls = 0; + peer.destroy = () => { + peer.destroyCalls++; + peer.destroyed = true; + return peer; + }; + return peer; + }); + let bindOptions; + let completeClose; + let closeCompleted = false; + let probeCalls = 0; + // Native createServer stays real, including its connection-listener registration. + Server.prototype.address = function () { + return { address: "127.0.0.1", family: "IPv4", port: 43219 }; + }; + Server.prototype.close = function (callback) { + completeClose = () => { + if (peers.some(peer => !peer.destroyed)) return false; + closeCompleted = true; + callback(); + return true; + }; + return this; + }; + Server.prototype.listen = function (options) { + probeCalls++; + bindOptions = options; + for (const peer of peers) this.emit("connection", peer); + this.emit("listening"); + return this; + }; + + const ports = await import(${JSON.stringify(portsUrl)}); + let settled = false; + let rejection; + const pending = (operation === "isPortAvailable" + ? ports.isPortAvailable(43117, "127.0.0.1") + : ports.findAvailablePort(0, "127.0.0.1")).then(value => { + settled = true; + return value; + }, error => { + settled = true; + rejection = error; + }); + // One event-loop turn drains promise reactions without time-based polling. + await new Promise(resolve => setImmediate(resolve)); + assert.equal(probeCalls, 1, "must intercept the real temporary Server instance"); + assert.deepEqual(bindOptions, { + port: operation === "isPortAvailable" ? 43117 : 0, host: "127.0.0.1", + }); + assert.equal(rejection, undefined, "probe must not reject before disposal assertions"); + assert.equal(typeof completeClose, "function", "server.close callback must be registered"); + assert.deepEqual(peers.map(peer => peer.destroyed), [true, true], + "probe must destroy both accepted peers"); + for (const peer of peers) { + const beforeError = peer.destroyCalls; + assert.doesNotThrow(() => peer.emit("error", new Error("peer reset"))); + assert.ok(peer.destroyCalls > beforeError, "socket errors must dispose the peer"); + } + await new Promise(resolve => setImmediate(resolve)); + assert.equal(settled, false, "destroying peers must not resolve before close callback"); + assert.equal(closeCompleted, false); + assert.equal(completeClose(), true); + const value = await pending; + console.log(JSON.stringify({ value, closeCompleted })); + `; + const child = Bun.spawnSync([process.execPath, "--eval", childSource], { + cwd: repoRoot(), + stdout: "pipe", + stderr: "pipe", + timeout: 5000, + }); + expect(child.exitCode, child.stderr.toString()).toBe(0); + expect(JSON.parse(child.stdout.toString())).toEqual({ + value: operation === "isPortAvailable" ? true : 43219, + closeCompleted: true, + }); + }, + 10000, + ); + test("resolves port 0 to a concrete ephemeral port", async () => { const selected = await findAvailablePort(0); From 0d9b6c2b20bf90b6da506fc6f6935845e31d6990 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 16:12:32 +0900 Subject: [PATCH 5/9] fix(server): dispose accepted peers in temporary port probes --- .../445_server_port_probe_disposal.md | 24 +++ src/server/ports.ts | 12 +- structure/01_runtime.md | 1 + tests/server/ports.test.ts | 156 +++++++++--------- 4 files changed, 114 insertions(+), 79 deletions(-) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index 3eb65e6bf6..7f97d7dd92 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -167,3 +167,27 @@ Remote RED must be repeated at an allocated CI handoff before source changes. The wrapper also used unavailable remote `rg` after the run; its final marker check now uses grep. Neither the wrapper exit127 nor the two wrong-reason failures count as a valid regression result. + +The corrected fixture was then moved to a private module constant so the test +callback stays15lines rather than embedding a long script in a function. The +child reads only the two explicitly supplied argv entries. Existing11cases +and all instrumentation/behavior assertions remain intact. + +RED2 is valid: explicit Bun1.4.0 ran both new cases in98ms; interception, +rejection absence and close registration passed, then both cases failed at +`probe must destroy both accepted peers`, actual `[false,false]` versus +`[true,true]`. Test exit1 and wrapper exit0 with the expected-RED marker are +recorded in `wp445-short-red2.log`. Original production source remained +unchanged. Stage2 is now authorized to implement only the planned two-site +disposal correction. Green verification still awaits an allocated slot. + +Stage2 implementation adds a six-line private `createProbeServer` and replaces +the two existing factory calls: source +10/−2, now171lines. It attaches the +socket-error disposal handler before immediate destroy and leaves success in +the existing server-close callbacks. Worker static review preserves all nine +exports/imports and bind-error/timeout/fallback/reserved-port logic. Main +inspected the complete diff and whitespace checks pass. The existing test +file now235lines (+100/−0 versus base), with all11original tests preserved and +a15line regression callback. Main added the single Runtime ownership row. +No local runtime tests ran. C must still establish restored GREEN, real-socket +controls, full gates, current-head CI and independent review before completion. diff --git a/src/server/ports.ts b/src/server/ports.ts index 4c5a857803..57f86327c2 100644 --- a/src/server/ports.ts +++ b/src/server/ports.ts @@ -1,5 +1,13 @@ import { createServer } from "node:net"; +/** Temporary bind probes must not let accepted peers hold server.close() open. */ +function createProbeServer(): ReturnType { + return createServer(socket => { + socket.on("error", () => socket.destroy()); + socket.destroy(); + }); +} + /** * True when an error means "this port/address is already bound" — the only bind failure * that is safe to answer with a retry on another port. Bun/Node surface it as @@ -15,7 +23,7 @@ export function isAddrInUse(err: unknown): boolean { export async function isPortAvailable(port: number, hostname = "127.0.0.1"): Promise { return await new Promise(resolve => { - const server = createServer(); + const server = createProbeServer(); // Fail closed: EACCES / EADDRNOTAVAIL / EPERM / unknown listen errors mean the // requested bind is not available. Only the listening event reports free. server.once("error", () => resolve(false)); @@ -133,7 +141,7 @@ export function setEphemeralPortAllocatorForTests( async function allocateEphemeralPort(hostname: string): Promise { if (ephemeralAllocator) return ephemeralAllocator(hostname); return await new Promise((resolve, reject) => { - const server = createServer(); + const server = createProbeServer(); server.once("error", reject); server.once("listening", () => { const address = server.address(); diff --git a/structure/01_runtime.md b/structure/01_runtime.md index 7a5139cfad..47e2b19ca5 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -13,6 +13,7 @@ | `src/config/paths.ts` | Resolves `OPENCODEX_HOME`, `config.json`, and owner-only directory hardening. | | `src/config/atomic-write.ts` | Shared synchronous/asynchronous temp-harden-rename writer and residual-temp failure contract. | | `src/config/process-state.ts` | Owns `ocx.pid`, `runtime-port.json`, cheap liveness, full command-line identity verification, and snapshot-guarded cleanup. | +| `src/server/ports.ts` | Owns bind availability and ephemeral-port selection. Temporary probes dispose accepted peers and wait for listener close before reporting success. | | `src/router.ts` | Provider/model selection before adapter dispatch. | | `src/types.ts` | Shared config, parsed request, adapter, and event types. | | `src/reasoning-effort.ts` | Codex reasoning-level definitions (`low`/`medium`/`high`/`xhigh`), per-model effort mapping, and catalog effort sanitization. | diff --git a/tests/server/ports.test.ts b/tests/server/ports.test.ts index bde29b215a..6471244c56 100644 --- a/tests/server/ports.test.ts +++ b/tests/server/ports.test.ts @@ -4,6 +4,84 @@ import { pathToFileURL } from "node:url"; import { findAvailablePort, isAddrInUse, isPortAvailable, PortUnavailableError, shouldPersistSelectedPort, waitForPortAvailable } from "../../src/server/ports"; import { repoPath, repoRoot } from "../helpers/repo-root"; +// Prototype overrides exist only inside the disposable child process. +const PORT_PROBE_PEER_DISPOSAL_CHILD = ` + import assert from "node:assert/strict"; + import { EventEmitter } from "node:events"; + import { Server } from "node:net"; + + const [operation, portsUrl] = process.argv.slice(-2); + const peers = Array.from({ length: 2 }, () => { + const peer = new EventEmitter(); + peer.destroyed = false; + peer.destroyCalls = 0; + peer.destroy = () => { + peer.destroyCalls++; + peer.destroyed = true; + return peer; + }; + return peer; + }); + let bindOptions; + let completeClose; + let closeCompleted = false; + let probeCalls = 0; + // Native createServer stays real, including its connection-listener registration. + Server.prototype.address = function () { + return { address: "127.0.0.1", family: "IPv4", port: 43219 }; + }; + Server.prototype.close = function (callback) { + completeClose = () => { + if (peers.some(peer => !peer.destroyed)) return false; + closeCompleted = true; + callback(); + return true; + }; + return this; + }; + Server.prototype.listen = function (options) { + probeCalls++; + bindOptions = options; + for (const peer of peers) this.emit("connection", peer); + this.emit("listening"); + return this; + }; + + const ports = await import(portsUrl); + let settled = false; + let rejection; + const pending = (operation === "isPortAvailable" + ? ports.isPortAvailable(43117, "127.0.0.1") + : ports.findAvailablePort(0, "127.0.0.1")).then(value => { + settled = true; + return value; + }, error => { + settled = true; + rejection = error; + }); + // One event-loop turn drains promise reactions without time-based polling. + await new Promise(resolve => setImmediate(resolve)); + assert.equal(probeCalls, 1, "must intercept the real temporary Server instance"); + assert.deepEqual(bindOptions, { + port: operation === "isPortAvailable" ? 43117 : 0, host: "127.0.0.1", + }); + assert.equal(rejection, undefined, "probe must not reject before disposal assertions"); + assert.equal(typeof completeClose, "function", "server.close callback must be registered"); + assert.deepEqual(peers.map(peer => peer.destroyed), [true, true], + "probe must destroy both accepted peers"); + for (const peer of peers) { + const beforeError = peer.destroyCalls; + assert.doesNotThrow(() => peer.emit("error", new Error("peer reset"))); + assert.ok(peer.destroyCalls > beforeError, "socket errors must dispose the peer"); + } + await new Promise(resolve => setImmediate(resolve)); + assert.equal(settled, false, "destroying peers must not resolve before close callback"); + assert.equal(closeCompleted, false); + assert.equal(completeClose(), true); + const value = await pending; + console.log(JSON.stringify({ value, closeCompleted })); +`; + const servers: Server[] = []; function close(server: Server): Promise { @@ -37,83 +115,7 @@ describe("port selection", () => { (operation) => { // Keep Server.prototype overrides out of this process and its real-socket tests. const portsUrl = pathToFileURL(repoPath("src", "server", "ports.ts")).href; - const childSource = ` - import assert from "node:assert/strict"; - import { EventEmitter } from "node:events"; - import { Server } from "node:net"; - - const operation = ${JSON.stringify(operation)}; - const peers = Array.from({ length: 2 }, () => { - const peer = new EventEmitter(); - peer.destroyed = false; - peer.destroyCalls = 0; - peer.destroy = () => { - peer.destroyCalls++; - peer.destroyed = true; - return peer; - }; - return peer; - }); - let bindOptions; - let completeClose; - let closeCompleted = false; - let probeCalls = 0; - // Native createServer stays real, including its connection-listener registration. - Server.prototype.address = function () { - return { address: "127.0.0.1", family: "IPv4", port: 43219 }; - }; - Server.prototype.close = function (callback) { - completeClose = () => { - if (peers.some(peer => !peer.destroyed)) return false; - closeCompleted = true; - callback(); - return true; - }; - return this; - }; - Server.prototype.listen = function (options) { - probeCalls++; - bindOptions = options; - for (const peer of peers) this.emit("connection", peer); - this.emit("listening"); - return this; - }; - - const ports = await import(${JSON.stringify(portsUrl)}); - let settled = false; - let rejection; - const pending = (operation === "isPortAvailable" - ? ports.isPortAvailable(43117, "127.0.0.1") - : ports.findAvailablePort(0, "127.0.0.1")).then(value => { - settled = true; - return value; - }, error => { - settled = true; - rejection = error; - }); - // One event-loop turn drains promise reactions without time-based polling. - await new Promise(resolve => setImmediate(resolve)); - assert.equal(probeCalls, 1, "must intercept the real temporary Server instance"); - assert.deepEqual(bindOptions, { - port: operation === "isPortAvailable" ? 43117 : 0, host: "127.0.0.1", - }); - assert.equal(rejection, undefined, "probe must not reject before disposal assertions"); - assert.equal(typeof completeClose, "function", "server.close callback must be registered"); - assert.deepEqual(peers.map(peer => peer.destroyed), [true, true], - "probe must destroy both accepted peers"); - for (const peer of peers) { - const beforeError = peer.destroyCalls; - assert.doesNotThrow(() => peer.emit("error", new Error("peer reset"))); - assert.ok(peer.destroyCalls > beforeError, "socket errors must dispose the peer"); - } - await new Promise(resolve => setImmediate(resolve)); - assert.equal(settled, false, "destroying peers must not resolve before close callback"); - assert.equal(closeCompleted, false); - assert.equal(completeClose(), true); - const value = await pending; - console.log(JSON.stringify({ value, closeCompleted })); - `; - const child = Bun.spawnSync([process.execPath, "--eval", childSource], { + const child = Bun.spawnSync([process.execPath, "--eval", PORT_PROBE_PEER_DISPOSAL_CHILD, "--", operation, portsUrl], { cwd: repoRoot(), stdout: "pipe", stderr: "pipe", From 30238bb0f7af5d9da65a072d48caaf9cf1fd063f Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 16:26:07 +0900 Subject: [PATCH 6/9] docs: replan explicit probe listener registration from Bun source proof --- .../445_server_port_probe_disposal.md | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index 7f97d7dd92..3e7e5468e9 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -68,7 +68,10 @@ the downstream recovery assertion. Recovery fixture cleanup is separate debt. - MODIFY `src/server/ports.ts`: add a small private temporary-server factory (under15lines) whose connection listener is installed before listen. It - registers a narrow socket-error disposal handler and immediately destroys + explicitly creates the server, registers a public `server.on("connection")` + handler, and returns that server. Do not use a constructor callback: the + pinned Bun implementation defers that callback's registration until native + accept. The handler registers narrow socket-error disposal and immediately destroys each accepted socket. Use it at the two existing `createServer()` sites. Keep success inside the real server-close callback and preserve all existing signatures, bind-error handlers, and caller behavior. @@ -79,8 +82,9 @@ the downstream recovery assertion. Recovery fixture cleanup is separate debt. delivers two accepted peers and only completes close after both are destroyed. Check socket error disposal, close-completion-before-result, and the independently specified selected port. Override listen/close/address only in the isolated - child, retaining the real createServer constructor and connection-listener - registration. Never replace network methods in the parent test process. + child, retaining the real createServer constructor and public EventEmitter + registration. Never replace network methods in the parent test process or + reach into Bun's private callback-storage symbols. Resolve source through `tests/helpers/repo-root.ts`; no new test file. - MODIFY `structure/01_runtime.md`: add one ownership row describing temporary port-probe socket disposal; no authentication or server-composition changes. @@ -191,3 +195,31 @@ file now235lines (+100/−0 versus base), with all11original tests preserved and a15line regression callback. Main added the single Runtime ownership row. No local runtime tests ran. C must still establish restored GREEN, real-socket controls, full gates, current-head CI and independent review before completion. + +## P re-plan after the first GREEN attempt + +The first GREEN attempt applied the correct source/test blobs but reported +11original passes and2new failures at peer disposal. The wrapper stopped +before any real-peer control. This is not a successful check and does not +establish a production regression. Main returned C→P before further repair. + +Pinned primary-source proof: [Bun net implementation at34cbb9a40](https://github.com/oven-sh/bun/blob/34cbb9a40/src/js/node/net.ts). +The constructor stores its callback in server options (lines3364–3365); native +accept prepends it immediately before emitting the connection event +(lines4021–4025, also1181–1189). A direct synthetic emit therefore bypassed +that deferred registration. Static assumptions about Node-style constructor +registration were wrong for this Bun version. + +Rejected alternatives: weakening the disposal assertions, reaching into a +private Bun symbol, or changing to a Node-only test would conceal the timing +contract. Explicit public `server.on("connection", handler)` registration +before listen makes the intended lifecycle real and observable without +runtime-private knowledge. Both production call sites and every test assertion +remain unchanged. Expected helper8lines/source173lines; this is a two-line +refinement of the private factory, not a wider behavior change. + +Re-audit this explicit-listener plan before B. Then repeat the two-case RED +control against baseline, verify GREEN plus all three real-peer experiments, +and toggle disposal off/on again. The focused wrapper must emit its captured +failure tail before exiting, so an early test error cannot hide the evidence. +Do not rerun during another owner's CI slot; #3636 currently owns it. From f47a8e39885a6c79ffdb7b50fb4594aae199a2da Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 16:29:07 +0900 Subject: [PATCH 7/9] fix(server): register probe connection disposal before listening --- .../445_server_port_probe_disposal.md | 8 ++++++++ src/server/ports.ts | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index 3e7e5468e9..b46a2f12e3 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -223,3 +223,11 @@ control against baseline, verify GREEN plus all three real-peer experiments, and toggle disposal off/on again. The focused wrapper must emit its captured failure tail before exiting, so an early test error cannot hide the evidence. Do not rerun during another owner's CI slot; #3636 currently owns it. + +Heisenberg independently approved the re-plan: explicit public registration +reaches both native acceptance and synthetic emission, with no private-symbol +dependency and no weakened assertion. B changed only the private factory +(+3/−1 versus the constructor-callback candidate). It now has8lines; source +has173lines and retains all9exports. Tests and docs were unchanged by the +worker. Main reviewed the diff. Renewed runtime evidence is still pending; +the prior static PASS does not substitute for this round's checks. diff --git a/src/server/ports.ts b/src/server/ports.ts index 57f86327c2..11fe76838a 100644 --- a/src/server/ports.ts +++ b/src/server/ports.ts @@ -2,10 +2,12 @@ import { createServer } from "node:net"; /** Temporary bind probes must not let accepted peers hold server.close() open. */ function createProbeServer(): ReturnType { - return createServer(socket => { + const server = createServer(); + server.on("connection", socket => { socket.on("error", () => socket.destroy()); socket.destroy(); }); + return server; } /** From e0eb1e031b530bbff03d27cae00e2c1dcf160811 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 16:49:55 +0900 Subject: [PATCH 8/9] docs: keep investigation records outside public devlog --- .../445_server_port_probe_disposal.md | 290 ++++-------------- 1 file changed, 57 insertions(+), 233 deletions(-) diff --git a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md index b46a2f12e3..b1b02ddd38 100644 --- a/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md +++ b/devlog/_plan/260905_now_split_train/445_server_port_probe_disposal.md @@ -1,233 +1,57 @@ -# 445 — Temporary port-probe peer disposal - -## Loop spec and authority - -C3 bounded behavior-fix prerequisite, separate from pure-move WP450. User -authorization covers scoped verification repairs and stacked PR maintenance; -local suites remain prohibited. Work stays in the bound a2c0 checkout, on -`codex/fix-port-probe-peer-disposal`, base `dev` at -`a687eb735afc7307f902816972c2f8fb522ed2f3`. Main owns Git/FSM/remote checks; -gpt-6-astra high workers have bounded file ownership. No merges, deployment, -live proxy changes, dependency installation outside isolated remote checkouts, -credential changes, or unrelated cleanup work. Time/tokens are user-unbounded; -individual subprocesses and probes remain bounded. - -Stop only when this repair has a reviewed PR, exact-head remote gates and CI -evidence. Then D returns to the suspended WP450 for its own re-plan/restack and -fresh verification. Its existing acceptance criteria are unchanged. Source -and receipt identity remain in the same checkout throughout each cycle. - -## Problem and evidence - -Both temporary TCP servers in `src/server/ports.ts` wait for `server.close()` -before resolving, but neither disposes connections accepted during the brief -bind probe. These listeners are not application servers and have no request -handler. An accepted peer can therefore hold selection open before startup -publishes runtime records. - -Remote experiment on Bun1.4.0, unchanged actual CI merge tree: a concurrent -TCP peer held `isPortAvailable()` for2s; closing only that peer released the -promise. A second experiment used aborting HTTP readiness requests: after5s -all fetches had settled, yet the probe remained pending another2s. The -experiments establish the socket-lifetime defect. They do not alone prove -that every historical CI recovery failure has the same cause. - -Competing hypotheses: H1 pre-bind wait; H2 early runtime failure with retained -handles; H3 probe/environment mismatch. The controlled peer-close toggle -rules out bind contention/permissions and runtime startup code for this -specific reproduction. The HTTP variant demonstrates that client abort is -not sufficient disposal. The prior CI instance has no live stack, so its -precise attribution remains unconfirmed until further evidence. - -A third control matched the recovery test's sequential polling: each HTTP -attempt aborts after2s, then waits100ms before the next attempt. It also held -the real probe at iteration0 beyond5s, with no active fetch, and remained held -for another2s after polling stopped. This removes aggressive overlapping -polling as a prerequisite for the reproduced defect. - -Unchanged head and CI merge-tree singleton/batch controls passed. Even the -complete original CI shard4/4 passed remotely with Bun1.4.0, isolate mode, -GUI built, and two-core affinity. This does not erase the failed hosted job. - -## Search, ownership, and rejected alternatives - -Main read the complete163-line ports module, its existing tests, reclaim -caller, and startup selection path. `isPortAvailable` is the primitive used -by availability/reclaim; `allocateEphemeralPort` repeats the same temporary -server lifetime for port0. Existing `setEphemeralPortAllocatorForTests` -bypasses the affected implementation and is unsuitable for regression proof. - -Keep this resource lifecycle in its existing owner; no new module, dependency, -export, global setter, timer, socket registry, or cycle. Do not change retry -deadlines, bind-error interpretation, ephemeral fallback policy, or reserved -port handling. Premature resolve, `unref`, `end`, and a timeout race leave the -resource problem intact and are rejected. Fix both same-owner instances, not -the downstream recovery assertion. Recovery fixture cleanup is separate debt. - -## Exact implementation scope - -- MODIFY `src/server/ports.ts`: add a small private temporary-server factory - (under15lines) whose connection listener is installed before listen. It - explicitly creates the server, registers a public `server.on("connection")` - handler, and returns that server. Do not use a constructor callback: the - pinned Bun implementation defers that callback's registration until native - accept. The handler registers narrow socket-error disposal and immediately destroys - each accepted socket. Use it at the two existing `createServer()` sites. - Keep success inside the real server-close callback and preserve all existing - signatures, bind-error handlers, and caller behavior. -- MODIFY `tests/server/ports.test.ts`: retain all original assertions. Add - deterministic subprocess-isolated regression coverage of the real - `isPortAvailable(port)` and `findAvailablePort(0)` implementations. Inside - each disposable test subprocess, a `node:net` Server-prototype method double - delivers two accepted peers and only completes close after both are destroyed. Check - socket error disposal, close-completion-before-result, and the independently - specified selected port. Override listen/close/address only in the isolated - child, retaining the real createServer constructor and public EventEmitter - registration. Never replace network methods in the parent test process or - reach into Bun's private callback-storage symbols. - Resolve source through `tests/helpers/repo-root.ts`; no new test file. -- MODIFY `structure/01_runtime.md`: add one ownership row describing temporary - port-probe socket disposal; no authentication or server-composition changes. -- This plan and carried000/003 are the only other tracked changes. - -Expected source change under25lines; test amendment under120lines; each file -remains below400lines. Main owns docs/SoT; worker owns only source/test files. -Any additional behavior or broader cleanup requires a separate P amendment. - -## Audit and verification - -Independent A review must verify both affected call sites, safe disposal before -listen, no premature close success, regression isolation, unchanged exports, -and exact scoped writes. An operational review checks the scheduling amendment: -new445before450,450suspendedpending with all evidence/criteria preserved. - -Before code change, run the new regression remotely against unchanged source -and require failure in both paths. After correction require green. Revert only -disposal in a disposable remote clone and require the regression to fail again; -restore before final checks. Run the real socket and aborted-fetch experiments -against the corrected source; they must terminate without client cooperation. -Local inspection may use diff/AST/bash syntax only, never local tests/typecheck. - -Final remote recipe follows003 and the already-reviewed WP450 recipe, with -this branch and explicit package Bun1.4.0 on PATH. From clean published head, -`cxc receipt test` must wrap local head/clean checks before and after SSH. -SSH creates a new `mktemp -d` clone; fetch/match the exact branch SHA; frozen -root+GUI install; build GUI; run typecheck; run focused -`tests/server/ports.test.ts`, `tests/server/port-reclaim.test.ts`, -`tests/update/update-stop-first.test.ts`, and -`tests/lab/core-lab-boundary.test.ts`; privacy; full `bun run test`; and final -HEAD/clean checks. Propagate all exits and preserve complete output. The -executable recipe is written and syntax-reviewed before A closes. - -Acceptance: deterministic red/green/revert-red; real peer experiments settle; -all named focused checks, typecheck/privacy/full suite exit0; unchanged public -API and ownership boundary; independent C review; exact-head CI green; PR open -with every template section and actual evidence. Never count this support -repair as resolving another modularization ledger row. - -## Stack map - -| Layer | Branch | Base | Scope | -|---|---|---|---| -| WP450 / PR3633 | codex/split-cli-status | this repair after rebase | original pure-move status extraction only | -| WP445 / PR pending | codex/fix-port-probe-peer-disposal | dev | temporary probe peer disposal only | - -Keep parent open until its child is retargeted appropriately; no merge is -authorized. WP450 needs fresh head-bound evidence after restacking. The old -4a71894 receipt remains historical proof, not the new head's acceptance. - -## A closure and CI scheduling - -Hooke passed the two-site disposal design, nine original exports, and isolated -regression plan. Both reviewers found a verifier error-propagation issue: -inline `test -z` around Git-status substitution could conceal Git failure. -All five sites now assign status in a standalone command before checking -emptiness. Hooke and Wegener independently closed that blocker with PASS; -the script passes Bash syntax checking. No runtime result is inferred. - -The user's latest instruction requires cross-task CI coordination: leave the -Windows task alone, message other owners, and schedule non-Windows CI one at -a time. Main has contacted the provider, registration, image, and Reserve -owners and will hold this repair's push/full verification until its slot. -Implementation and static review may proceed while that queue drains. Never -cancel another task's run without confirming ownership and communicating the -chosen order. Existing successful job evidence must be preserved where the -CI platform supports rerunning only failed/cancelled jobs. - -## B regression-harness correction - -The first two-case remote run completed in97ms and failed both cases, but -those failures were not accepted as RED evidence: Bun1.4.0 did not route the -native named createServer import through the child `mock.module` replacement. -The double reported zero factory calls and the real probe completed before -the controlled close flag. This tests the broken double, not peer disposal. -Product source remains unchanged. - -The test-only repair uses child-local Server prototype method overrides so -the real constructor retains connection-listener registration while the -double controls listen events, address and close completion. This changes -only instrumentation, not the intended behavioral assertions or public API. -Remote RED must be repeated at an allocated CI handoff before source changes. -The wrapper also used unavailable remote `rg` after the run; its final marker -check now uses grep. Neither the wrapper exit127 nor the two wrong-reason -failures count as a valid regression result. - -The corrected fixture was then moved to a private module constant so the test -callback stays15lines rather than embedding a long script in a function. The -child reads only the two explicitly supplied argv entries. Existing11cases -and all instrumentation/behavior assertions remain intact. - -RED2 is valid: explicit Bun1.4.0 ran both new cases in98ms; interception, -rejection absence and close registration passed, then both cases failed at -`probe must destroy both accepted peers`, actual `[false,false]` versus -`[true,true]`. Test exit1 and wrapper exit0 with the expected-RED marker are -recorded in `wp445-short-red2.log`. Original production source remained -unchanged. Stage2 is now authorized to implement only the planned two-site -disposal correction. Green verification still awaits an allocated slot. - -Stage2 implementation adds a six-line private `createProbeServer` and replaces -the two existing factory calls: source +10/−2, now171lines. It attaches the -socket-error disposal handler before immediate destroy and leaves success in -the existing server-close callbacks. Worker static review preserves all nine -exports/imports and bind-error/timeout/fallback/reserved-port logic. Main -inspected the complete diff and whitespace checks pass. The existing test -file now235lines (+100/−0 versus base), with all11original tests preserved and -a15line regression callback. Main added the single Runtime ownership row. -No local runtime tests ran. C must still establish restored GREEN, real-socket -controls, full gates, current-head CI and independent review before completion. - -## P re-plan after the first GREEN attempt - -The first GREEN attempt applied the correct source/test blobs but reported -11original passes and2new failures at peer disposal. The wrapper stopped -before any real-peer control. This is not a successful check and does not -establish a production regression. Main returned C→P before further repair. - -Pinned primary-source proof: [Bun net implementation at34cbb9a40](https://github.com/oven-sh/bun/blob/34cbb9a40/src/js/node/net.ts). -The constructor stores its callback in server options (lines3364–3365); native -accept prepends it immediately before emitting the connection event -(lines4021–4025, also1181–1189). A direct synthetic emit therefore bypassed -that deferred registration. Static assumptions about Node-style constructor -registration were wrong for this Bun version. - -Rejected alternatives: weakening the disposal assertions, reaching into a -private Bun symbol, or changing to a Node-only test would conceal the timing -contract. Explicit public `server.on("connection", handler)` registration -before listen makes the intended lifecycle real and observable without -runtime-private knowledge. Both production call sites and every test assertion -remain unchanged. Expected helper8lines/source173lines; this is a two-line -refinement of the private factory, not a wider behavior change. - -Re-audit this explicit-listener plan before B. Then repeat the two-case RED -control against baseline, verify GREEN plus all three real-peer experiments, -and toggle disposal off/on again. The focused wrapper must emit its captured -failure tail before exiting, so an early test error cannot hide the evidence. -Do not rerun during another owner's CI slot; #3636 currently owns it. - -Heisenberg independently approved the re-plan: explicit public registration -reaches both native acceptance and synthetic emission, with no private-symbol -dependency and no weakened assertion. B changed only the private factory -(+3/−1 versus the constructor-callback candidate). It now has8lines; source -has173lines and retains all9exports. Tests and docs were unchanged by the -worker. Main reviewed the diff. Renewed runtime evidence is still pending; -the prior static PASS does not substitute for this round's checks. +# 445 — Runtime verification prerequisite + +## Scope and workflow + +C3 independent runtime-maintenance prerequisite for the modularization train. +PR #3640 uses branch `codex/fix-port-probe-peer-disposal`, base `dev`. +Its production and test diff is the review surface. Investigation, negative +controls, failure analysis and reproduction records remain in ignored scratch, +not public devlog. Publication of the final retrospective waits for release. + +Bound session: `01a06e97-b9d8-7250-8204-bb788338c288`; same a2c0 checkout +owns implementation, persisted PABCD and receipts. Main owns Git/PR/CI. +Delegation uses gpt-6-astra high with disjoint source/test ownership. +No merge, release, live-service change or repository-wide setting change. + +## Planned files and acceptance + +- `src/server/ports.ts`: bounded existing-owner maintenance; preserve public + exports, caller interfaces, error handling and selection policies. +- `tests/server/ports.test.ts`: scoped regression coverage; preserve the + original test cases and isolate test doubles from the parent process. +- `structure/01_runtime.md`: ownership row only. +- This public scope record and the carried000/003 workflow documents. + +Keep source/tests below400lines and added functions below50lines. Do not +weaken assertions, alter verification thresholds or mark a failed check passed. + +All runtime verification is remote. Use the reviewed source-bound receipt +recipe stored in ignored evidence: check clean expected HEAD before/after +SSH, create a fresh isolated clone, match fetched branch SHA, frozen dependency +setup, explicit package Bun1.4.0, build, typecheck, focused subsystem/boundary +tests, privacy, full suite, and final clean HEAD. Preserve full output and +actual exits. No local suites or typecheck; no shared-checkout reset. + +Independent review, exact-head remote gates and hosted CI must pass. A prior +head's results do not establish a later head. Detailed verification records +are kept with private receipt evidence; no completion is inferred from a plan. + +## Continuation and coordination + +This work does not close a modularization ledger row. D resumes suspended +WP450 for its own P/A, restack and fresh verification; do not count it done. +PR #3633 remains independent until that controlled restack is performed. + +The user requires conversational one-at-a-time non-Windows CI coordination. +Windows-owner work remains excluded. Changes that start CI, including pushes, +retargeting and landing, require the scheduled slot. Code/static review may +continue while waiting. Scope authority is already granted; a queue wait is +not a request for more user permission. + +## Review disposition + +A reviewer identified that the previous version mixed investigation records +with this public scope document. Those details were moved to ignored scratch +and removed from the current public document. Earlier published commits may +still be accessible; this change is not a history-purge claim. The actual +source/test review and all verification requirements remain unchanged. From d2b4a81c61294c3c9ae7a2d58a01397167b120d0 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 17:12:39 +0900 Subject: [PATCH 9/9] docs: include binding parent decisions in plan verification --- devlog/_plan/260905_now_split_train/000_plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlog/_plan/260905_now_split_train/000_plan.md b/devlog/_plan/260905_now_split_train/000_plan.md index adfc936873..352583db36 100644 --- a/devlog/_plan/260905_now_split_train/000_plan.md +++ b/devlog/_plan/260905_now_split_train/000_plan.md @@ -55,7 +55,7 @@ the closeout tallies both and only the first counts as resolved. | WP | Deliverable | Depends on | Verifier | |---|---|---|---| -| wp1 | 000–002 + every layer's decade doc (010…750) at diff level | — | docs checks (numbered only, every layer has a doc, every NOW file appears in exactly one stack); privacy scan | +| wp1 | 000–003, including binding parent decisions, + every layer's decade doc (010…750) at diff level | — | docs checks (numbered only, every layer has a doc, every NOW file appears in exactly one stack, 003 amendments agree with 000/002 and the layer plans); privacy scan | | wp2… | one layer per work-phase, dependency-ordered by the base edges in 002; independent groups may be interleaved | its declared base layer, if any | the current decade document's Verification and Accept criteria sections | Total: 77 implementation layers across 21 stacks (002_layer_map.md; 105 and