Fix CLI port mismatch and centralize local dev URLs#1985
Fix CLI port mismatch and centralize local dev URLs#1985nick-inkeep wants to merge 5 commits intomainfrom
Conversation
Bug: `inkeep init --local` hardcoded `manageUi: 'http://localhost:3001'` instead of importing the `LOCAL_REMOTE` constant (which was updated to port 3000 in PR #1752). The device-code auth flow sent users to the wrong port, causing ERR_CONNECTION_REFUSED for every new developer. Fix: - Import LOCAL_REMOTE in init.ts, replacing 4 hardcoded localhost URLs - Centralize fallback URLs in profile-config.ts and config.ts to use LOCAL_REMOTE instead of inline strings - Remove dead DEFAULT_LOCAL_PROFILE constant (never imported since creation in PR #1395) Additionally, split `profile add` into three remote type options: - Cloud: for Inkeep Cloud deployments (env defaults to 'production') - Local: for local dev (URLs from LOCAL_REMOTE, credential auto-set to 'none', no URL/credential prompts, env defaults to 'development') - Custom: for self-hosted/staging (placeholder hints, no pre-filled defaults, URL validation rejects empty input) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix 3 init test expectations: manageUi 3001 → 3000
- Add 9 tests for profileAddCommand covering all three paths:
Local (LOCAL_REMOTE URLs, credential 'none', no URL prompts),
Cloud ('cloud' remote string), Custom (URL prompts + credential)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cli-reference.mdx: - Fix profile YAML example (manageApi → api, remove runApi, credential 'none' for local) - Add inkeep login/logout as proper command sections - Update profile add description for Cloud/Local/Custom options - Fix init --local description (drop 'self-hosted', fix stale port 3003) - Fix push options: --agents-manage-api-url → --agents-api-url - Fix env vars: INKEEP_AGENTS_MANAGE_API_URL → INKEEP_AGENTS_API_URL - Fix CI priority table field names workspace-configuration.mdx: - Fix CLI flag: --agents-manage-api-url → --agents-api-url - Fix env vars table and code examples - Fix config field: agentsManageApiUrl → agentsApi.url setup-profile.mdx: - Update Step 1 to describe Cloud/Local/Custom options troubleshooting.mdx: - Add CLI Issues section (login failures, device code expiry, push auth) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
There was a problem hiding this comment.
PR Review Summary
(4) Total Issues | Risk: Medium
🟠⚠️ Major (2) 🟠⚠️
🟠 1) profile.ts:253-259 profileCurrentCommand warns about credential: 'none' keychain missing
Issue: The profileCurrentCommand function checks if the credential exists in keychain for ALL profiles, including local profiles that use credential: 'none'. This will display a misleading warning: Credential 'none' not found in keychain. Run "inkeep login" to authenticate.
Why: This contradicts the PR's intent that local profiles don't require authentication. The same guard (if (credential !== 'none')) was correctly added to profileAddCommand at line 194, but the parallel function profileCurrentCommand wasn't updated. Users running inkeep profile current on a local profile will see a confusing warning telling them to authenticate when they don't need to.
Fix: Add the same guard in profileCurrentCommand:
// Check if credential exists (skip for 'none')
if (profile.credential !== 'none') {
const credentialExists = await profileManager.checkCredentialExists(profile.credential);
if (!credentialExists) {
console.log();
console.log(chalk.yellow('⚠'), `Credential '${profile.credential}' not found in keychain.`);
console.log(chalk.gray(' Run "inkeep login" to authenticate.'));
}
}Refs:
🟠 2) commands/config.ts:90 Hardcoded localhost URL contradicts PR's centralization goal
Issue: The config set command still hardcodes 'http://localhost:3002' instead of using LOCAL_REMOTE.api. This is the same class of bug the PR aims to fix.
Why: The PR's stated goal is to centralize localhost URLs using the LOCAL_REMOTE constant to prevent future drift. This file was missed. If LOCAL_REMOTE.api changes in the future, this file would remain out of sync—recreating the exact bug pattern this PR addresses.
Fix: Import LOCAL_REMOTE and use it for the default:
import { LOCAL_REMOTE } from '../utils/profiles';
// In the config creation template:
apiUrl: '${key === 'apiUrl' ? value : LOCAL_REMOTE.api}',Refs:
- commands/config.ts:90 — hardcoded URL
- utils/config.ts:343-345 — correct pattern using LOCAL_REMOTE (this PR)
🟡 Minor (1) 🟡
🟡 1) url.ts:34 Dead code fallback not using LOCAL_REMOTE
Issue: The buildAgentViewUrl function has a hardcoded 'http://localhost:3000' fallback that doesn't use LOCAL_REMOTE.manageUi.
Why: While the PR description acknowledges this as dead code (zero callers) tracked in PR #1984, it represents inconsistency in the centralization effort. If the function is ever resurrected, it would use a potentially stale hardcoded value.
Fix: Either add a TODO comment marking it for removal, or update to use LOCAL_REMOTE.manageUi for consistency until removal:
// TODO: Remove this dead code (tracked in PR #1984)
const baseUrl = normalizeBaseUrl(manageUiUrl || LOCAL_REMOTE.manageUi);Refs:
💭 Consider (2) 💭
💭 1) profile.test.ts Missing test for existing profile conflict
Issue: When profileManager.getProfile(profileName) returns a truthy value (lines 67-71), the command should error with "Profile already exists" and exit. No test covers this error path.
Why: Could allow silent profile overwrites if the check breaks. The mock returns undefined by default but there's no test verifying behavior when a profile exists.
Refs: profile.ts:67-71
💭 2) init.test.ts Tests use literal URLs instead of LOCAL_REMOTE constant
Issue: The existing init.test.ts tests use hardcoded 'http://localhost:3000' and 'http://localhost:3002' strings, while the new profile.test.ts correctly imports and uses LOCAL_REMOTE. This inconsistency means if LOCAL_REMOTE values change, init.test.ts assertions would fail with confusing "expected vs actual" messages rather than catching regressions.
Why: The original bug was caused by hardcoded values diverging from the constant. Using LOCAL_REMOTE in test assertions would provide an additional layer of protection.
Refs:
🧹 While You're Here (1) 🧹
🧹 1) profile-config.ts:47-57 Catch block silently swallows all profile errors
Issue: The catch block catches ALL exceptions from profile loading and silently returns default config. This swallows ProfileError exceptions that indicate real configuration problems (PARSE_ERROR, VALIDATION_ERROR) in addition to the expected "no profile configured" case.
Why: If a user has a malformed profiles.yaml file (YAML syntax error, invalid schema), the CLI silently uses localhost defaults without warning. The user would see requests going to http://localhost:3002 instead of their configured remote, with no indication of why.
Fix: Differentiate between "no profiles configured" (expected) and actual errors:
} catch (error) {
if (error instanceof ProfileError && !profileManager.profilesFileExists()) {
// No profile file at all - use defaults
return { /* defaults */ };
}
// Profile file exists but is broken - let user know
throw error;
}Refs: profile-config.ts:47-57
🚫 REQUEST CHANGES
Summary: This PR correctly fixes the CLI port mismatch bug and does good work centralizing localhost URLs. However, two issues should be addressed before merge: (1) profileCurrentCommand needs the same credential !== 'none' guard that was added to profileAddCommand, and (2) commands/config.ts:90 has a hardcoded URL that contradicts the PR's centralization goal. Both are quick fixes that complete the PR's intent.
Discarded (5)
| Location | Issue | Reason Discarded |
|---|---|---|
slack-api.ts |
Unrelated change adding "Last Used" column | Diff artifact from base branch comparison; not part of this PR's changes. |
Docs credential: none |
Could add inline comment explanation | LOW confidence; docs are appropriately simplified and inkeep login section explains the sentinel. |
| Test cancellation flows | Missing tests for user cancellation | Expansion of scope; existing tests cover happy paths adequately. |
| Test URL validation | Missing validation rejection tests | Expansion of scope; inline validation is simple and low-risk. |
| Test switch confirmation | Missing switch profile confirmation test | INFO-level convenience feature; not a critical path. |
Reviewers (7)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-product |
3 | 1 | 0 | 0 | 0 | 0 | 2 |
pr-review-consistency |
5 | 1 | 1 | 0 | 0 | 0 | 3 |
pr-review-tests |
6 | 0 | 1 | 0 | 0 | 0 | 5 |
pr-review-errors |
4 | 0 | 0 | 1 | 0 | 0 | 3 |
pr-review-docs |
1 | 0 | 0 | 0 | 0 | 0 | 1 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-comments |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 19 | 2 | 2 | 1 | 0 | 0 | 14 |
Note: Many reviewer findings were filtered as expansions of scope (test coverage improvements) or INFO-level observations. The core bug fix and centralization work are solid.
…t consistency - Add `credential !== 'none'` guard to `profileCurrentCommand` to match the guard already in `profileAddCommand`. Without this, `inkeep profile current` on a local profile warns about 'none' missing from keychain. - Use LOCAL_REMOTE.api in commands/config.ts template instead of hardcoded 'http://localhost:3002' for centralization consistency. - Use LOCAL_REMOTE.manageUi in init.test.ts assertions instead of hardcoded literals, consistent with profile.test.ts pattern. 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 Summary
This is a re-review scoped to 1 commit (f2f80f8f) since the last automated review.
Prior feedback status: ✅ Fully addressed
| Prior Issue | Status | Fix |
|---|---|---|
🟠 profileCurrentCommand missing credential !== 'none' guard |
✅ Fixed | Guard added at lines 253-261 |
🟠 config.ts:90 hardcoded localhost URL |
✅ Fixed | Now uses LOCAL_REMOTE.api |
💭 Test consistency (use LOCAL_REMOTE in test assertions) |
✅ Partially addressed | 2 of 3 instances updated |
Delta Changes Verified
All three changes in the delta are correct:
-
profile.ts:253-261— Added thecredential !== 'none'guard toprofileCurrentCommand, matching the existing pattern inprofileAddCommand(line 193). Local profiles withcredential: 'none'now correctly skip the keychain warning. -
config.ts:4,91— AddedLOCAL_REMOTEimport and replaced the hardcoded'http://localhost:3002'withLOCAL_REMOTE.api. This completes the URL centralization goal for the CLI package. -
init.test.ts:4,277,338— Updated test assertions to useLOCAL_REMOTE.manageUiinstead of hardcoded strings. This improves test resilience if the constant changes.
💭 Consider
💭 1) init.test.ts:314 One remaining hardcoded URL in test assertions
Issue: Line 314 still uses 'http://localhost:3000' while lines 277 and 338 were updated to use LOCAL_REMOTE.manageUi.
Why: Minor inconsistency within the same test file. Not blocking — the test would correctly fail if values diverge. This is an extension of the prior "Consider" item that was partially addressed.
✅ APPROVE
Summary: All prior Major issues have been addressed. The delta commit cleanly implements the requested fixes with no new issues introduced. The URL centralization and credential guard patterns are now consistent across the CLI codebase. Ship it! 🚀
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
1 | 0 | 1 | 0 | 0 | 0 | 0 |
| Total | 1 | 0 | 1 | 0 | 0 | 0 | 0 |
Note: Minimal reviewer set dispatched for delta scope (3 files, all addressing prior feedback).
Replace all hardcoded 'http://localhost:3002' and remaining 'http://localhost:3000' literals with LOCAL_REMOTE.api and LOCAL_REMOTE.manageUi in mock return values and assertions, so tests stay correct if the canonical defaults ever change. 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 Summary
This is a re-review scoped to 1 commit (973b0413) since the last automated review.
Prior feedback status: ✅ Fully addressed
| Prior Issue | Status | Fix |
|---|---|---|
💭 Test consistency (use LOCAL_REMOTE in test assertions) |
✅ Fixed | All remaining hardcoded URLs now use LOCAL_REMOTE constants |
Delta Changes Verified
The single commit in this delta (test(init): use LOCAL_REMOTE constants consistently in init tests) addresses the remaining "Consider" item from the prior review:
init.test.ts — 14 occurrences of hardcoded localhost URLs ('http://localhost:3002', 'http://localhost:3000') have been replaced with LOCAL_REMOTE.api and LOCAL_REMOTE.manageUi constants throughout the test file.
This completes the URL centralization effort. If LOCAL_REMOTE values change in the future, test assertions will automatically stay in sync rather than producing confusing "expected vs actual" failures.
✅ APPROVE
Summary: The final "Consider" item from the prior review has been addressed. Test assertions now consistently use LOCAL_REMOTE constants throughout init.test.ts, completing the URL centralization goal. All prior Major issues were already addressed in earlier commits. This PR is ready to ship! 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: No sub-reviewers dispatched — delta was a single test file addressing the remaining "Consider" item from the prior APPROVED review. Full reviewer coverage was already completed in prior review runs.
|
Superseded by #1987 (clean commit history, no review noise). |
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 developer running local setup — 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.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.Changes
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)2. Centralize runtime fallbacks
profile-config.tsandconfig.tsboth had hardcoded localhost URL fallbacks. Replaced withLOCAL_REMOTEreferences so they stay in sync with the constant.3. Split
profile addinto Cloud/Local/CustomThe old "Custom" option served both local dev AND remote/staging users with conflicting defaults (pre-filled localhost URLs confused self-hosted users). Split into three options with audience-appropriate defaults:
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.https://your-agents-api.example.com) with no pre-filled default — prevents accidentally accepting wrong defaults. Validation rejects empty input.credential === 'none'—'none'is a sentinel meaning "no auth needed", warning about it missing from keychain is noise.4. Remove dead
DEFAULT_LOCAL_PROFILEconstantExported from
types.tsbut never imported by any file since creation in PR #1395. Removed to avoid misleading future developers.5. Tests
manageUi: 'http://localhost:3001'to'http://localhost:3000'profileAddCommandcovering all three paths (Local, Cloud, Custom)6. Documentation
cli-reference.mdxmanageApi→api, removerunApi, credentialnone). Add login/logout command sections. Update profile add description. Fix init --local description. Fix push options/env vars/CI table (--agents-manage-api-url→--agents-api-url, env vars updated).workspace-configuration.mdxagentsApi.url,INKEEP_AGENTS_API_URL,--agents-api-url).setup-profile.mdxtroubleshooting.mdxNot changing
utils/url.tsline 34 — Dead code (buildAgentViewUrlhas zero callers). Removal tracked in PR Remove dead buildAgentViewUrl utility #1984.config.tslines 32, 37 — JSDoc@defaultannotations consumed by fumadocs-typescript for the docs site. Values are already correct (localhost:3002,localhost:3000). Can't reference constants in JSDoc tags (they're parsed as plain text).Test plan
Automated
pnpm test— 632 tests pass (5 skipped), 41 test files, 0 failuresManual end-to-end (built from source)
inkeep init --local --no-interactive→manageUi: 'http://localhost:3000'in profile,agentsApi.url: 'http://localhost:3002'in configinkeep profile add→ Local → no URL/credential prompts, env defaults todevelopment, profile stored withmanageUi: http://localhost:3000inkeep profile add→ Custom → placeholder hints visible, empty validation rejects with "URL is required", credential keychain warning shownlogin.tsconstructs URL fromprofile.remote.manageUi→http://localhost:3000/device?user_code=...for local profiles🤖 Generated with Claude Code