Repository navigation
feat(cli): add config set-cookie and config clear-credential - #4765
johan456789 wants to merge 2 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex review: blocked before merge. What this changesAdds CLI commands to save manual provider cookies and selectively clear saved API keys or cookies. Example: Configure a Claude cookie from a Linux terminal.
Review scores
ProductKind: Feature · Worth it: Needs a maintainer decision · Fix scope: Partial Merge readiness⛔ Blocked before merge - 2 items remain Keep open for an owner decision on the new commands and repair of the cookie-support gate. Priority: P2 Decision needed
Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe CLI validates provider arguments and updates the shared configuration store, whose credential fields feed provider settings and fetchers. flowchart LR
A[CLI arguments and stdin] --> B[Provider capability validation]
B --> C[Credential config mutation]
C --> D[Shared private config store]
D --> E[Provider settings snapshot]
E --> F[Provider fetcher]
Technical reviewBest possible solution: Extend the descriptor-driven cookie gate to recognize existing settings-backed consumers such as Augment while retaining the shared config store and selective clearing behavior. Do we have a high-confidence way to reproduce the issue? Source inspection establishes that set-cookie rejects Augment: its advertised sources omit web and its credential adapter projects no environment value, although its settings-backed fallback consumes manual cookies. Is this the best way to solve the issue? Reusing the shared config store is appropriate, but advertised source modes and environment projection alone do not establish whether a provider consumes config cookies. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against d34fbb47afe7. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 25, 2026, 1:56 AM ET / 05:56 UTC (Revision 2). |
|
Proof for the review items above: a fresh config, an existing v0.81.0 config where only the requested fields change, and v0.81.0 reading the result (no new config fields). Real binaries on Linux against a scratch Fresh config: set, reject, clear, and no-op clear. $ printf 'sessionKey=PLACEHOLDER' | codexbar config set-cookie --provider claude --stdin
Config: stored manual cookie for Claude and enabled
$ jq -c '.providers[] | select(.id == "claude")' config.json
{"cookieHeader":"sessionKey=PLACEHOLDER","cookieSource":"manual","enabled":true,"id":"claude"}
$ stat -c %a config.json
600
$ codexbar config set-cookie --provider elevenlabs --cookie a=PLACEHOLDER
elevenlabs does not support config cookies.
[exit 1]
$ codexbar config clear-credential --provider claude
Config: cleared manual cookie for Claude
$ jq -c '.providers[] | select(.id == "claude")' config.json
{"enabled":true,"id":"claude"}
$ codexbar config clear-credential --provider claude
Config: no stored API key or manual cookie for ClaudeExisting v0.81.0 config: the diff of the whole file shows only the requested changes, and v0.81.0 still reads the result. No new config fields are introduced. $ codexbar-0.81.0 config set-source --provider claude --source cli
$ codexbar-0.81.0 config enable --provider grok
$ printf 'sk-or-PLACEHOLDER' | codexbar-0.81.0 config set-api-key --provider openrouter --stdin
$ cp config.json before.json
$ printf 'sessionKey=PLACEHOLDER' | codexbar config set-cookie --provider claude --stdin --no-enable
$ codexbar config clear-credential --provider openrouter --api-key
$ diff <(jq -S . before.json) <(jq -S . config.json)
15a16,17
> "cookieHeader": "sessionKey=PLACEHOLDER",
> "cookieSource": "manual",
134d135
< "apiKey": "sk-or-PLACEHOLDER",
$ codexbar-0.81.0 config validate
Config: OKOn the decision item: enabling on set matches @clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Summary
The CLI can store an API key (
config set-api-key) but has no way to store a manual cookie or to remove a stored credential. Settings can do both, so on macOS this only matters for scripting. On Linux, where there is no app and browser cookie import is unavailable, manual cookies are the only way to use cookie providers, and today the only route is hand-editingconfig.json.This PR adds the two missing commands, modeled on
set-api-key(#978) andset-source:config set-cookieconfig set-cookie --provider <name> (--cookie <header>|--stdin) [--no-enable]Writes
cookieHeaderand setscookieSource: manual, the same two fields Settings writes when you paste a manual cookie (SettingsStore+Config.swift).Trims input, rejects empty and multi-line values, and rejects
--cookietogether with--stdin.Enables the provider by default (
--no-enableto opt out), likeset-api-key.Output never contains the cookie. JSON is
{provider, enabled, configPath}, matchingset-api-key.Only providers that actually read a config cookie are accepted, in the spirit of Add config API key CLI #978's "only accepts consumed config keys". A provider qualifies if its fetch plan offers the
websource (the same descriptor dataset-sourcevalidates against), or if its credential adapter projects the config cookie into the fetch environment. DeepSeek is the one case today: it stores its platform token incookieHeader. Everything else gets<name> does not support config cookies.and the config is not touched.At this commit 50 provider IDs qualify. A test checks every registered alias against the rule.
config clear-credentialconfig clear-credential --provider <name> [--api-key] [--cookie]apiKeyandcookieHeader.--api-keyor--cookielimits it to one.manualcookieSourceto the provider default, since a manual source with no header has nothing to send. Non-manual sources (for exampleoff) are kept.apiKeyRegion, matchingMoonshotSettingsStore.source, token accounts, and all other fields are left alone. Removing token accounts is out of scope here.apiKey.clearedlist. JSON is{provider, cleared: ["apiKey", "cookieHeader"], configPath}.Commits
feat(cli): add config set-cookie for manual cookiesfeat(cli): add config clear-credential to remove stored credentialsEach commit adds its command, help text, docs (
docs/cli-configuration.md,docs/cli.md, plus a pointer in the manual-cookie section ofdocs/configuration.md), a CHANGELOG line under 0.81.1, and tests. Both commands are also added to the existingCLICommonOptionsTestspath list and theCLIPluginConfigPreservationTestsmatrix.Commands run (Linux, x86_64, Swift 6.4)
swift build --product CodexBarCLI: clean.CLIConfigCookieCommandTests,CLIConfigClearCredentialCommandTests,CLIConfigCommandTests,CLIConfigSourceCommandTests,CLICommonOptionsTests: 42 tests passed.CodexBarTestsis macOS-only, so I ran these by temporarily copying the files intoTestsLinuxwith a LinuxTestBuildProductspath. The copies are not committed.swift test(fullCodexBarLinuxTests): everything passes except 4 tests inPlatformGatingTests(Claude auto pipeline, OpenCodex log URL). These fail identically on unmodifiedmain(d34fbb47a) on this machine, so they come from the local environment and are unrelated../Scripts/lint.sh lint-linux: 0 violations in 3116 files.CODEXBAR_CONFIG:elevenlabsCLIPluginConfigPreservationTestsscenario, which does not compile on Linux)0600Not run
make test/make check, andCLIPluginConfigPreservationTests(needs the macOS test target). PR CI covers these.Notes for review
websource instead of a provider's cookie-settings reader. I tried the reader first, but it is not wired for every provider (Claude builds its own settings section, for example), so it would wrongly reject providers that do read manual cookies.ProviderCookieSettingsResolveruses the config header when no token account is selected. Adding token-account entries from the CLI would be a separate change.