Repository navigation
fix(browser-surface): drop the ChatGPT surface-retention registry - #610
Merged
Merged
Conversation
This was referenced Oct 28, 2026
The runtime kept a ChatGPT n.eko surface process alive past the idle TTL and capacity reclaim, on the belief that its login lived only in the browser process. It does not: the ChatGPT session token is a persistent cookie in the profile, and the profile is a host bind mount that survives a container stop. So an idle stop followed by a restart keeps the login, and the retention rule protects nothing. The rule also never reached shipped builds. It came from the optional @pdpp/polyfill-connectors/browser-surface-policy export, which is a dev dependency; the Docker image and the desktop stage install with --omit=dev, so connectorRetainsSurfaceProcess() was always false there. Only source checkouts and CI saw a retained ChatGPT surface. Removed: - retained-surface-connectors.ts and the optional browserPolicy module. - The retainSurfaceProcess flag on lease acquire. - Boot re-derivation of `retained` on rehydrated surfaces and leases. - The surface-cap reserve check and the retained-count sizing of the host-mode default cap. The default is now 1, which is what shipped builds already used. - The health summary's process-bound replacement-receipt read. Only retained connectors reached it. The replacement-receipt ledger still records replacements but nothing reads it now; deleting it (with its tables) is a separate change. The store keeps the `retained` column, which mirrors the SDK surface type. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Source checkouts loaded the dev-only retention policy, so the host-mode default cap counted ChatGPT and came out as 2. Shipped builds never loaded it and already used 1. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
The desktop stage installs @pdpp/polyfill-connectors as a production dependency, so desktop builds did load the ChatGPT retention rule. Two consequences of removing it needed handling: - The host-mode default cap was retained count + 1, which is 2 on desktop. Keep 2 as a plain default so desktop concurrency does not change. - Desktop rows written with retained = 1 would reload as retained and never be idle-stopped. The store no longer restores the flag; the next write clears the column. Also drop the boundary tests that exercised the SDK's retained handling, which the reference implementation no longer uses. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
…t unused With the process-bound read gone from the health summary, nothing in production imports replacement-receipt-reader.ts or replacement-generation-currentness.ts, and the summary's receipt-evidence wrapper only forwarded the last-successful-run receipt. Delete both modules and their tests, and read that receipt directly. The ledger's lifecycle hooks, which still read and write receipts, are unchanged. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
Nothing in the reference server marks a surface retained any more, so the idle sweep comments and a test title no longer need to exclude one. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
tnunamak
force-pushed
the
fix/remove-surface-retention-registry
branch
from
October 29, 2026 02:44
0be1b9d to
b41f2fa
Compare
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
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.
The runtime kept a ChatGPT n.eko surface process alive past the idle TTL and capacity reclaim, on the belief that its login lived only in the browser process. It does not. The ChatGPT session token is a persistent cookie in the profile, and the profile is a host bind mount that survives a container stop, so an idle stop and restart keeps the login.
The Docker image never loaded the rule: it came from the optional
@pdpp/polyfill-connectors/browser-surface-policyexport, a dev dependency, and the image installs with--omit=dev. The desktop stage adds the polyfill tarball as a production dependency, so desktop builds did load it. This PR keeps desktop behavior where it matters: the host-mode default cap stays 2, and rows that desktop wrote withretained = 1no longer reload as retained.Removed:
retained-surface-connectors.tsand the optionalbrowserPolicymodule (server/polyfill-connectors-runtime.ts).retainSurfaceProcessflag on lease acquire (run-coordinator.ts).retainedon rehydrated surfaces and leases (server/index.ts).browser-surface-leases.ts). The default is now a plain 2, the value desktop builds used. It rises from 1 to 2 only where the old code counted no retained connector: an explicitPDPP_NEKO_MANAGED_CONNECTORSlist without ChatGPT, or a host build that could not load the policy module.retainedfrom the store (browser-surface-lease-store.ts). The column is no longer read, and the next write clears it.health-summary-adapter.ts), which only retained connectors reached, and the two modules only it used:replacement-receipt-reader.tsandreplacement-generation-currentness.ts.Not in this PR: the replacement-receipt ledger still records replacements, but nothing reads it now. Deleting it and its tables is a separate change. That change also removes the health projection's credential-continuity states (
replacement_pending,rehydration_false,indeterminate,continuity_proven), which no production path can reach now; they appear in the API output, so removing them is an API change. The unusedretainedcolumn can be dropped with the ledger tables. The matching registry deletion in data-connectors is PDP-Connect/data-connectors#574.Checks: both typechecks pass.
server-neko-runtime-config,browser-surface-leases,controller-browser-surface-leases,replacement-receipt-ledger-lifecycle,connection-healthand the ephemeral health acceptance files pass after the retention tests were removed. A new controller test persists a surface withretained = 1, reloads it, and asserts that idle cleanup stops it. Biome error counts on the touched files match main.Assisted-by: AI