Fix credential setup (PKCE OAuth) and restore broad lang_path_pattern#25
Merged
KHAEntertainment merged 7 commits intomasterfrom Mar 29, 2026
Merged
Conversation
- Add src/bridge/oauth_setup.py: stdlib-only PKCE OAuth helper that flows browser → OpenRouter → localhost:3000 → config.json; API key never passes through Claude's context window - Fix commands/setup.sh: remove infinite self-exec bug (exec "$SCRIPT_DIR/setup.sh" called itself when no key found); replace with guidance message + exit 1 - Rewrite commands/setup.md: instruct Claude to invoke oauth_setup.py via Bash (200s timeout), present auth URL to user, report success/failure; includes xAI direct fallback path - Fix get_api_key() in both grok_bridge.py copies: add ~/.claude/grok-swarm.local.md reading (legacy path written by old setup.sh, was dead code before) - Update README.md and INSTALL.md: replace ./scripts/setup.sh steps for Claude Code users with /grok-swarm:setup OAuth instructions Security: key travels browser → OpenRouter → localhost:3000 → disk, never seen by Claude. Port 3000 conflict detection included. https://claude.ai/code/session_01No9S6TZbfTgocHwHYWwxRN
Upstream commits narrowed lang_path_pattern to absolute-only paths and reverted the Issue #14 absolute-path rebasing in _safe_dest(). Re-apply both fixes: - lang_path_pattern: r'^(\w+):([^\s\n]+)\n' (relative AND absolute) - _safe_dest(): rebase absolute paths to filename-only with a warning instead of raising ValueError (which silently dropped Grok output) https://claude.ai/code/session_01No9S6TZbfTgocHwHYWwxRN
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR replaces local/CLI API-key setup with a PKCE OAuth browser flow ( Changes
Sequence DiagramsequenceDiagram
participant User as User (Claude Code)
participant Claude as Claude Code\n(/grok-swarm:setup)
participant OAuth as oauth_setup.py\n(Local Server)
participant AuthServer as OpenRouter\nOAuth Server
participant Storage as Config File\n(~/.config/grok-swarm/config.json)
User->>Claude: Run /grok-swarm:setup
Claude->>OAuth: Invoke oauth_setup.py
OAuth->>OAuth: Start HTTP server on localhost:3000
OAuth->>AuthServer: Build PKCE challenge & auth URL
OAuth-->>Claude: Return auth URL
Claude-->>User: Show authorization link
User->>AuthServer: Authorize in browser
AuthServer->>OAuth: Redirect to localhost:3000 with code
OAuth->>OAuth: Capture code and exchange for token
AuthServer-->>OAuth: Return access token (API key)
OAuth->>Storage: Persist API key (mode 0600)
OAuth-->>Claude: Report success
Claude-->>User: Setup complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
INSTALL.md (1)
177-184:⚠️ Potential issue | 🟡 MinorAdd a language hint to this fenced block to satisfy markdown linting.
Line 177 opens a code fence without a language tag (MD040).
🔧 Proposed fix
-``` +```text # Authorize via OAuth (no API key in-context) /grok-swarm:setup🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@INSTALL.md` around lines 177 - 184, The fenced code block starting with the lines "# Authorize via OAuth (no API key in-context)" and "/grok-swarm:setup" lacks a language hint (MD040); update the opening fence to include a language tag such as "text" (e.g., change "```" to "```text") so the block becomes fenced with a language hint, ensuring markdown linting passes for the snippet containing "/grok-swarm:setup" and the following example "/grok-swarm:analyze Review my auth module for security issues".
🧹 Nitpick comments (1)
src/bridge/oauth_setup.py (1)
99-100: Clear callback state before starting a new OAuth attempt.Line 99-Line 100 uses global state; without reset, a second in-process run can consume stale code and skip waiting.
♻️ Proposed fix
def run_oauth_flow() -> int: @@ - if not _check_port_available(): + _received_code.clear() + + if not _check_port_available():Also applies to: 157-163, 206-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bridge/oauth_setup.py` around lines 99 - 100, Reset the module-level callback state _received_code before starting any new OAuth attempt: identify the functions that initiate the OAuth flow and wait for the callback (the routines that build the auth URL and call the polling/wait logic) and set _received_code = [] at the start of each such function so stale codes are not consumed by subsequent runs; apply the same reset in the places matching the other ranges mentioned (the other OAuth-starting routines around the 157-163 and 206-213 regions) to ensure each attempt begins with a clear state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/commands/setup.md`:
- Around line 34-40: The docs mention a "200-second timeout" but the example
invoking oauth_setup.py via PLUGIN_ROOT does not show where that timeout is
applied; update the text around the PLUGIN_ROOT/python3 example to state that
the 200s limit is enforced by the surrounding Bash tool invocation (or CI
runner) and either add a clear note naming that tool (e.g., the Bash `timeout`
wrapper used by the runner) or replace the example with an explicit
command-level timeout invocation for oauth_setup.py so readers know exactly how
the 200s is enforced; reference the oauth_setup.py invocation and the
PLUGIN_ROOT variable so it's clear which command to change or annotate.
In `@src/bridge/oauth_setup.py`:
- Around line 163-173: The pre-check using _check_port_available and the
subsequent HTTPServer creation (where CALLBACK_PORT is used) leaves a race
leading to an unhandled OSError if another process binds between the check and
server.bind; modify the code around HTTPServer(...) in oauth_setup.py to handle
bind-time races by wrapping the server creation/serve_forever start in a
try/except that catches OSError (or socket.error), retries a small number of
times or returns a clear error code/message, and closes any partially created
server if needed; reference _check_port_available, CALLBACK_PORT, and the
HTTPServer instantiation to locate the spot to add the exception handling and
optional retry/backoff logic.
- Around line 125-133: The _exchange_code function builds the POST payload but
omits the code_verifier required by the PKCE flow; modify the payload
construction in _exchange_code to include "code_verifier": code_verifier (i.e.,
JSON payload should contain both "code" and "code_verifier") before encoding and
sending the request to OPENROUTER_TOKEN_URL so the token exchange succeeds with
the S256 challenge previously sent.
---
Outside diff comments:
In `@INSTALL.md`:
- Around line 177-184: The fenced code block starting with the lines "#
Authorize via OAuth (no API key in-context)" and "/grok-swarm:setup" lacks a
language hint (MD040); update the opening fence to include a language tag such
as "text" (e.g., change "```" to "```text") so the block becomes fenced with a
language hint, ensuring markdown linting passes for the snippet containing
"/grok-swarm:setup" and the following example "/grok-swarm:analyze Review my
auth module for security issues".
---
Nitpick comments:
In `@src/bridge/oauth_setup.py`:
- Around line 99-100: Reset the module-level callback state _received_code
before starting any new OAuth attempt: identify the functions that initiate the
OAuth flow and wait for the callback (the routines that build the auth URL and
call the polling/wait logic) and set _received_code = [] at the start of each
such function so stale codes are not consumed by subsequent runs; apply the same
reset in the places matching the other ranges mentioned (the other
OAuth-starting routines around the 157-163 and 206-213 regions) to ensure each
attempt begins with a clear state.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19028218-b057-46de-b564-54931be39cb5
📒 Files selected for processing (8)
INSTALL.mdREADME.mdplatforms/claude/commands/setup.mdplatforms/claude/commands/setup.shskills/grok-refactor/bridge/grok_bridge.pysrc/agent/grok_agent.pysrc/bridge/grok_bridge.pysrc/bridge/oauth_setup.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/commands/setup.md`:
- Around line 23-25: The script derives BRIDGE_DIR and PLUGIN_ROOT using dirname
"$0", which is invalid in this context; replace those $0-based derivations with
an absolute lookup using find (the same pattern already used for locating
oauth_setup.py) so BRIDGE_DIR and PLUGIN_ROOT are set by resolving the actual
oauth_setup.py (or plugin root) path via find and pwd, and update the three
blocks that set BRIDGE_DIR/PLUGIN_ROOT (the block that sets BRIDGE_DIR before
calling oauth_setup.py, the Step 2 block that sets PLUGIN_ROOT, and the xAI
Direct Users block) to use that find-based absolute path resolution instead of
dirname "$0".
- Around line 43-45: Update the shell wrapper timeout from "timeout 200s" to
"timeout 240s" and revise the explanatory note to state that the outer timeout
must exceed the script's internal OAUTH_TIMEOUT_SECS (180s) plus roughly 30s for
token exchange, so a 240s wrapper provides a safe margin; mention
OAUTH_TIMEOUT_SECS by name in the note to link the two timeouts and explain the
rationale.
In `@src/bridge/oauth_setup.py`:
- Around line 241-242: Remove any echoing of the API key: delete the masked =
api_key[:12] ... line and change the print that references Key: {masked} to only
confirm success and the saved file (e.g., "Success! API key saved to
CONFIG_FILE") so no portion of api_key is written to stdout; locate the api_key
variable, the masked symbol, CONFIG_FILE, and the print statement in
oauth_setup.py to make this change.
- Around line 275-277: Update the note in oauth_setup.py so it accurately
reflects how grok_bridge retrieves the key: replace the misleading mention of
the environment variable XAI_API_KEY with a statement that grok_bridge reads the
"api_key" field from the JSON config file (e.g.,
~/.config/grok-swarm/config.json); ensure the wording references the grok_bridge
module/reader (grok_bridge) and the "api_key" JSON field so readers are pointed
to the correct knob.
- Line 45: Update type annotations to be Python 3.8 compatible by replacing
built-in generics with typing types: change the return annotation of
_generate_pkce_pair from tuple[str, str] to typing.Tuple[str, str] and replace
any occurrences of list[str] (e.g., the variable referenced around line 99) with
typing.List[str]; add the necessary imports (from typing import Tuple, List) at
the top of the module and update the annotations to use Tuple[...] and List[...]
accordingly so the module imports on Python 3.8.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5686e134-f0db-4d9e-8f00-03cc1a02e7f7
📒 Files selected for processing (2)
platforms/claude/commands/setup.mdsrc/bridge/oauth_setup.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 5 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
platforms/claude/commands/setup.md (1)
37-39:⚠️ Potential issue | 🟡 MinorSync the prose with the 240s wrapper.
Line 38 still says 200 seconds, while Line 46 and Lines 49-52 document 240s. Two clocks for one flow is an easy way to make operators second-guess the instructions.
📝 Tiny wording fix
-Locate `oauth_setup.py` in the plugin's `src/bridge/` directory and run it -with a 200-second timeout. The timeout is enforced by the Bash tool invocation +Locate `oauth_setup.py` in the plugin's `src/bridge/` directory and run it +with a 240-second timeout. The timeout is enforced by the Bash tool invocationAlso applies to: 46-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/claude/commands/setup.md` around lines 37 - 39, Update the prose in platforms/claude/commands/setup.md so the timeout values match the actual wrapper (change any mention of "200 seconds" for running oauth_setup.py to "240 seconds"); search for the string oauth_setup.py and the numeric "200" in that file and replace the inconsistent instances so Lines describing the Bash tool invocation and the wrapper all consistently state 240 seconds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/commands/setup.md`:
- Line 18: Replace the fragile `dirname $(find ...)` invocations used to invoke
oauth_setup.py with a two-step check: first assign the result of the find
command to a variable (e.g., OAUTH_PATH), test it with `[ -n "$OAUTH_PATH" ]`,
and only then call dirname on that variable to run python3 "$(dirname
"$OAUTH_PATH")/oauth_setup.py" --check so the missing-find case doesn't emit a
dirname "missing operand" error; update all occurrences (the Step 1 quiet
--check path and the fallback paths matching the existing patterns) to follow
this pattern. Also update the Step 2 documentation text that currently says
"200-second timeout" to state "240 seconds" so it matches the actual `timeout
240s` used in the script.
In `@src/bridge/oauth_setup.py`:
- Line 100: The module-global list _received_code retains previous callback
values across runs; modify run_oauth_flow to clear this buffer at start (e.g.,
call _received_code.clear() or assign [] to _received_code) so the waiting loop
in run_oauth_flow will not see stale codes from prior flows and will properly
wait for a fresh callback; locate the reset at the top of the run_oauth_flow
function before any loop/wait logic that checks _received_code.
- Around line 241-242: The call to _save_key(api_key) can raise OSError (e.g.,
permission denied, disk full) and should be wrapped in a try/except to mirror
the error-handling used for _exchange_code(); catch OSError (or Exception if
broader coverage is desired), print a concise error message that includes the
exception and the CONFIG_FILE target to stderr, and call sys.exit(1) so the CLI
exits gracefully instead of letting a traceback bubble up.
- Around line 82-93: The _save_key function currently writes the config with
default permissions then calls CONFIG_FILE.chmod, exposing the key briefly;
change it to create and write the JSON into a securely-permissioned temporary
file (use os.open or tempfile in CONFIG_DIR with flags to create mode 0o600),
write the JSON bytes, fsync the file descriptor, close it, and then atomically
rename/replace the temp file to CONFIG_FILE (os.replace) so the config is
created with 0600 from the first write and never world-readable; keep
CONFIG_DIR.mkdir(...) and continue to populate the JSON dict as before.
---
Duplicate comments:
In `@platforms/claude/commands/setup.md`:
- Around line 37-39: Update the prose in platforms/claude/commands/setup.md so
the timeout values match the actual wrapper (change any mention of "200 seconds"
for running oauth_setup.py to "240 seconds"); search for the string
oauth_setup.py and the numeric "200" in that file and replace the inconsistent
instances so Lines describing the Bash tool invocation and the wrapper all
consistently state 240 seconds.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7d035d0-f433-4fd3-81d2-dd6a9da73d63
📒 Files selected for processing (2)
platforms/claude/commands/setup.mdsrc/bridge/oauth_setup.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/claude/commands/setup.md`:
- Around line 85-89: Update the fenced code block that contains the shell
commands (the triple backticks surrounding the lines with mkdir -p
~/.config/grok-swarm, echo '{"api_key": "sk-or-v1-..."}' >
~/.config/grok-swarm/config.json, and chmod 600
~/.config/grok-swarm/config.json) to include the language specifier "bash"
immediately after the opening ``` so the block starts with ```bash for proper
syntax highlighting and to satisfy MD040.
In `@src/bridge/oauth_setup.py`:
- Around line 259-266: The code calls sys.exit(1) inside the except around
_save_key, which breaks the function's promised int return contract; change that
sys.exit(1) into return 1 so the function consistently returns status codes
(keep the stderr print that includes CONFIG_FILE and exc), ensuring the
surrounding function (the one that calls _save_key and is annotated -> int)
always returns 0 or 1 rather than exiting the process.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ca15bbf-aa7c-4a1f-862c-73ffb5086581
📒 Files selected for processing (2)
platforms/claude/commands/setup.mdsrc/bridge/oauth_setup.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
2 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.
Summary
src/bridge/oauth_setup.py: stdlib-only PKCE OAuth helper for OpenRouter. API key flows browser → OpenRouter →localhost:3000→~/.config/grok-swarm/config.json, never through Claude's context window. Includes--checkflag, port-3000 conflict detection, and--provider xaipath for manual credential instructions.commands/setup.sh: removes infinite self-exec bug (exec "$SCRIPT_DIR/setup.sh"called itself when no key found) → prints guidance + exits 1.commands/setup.md: instructs Claude to invokeoauth_setup.pyvia Bash (200s timeout), present auth URL to user, report result.get_api_key()in both bridge copies: now also reads~/.claude/grok-swarm.local.md(legacy path written by oldsetup.sh, was dead code before).README.md/INSTALL.md: replace./scripts/setup.shClaude Code install steps with/grok-swarm:setupOAuth instructions.lang_path_pattern(broad match): master currently hasr'^(\w+):(/[^\s\n]+...)'which only matches absolute paths — relative paths like```python:src/cli.pyare silently dropped. Restored tor'^(\w+):([^\s\n]+)\n'._safe_dest()absolute path rebasing: master raisesValueErrorfor absolute paths causing silent skips. Restored the rebase-to-filename behavior with a warning.Test plan
python3 src/bridge/oauth_setup.py --checkexits 1 with no output when no key configuredpython3 src/bridge/oauth_setup.py --provider xaiprints manual instructions_safe_dest()with absolute path → warns and writes to output dir (no silent skip)lang_path_patternmatches```python:src/cli.py(relative path)./scripts/setup.shfor Claude Code usershttps://claude.ai/code/session_01No9S6TZbfTgocHwHYWwxRN
Summary by CodeRabbit
New Features
Documentation
Improvements
Bug Fixes