fix(sdk): bound stale broker lock cleanup, add startup failure diagnostics (#3963) - #4015
Open
Yeachan-Heo wants to merge 224 commits into
Open
fix(sdk): bound stale broker lock cleanup, add startup failure diagnostics (#3963)#4015Yeachan-Heo wants to merge 224 commits into
Yeachan-Heo wants to merge 224 commits into
Conversation
added 30 commits
August 7, 2026 23:13
Long-running sessions retained complete transcript graphs in memory even after compaction. Add disposable disk sidecars, bounded lazy reads, typed context overflow handling, rollout settings, and focused parity and large-session coverage while keeping transcript v5 authoritative. Lore-id: 4ea1286f Constraint: transcript v5 remains authoritative and sidecars stay disposable Constraint: default rollout mode remains shadow Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: coding-agent package check and 92 focused session tests Not-tested: full crash-injection matrix and streaming 1 GiB fork transform
File and in-memory session deletion now have focused regression coverage proving disposable spill index, tail, and commit siblings are removed with their transcript. Lore-id: 1c93bc21 Confidence: high Scope-risk: narrow Reversibility: easy Tested: coding-agent package check and 24 focused integration/storage tests
Whole-session rewrites, model staging, entry updates, and explicit or nested startup now rehydrate the authoritative graph before mutation, rebuild disposable sidecars after publication, and run the bounded context preflight before returning an opened manager. Lore-id: 56c10a8d Constraint: never mutate an incomplete retired graph Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: coding-agent package check and 21 focused session tests Not-tested: managed crash-recovery matrix
Run one hundred file and memory session create-delete cycles and assert no derived sidecar or temporary overlay artifacts remain. Lore-id: 9c7f124e Confidence: high Scope-risk: narrow Reversibility: easy Tested: 20 session memory storage contract tests
Subscribe each AgentSession to sessionMemory.mode changes so enabled, shadow, and off transitions reach the active SessionManager instead of being frozen at construction. Dispose the listener on normal and forced teardown. Lore-id: a74e9b31 Confidence: high Scope-risk: narrow Reversibility: easy Tested: coding-agent package check and 99 focused session tests
Prepared transition and async load frames retained full parsed transcript arrays after cold retirement. Drop adopted transition references, clear source snapshots promptly, replace oversized eager maps, reset materialization caches, and remove runtime dictionary/path-set duplication. Strict capture now uses descriptor-bound range passes and staged fork publication without a second whole-file string, preserves final non-newline records, and bounds checkpoint materialization. Lore-id: d8183f42 Constraint: transcript v5 remains authoritative Constraint: staged fork publication must remain no-replace Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: coding-agent package check, 100 focused tests, and opt-in 1M-record retirement soak Not-tested: transcript-linear fork prepass removal and managed five-class crash recovery
Only retire against a compaction reachable from the active leaf, preventing abandoned branches from authorizing cold history removal. Treat disposable sidecar build failures as an eager fallback and clean partial derived files without aborting authoritative transcript opens. Lore-id: e4db18c7 Confidence: high Scope-risk: narrow Reversibility: easy Tested: 13 session-memory integration tests and coding-agent typecheck
Reserve accountant and hot-suffix capacity before mutating or persisting an append. When the fixed budget cannot admit the entry, rehydrate the authoritative transcript and continue eagerly instead of silently exceeding the memory guarantee. Bind retirement to the active-branch compaction used by the sidecar build. Lore-id: 64bd44a9 Constraint: memory budgets remain fixed implementation constants Confidence: high Scope-risk: medium Reversibility: easy Tested: 14 session-memory integration tests and coding-agent typecheck
Sidecar indexing now serializes candidate bytes without externalizing blobs or mutating persistence state. Patch-bearing transcripts fail closed to eager mode because applied patch records shift authoritative raw offsets until the bounded raw-line indexer is available. Lore-id: 8b21c9e6 Confidence: high Scope-risk: narrow Reversibility: easy Tested: 15 session-memory integration tests and coding-agent typecheck
Full cold hydration now invalidates materialized snapshots and persistence tokens. Branch mutations discard their sidecar proof, accounting, and derived files so a later enabled transition must rebuild against the new active branch rather than reusing stale compaction authority. Lore-id: b57fc91d Confidence: high Scope-risk: medium Reversibility: easy Tested: 16 session-memory integration tests and coding-agent typecheck
Resident-store commits now reapply enabled retirement after rebuilding sidecars, while off mode refuses to recreate derived files. Session file switches therefore preserve the active rollout policy rather than silently reverting to shadow-like behavior. Lore-id: c73d211b Confidence: high Scope-risk: medium Reversibility: easy Tested: 17 session-memory integration tests and coding-agent typecheck
Expose retained-authority descriptor capture for managed descendants and use it for commit publication and reopen classification. Linux retained roots no longer copy the complete transcript merely to obtain dev, inode, size, and time identity; authority-absent platforms retain their existing verified fallback. Lore-id: 19a8cf63 Confidence: high Scope-risk: narrow Reversibility: easy Tested: managed descriptor test, 17 session-memory integration tests, and coding-agent typecheck
Parse the checked marker bytes, validate descriptor/base/tail/terminal proof, and derive the five reopen classes from observed managed transcript identity instead of hard-coded booleans. Record both pre-rebuild recovery evidence and the post-publication current marker state. Lore-id: f26c084e Confidence: medium Scope-risk: wide Reversibility: easy Tested: 62 marker, sidecar, integration, and managed append tests plus coding-agent typecheck Not-tested: crash-window append-tail recovery
Reserve tail metadata before transcript publication, append exact persisted line evidence to the cold index and rolling tail, advance the in-memory terminal proof, then replace the managed commit marker. Sidecar failures hydrate and demote to eager state instead of invalidating the authoritative append. Lore-id: 2d640f3c Constraint: transcript append remains authoritative Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: 107 focused tests, 1M-record soak, and coding-agent package check Not-tested: process-crash injection between transcript, tail, and marker fsync boundaries
Exercise transcript-ahead recovery from a simulated crash after the authoritative append and verify corrupt terminal marker proof is classified, rebuilt, and republished as exact. Lore-id: c4b0216a Confidence: high Scope-risk: narrow Reversibility: easy Tested: transcript-ahead and corrupt-marker managed reopen scenarios plus coding-agent typecheck
Current v5 transcripts without patch records now parse directly into the fork entry graph instead of retaining a second raw-record array, and fresh-header replacement avoids another history array plus spread copy. Patch-bearing transcripts retain the compatibility reducer path. Lore-id: 5d912e7f Constraint: preserve transcript-v5 patch compatibility Confidence: medium Scope-risk: medium Reversibility: easy Tested: 7 descriptor-bound capture/fork integration tests and coding-agent typecheck Not-tested: four known managed path-security assertions in session-manager-resume-readonly remain environment-failing
Keep later compactions on the authenticated cold tail instead of hydrating and rebuilding the retired prefix, mutate the bounded tail record array rather than copying it on every append, and rebuild resident text storage from only the hot suffix after retirement. Add an opt-in 120k-record process RSS gate that measures three post-compaction growth cycles. Lore-id: 79c5f83a Constraint: steady accounted session state must remain at or below 64 MiB Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: opt-in 120k RSS plateau gate, 61 integration/revision/sidecar tests, and coding-agent typecheck Not-tested: first-open eager RSS peak remains outside the plateau assertion
Provider context construction now stops walking parent links once it reaches the active compaction's first kept entry. This preserves summary plus kept history semantics without lazily hydrating provider-invisible cold ancestors. Lore-id: e418ad90 Confidence: high Scope-risk: medium Reversibility: easy Tested: bounded provider-context cold-read regression and coding-agent typecheck
Explicit and custom-storage opens now derive session identity and cwd from descriptor-validated bounded range inspection before initialization. This removes the first full parsed entry graph that was immediately discarded before the authoritative load, reducing the 120k fixture's observed eager RSS delta from about 448 MiB to 317 MiB. Lore-id: a703d5bf Confidence: high Scope-risk: medium Reversibility: easy Tested: 54 session memory/storage/context tests, coding-agent typecheck, and 120k RSS worker
Explicit and memory-backed transcripts now publish, replace, and classify the same descriptor-bound commit marker as managed sessions. This gives enabled explicit resumes authenticated base/tail evidence instead of leaving their disposable sidecars permanently uncommitted. Lore-id: b12c7e44 Confidence: high Scope-risk: medium Reversibility: easy Tested: 40 session memory/storage tests and coding-agent typecheck
Pass the configured session-memory mode into explicit, managed, and continue startup paths before transcript hydration. Enabled resumes can now select bounded sidecar reopen behavior at the load seam instead of learning the setting only after an eager manager has already materialized history. Lore-id: f51e72ac Confidence: high Scope-risk: medium Reversibility: easy Tested: coding-agent typecheck and 29 settings/session-memory tests
Persist bounded reducer, label, usage, leaf, and retirement metadata in descriptor-bound commit markers. Enabled explicit resumes validate the base digest, marker descriptor, tail chain, terminal proof, and every hot record digest, then load only the header plus authenticated tail without a full transcript read. Ordinary explicit appends checkpoint markers at compaction boundaries while remaining safely classifiable as transcript-ahead between checkpoints. Lore-id: 64e19f2c Constraint: transcript v5 remains authoritative and sidecars remain disposable Constraint: lazy reopen RSS delta must not exceed 64 MiB on the 120k fixture Confidence: medium Scope-risk: wide Reversibility: flag-off Tested: 110 focused tests, two opt-in 120k RSS gates, and coding-agent package check Not-tested: managed lazy reopen still uses the eager compatibility path
Revalidate transcript identity after every base/tail/header read, require marker leaf and retirement boundaries to match the authenticated tail, validate bounded labels, reducer state, and usage totals, and continue the marker generation fence after adoption. Corrupt metadata now fails closed to the eager authoritative transcript path. Lore-id: d84fa0e2 Confidence: high Scope-risk: medium Reversibility: easy Tested: 110 focused tests, two 120k RSS gates, and coding-agent typecheck
Expose whether bounded reopen was attempted, whether authenticated adoption succeeded, and whether startup fell back to the eager authoritative path. This makes enabled rollout failures observable without adding user-tunable thresholds or weakening fail-closed behavior. Lore-id: 86d91f0c Confidence: high Scope-risk: narrow Reversibility: easy Tested: lazy success/corrupt fallback integration and coding-agent typecheck
Record the authenticated explicit-path resume behavior, measured 120k RSS gates, and the retained-authority limitation that keeps managed resumes on the eager compatibility path. Lore-id: a6f703d9 Confidence: high Scope-risk: narrow Reversibility: easy Tested: changelog wording against current verified behavior
Remove the redundant structured clone from legacy direct fork loading and apply the configured session-memory mode before fork construction. Enabled forks now rebuild and retire compacted history immediately after durable publication instead of retaining the complete source graph until AgentSession applies settings later. Lore-id: 7bfcde91 Confidence: high Scope-risk: medium Reversibility: easy Tested: 42 direct-fork/resident tests, enabled fork retirement integration, and coding-agent typecheck
Allow descriptor-bound captured forks to inherit an enabled memory mode and rebuild disposable sidecars only after the staged destination is durably published. The returned manager then releases compacted source history instead of retaining the complete compatibility parse graph. Lore-id: 71da3f86 Confidence: high Scope-risk: medium Reversibility: easy Tested: captured memory-storage fork retirement integration and coding-agent typecheck Not-tested: captured fork parsing peak remains transcript-linear
Exercise descriptor-mismatched transcript-ahead and terminal-proof-mismatched tail-ahead states through enabled startup. Both cases must reject lazy adoption, parse the authoritative transcript eagerly, and preserve the durable successor content. Lore-id: 43ac3d11 Confidence: high Scope-risk: narrow Reversibility: easy Tested: transcript-ahead/tail-ahead integration
Revalidate captured fork sources after destination publication, classify destination replacement and quarantine collisions at the strict fork boundary, and recheck picker consent immediately after prepared-receipt publication. A source swap after the publication callback now rolls back the prepared receipt instead of leaving migration authority behind. Test fault injection now targets staged-writer and native receipt publication seams used by the current implementation. Lore-id: c0a58e43 Constraint: foreign destination evidence must never be deleted Confidence: high Scope-risk: wide Reversibility: easy Tested: 39 pass, 1 skip resume/read-only authority tests and coding-agent typecheck
Stream eligible transcript-v5 sessions through two bounded range-read passes, authenticate the disposable index in commit metadata, and materialize only the hot suffix. Fixed-size duplicate detection, byte-bounded labels, periodic collection, and process gates keep the measured 120k first-open RSS delta within 64 MiB while preserving eager fallback for branches and unsupported shapes. Full hydration now becomes an explicit eager transition, patch appends invalidate cold offsets, staged model promotion is fenced by leaf revision, and label, reducer, and usage metadata are ordered before sidecar commit publication. Lore-id: 8b742e1c Constraint: transcript v5 remains authoritative and sidecars remain disposable Constraint: managed retained-authority startup remains eager Confidence: high Scope-risk: wide Reversibility: easy Tested: 198 pass, 1 skip across nine focused suites; 3 process RSS gates; coding-agent check and typecheck
fix(session): retire managed cold history safely
…ns (#4179) Issue #4178 tracked four defects shipped in the merged quick-lane PR (#4177, dev 1a28f92). The preserved owner-repair candidate covered three; this commit completes the set and hardens the regressions: - cli.ts: load the quick-lane command through a top-level import and return the imported class from the registry loader, replacing the runtime inline import() (repository top-level-import contract). Registration/lazy-load behavior is unchanged: the loader still resolves to the same command class on demand. - classify.ts: recognize long decimal issue references (#123456 and up) as issue/PR anchors without reopening hex-color false positives — the issue-number pattern now excludes only 3-digit rgb short forms and tokens containing hex letters, so pure-decimal references of any length stay quick while #rgb/#rrggbb/#rrggbbaa colors stay deep. - classify.ts: recognize extension-only dotfiles (.env, .env.example) as concrete file-path anchors by allowing a leading-dot filename segment in the file-path pattern. - classify.ts: preserve path casing when counting distinct file paths for the multi-file breadth exclusion, so src/Foo.ts and src/foo.ts are two distinct paths on case-sensitive filesystems and force the deep lane (fail-closed; case-insensitive hosts may over-exclude, never under-exclude). - Tests: golden regression block covering all four defects plus an explicit color-discrimination guard, a case-sensitivity platform contract test, and a lazy-load-through-registry regression for the top-level-import change. Lore-id: 4178 Constraint: do not weaken lazy command registration/loading behavior Constraint: keep hex-color discrimination fail-closed Rejected: lowercasing path anchors | collapses case-distinct files Rejected: keeping inline import() | violates top-level-import contract Confidence: high Scope-risk: narrow Reversibility: trivial Tested: quick-lane classify+cli (61) + cli-command-registry (10) + cli-command-surface (24) + issue-4178 adversarial probe (17/17) + coding-agent typecheck + biome Not-tested: full native-dependent suites (CI legs) Co-authored-by: Yeachan Heo <Yeachan-2@users.noreply.github.com>
The macOS verification gate only understood iTerm2 profiles, so normal Terminal.app sessions were rejected or left without an actionable check. Validate Terminal.app default and startup profiles for Use Option as Meta key, retain the iTerm2 path, and document the required profile setting. Constraint: Option shortcuts require the terminal to preserve the modifier as Meta/Esc. Confidence: high Scope-risk: low Reversibility: straightforward Tested: Terminal.app profile verification, failure diagnostic, 102 focused keybinding tests, gjc smoke test Not-tested: manual hardware keypress without synthesized CGEvent
…eue eviction (#4176) (#4180) The virtual-integration job pinned a single global, non-branch-scoped concurrency group (dev-ci-virtual-integration, cancel-in-progress: false). GitHub keeps at most one running plus one pending entry per group, so with N open PRs each newly queued run displaces the previously pending one, which surfaces as a `cancelled` required check with zero executed steps and zero real test failures. That silently gates the whole merge queue under ordinary parallel traffic and makes genuine cancellations unreadable. Scope the job-level group per merge candidate (per PR/head ref) so an unrelated PR can never evict this run while it waits behind a single global slot, while a fresh push to the same head still cancels its stale run via cancel-in-progress (same-ref stale-run cancellation and bounded per-ref load are preserved). Manual dispatch validation keeps its own non-cancelling lane so re-validations cannot race a different base/head merge candidate. Update the pinned topology guard test to assert the per-candidate group key and add an eviction-semantics regression test that evaluates the live workflow expression: unrelated PRs map to distinct groups, same-ref re-push keeps the same group with cancel-in-progress true, and dispatch stays non-cancelling. Lore-id: 4176 Confidence: high Scope-risk: low Reversibility: reversible Tested: bun test scripts/dev-ci-guard-topology.test.ts; ci-virtual-integration, ci-risk-canary-manifest, check-workflow-permissions, check-workflow-yaml; tsc -p tsconfig.tools.json Not-tested: live GitHub Actions run (requires a merge to dev that is out of scope) Co-authored-by: Yeachan Heo <Yeachan-2@users.noreply.github.com>
…k check:ts (#4173) * fix(sdk): keep broker teardown identity provable after a workspace is deleted The identity fence that gates every session.close signal read only the spawn-time marker inside the session's own workspace state root. Deleting that workspace while its host kept running destroyed the sole evidence the broker would accept, so the host survived as an orphan still serving the source it started with and `restart:sdk-broker --close-session-hosts` named the same close_refused sessions on every run -- precisely the stale-code failure the flag exists to prevent. Hosts now publish their own OS start incarnation into the broker-owned session index, which outlives any workspace, and reconciliation carries it so the fence is not silently disarmed for lifecycle sessions. Lore-id: 7c41ab9e Constraint: a pid is reusable -- never signal one whose current OS incarnation no longer matches what was published Constraint: a marker naming a different process is contradiction, not absence, and must still refuse Rejected: mirror the spawn marker into the agent dir | introduces durable per-session files with their own leak and cleanup lifecycle Rejected: signal any pid whose argv looks like a session host | argv is not identity and cannot fence pid reuse Confidence: high Scope-risk: moderate Reversibility: safe Directive: any event that republishes a session record must preserve every identity fact its host published Tested: close of a live host whose workspace state root was deleted, refusal on a recycled incarnation, broker-side retirement of a host that died before unregistering, real restart:sdk-broker --close-session-hosts end to end Not-tested: windows session hosts * fix(ci): drop the dead pre-rename prefix that broke check:ts The session-sdk-notifications risk class matched on `packages/coding-agent/src/notifications/`, a directory that no longer exists since the bus moved under `src/sdk/`. The rename scanner forbids that literal anywhere in tracked files, so `check:ts` failed at its final gate -- after paying the full multi-minute run and without ever reaching the workspace typechecks the `&&` chain guards. The sibling `src/sdk/` prefix already covers the bus, so removing the dead entry restores the gate without narrowing which changes select the canaries. Lore-id: 3fd8c206 Confidence: high Scope-risk: narrow Reversibility: safe Tested: verify-gjc-sdk-rename passes, canary manifest and virtual integration suites unchanged * perf(ci): run manifest receipts through an opt-in bounded pool Manifest receipts ran strictly one at a time, so the SDK adapter parity gate spent 735s serializing 588 `bun test` process startups and alone accounted for three quarters of `check:ts`. Receipts are independent by construction -- each is its own process -- so they now run through a bounded worker pool that preserves the evidence exactly: one process, one argv, one receipt line, output replayed in manifest order, and a failure that stops further claims and exits non-zero. Concurrency is never assumed: it stays sequential unless a caller opts a specific manifest in, because the telegram baseline receipts each spawn a detached broker and load Node-API addons and fail when they race. Lore-id: b95e07d4 Constraint: the gate must stay fail-closed -- in-flight receipts drain, unclaimed receipts are never reported as evidence Constraint: only manifests measured safe may opt in; sequential remains the default Rejected: collapse each file's rows into one process | per-row process isolation is the evidence this manifest exists to produce Rejected: default every manifest to parallel | telegram baseline reproducibly fails at concurrency 8 and passes at 1 Confidence: high Scope-risk: narrow Reversibility: safe Tested: parity manifest 735s to 335s with 588 clean receipts at concurrency 8 and 16, telegram baseline unchanged at 99s, check:sdk-closure 927s failing to 584s passing, check:ts 990s failing to 626s green Not-tested: concurrency above 16 (saturated at 8 on this host)
Page exceptions and console errors were invisible to browser callers when an interaction otherwise succeeded. Capture only bounded metadata and drain it into the next successful act/run response without retaining messages, values, arguments, or stacks. Constraint: keep at most 20 entries per tab and mask URL queries Rejected: domain sandbox and irreversible-action gating | raw Puppeteer run makes those separate policy designs Confidence: high Scope-risk: narrow Reversibility: easy Tested: 24 browser tests; coding-agent typecheck; live Chromium exception and console probe
Maintainer review 4896349024 on PR #4175 reproduced raw pathname leakage into persisted tool results, page-controlled error class names, and an unbounded serialized diagnostics block. Close all three and make the feature opt-in: - Emit http(s) URLs origin-only so path-embedded tokens (signed URLs, invite/ reset links, per-tenant ids) can never reach persisted tool results; non-URL values are hashed irreversibly instead of echoed, and about: paths are restricted to safe fixed names. - Map exception class names through a fixed allowlist of built-in error classes; page-controlled names like CustomerAlice123 are omitted, never echoed. - Bound serialization: url field capped at 256 chars with a visible ellipsis marker and the whole block capped at 4 KiB, shedding oldest entries first and marking runtimeDiagnosticsTruncated: true (never silent). - Make capture opt-in via open(..., { diagnostics: true }) so the extra CDP session and per-console-event traffic are opted into; docs, prompt, and changelog updated. - Add a WorkerCore-level emission test proving diagnostics reach the result (ordering, drain-on-success, retention after failure, byte bound, opt-in off) via a loadPuppeteerInWorkerForTest seam, plus extended unit tests. Constraint: origin-only URLs or irreversible bounded paths only Constraint: fixed allowlist for error class names Constraint: explicit truncation/drop metadata and total byte cap Rejected: query-masked pathname retention | path segments can carry secrets Rejected: identifier-regex class validation | pages control those names Tested: 16 focused diagnostics tests; 42 supervisor/action browser tests; coding-agent check:types; Biome Not-tested: live Chromium probe in this environment Lore-id: 4175-repair Confidence: high Scope-risk: narrow Reversibility: easy Supersedes: 412bd44 review posture
Resolve the current dev conflicts while retaining the Terminal.app Option+Q verifier and the upstream queued-continuation behavior. Confidence: high Scope-risk: medium Tested: conflict-free tree and focused queue/keybinding suite
Terminal.app sends Option+Arrow as an ESC-wrapped CSI sequence when Option is configured as Meta. The input buffer previously split the prefix, so queued-message selection could not see alt+up/down. Preserve the wrapped sequence and decode it in the native matcher so selector restore, delete, and reorder flows receive the intended shortcut. Constraint: preserve existing CSI modifier keybindings Confidence: high Scope-risk: medium Reversibility: easy Tested: cargo fmt, pi-natives key tests, focused TUI and coding-agent tests, package checks, CLI smoke test
…4168) A code-less lifecycle startup failure was rewritten into "No ready SDK endpoint remains available.", which named a resource that was never exhausted. The live ledger records phase "startup", reason "pending", message "SDK startup did not complete before readiness cutoff." and endpointGeneration null -- startup never reached generation recording, so no endpoint or broker registration existed to run out of. Operators chasing this were sent looking for a leak. The actual limit is the semantic-readiness budget: 10s default minus two 1s cleanup/termination windows leaves the child about 8s to become ready. Lore-id: 4146a3c1 Constraint: keep the generic text for a genuinely absent failure -- only a present message replaces it Rejected: raise the readiness budget | hides whatever is slow instead of naming it Rejected: keep the generic message and log the real one | operators read the error, not the ledger Confidence: high Scope-risk: narrow Reversibility: easy Tested: restoring the generic message turns 62 pass / 0 fail into 60 pass / 2 fail Not-tested: which startup stage consumes the 8s -- that needs per-stage timing in the failure artifact
The #4006 schema change already lets provider/model IDs containing colon tags (e.g. ollama-cloud/deepseek-v4-flash:0731) parse and resolve whole, with only a recognized effort token consumed from the final colon. No tracked test covered the numeric-tag form through the real surfaces, so issue #4187's exact scenario (plus its :xhigh variant) could regress silently. Add regression coverage across schema validation, resolver resolution, profile activation, and custom preset creation, proving a colon-bearing mapping no longer blocks loading or saving other presets. Invalid ambiguous forms remain rejected by the existing red-team cases. Lore-id: 4187 Confidence: high Scope-risk: low Reversibility: revert-commit Tested: model-profiles-schema, model-resolver, model-profile-activation, custom-model-preset-creation, model-profiles-redteam, model-profiles-catalog (220 pass) Not-tested: macOS/Bun 1.3.14 runtime path from the report (covered by the same source under Linux)
Terminal.app exposes one Use Option as Meta key profile setting for both the left and right Option keys. Make that contract explicit in the parser, verifier, capture helper, documentation, and regression coverage so either key follows the same Meta/Esc path for queued-message controls. Constraint: preserve the existing terminal-independent alt key IDs Confidence: high Scope-risk: low Reversibility: easy Tested: Rust keys, focused TUI/coding-agent tests, package checks, shell/Python syntax
…ons (#4188) PR #4151 added a managed sidecar cache that allocates its own `i-` instance directory under the shared resident-cache root, and a blanket rejection of nested relative reads whenever retained root authority is unavailable. Both landed on dev red: `moveTo` tripped the "exactly one instance dir" assumption in seven tests, and `fork()` artifact copying hit `managed_nested_path_unsupported` on every non-Linux platform. Nested no-authority reads now verify each intermediate directory component as a real same-device directory instead of being refused, which is strictly stronger than the pre-#4151 lexical containment check. Tests that need the resident text store address it through the manager rather than counting directories under the root. Lore-id: 7c4e19a2 Constraint: sidecar cache and resident text store share one cache root -- tests must not infer either from instance-dir counts Rejected: give sidecar caches a separate root | GC sweep and managed authority both bind to the single resident-cache root Rejected: drop the nested-read guard entirely | O_NOFOLLOW only covers the leaf Confidence: high Scope-risk: narrow Reversibility: easy Tested: session-resident-{lifecycle,cache,transition-seam,cache-root-derivation}, ultragoal-redteam-resident-cache, session-manager, task-managed-descendants Not-tested: Linux retained-authority nested read path (guard is a no-op there)
Retained append identity and absence codes can be emitted after bytes are written, so treating them as pre-commit allowed unsafe causal rollback assumptions. Keep only size preflight failures direct and lock both committed terminal paths with physical append-count evidence. Lore-id: 8c2f1d4a Constraint: pre-commit injected header patch failures must preserve their causal error and source authority Constraint: post-write identity_mismatch and not_found outcomes must remain committed-outcome-uncertain and must not be retried Rejected: update the expected error string | would erase the pre-commit versus post-commit contract Confidence: high Scope-risk: narrow Reversibility: clean Tested: focused cwd patch suite 10 repetitions; title-source suite; move-to and session-storage suites; full session-manager suite; coding-agent package check
…or-contract fix(session): preserve managed append uncertainty
…selector test(models): cover numeric colon-tagged model selectors
fix(coding-agent): harden queued prompts and macOS Option+Q
…stics feat(browser): surface bounded runtime diagnostics
…-replacement fix(secrets): keyed replace-mode replacement derivation (#4166)
…er and model id (P2) Resolves the same-ID cross-provider collision from the 2026-08-09 signed owner verdict: a user override for openai-codex/gpt-5.6-sol no longer exempts an unrelated Codex-transport extension model with the same id (mutation tooth: extension/gpt-5.6-sol at 1_000_000 plus an override-map entry for gpt-5.6-sol now caps to 372_000). Exemptions are keyed by codexContextOverrideKey(provider, modelId). Also fixes the latent .ceiling/.fallback type error in the override diagnostics. Lore-id: pr4024-p2 Tested: policy-level and registry-level same-ID cross-provider mutation teeth Confidence: high Scope-risk: narrow Reversibility: simple
…erride keys (cohort gen-2) Generation-1 cohort findings on the PR #4024 repair: - Cleaner BLOCKING: applyModelOverride silently dropped non-positive/NaN contextWindow overrides for non-Codex providers with no diagnostic, contradicting the CHANGELOG 'ignored with a warning' contract. The drop site now warns for every non-Codex provider (Codex-tier is already diagnosed at collect time), and a registry test locks the behavior. - Architect P3a: mixed-case modelOverrides config keys were exempted from the Codex cap without the value being merged (exact-case apply lookup vs lowercased exemption key). Override keys are now normalized to lowercase at the constructor boundary and all lookup sites match case-insensitively; a mixed-case regression test locks value-merge + exemption alignment. - Architect P3c: policy doc prose named removed ceiling/fallback fields; now names enforced / CODEX_GENERIC_CONTEXT_WINDOW. - Architect P3b (registerProvider OAuth exemption-before-merge) accepted as pre-existing risk, not changed in this lane. Lore-id: pr4024-cohort-gen2 Tested: registry Codex/override subset 8/8 under CI-like env; full registry suite 264/266 (2 pre-existing env-leak failures, reproduced on pristine dev); both package typechecks clean; biome clean Confidence: high Scope-risk: narrow Reversibility: simple
Runtime provider registration replaced configured Codex models after the override merge, allowing the cap exemption to preserve a stale registered context window. Apply the configured override before the final cap so the explicit user value remains authoritative.\n\nLore-id: pr4024-register-provider\nConstraint: preserve provider-qualified Codex override exemptions\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: model-registry contextWindow override subset; coding-agent and ai type checks\nNot-tested: full registry suite has two pre-existing credential-state failures
…stics (#3963) Rebuild of the original PR commit on current dev (d1d4807). The lock artifact reaper and corrupt-ledger rotation hunks are already merged on dev via #4035 and are dropped here; what remains is the durable startup-failure marker and typed BrokerStartupError surface, reconciled with dev's spawn-log stderr capture (file-based, not a pipe). Lore-id: 3963 Constraint: broker lifecycle cleanup/recovery only; no #3955-#3958 overlap Directive: bounded cleanup by age/count; typed bounded diagnostics
Repair pass on top of the #3963 rebuild. Adopts the reviewer's P1 source-contract requirements for the marker code (namespace node:path import, Bun.file/Bun.write for marker I/O with node:fs/promises reserved for the directory op), folds the bounded stderr excerpt into the typed BrokerStartupError message so the pre-existing 'Broker stderr:' contract in sdk-broker-lock-artifacts.test.ts holds, and pins the marker surface with a focused suite plus a real spawned-broker integration test proving the marker reason reaches the caller as a typed error. Lore-id: 3963 Directive: bounded cleanup by age/count; typed bounded diagnostics Tested: 12 new lifecycle-cleanup tests, 229 focused broker tests, mutation 60/2 (0 PR-surface survivors) Confidence: high Scope-risk: low Reversibility: low
A failure artifact can be durable before the detached host has exited and removed its endpoint. Retry the exact proof within a bounded grace window so callers retain the typed startup cause without weakening cleanup verification. Lore-id: 4015-cleanup-proof\nConfidence: high\nScope-risk: narrow\nReversibility: straightforward\nTested: sdk-broker-lifecycle-e2e factory failure and owned startup failure regressions; coding-agent check
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2f567515f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const timing = lifecycleTiming(broker); | ||
| const deadline = timing.now() + STARTUP_CLEANUP_PROOF_GRACE_MS; | ||
| for (;;) { | ||
| const proof = await exactCleanupProof(broker, root, id, expected, evidence); |
There was a problem hiding this comment.
For every normal lifecycle request where evidence is undefined—including successful create, fork, and resume operations—exactCleanupProof can never return a proof, yet this loop still sleeps until the full 500 ms grace expires. This adds a fixed half-second delay while the broker's per-target request chain remains held, and a test timing implementation whose now() does not advance can loop indefinitely; return immediately unless the root, session ID, marker, and qualifying rollback evidence needed for a proof are present.
Useful? React with 👍 / 👎.
Startup marker helpers must not pull the broker implementation into SDK client entrypoints. Keep the process completion helper isolated and move shared diagnostic serialization to its own dependency-safe module. Lore-id: 4015-sdk-closure\nConfidence: high\nScope-risk: narrow\nReversibility: straightforward\nTested: coding-agent check; sdk-broker-lifecycle-cleanup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3963
Problem
Three unbounded/undebuggable states in the SDK broker lifecycle (evidence from #3963):
#reclaimStaleLockrenamed dead-owner lock dirs to.broker.lock.stale-<sha256>but nothing ever removed them; 54 accumulated on the reporter's machine, plus legacybroker-restart-backup-*/broker-stale-backup-*dirs.broker-internalis spawned withstdio: "ignore", so acode=0exit before discovery left the caller with only a bare "exited before discovery" message.lifecycle-ledger.jsonl.corruptgrows unbounded —#quarantineappended every malformed row with no cap/rotation; the reporter observed 126 MB.Fix (broker lifecycle lane only; no #3955-#3958 SDK protocol/API overlap)
broker.ts): on broker startup as the lock owner, reap.broker.lock.stale-*tombstones and legacybroker-*-backup-*dirs with bounded retention — entries older than a 24h grace age are removed, and only the newest 10 are retained. The livebroker.lock/owner.jsonis never matched (fail-closed); reaper failures never block startup.internal.ts,ensure.ts,commands/sdk.ts): the broker-internal process writes a durable boundedsdk/broker.startup-failure.jsonmarker before any clean exit without owned discovery;ensureBrokerclears stale markers at spawn, captures a bounded 4 KB stderr tail, and raises a typedBrokerStartupError(code: "broker_startup_failed") carrying{ exitCode, signal, reason, stderrExcerpt }— surfacing the marker reason instead of a bare code=0.lifecycle-ledger.ts):#quarantinerotateslifecycle-ledger.jsonl.corruptwhen it exceeds an 8 MB cap, retaining only the newest 2 rotated snapshots, so total retained corrupt bytes stay bounded.Orphaned session-host lifecycle analysis
watchSessionHostBrokerLiveness(10-minute absence grace, 15s poll) plus the host self-disposal path are present and covered bysdk-broker-self-reap-process.test.ts/sdk-broker-lifecycle-e2e.test.ts. No concrete gap was proven in the current code path, so this lane ships the analysis as a finding (no host-code change): the 40 observed orphans predate the 0.12.6 self-disposal fix or were captured inside the 10-minute grace window; the broker-side fixes above (typed diagnostics + stale-lock reaping) address the surrounding wedge.Tests / verification
sdk-broker-lock-reaper.test.ts(age/count bounds, legacy backup coverage, live-lock immunity, startup reaping) andsdk-broker-lifecycle-cleanup.test.ts(marker write/read/clear, reason bound, malformed marker, typed error shape, corrupt rotation).sdk-broker-lock-reaper,sdk-broker-lifecycle-cleanup,sdk-broker-restart,sdk-broker,sdk-broker-transport,sdk-broker-lifecycle-e2e,fixture-broker-cleanup,sdk-broker-source-isolation(163 pass, 0 fail).bun --cwd=packages/coding-agent run check:typesclean. (Rootcheck:tshas one pre-existing lint error insmithery-env-trust.test.tspresent atorigin/dev— untouched by this PR.)pkill -f "sdk broker-internal"crash-loop reproduction (external process-kill scenario).