Skip to content

fix: release train 4 bug-hardening batch (adapter bounds, SSH Link on PowerShell, sibling-home client sync) #6113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidge-jun merged 4 commits into from
Sep 27, 2026
Merged

fix: release train 4 bug-hardening batch (adapter bounds, SSH Link on PowerShell, sibling-home client sync) #6113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10eca73
fix(adapters): bound coding-agent tool state and linearize ID reminting
lidge-jun Sep 27, 2026
9f50bdd
fix(link): invoke sh bare and surface non-UTF-8 ssh errors
lidge-jun Sep 27, 2026
f1f903b
fix(cli): keep a second-home proxy off a live proxy's client config
lidge-jun Sep 27, 2026
87afa86
docs(devlog): record release train 4 bug hardening lane
lidge-jun Sep 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions devlog/_plan/260927_release_train_4/bug-hardening/000_plan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Release train 4 — bug hardening lane

At the 2026-09-27 inventory, `origin/dev` was `24b2f39b77`. This lane will carry small, testable crash and resource bounds and make SSH Link failures diagnosable. It will keep policy-changing, unreachable or unsafe restart proposals open with concrete reasons. Every adopted change is rebased onto the then-current `dev`, reviewed, run through focused regressions and exact-head CI, and merged through a lane-owned PR.

## Loop specification

- Archetype and trigger: satisfy-spec release integration for the assigned PRs and issues, requested by the train coordinator.
- Goal and stop: record a decision for every assigned item; land only safe, validated changes; close solved issues and superseded PRs with links; check the resulting `dev` CI. Stop only when those actions and the evidence ledger in this directory are complete, or report a concrete external blocker.
- Scope: this worktree and `codex/t4-bug-hardening-*` branches; `devlog/_plan/260927_release_train_4/bug-hardening/` for plans; the exact source, tests, contracts and user docs in the phase files. GitHub writes are limited to this lane's branches and PRs, dispositions on assigned issues and candidate PRs, and permitted merges to `dev`.
- Non-goals: main/preview, releases, tags, deployment, version changes, contributor-fork pushes, picker CA/`src/claude/intercept`, account-pool changes, GUI PRs, provider-compatibility PRs, client-integration PRs, and other lanes' worktrees.
- Verifiers: focused commands in 010–040 directly name the changed test files; `bun run test:changed` traverses imports from changed TS files; `bun run typecheck` checks TS; `bun run privacy:scan` reads tracked devlog and source; `bun run structure:check` checks structure links and ownership. Run `test:changed`/full in a same-commit throwaway checkout under `/private/tmp/t4-bug-hardening-verify` because this source checkout under `~/.codex` triggers protected-cleanup failures; never bypass the guard. The PR's pull_request CI must complete every requested job at its exact head; after landing, dispatch and inspect Cross-platform CI on exact `dev`. The local full suite is omitted only under the seven-lane resource exception, with commands/results and CI coverage documented in each PR.
- Memory artifact: this numbered roadmap and phase documents, each batch PR Verification section, CI run links, and the final outcome in `050_disposition_and_ci.md`.
- Terminal outcomes: DONE means the adopted changes, dispositions, closures and dev CI are evidenced; NOOP means a candidate is already present with source proof; NEEDS_HUMAN means an owner contract or security decision is absent; BLOCKED means an external condition repeatedly prevents useful progress; UNSAFE means a candidate's regression/security cost exceeds its validated benefit. No time or token limit was set by the coordinator.
- Escalation: preserve an explicit maintainer objection or unsafe auth/credential contract instead of integrating it. No local test can substitute for an upstream WHAM window-completeness decision. GitHub writes and merge authority are the coordinator's explicit lane grant, bounded by current-head CI and `MAINTAINERS.md`.
- Tool, credential and write bounds: local Bun/Swift and `gh` with the signed-in maintainer account; no new credential acquisition. Source writes stay in this worktree, and sensitive unpublished triage stays in `.tmp/`. Wall-clock and token budgets are unbounded by request; CI and risk gates still stop unsafe integration.

