Fix CLI port mismatch and centralize local dev URLs#1983
Fix CLI port mismatch and centralize local dev URLs#1983nick-inkeep wants to merge 12 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the unused DEFAULT_LOCAL_PROFILE constant from types.ts and its re-export from index.ts. This constant was created in PR #1395 but never imported by any other file. Removing it avoids confusion about what to use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The core bug: inkeep init --local wrote manageUi: 'http://localhost:3001' but the dashboard runs on port 3000. Fix by importing LOCAL_REMOTE and replacing all 4 hardcoded localhost URLs in init.ts. Update 3 test expectations from 3001 to 3000. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nfig.ts Replace hardcoded localhost URLs with LOCAL_REMOTE constant references in profile-config.ts (fallback when profile system fails) and config.ts (lowest-priority default in config resolution). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add LOCAL_REMOTE import and "Local" option to remote type select - Local path: auto-sets URLs from LOCAL_REMOTE, credential to 'none', environment defaults to 'development', skips URL and credential prompts - Custom path: removes localhost defaults, uses descriptive placeholder hints for self-hosted/staging users, validates non-empty input - Cloud/Custom environment defaults to 'production' - Skip "credential not found in keychain" warning when credential is 'none' Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix stale profile YAML example: use 'api' field (not 'manageApi'), remove non-existent 'runApi' field, use credential 'none' for local - Add inkeep login and inkeep logout as proper SkillRule command sections placed after inkeep profile and before inkeep add - Update profile add description to reflect Cloud/Local/Custom options Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add CLI Issues section to troubleshooting.mdx with subsections for login failures, device code expiry, and push auth errors - Fix Step 1 in setup-profile.mdx to describe all three remote type options (Inkeep Cloud, Local, Custom) instead of only Custom Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(5) Total Issues | Risk: Medium
🔴❗ Critical (1) ❗🔴
🔴 1) prd.json Committed file that's in .gitignore
file: prd.json
Issue: The file prd.json was added and committed in commit 751a050f despite being explicitly listed in .gitignore (line 95). This is an internal Ralph automation artifact used for autonomous agent workflows and should NOT be committed to the repository.
Why:
- Pollutes the repository with internal tooling artifacts
- Creates confusion about whether this is an authoritative configuration file
- Contradicts the established convention where
.ai-dev/prd-template.jsonis the canonical template andprd.jsonis expected to be a local working file
Fix: Remove prd.json from the PR:
git rm --cached prd.json
git commit --amend # or create a new commitRefs:
- .gitignore line 95
- .ai-dev/prd-template.json — the canonical template location
🟠⚠️ Major (1) 🟠⚠️
🟠 1) agents-cli/src/commands/profile.ts:74-179 Missing test coverage for new profileAddCommand flows
Issue: The profileAddCommand function has no unit tests, and this PR introduces significant new behavior:
- New 'local' remote type option (lines 78, 92-93) that auto-sets URLs from
LOCAL_REMOTEand skips URL prompts - Credential auto-set to
'none'for local profiles without prompting (lines 159-160) - Modified 'custom' flow with stricter URL validation (lines 94-136)
Why: The original port 3001 bug demonstrates how easily URL mismatches can slip through without tests. If a future change accidentally removes the remoteType === 'local' branch or changes the LOCAL_REMOTE reference, users selecting 'Local' during inkeep profile add could be prompted for URLs or receive incorrect defaults, breaking the intended zero-prompt local setup flow.
Fix: Add unit tests for profileAddCommand in a new test file agents-cli/src/__tests__/commands/profile.test.ts:
describe('profileAddCommand', () => {
it('should create local profile with LOCAL_REMOTE URLs without URL prompts', async () => {
vi.mocked(p.select).mockResolvedValueOnce('local');
// ... mock other prompts
await profileAddCommand('test-local');
// Verify p.text was NOT called for API/ManageUI URLs
expect(p.text).not.toHaveBeenCalledWith(
expect.objectContaining({ message: expect.stringContaining('API URL') })
);
// Verify profile created with LOCAL_REMOTE values
expect(mockProfileManager.addProfile).toHaveBeenCalledWith('test-local', {
remote: { api: 'http://localhost:3002', manageUi: 'http://localhost:3000' },
credential: 'none',
environment: 'development',
});
});
});Refs:
- init.test.ts pattern — existing test pattern to follow
🟡 Minor (2) 🟡
🟡 1) agents-docs/content/guides/cli/setup-profile.mdx:33-36 Docs don't clarify when to use Local vs Custom for self-hosted deployments
Issue: The setup-profile guide describes the three options but doesn't explain the distinction clearly. A customer running a self-hosted deployment locally (e.g., Docker Compose) might be confused about whether to choose 'Local' or 'Custom'.
Why: The key distinction is:
- Local: localhost URLs, no auth, development mode (for Quick Start workspace)
- Custom: any URLs, auth required, production mode (for remote deployments)
But this isn't explicitly stated, leading to potential confusion.
Fix: Update the text to clarify:
1. **Remote type**:
- Select **Inkeep Cloud** if you have an Inkeep Cloud account (URLs configured automatically)
- Select **Local** for local development with the Quick Start workspace (localhost URLs, no authentication)
- Select **Custom** for self-hosted or staging deployments that require authenticationRefs:
🟡 2) agents-docs/content/typescript-sdk/cli-reference.mdx:59-63 init --local docs conflate 'local' and 'self-hosted'
Issue: The --local flag description says "Use --local for self-hosted or local development" but the new profile add flow distinguishes these as separate concepts (Local vs Custom). This terminology mismatch could confuse users who use both commands.
Why: init --local behavior (prompts for URLs, accepts defaults) is a hybrid that doesn't cleanly map to either the new "Local" (no prompts) or "Custom" (no defaults) profile options.
Fix: Update the description to clarify scope:
Use `--local` for local development with the Quick Start workspace:
- Prompts for tenant ID and API URL (defaults to localhost)
- Creates `inkeep.config.ts` with your local configuration
- Sets up a `local` profile with no authentication required
For self-hosted deployments that require authentication, use `inkeep profile add` and select **Custom**.Refs:
Inline Comments:
- 🟡 Minor:
profile.ts:140Environment default behavior change - 🧹 While You're Here:
cli-reference.mdx:60Incorrect port 3003 (pre-existing) - 🧹 While You're Here:
profile.ts:253-259profile currentcredential warning (pre-existing)
💭 Consider (3) 💭
💭 1) profile.ts:77-79 'Local' option name could be clearer
Issue: The label "Local" could be interpreted as "my local self-hosted instance" rather than "the unauthenticated local dev setup". The hint helps but the label alone could mislead.
Fix: Consider renaming to "Local Dev" or "Dev Server", or update the hint to "Quick Start local development (no auth, localhost defaults)".
💭 2) profile.ts:95-125 Validation inconsistency between Custom and init flows
Issue: Custom URL prompts now require non-empty input, while init.ts allows empty values. This may be intentional (Custom should require explicit URLs) but the patterns diverge.
Fix: Consider whether init.ts should also reject empty URLs for consistency, or document why the difference is intentional.
💭 3) init.test.ts Tests use hardcoded strings vs LOCAL_REMOTE constant
Issue: Tests assert hardcoded URL strings rather than importing LOCAL_REMOTE. If the constant changes, tests would need manual updates.
Fix: Optionally import LOCAL_REMOTE in tests for future-proofing. Low priority.
💡 APPROVE WITH SUGGESTIONS
Summary: The core bug fix is correct and the centralization of LOCAL_REMOTE is a solid improvement. Two items deserve attention before merge:
- Remove
prd.json— This file is in.gitignoreand shouldn't be committed - Consider adding test coverage for the new
profileAddCommandflows — The original port bug demonstrates the value of tests for this code path
The documentation and minor UX considerations can be addressed in this PR or as follow-ups.
Discarded (6)
| Location | Issue | Reason Discarded |
|---|---|---|
profile.ts:140 |
Environment default pattern diverges from Cloud | Duplicate — same issue covered by product reviewer as Minor |
types.ts:125-128 |
LOCAL_REMOTE centralization | Positive observation, not an issue |
cli-reference.mdx:808-850 |
Login/logout sections follow SkillRule pattern | Positive observation, not an issue |
cli-reference.mdx:739-753 |
YAML example corrected | Positive observation, not an issue |
types.ts:128 |
Dead code removal | Positive observation, not an issue |
init.test.ts |
Keychain warning skip untested | INFO + LOW confidence — too minor to surface |
Reviewers (6)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
1 | 1 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
5 | 1 | 2 | 0 | 0 | 0 | 2 |
pr-review-product |
5 | 2 | 1 | 1 | 1 | 0 | 0 |
pr-review-docs |
1 | 0 | 0 | 1 | 0 | 0 | 0 |
pr-review-consistency |
8 | 0 | 0 | 0 | 0 | 0 | 8 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 20 | 4 | 3 | 2 | 1 | 0 | 10 |
Note: Consistency reviewer returned mostly positive observations confirming the changes follow established patterns.
| } | ||
|
|
||
| // Get environment name | ||
| const envDefault = remoteType === 'local' ? 'development' : 'production'; |
There was a problem hiding this comment.
🟡 Minor: Custom profiles default to 'production' — verify this is intentional
Issue: The environment default is now 'production' for both Cloud and Custom profiles. Previously, non-cloud profiles defaulted to 'development'. This changes behavior for self-hosted users running profile add.
Why: Users setting up a staging environment (the hint says "Self-hosted or staging deployment") may be surprised their profile defaults to production. This is a subtle behavior change that could cause confusion if unintentional.
Fix: If intentional, consider either:
- Adding a comment explaining the rationale
- Updating the hint to
"Self-hosted deployment"(removing "staging") - Or leaving as-is if production is the safer default for authenticated deployments
Refs:
- Previous behavior — non-cloud defaulted to
development
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(2) Total Issues | Risk: Medium
Delta Review: This review is scoped to the changes since the last automated review (
1694f6cd). The delta is a single new file:profile.test.ts(+182 lines).
🔴❗ Critical (0) ❗🔴
No new Critical issues in the delta.
🟠⚠️ Major (0) 🟠⚠️
No new Major issues in the delta.
The new profile.test.ts successfully addresses the Major finding from the prior review about missing test coverage for profileAddCommand. The tests cover:
- ✅ Local flow:
LOCAL_REMOTEURLs,credential: 'none', no URL prompts,developmentdefault - ✅ Cloud flow:
remote: 'cloud',productiondefault - ✅ Custom flow: URL prompts,
productiondefault, keychain check
🟡 Minor (0) 🟡
No new Minor issues in the delta.
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
profile.test.ts:127Add explicit assertion for noinitialValueon Custom URL prompts
🕐 Pending Recommendations (2)
Issues from the prior review that remain unresolved:
- 🔴
prd.jsonCommitted file that's in.gitignore— should be removed from the PR - 🟡
profile.ts:140Custom profiles default to 'production' — verify this is intentional
🚫 REQUEST CHANGES
Summary: The delta (profile.test.ts) is well-written and addresses the prior Major finding about missing test coverage. However, the Critical issue from the prior review remains unresolved: prd.json is still committed despite being in .gitignore. Please remove it:
git rm --cached prd.json
git commit -m "chore: remove prd.json (should not be committed)"
git pushOnce prd.json is removed, this PR should be ready to merge.
Discarded (0)
No findings discarded in this delta review.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (direct) |
1 | 0 | 0 | 0 | 1 | 2 | 0 |
| Total | 1 | 0 | 0 | 0 | 1 | 2 | 0 |
Note: Delta review — single test file analyzed directly by orchestrator without spawning subagents.
| }); | ||
|
|
||
| describe('Custom remote type', () => { | ||
| it('should prompt for URLs with no initialValue', async () => { |
There was a problem hiding this comment.
💭 Consider: Add explicit assertion for no initialValue on Custom URL prompts
Issue: The test title says "should prompt for URLs with no initialValue" but doesn't explicitly assert this. The test relies on the mock call sequence implicitly.
Why: An explicit assertion would make the test intention clearer and catch regressions if someone accidentally adds initialValue to Custom prompts.
Fix: Add an assertion like:
expect(p.text).toHaveBeenCalledWith(
expect.objectContaining({
message: 'Agents API URL:',
initialValue: undefined,
})
);Refs:
- profile.ts:95-107 — Custom URL prompt implementation
- Add comment explaining why Custom/Cloud default to 'production' - Drop 'self-hosted' from init --local description (no-auth profile isn't suitable for self-hosted deployments that require auth) - Fix stale port 3003 reference (runApi removed in mono-API merge) - Direct self-hosted users to 'inkeep profile add' Custom option Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Medium
Delta Review: Scoped to changes since commit
beea015a. Delta is +7/-5 lines across 2 files.
🔴❗ Critical (0) ❗🔴
No new Critical issues in the delta.
🟠⚠️ Major (0) 🟠⚠️
No new Major issues in the delta.
🟡 Minor (0) 🟡
No new Minor issues in the delta.
💭 Consider (0) 💭
No new Consider items in the delta.
✅ Delta Changes Assessment
The delta commits address prior feedback well:
-
profile.ts:139— Added clarifying comment explaining the environment default logic ('development'for local,'production'for cloud/custom). This directly addresses the prior Minor feedback about verifying the intent behind the behavior change. -
cli-reference.mdx— Fixed--localflag documentation:- Removed confusing "self-hosted" terminology from the flag description
- Clarified
--localis for "local development" with no auth required - Added explicit guidance: "For self-hosted deployments that require authentication, use
inkeep profile addand select Custom."
This addresses the prior Minor finding about the
init --localdocs conflating local and self-hosted concepts.
🕐 Pending Recommendations (1)
- 🔴
prd.json— Committed file that's in.gitignore— should be removed from the PR
🚫 REQUEST CHANGES
Summary: The delta changes are well-executed and address the prior Minor feedback. However, the Critical issue from prior reviews remains unresolved: prd.json is still committed despite being in .gitignore. Please remove it:
git rm --cached prd.json
git commit -m "chore: remove prd.json (should not be committed)"
git pushOnce prd.json is removed, this PR is ready to merge. 🚀
Discarded (0)
No findings discarded.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (direct) |
0 | 0 | 0 | 0 | 0 | 1 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Minimal delta — reviewed directly by orchestrator without spawning subagents.
The mono-API merge (PR #1458) unified the config to a single `agentsApi` field, but several docs and the test-agents config still referenced the old `agentsManageApi`/`agentsRunApi` fields which don't exist in the TypeScript types. - workspace-configuration.mdx: fix 3 code examples to use `agentsApi` - cli-reference.mdx: fix config example and priority table - test-agents/inkeep.config.ts: fix to use `agentsApi` Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Medium
Delta Review: Scoped to changes since commit
b510f88b. Delta is 1 commit (+11/-20 lines) across 3 files.
🔴❗ Critical (0) ❗🔴
No new Critical issues in the delta.
🟠⚠️ Major (0) 🟠⚠️
No new Major issues in the delta.
🟡 Minor (0) 🟡
No new Minor issues in the delta.
💭 Consider (0) 💭
No new Consider items in the delta.
✅ Delta Changes Assessment
The delta commit (bcf913ef) correctly fixes stale field name references from the mono-API merge:
-
cli-reference.mdx— Fixed config example and priority table:agentsManageApiUrl→agentsApi.url--agents-manage-api-url→--agents-api-url- Config example now uses nested
agentsApi: { url: ... }format
-
workspace-configuration.mdx— Fixed 3 code examples:- Replaced
agentsManageApi/agentsRunApiwith unifiedagentsApi - Fixed defaults pseudocode to show
agentsApiUrlandmanageUiUrl
- Replaced
-
test-agents/inkeep.config.ts— Fixed actual config file:- Changed
agentsManageApi/agentsRunApitoagentsApi - This was broken code — the old field names don't exist in TypeScript types
- Changed
All changes are correct and align with the actual InkeepConfig types in agents-cli/src/utils/config.ts.
🕐 Pending Recommendations (1)
- 🔴
prd.json— Committed file that's in.gitignore(line 95) — should be removed from the PR
🚫 REQUEST CHANGES
Summary: The delta changes are excellent — they fix real bugs in documentation and the test config where stale field names from before the mono-API merge were causing incorrect examples. However, the Critical issue from prior reviews remains unresolved: prd.json is still committed despite being in .gitignore.
Please remove it:
git rm --cached prd.json
git commit -m "chore: remove prd.json (should not be committed)"
git pushOnce prd.json is removed, this PR is ready to merge. The core bug fix (port 3001 → 3000), URL centralization, profile add UX improvements, and documentation updates are all well-executed. 🚀
Discarded (0)
No findings discarded.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (direct) |
0 | 0 | 0 | 0 | 0 | 1 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Minimal delta (doc field name fixes) — reviewed directly by orchestrator without spawning subagents.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Review: Scoped to changes since commit
bcf913ef. Delta is 1 commit (+0/-105 lines) — removal ofprd.json.
🔴❗ Critical (0) ❗🔴
No Critical issues.
🟠⚠️ Major (0) 🟠⚠️
No Major issues.
🟡 Minor (0) 🟡
No Minor issues.
💭 Consider (0) 💭
No Consider items.
✅ Prior Issues Resolved
The delta commit (7a5460b4) removes prd.json from the repository. This resolves the Critical issue flagged in prior reviews:
- ✅
prd.json— Committed file that was in.gitignore(line 95) has been removed
All other feedback from prior reviews was already addressed in earlier commits:
- ✅ Tests added for
profileAddCommand(commitbeea015a) - ✅ Environment default clarified with comment (commit
b510f88b) - ✅
init --localdocs clarified (commitb510f88b) - ✅ Stale field name references fixed (commit
bcf913ef)
✅ APPROVE
Summary: The PR is now ready to merge! 🚀
The core bug fix (port 3001 → 3000), URL centralization via LOCAL_REMOTE, the improved profile add UX with Local/Cloud/Custom paths, comprehensive test coverage, and documentation updates are all well-executed. The final blocker (prd.json) has been removed.
What this PR accomplishes:
- Fixes the immediate bug —
init --localnow creates profiles with the correctmanageUi: 'http://localhost:3000' - Prevents recurrence — Centralizes all localhost URLs through
LOCAL_REMOTEconstant - Improves UX —
profile addnow has clear Local/Cloud/Custom paths with appropriate defaults for each audience - Adds test coverage — 9 new tests for
profileAddCommandcovering all three flows - Updates documentation — CLI reference, troubleshooting guide, and setup-profile guide all updated
Discarded (0)
No findings discarded.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (direct) |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Trivial delta (single file deletion resolving prior blocker) — reviewed directly by orchestrator without spawning subagents.
… docs - workspace-configuration.mdx: --agents-manage-api-url → --agents-api-url, INKEEP_AGENTS_MANAGE/RUN_API_URL → INKEEP_AGENTS_API_URL, agentsManageApiUrl → agentsApi.url in all code examples - cli-reference.mdx: push options/examples used stale --agents-manage-api-url and --agents-run-api-url (only --agents-api-url exists in source), CI table and env vars section updated to match Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Review: Scoped to changes since commit
7a5460b4. Delta is 1 commit (+20/-19 lines) across 2 documentation files.
🔴❗ Critical (0) ❗🔴
No Critical issues.
🟠⚠️ Major (0) 🟠⚠️
No Major issues.
🟡 Minor (0) 🟡
No Minor issues.
💭 Consider (0) 💭
No Consider items.
✅ Delta Changes Assessment
The delta commit (7ac8f2b5) fixes remaining stale CLI documentation that was missed in the prior commit (bcf913ef):
1. cli-reference.mdx
- Fixed push command options:
--agents-manage-api-url/--agents-run-api-url→--agents-api-url - Fixed push examples: removed reference to non-existent
--agents-run-api-urlflag - Fixed CI precedence table:
agentsManageApiUrl→agentsApi.url - Fixed environment variables section:
INKEEP_AGENTS_MANAGE_API_URL/INKEEP_AGENTS_RUN_API_URL→INKEEP_AGENTS_API_URL
2. workspace-configuration.mdx
- Fixed CLI override example:
--agents-manage-api-url→--agents-api-url - Fixed environment variable examples and table: removed stale
INKEEP_AGENTS_MANAGE_API_URL/INKEEP_AGENTS_RUN_API_URL - Fixed dynamic config examples:
agentsManageApiUrl→agentsApi: { url: ... }(nested format) - Fixed multi-workspace config examples: same nested format update
Verification: I confirmed against CLI source (agents-cli/src/index.ts:116,165) that only --agents-api-url exists, and only INKEEP_AGENTS_API_URL is read in ci-environment.ts. These documentation fixes are correct and necessary.
🕐 Pending Recommendations (1)
- 💭
profile.test.ts:127— Consider adding explicit assertion for noinitialValueon Custom URL prompts (optional, low priority)
✅ APPROVE
Summary: The delta is a straightforward documentation fix that aligns CLI reference docs with the actual mono-API implementation. All prior blockers have been resolved:
- ✅
prd.jsonremoved (commit7a5460b4) - ✅ Environment default clarified with comment (commit
b510f88b) - ✅
init --localdocs clarified (commitb510f88b) - ✅ Config field names fixed (commit
bcf913ef) - ✅ CLI flag/env var references fixed (this commit
7ac8f2b5)
This PR is ready to merge. 🚀
Discarded (0)
No findings discarded.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (direct) |
0 | 0 | 0 | 0 | 0 | 1 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: Trivial delta (docs-only fixes) — reviewed directly by orchestrator without spawning subagents.
|
Superseded by #1985 — clean branch with the same changes reorganized into 3 logical commits and no review comment noise. |
1 participant
Problem
inkeep init --localcreates a CLI profile withmanageUi: 'http://localhost:3001', but the management dashboard runs on port 3000. This causes the device-code authentication flow to fail for every new user runningpnpm setup-dev— the CLI directs the browser tohttp://localhost:3001/device?user_code=XXXwhich doesn't exist, resulting in ERR_CONNECTION_REFUSED and eventually "Device code expired."How it happened
112b5c765(PR #1395)--localflag. CreatedLOCAL_REMOTEconstant intypes.tsAND hardcodedmanageUi: 'http://localhost:3001'inline ininit.ts. Both used port 3001. No mismatch yet.b63b786c7(PR #1458)init.tssurvived untouched.96de89839(PR #1752)LOCAL_REMOTE.manageUifrom 3001 → 3000. Did not touchinit.ts. The inline hardcoded'http://localhost:3001'was missed.Root cause:
init.tshardcodes the URL inline instead of importing theLOCAL_REMOTEconstant, so updating the constant had no effect on the actual profile creation. Tests asserted 3001, so they passed despite the bug.Beyond the immediate bug, the same localhost URLs are hardcoded as string literals in 6+ files across the CLI package.
LOCAL_REMOTEalready exists intypes.tsbut was barely used, creating a class of bug where one file gets updated and others don't.Solution
Change 1: Fix the bug — use
LOCAL_REMOTEininit.tsImport
LOCAL_REMOTEfrom the profiles barrel and replace all 4 hardcoded localhost URLs:manageUi: 'http://localhost:3001'→LOCAL_REMOTE.manageUi(the bug)Change 2: Centralize runtime fallbacks in
profile-config.tsThis file has a fallback that returns hardcoded localhost URLs when the profile system fails entirely. Replaced with
LOCAL_REMOTEreferences so they stay in sync.Change 3: Centralize runtime fallbacks in
config.tsSame pattern — lowest-priority defaults in config resolution used hardcoded strings. Now uses
LOCAL_REMOTE.Change 4: Split
profile add"Custom" into Local/Custom/CloudProblem: The "Custom" option in
profile addserved both local dev AND remote/staging users, but pre-filled localhost URLs. Sinceinit --localhandles initial local profile creation,profile addCustom was primarily used for remote/self-hosted setups — pre-fillinghttp://localhost:3002was misleading for that audience. Additionally, cloud-first users who later wanted to add a local profile had no clean path.Options considered:
placeholder: 'http://localhost:3002'without initialValue still suggests localhost is expected input.Solution: Three remote type options:
LOCAL_REMOTE, credential auto-set to'none', no URL/credential prompts)Key decisions:
'none'— local dev servers don't require auth, andinit --localalready does this. Prompting would confuse users and trigger a spurious "credential not found in keychain" warning.initialValue, only placeholder hints (https://your-agents-api.example.com) — prevents users from accidentally pressing Enter and accepting wrong defaults. Validation rejects empty input.'production'for Cloud and Custom,'development'for Local — self-hosted/staging deployments are typically production-grade; only localhost dev should default to development.credential === 'none'regardless of how the profile was created —'none'is a sentinel meaning "no auth needed", warning about it missing from keychain is always noise.Change 5: Update init tests
Updated 3 test expectations from
manageUi: 'http://localhost:3001'to'http://localhost:3000'.Change 6: Remove dead
DEFAULT_LOCAL_PROFILEconstantDEFAULT_LOCAL_PROFILEwas exported fromtypes.tsbut never imported by any file since its creation in PR #1395. It can't be used ininit.ts(apiUrl is user-configurable) orprofile.ts(credential/environment are prompted). Two refactors have already spent effort updating a constant nothing consumes. Removed to avoid misleading future developers.Change 7: Add unit tests for
profileAddCommand(review feedback)Added 9 tests covering all three paths (Local/Cloud/Custom), verifying:
LOCAL_REMOTEURLs without URL prompts'none'without prompting'cloud'remote stringDocumentation changes
cli-reference.mdxmanageApi→api, removedrunApi, fixedcredential: inkeep-local→nonecli-reference.mdxcli-reference.mdxcli-reference.mdxprofile addCustomcli-reference.mdx--agents-manage-api-url/--agents-run-api-url→--agents-api-url, env vars updatedtroubleshooting.mdxsetup-profile.mdxworkspace-configuration.mdxagentsManageApiUrl→agentsApi.url, env vars and CLI flags updatedWhat we're NOT changing (and why)
utils/url.tsline 34 — Dead code.buildAgentViewUrlhas zero production callers.config.ts(src root) lines 32, 37 — JSDoc@defaultannotations. Zero runtime effect. Can't reference constants in JSDoc@defaulttags.Test plan
Automated tests
vitest run— 632 tests pass (5 skipped), 41 test files, 0 failuresvitest run src/__tests__/commands/init.test.ts— 10 tests passvitest run src/__tests__/commands/profile.test.ts— 9 tests passpull-v3/only)Manual end-to-end verification (built from source, not published)
inkeep init --local --no-interactive→ verifiedmanageUi: 'http://localhost:3000'in profile,agentsApi.url: 'http://localhost:3002'in configinkeep profile add test-local→ select Local → verified no URL/credential prompts, env defaults todevelopment, profile stored withmanageUi: http://localhost:3000inkeep profile add test-custom→ select Custom → verified placeholder hints (https://your-agents-api.example.com), empty validation rejects with "URL is required", accepts valid URLs, credential keychain warning shownlogin.ts:234constructs URL fromprofile.remote.manageUi→http://localhost:3000/device?user_code=...for local profilesFull spec:
SPEC-fix-cli-port-mismatch.md🤖 Generated with Claude Code