Repository navigation
Conversation
Applications need to set their own environment values without removing other settings people added to their agent configuration. Retaining specified optional arguments also lets checks agree with a prior setup.
Callers need to know when an old environment key requires manual removal and which arguments can be retained without their values.
A saved boolean flag must not override an explicit false value. Keep the order and repetitions of an already valid registration so a check agrees with the command the agent will run.
Moving a flag across a positional argument can change the command an agent runs. Keep its position when setup updates the required arguments, and recognize boolean values saved in the flag token.
An agent command can interpret a flag differently depending on its position and scope. Keep the application's complete argument list while preserving user-owned environment settings.
The remaining hand-added flag case exercises the same outdated result.
Keep coverage at the merge and client boundaries while removing cases that exercise the same behavior through another fixture.
roborev: Combined Review (
|
An environment key can outlive the app setting that first supplied it. Callers need to know that removing the setting from the app alone leaves the existing value in place.
roborev: Combined Review (
|
rodboev
marked this pull request as ready for review
November 13, 2026 23:33
rodboev
marked this pull request as draft
November 14, 2026 00:31
Rerunning setup could discard user settings when authentication was enabled, or leave a stale token reference when authentication was disabled. Keep unrelated settings while making the bearer reference follow the application.
A non-string header value already fails the bearer pattern, so the separate type check never changes which headers setup removes.
An existing authorization header can conflict with a new bearer token reference. Remove that header when the application supplies the reference so the resulting entry follows the same rule as a new registration.
Keep each ownership behavior at the boundary that can show it failing. The stale bearer check also needs to prove that configuration is checked before an agent starts.
Claude and Codex each needed their own loop to drop a saved authorization header. One shared check now covers both cases, so the next change to bearer handling edits one place. Setup writes the same entries as before.
Exercise configuration ownership where setup performs the merge, while keeping checks for new and existing environment keys in the same case.
The native entry writer already limits fields for these clients, so the allowlist cannot affect their setup and duplicates that contract.
An existing bearer reference may belong to the user even when it has the same shape as one written by an application. Removing it without evidence can break a working connection after setup runs. Applications can identify their previous variable when disabling auth. Supplied headers also need to take precedence over environment-backed headers so the client sends the value the application requested.
Only Codex entries carry a bearer variable field, so the merge needs no separate Codex check. An empty previous variable only matches values setup never writes, so it needs no guard either. Two ownership cases repeated paths other cases already cover, and the research notes no longer describe the bearer setting as app-owned.
A caller can disable authentication while supplying another header. That combination needs its own check because a merge can mistakenly retain the old bearer only when new headers are present.
Codex can reject a connection when a saved bearer variable remains after an application supplies an Authorization header. Treat both forms as the same authentication slot so the supplied value takes effect. Reject malformed previous variable names so a caller cannot silently miss the credential it intended to remove.
The authorization fix rewrote how the Claude Authorization header map is built, but the result is the same as before. Keeping the original lines leaves that code untouched by this change, so reviewers see only the lines that change behavior.
Install cases already cover preservation without ownership evidence. Keep the Check case focused on detecting a caller-owned stale bearer.
rodboev
changed the title
roborev: Combined Review (
|
rodboev
marked this pull request as ready for review
November 14, 2026 01:42
This branch has not been deployed
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.
Users who tune an app's MCP server in Claude Code or Codex keep those settings when the app reinstalls it with env values, headers, or a bearer reference. Before, an app that supplied
BearerTokenEnvVarasAPP_TOKENreplaced the whole entry, so a user'sUser='keep'header,tool_timeout_sec=20, and Codexenabled=falsedisappeared. Now they stay.Before, rerunning setup with only the URL after turning off authentication left
bearer_token_env_var='APP_TOKEN'in place, and Codex refused to connect whileAPP_TOKENwas unset. An app now names that variable in the newServer.PreviousBearerTokenEnvVarfield, and setup removes it. When the app omits authentication, setup keeps any bearer setting the app doesn't name.App values win for the env and header keys the app supplies, and keys it stops supplying stay until the user removes them. Args follow the app, so a hand-added flag such as
--writemakes Check report the server as outdated. Check uses the same merge as Install.Needed by kenn-io/docbank#1491
Needed by kenn-io/forge#1997