## Candidate decisions

The disposition is a plan, not a merge claim. Current PR pages and source were inspected against `24b2f39b77`; every adoption is rechecked against the later integration head.

| Item | Decision | Grounded reason and next proof |
| --- | --- | --- |
| [#6081](https://github.com/lidge-jun/opencodex/pull/6081) | Carry current head `8a4399f` with a single-parser admission fix in B1 | `src/adapters/coding-agent/protocol.ts` retains unbounded argument fragments until close; the proposal charges the shared translator budget and releases it. The new head removed an inconsistent double raw-start counter, but the existing parser-owned cap still runs after block insertion. Move that one cap before insertion for nonempty IDs, preserving the existing error and empty-ID semantics. Check interleaving, index reuse, abort cleanup and Qoder's shared parser. |
| [#6083](https://github.com/lidge-jun/opencodex/pull/6083) | Reimplement in B1, with author credit | Current `src/adapters/openai-chat/tool-call-id-remint.ts:28-39` probes from 2 on each repeat. The proposed 62-character key restarts searches after the suffix widens at `-10`; add a cursor for the actual truncation domain and a cross-width regression. |
| [#6082](https://github.com/lidge-jun/opencodex/pull/6082) | Carry as-is in B2 | `app/Sources/NativeTray/Models.swift:160-171` can convert a finite oversized percentage to `Int` and trap. The two guarded conversions and test are small and independent of GUI PRs. |
| [#6085](https://github.com/lidge-jun/opencodex/pull/6085) | Hold, leave open; B3 disposition | The proposal's start retries and post-deadline recovery can race a late replacement. A narrowed carry also needs production phase evidence to separate pre-launch refusal, late health, and post-health integration failure; the current boolean collapses them. Preserve existing once-only/fail-closed behavior and comment with the reconsideration gate. |
| [#6076](https://github.com/lidge-jun/opencodex/pull/6076) | Hold, leave open | Pairing grants are hub-only (`src/server/gui-session.ts:263,319`), while Child join requires standalone. The proposed gate makes actual Child join unreachable; old local dashboards would also receive 403 without an upgrade path. A separately reviewed standalone operator credential and GUI denial reason are required. Keep unpublished security reasoning in scratch. |
| [#5964](https://github.com/lidge-jun/opencodex/pull/5964) | Hold, leave open | Its markup rule reverses the observed prose-prefixed text-only MiMo tool call in `tests/providers/command-code-tool-text-prose-split.test.ts:91-122`. An explicit compatibility/security decision is needed. |
| [#6030](https://github.com/lidge-jun/opencodex/pull/6030) | Hold, leave open | The 17-path draft conflicts with current `dev`; its launchd rewrite can change the plist without reloading the live job and must preserve package-local runtime and non-PATH environment. A narrowed service-owner reimplementation needs Linux/macOS lifecycle proof. |
| [#5831](https://github.com/lidge-jun/opencodex/pull/5831) | Hold, leave open | The two-window WHAM exception is plausible, but a current maintainer explicitly withheld approval until the provider/owner confirms that omitted tertiary means no governing short window. It also changes credential-generation publication and has no complete current-head CI. |
| [#5539](https://github.com/lidge-jun/opencodex/pull/5539) | Hold, leave open | The broad mapper change reverses `tests/responses/openai-responses-passthrough.test.ts:867` for unconfigured providers; no current-dev request reproduces the 400. Retain the intent for a provider-scoped repro, not the stale branch. |
| [#6088](https://github.com/lidge-jun/opencodex/issues/6088) | Implement in B4 | `src/link/ssh-argv.ts:180-185` quotes the command name as data, which PowerShell parses differently. `src/link/ssh-runner.ts:96-124` fatal-decodes stderr before a sanitized hint can describe the real error. Keep stdout strict and bound/redact diagnostic stderr. |
| [#4956](https://github.com/lidge-jun/opencodex/issues/4956) | Keep open, comment partial scope | #5014 and later isolated fixture/CI changes address concrete hangs, but do not prove the Bun child-process failure family solved. A silent cancelled CI leg is not green. |
| [#4761](https://github.com/lidge-jun/opencodex/issues/4761) | Keep open, comment scope decision | `src/codex/app-server-restart-service.ts:149` still invokes desktop-shell restart and warns about unsaved state. App-server-only restart does not currently refresh the picker; changing consent/UI belongs to the other lane. |

## Dependency-ordered work phases

The roadmap is this docs-only PABCD cycle. Production work starts after its audit and Check. Independent source slices are integrated serially so each next branch starts at current `origin/dev` and inherits any merged contract changes.

| Cycle | Decade doc | Deliverable |
| --- | --- | --- |
| B1 | [010_adapter_bounds.md](010_adapter_bounds.md) | CodeBuddy retained-argument cap and linear tool-call-ID reminting, one reviewable adapter hardening PR. |
| B2 | [020_native_quota.md](020_native_quota.md) | NativeTray percentage conversion guard, separate Swift PR. |
| B3 | [030_restart_transaction.md](030_restart_transaction.md) | Record the restart safety audit and comment on #6085; no source PR from this cycle. |
| B4 | [040_link_ssh.md](040_link_ssh.md) | SSH argv and diagnostic-error fix for #6088, separate Link PR. |
| B6 | 060_sibling_home_client_sync.md (written in its own P) | Coordinator-added bug: an instance started with an independent `OPENCODEX_HOME` while another proxy runs must not rewrite global client configs (`~/.grok/config.toml` and the same class of Codex/Claude sync), separate PR. |
| Closure | [050_disposition_and_ci.md](050_disposition_and_ci.md) | Candidate comments/closures, exact merged SHAs, and final `dev` CI. |

### Resumption amendment (session `01a0e37e`, 2026-09-28)

The previous session stopped after this roadmap and the B1 plan; no source had been edited and `origin/dev` was still `24b2f39b77` with unchanged candidate heads, so every disposition above still stands. The resumed goalplan runs three work-phases. wp1 builds B1, B2 and B4 in one PABCD cycle because their write sets are disjoint (`src/adapters/**` + CodeBuddy/remint tests; `app/Sources/NativeTray*`; `src/link/ssh-*` + Link tests), then publishes them as three separate lane PRs cut from `origin/dev`, so each keeps its own review surface, security note and exact-head CI. Each PR is rebased onto whatever `dev` is when it merges. wp2 is B6 with its own 060 decade doc. wp3 is B3's #6085 comment plus the Closure row. The serial-integration rule above still holds at merge time: a later PR is rebased and re-verified after an earlier one lands.

## Architecture consultation

Read-only architect proposal handle `01a0e33e-d6cf-75f1-8873-e262e9c7d68e`, current-dev source anchors in its report. A1 accepted: `devlog/` holds open decisions; `structure/` changes only when its present-tense contract changes. A2 amended: #5964 is held because it reverses a live regression, and #6081/#6083 share one bounded adapter-hardening PR; #6082 stays separate for the Swift gate. A3 amended after independent audit: #6085 is held because safe recovery requires phase evidence across the real CLI start adapters; #6030 remains held until its loaded-launchd behavior and environment preservation can be proved. A4 accepted: hold unreachable #6076 and keep #6088 as its own SSH change. A5 accepted: retain current quota and effort contracts until the named missing evidence arrives. A6 accepted: #4956 cannot be closed from partial fixture fixes, and #4761 requires a separate desktop/GUI scope decision. The earlier B4 verifier corrections are in 040. Final architect reflection and independent audit are recorded before this roadmap is locked.

## Roadmap audit outcome

The independent reviewer `01a0e34b-f829-7c50-9eac-c12b93690513` gave a final `VERDICT: PASS` after the restart race, changed #6081 head, and B4 dependency findings were folded into the staged plan. Its final pre-scan found no blocking issue; `git diff --cached --check` and `bun run privacy:scan` passed. The architect's final reflection was `ALIGNED`. This locks decisions for the first implementation pass; each later P rechecks its decade doc against current `dev`.
39 changes: 39 additions & 0 deletions devlog/_plan/260927_release_train_4/bug-hardening/010_adapter_bounds.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# B1 — bounded adapter state

Depends on the locked roadmap. The prior D closed at roadmap commit `1c2ac3cb79` and directed this B1 adapter-bound phase. Fresh `origin/dev` remains `24b2f39b77`; #6081 is now `8a4399f` and #6083 is `ccfb8e63`. Carry #6081 with the single-parser admission repair and reimplement #6083's cross-width cursor on one lane-owned branch. The combined thesis is that untrusted upstream tool-call identifiers and argument fragments use bounded time and memory before client emission. Do not change provider routing or command-code markup.

## Exact file map

| Path | Change from current behavior |
| --- | --- |
| `src/adapters/coding-agent/protocol.ts` | MODIFY: `OpenToolBlock` retains a budget identity; opening, each argument delta, replacement and close charge/release the existing `TranslatorBudget`. Add one cleanup function for blocks left open at turn end. Current `argParts.push` at the input delta has no retained-byte charge. Preserve ordered event emission. Put the existing turn-call ceiling in the parser's `content_block_start` path, after confirming a nonempty ID but **before** inserting a block, and expose a process-local limit-exceeded signal to the caller. |
| `src/adapters/coding-agent/turn.ts` | MODIFY: pass the incoming budget and bridge call ceiling to parse state; map budget overflow to `translation_buffer_limit`, parser call-limit signal to the established `tool_call_limit`, and release reservations in `finally`. Remove the old post-map call-limit check so there is one authoritative count. Do not restore #6081's removed double raw-start counter, which counted empty-ID frames inconsistently. |
| `src/adapters/openai-chat/tool-call-id-remint.ts` | MODIFY: preserve first occurrences and the `-<n>` family. Replace per-repeat probing from 2 with a next-suffix cursor keyed by `(suffix digit width, sanitized base prefix retained under 64 characters)`; when `-9` becomes `-10`, resume in the 61-character collision group instead of restarting a 62-character group. A probe advances that group's cursor before a later repeat can retry it, and an emitted candidate is reserved immediately. Do not adopt #6083's fixed 62-character key unchanged or change unrelated IDs with one global suffix counter. |
| `tests/providers/codebuddy-protocol.test.ts`, `tests/providers/codebuddy-tool-bridge-turn.test.ts` | MODIFY: before/after tests for over-budget fragments, interleaved blocks, same-index reuse and abort cleanup. A seventeenth valid start is refused before another block is inserted and emits no tool event; an empty-ID frame does not consume a slot. |
| `tests/providers/qoder-adapter.test.ts` | MODIFY only if a shared-parser regression needs an explicit Qoder assertion; otherwise run the existing file and record no change. |
| `tests/adapters/openai/openai-chat-tool-call-id-remint.test.ts` | MODIFY: instrument occupied-set probes in a synchronous `try/finally`; cover thousands of repeats and distinct 62-character sibling IDs that converge when the suffix widens. Assert uniqueness, length <= 64 and preserved first occurrence. The cross-width case must fail on #6083's current head. |
| `structure/providers-and-adapters.md` | MODIFY the CodeBuddy buffer and unique-ID paragraphs to state the new present-tense budget and width-aware collision cursor. Correct #6081's stale raw-start wording to describe one parser-owned valid-ID admission check before allocation. Review other mapped adapter docs for contradictions; do not copy the pending plan into structure. |

No new test file is expected. If one is needed to avoid the file-size ratchet, register it in both `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`; never raise a cap.

## Audit and activation

The main audit reads the complete #6081 diff and its budget owner, verifies that reservations release on close, malformed index reuse, thrown decode and abort, and confirms the existing 8 MiB JSONL ceiling remains separate. For #6083, use candidate-width groups rather than the original ID as the cursor key; assert that no occupied candidate is probed again under converging prefixes. A rewritten nonconforming collision becomes conforming; an unoccupied first occurrence, even if nonconforming, retains the existing byte-identical fast path.

Trigger tests: a fifth byte after a four-byte retained budget emits `translation_buffer_limit` with no tool-call event and a zero active reservation; a seventeenth valid start triggers `tool_call_limit` before block allocation; an empty-ID start does not consume a slot; sibling IDs at `-9`/`-10` keep probe count proportional to emissions. The new buffer and remint regressions run red on the prior behavior and green on the patch. The new parser limit and exceeded signal are in-memory: `turn.ts` creates the limit in parse state, `protocol.ts` sets the signal, `turn.ts` consumes it for the error response; serialization and deserialization are N/A.

Security review records assets (process memory and client tool-call identity), entrypoint (untrusted upstream stream), trust boundary (provider output to adapter state), attacker capability (repeated IDs/fragments), and controls (per-turn/per-call budget, monotone cursors, cleanup). This is input/resource hardening; no auth or credential check is relaxed. Review the PR for raw argument or token logging.

## Verification and delivery

Before planning, the baseline command `bun test tests/adapters/openai/openai-chat-tool-call-id-remint.test.ts tests/providers/codebuddy-protocol.test.ts tests/providers/codebuddy-tool-bridge-turn.test.ts tests/providers/qoder-adapter.test.ts` ran after a frozen install: 88 pass, 0 fail. It reads every listed test target directly. During B1 run those focused files in the dedicated worktree. Run `bun run test:changed` and any full suite in a throwaway `/private/tmp/t4-bug-hardening-verify` checkout pinned to the exact B1 commit, because this checkout under `~/.codex` triggers protected-cleanup failures; do not bypass those guards. Run `bun run typecheck`, `bun run structure:check`, `bun run privacy:scan`, file-size and test-layout guards. Record exact commands/results, the seven-lane local full-suite exception, and exact-head hosted CI in the PR.

## B1 architect decisions

Read-only architect handle `01a0e33e-d6cf-75f1-8873-e262e9c7d68e` proposed B1-A1 through B1-A5 against #6081 `8a4399f`, #6083 `ccfb8e63` and `dev` `24b2f39b77`. Main accepts B1-A1 single valid-ID parser admission owner; B1-A2 reuse of the shared budget and separate JSONL line ceiling; B1-A3 width-aware prefix cursors rather than one global counter or hash; B1-A4 activation tests for the 17th valid block, empty ID, fragment overflow, cleanup and cross-width siblings; and B1-A5 structure-contract and exact-head gates. The file map and tests above encode those decisions. Reflection on this plan revision precedes independent A audit.

The PR body carries `Co-authored-by` for #6081/#6083 contributors. Merge only when current `origin/dev` is an ancestor of the PR head, every required current-head check is successful, and correct review findings are resolved. After merge, thank and close both source PRs with the merged PR/SHA.

## A-phase fold (session `01a0e37e`, auditor `01a0e381-33d6`, NEAR-PASS)

Qoder runs the same parser through the shared turn runner without a tool bridge, so a bridge-supplied call ceiling would leave it unbounded, and an opened block retains its ID and name even with zero argument bytes. The fold: the parser owns a block-count ceiling that applies whether or not a bridge is present (the bridge ceiling, when present, is the tighter of the two), and the retained-byte charge covers the block's ID and name as well as argument fragments. Add a Qoder regression in `tests/providers/qoder-adapter.test.ts` that drives more valid starts than the parser ceiling and asserts the terminal error with no further block allocation, and one that proves ID/name bytes count against the budget.
Loading
Loading