fix(cli): honor local context for memory and token auth#1011
Conversation
📝 WalkthroughWalkthroughLocal bootstrap now prefers session_token from /api/local-init for the CLI accessToken and stores device_token as localWorkerToken; a new getAgentApiToken exposes worker tokens for agent APIs. Memory URL resolution derives /mcp endpoints for loopback contexts. Chat/dev commands and tests updated to use the new helpers. ChangesLocal Context Authentication and Token Flow
Local Loopback Memory URL Derivation
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/internal/__tests__/context.test.ts (1)
146-163: ⚡ Quick winAdd an IPv6 loopback regression assertion in this test case.
This scenario currently validates
localhostand127.0.0.1; addinghttp://[::1]:8787/api/v1 -> http://[::1]:8787/mcpcoverage will lock the loopback contract across all supported local forms.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/internal/__tests__/context.test.ts` around lines 146 - 163, Add an IPv6 loopback context and assertions mirroring the IPv4/localhost checks: extend the test's configData to include a new context (e.g. name it "localIPv6") with url "http://[::1]:8787/api/v1", then call getMemoryUrl("localIPv6") and assert it returns "http://[::1]:8787/mcp", and verify findContextByMemoryUrl("http://[::1]:8787/mcp")?.name === "localIPv6"; this uses the existing getMemoryUrl and findContextByMemoryUrl helpers and preserves the same pattern as the existing localhost/127.0.0.1 assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/internal/__tests__/context.test.ts`:
- Around line 146-163: Add an IPv6 loopback context and assertions mirroring the
IPv4/localhost checks: extend the test's configData to include a new context
(e.g. name it "localIPv6") with url "http://[::1]:8787/api/v1", then call
getMemoryUrl("localIPv6") and assert it returns "http://[::1]:8787/mcp", and
verify findContextByMemoryUrl("http://[::1]:8787/mcp")?.name === "localIPv6";
this uses the existing getMemoryUrl and findContextByMemoryUrl helpers and
preserves the same pattern as the existing localhost/127.0.0.1 assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64d546e8-0686-44dc-95fb-c974c3df2285
📒 Files selected for processing (5)
packages/cli/src/commands/dev.tspackages/cli/src/internal/__tests__/context.test.tspackages/cli/src/internal/__tests__/credentials.test.tspackages/cli/src/internal/context.tspackages/cli/src/internal/credentials.ts
buremba
force-pushed
the
fix-local-context-auth
branch
from
4f7010d to
b9c1a65
Compare
buremba
force-pushed
the
fix-local-context-auth
branch
from
b9c1a65 to
9fc30a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/internal/__tests__/context.test.ts (1)
146-163: ⚡ Quick winAdd test coverage for IPv6 loopback context.
The test verifies IPv4 loopback (
localhostand127.0.0.1) behavior but doesn't cover IPv6 loopback ([::1]). Adding a test case for an IPv6 loopback context would improve coverage and help catch the IPv6 bracket handling issue flagged incontext.ts.🧪 Suggested test case for IPv6 loopback
test("derives local memory URL from an IPv6 loopback context URL", async () => { const configData = { currentContext: "local-v6", contexts: { lobu: { url: "https://app.lobu.ai/api/v1" }, "local-v6": { url: "http://[::1]:8787/api/v1" }, }, }; readFileSpy.mockResolvedValue(JSON.stringify(configData)); expect(await getMemoryUrl("local-v6")).toBe("http://[::1]:8787/mcp"); expect( (await findContextByMemoryUrl("http://[::1]:8787/mcp"))?.name ).toBe("local-v6"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/internal/__tests__/context.test.ts` around lines 146 - 163, Add a new unit test in packages/cli/src/internal/__tests__/context.test.ts that mirrors the existing IPv4 loopback case but uses an IPv6 loopback address; set a context URL to "http://[::1]:8787/api/v1", mock the config read like the other tests, and assert that getMemoryUrl("local-v6") returns "http://[::1]:8787/mcp" and that findContextByMemoryUrl("http://[::1]:8787/mcp")?.name === "local-v6" to ensure bracketed IPv6 host handling in getMemoryUrl and findContextByMemoryUrl is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/internal/context.ts`:
- Around line 512-521: isLoopbackContextUrl fails to detect bracketed IPv6
hostnames because new URL(...).hostname returns "[::1]"; update the
isLoopbackContextUrl function to normalize the parsed hostname by stripping
surrounding brackets if present before comparing: obtain hostname via new
URL(input).hostname, remove a leading "[" and trailing "]" when both exist, then
check equality against "localhost", "127.0.0.1", and "::1" (keeping the existing
try/catch behavior) so bracketed IPv6 URLs like "http://[::1]:8787/..." are
correctly recognized as loopback.
In `@packages/cli/src/internal/credentials.ts`:
- Around line 169-176: The migration only calls tryLocalInit when
localWorkerToken is missing, but we also need to heal cases where accessToken is
still the old worker PAT (accessToken === localWorkerToken/device_token); update
the branch that checks isLoopbackContext(contextName) to also trigger remint
when creds.accessToken equals creds.localWorkerToken (or device token field) so
tryLocalInit(contextName) is run and creds replaced; locate the block using
isLoopbackContext, creds, tryLocalInit and getToken and add the equality check
for accessToken->localWorkerToken to force re-minting and persisting the new
session token.
---
Nitpick comments:
In `@packages/cli/src/internal/__tests__/context.test.ts`:
- Around line 146-163: Add a new unit test in
packages/cli/src/internal/__tests__/context.test.ts that mirrors the existing
IPv4 loopback case but uses an IPv6 loopback address; set a context URL to
"http://[::1]:8787/api/v1", mock the config read like the other tests, and
assert that getMemoryUrl("local-v6") returns "http://[::1]:8787/mcp" and that
findContextByMemoryUrl("http://[::1]:8787/mcp")?.name === "local-v6" to ensure
bracketed IPv6 host handling in getMemoryUrl and findContextByMemoryUrl is
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a78e9ba-dd70-4d45-9ec2-2c00c39148d9
📒 Files selected for processing (7)
packages/cli/src/commands/chat.tspackages/cli/src/commands/dev.tspackages/cli/src/internal/__tests__/context.test.tspackages/cli/src/internal/__tests__/credentials.test.tspackages/cli/src/internal/context.tspackages/cli/src/internal/credentials.tspackages/cli/src/internal/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/cli/src/internal/tests/credentials.test.ts
| function isLoopbackContextUrl(input: string): boolean { | ||
| try { | ||
| const { hostname } = new URL(input); | ||
| return ( | ||
| hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1" | ||
| ); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
IPv6 loopback detection fails for bracketed addresses.
The hostname property of a parsed IPv6 URL includes brackets. For example, new URL("http://[::1]:8787").hostname returns "[::1]", not "::1" (as noted in the comment at line 432). The check hostname === "::1" will never match a stored context URL like "http://[::1]:8787/api/v1", causing isLoopbackContextUrl to return false and preventing local memory URL derivation for IPv6 loopback contexts.
🔧 Proposed fix to handle bracketed IPv6 addresses
function isLoopbackContextUrl(input: string): boolean {
try {
const { hostname } = new URL(input);
return (
- hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1"
+ hostname === "localhost" ||
+ hostname === "127.0.0.1" ||
+ hostname === "::1" ||
+ hostname === "[::1]"
);
} catch {
return false;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isLoopbackContextUrl(input: string): boolean { | |
| try { | |
| const { hostname } = new URL(input); | |
| return ( | |
| hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1" | |
| ); | |
| } catch { | |
| return false; | |
| } | |
| } | |
| function isLoopbackContextUrl(input: string): boolean { | |
| try { | |
| const { hostname } = new URL(input); | |
| return ( | |
| hostname === "localhost" || | |
| hostname === "127.0.0.1" || | |
| hostname === "::1" || | |
| hostname === "[::1]" | |
| ); | |
| } catch { | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/internal/context.ts` around lines 512 - 521,
isLoopbackContextUrl fails to detect bracketed IPv6 hostnames because new
URL(...).hostname returns "[::1]"; update the isLoopbackContextUrl function to
normalize the parsed hostname by stripping surrounding brackets if present
before comparing: obtain hostname via new URL(input).hostname, remove a leading
"[" and trailing "]" when both exist, then check equality against "localhost",
"127.0.0.1", and "::1" (keeping the existing try/catch behavior) so bracketed
IPv6 URLs like "http://[::1]:8787/..." are correctly recognized as loopback.
| } else if ( | ||
| !creds.localWorkerToken && | ||
| (await isLoopbackContext(contextName)) | ||
| ) { | ||
| // Heal credentials saved by older CLIs that stored only the local-init | ||
| // worker PAT as accessToken. Re-mint so admin REST/MCP get the session | ||
| // token while chat keeps the companion worker PAT. | ||
| creds = (await tryLocalInit(contextName)) ?? creds; |
There was a problem hiding this comment.
Also heal loopback creds when accessToken is still the worker token.
This migration only reruns /api/local-init when localWorkerToken is missing. But Lines 224-228 now persist accessToken === localWorkerToken === device_token for older local servers, so after that server is upgraded to return session_token, getToken() will keep returning the stale worker PAT and admin REST/MCP stays broken until the user manually deletes credentials.
Proposed fix
- } else if (
- !creds.localWorkerToken &&
- (await isLoopbackContext(contextName))
- ) {
+ } else if (
+ (await isLoopbackContext(contextName)) &&
+ (!creds.localWorkerToken ||
+ creds.accessToken === creds.localWorkerToken)
+ ) {
// Heal credentials saved by older CLIs that stored only the local-init
// worker PAT as accessToken. Re-mint so admin REST/MCP get the session
// token while chat keeps the companion worker PAT.
creds = (await tryLocalInit(contextName)) ?? creds;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/internal/credentials.ts` around lines 169 - 176, The
migration only calls tryLocalInit when localWorkerToken is missing, but we also
need to heal cases where accessToken is still the old worker PAT (accessToken
=== localWorkerToken/device_token); update the branch that checks
isLoopbackContext(contextName) to also trigger remint when creds.accessToken
equals creds.localWorkerToken (or device token field) so
tryLocalInit(contextName) is run and creds replaced; locate the block using
isLoopbackContext, creds, tryLocalInit and getToken and add the equality check
for accessToken->localWorkerToken to force re-minting and persisting the new
session token.
2 participants
Summary
https://lobu.ai/mcp/api/local-initagainst the context originlobu chat/ gateway agent APIFixes #1008
Tests
bun test packages/cli/src/internal/__tests__/context.test.ts packages/cli/src/internal/__tests__/credentials.test.ts packages/cli/src/commands/memory/_lib/openclaw-auth.test.tsbunx biome check --config-path config/biome.config.json packages/cli/src/internal/credentials.ts packages/cli/src/internal/__tests__/credentials.test.tscd packages/cli && bunx tsc -p tsconfig.json --noEmittsc --noEmitSummary by CodeRabbit
Tests
Changes
Exports