feat: add switch-org command to change NodeSource organizations - #58
feat: add switch-org command to change NodeSource organizations#58brycebaril wants to merge 2 commits into
Conversation
Credentials are a single shared file with no way to re-authenticate into a different org once signed in. Adds `nsolid-plugin switch-org` (plus an `ns-switch-org` skill) that forces a fresh OAuth round-trip, reusing NodeSource's existing multi-org sign-in picker, and reports harness-specific follow-up guidance (reconnect vs. re-run install) depending on whether a native plugin, a fallback install, or both are present for the target harness. Also fixes mcpUrl derivation to always use the org's UUID instead of consoleUrl's hostname label, which broke for aliased consoles — the alias's .mcp. subdomain was never a real ingress route. Caps node --test concurrency in scripts/run-tests.mjs, which had no limit and could starve an already-busy machine badly enough to look hung rather than just slow — hit in practice via the pre-commit hook. Fixes two skill-linker tests that hardcoded a regular Windows symlink for their fixtures, which needs elevated privileges Windows doesn't grant by default — unlike the junction links the production code actually creates on Windows, which they now match.
WalkthroughThe PR adds forced organization switching, organization-specific MCP URL derivation, harness refresh guidance, the ChangesOrganization switching
Test-runner concurrency
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@CLAUDE.md`:
- Line 66: Update the CLI dispatch list in the documentation to include
switch-org alongside setup, install, uninstall, doctor, and restore, keeping the
architecture guidance aligned with the commands supported by src/cli.ts.
- Around line 20-28: Update the test-running documentation so single-file
execution uses the supported scripts/run-tests.mjs runner rather than invoking
node --test directly. Either add single-file selection support to run-tests.mjs
and document that command, or replace the single-file example with the existing
package-level node scripts/run-tests.mjs core command; retain the warning
against raw shell globs.
In `@packages/core/src/auth/mcp-url.ts`:
- Around line 26-27: The trusted SaaS suffix check must enforce a DNS-label
boundary, accepting only the exact suffix or values ending in
“.saas.nodesource.io”; update the validation in
packages/core/src/auth/mcp-url.ts (lines 26-27) and
scripts/plugin-generators.mjs (lines 241-244). Regenerate scripts/mcp-wrapper.js
(lines 97-100) using the prescribed sync command rather than editing it
manually, add a regression test for alias.evilsaas.nodesource.io in
packages/core/test/unit/auth/mcp-url.test.ts (lines 30-40), and run pnpm
plugin:check.
In `@packages/core/src/cli.ts`:
- Around line 441-465: Update the CLI usage/help text option scopes for
--accounts-url and --quiet to include switch-org. Keep the existing setup scope
for --accounts-url and install scope for --quiet, adding switch-org without
changing the switch-org command behavior around requireHarness and setup.
In `@README.md`:
- Around line 71-73: Update the earlier mcpUrl derivation wording in the setup
documentation to describe deriving the route from the organization UUID and
using consoleUrl only to determine the environment suffix. Remove any statement
that mcpUrl is created by transforming the consoleUrl hostname, keeping the
documented override and failure behavior consistent with the explanation around
“mcpUrl derivation.”
In `@scripts/run-tests.mjs`:
- Around line 99-100: Validate NSOLID_TEST_CONCURRENCY before assigning
concurrency, accepting only integer values from 1 through 4294967295. For
invalid, negative, fractional, or out-of-range values, fall back to
DEFAULT_CONCURRENCY or fail with a clear error, while preserving the existing
default when the variable is unset.
In `@skills/ns-switch-org/SKILL.md`:
- Line 37: Update the nsolid-plugin invocation in the switch-org command to pin
an explicit package version matching the release shipped with this skill, rather
than resolving the current registry version. Preserve the existing --harness
argument and command behavior.
- Line 36: Update the code fence at line 36 in SKILL.md to specify the shell
language by changing the opening fence to use sh, while leaving the enclosed
command content unchanged.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45dc4380-dd26-4131-828b-8a8a181e054b
📒 Files selected for processing (22)
.claude-plugin/plugin.jsonCLAUDE.mdREADME.mdbundle.jsonpackages/core/README.mdpackages/core/bundle.jsonpackages/core/src/auth/auth-manager.tspackages/core/src/auth/mcp-url.tspackages/core/src/cli.tspackages/core/src/index.tspackages/core/src/types.tspackages/core/src/utils/format.tspackages/core/test/integration/auth/auth-manager.test.tspackages/core/test/integration/cli-help.test.tspackages/core/test/integration/installer.test.tspackages/core/test/unit/auth/mcp-url.test.tspackages/core/test/unit/skills/skill-linker.test.tspackages/core/test/unit/utils/format.test.tsscripts/mcp-wrapper.jsscripts/plugin-generators.mjsscripts/run-tests.mjsskills/ns-switch-org/SKILL.md
| Run a single test file directly (the package-level `pnpm test` wraps this same runner): | ||
|
|
||
| ```bash | ||
| node --experimental-test-module-mocks --import tsx/esm --test packages/core/test/unit/skills/skill-copier.test.ts | ||
| ``` | ||
|
|
||
| Scope `pnpm test` to one package: `node scripts/run-tests.mjs core`. | ||
|
|
||
| **Do not use `node --test 'packages/*/test/**/*.test.ts'` with a shell glob** — `scripts/run-tests.mjs` exists specifically because Node's internal glob matcher silently matches zero files on Windows (exits 0 with "0 tests"). It discovers `*.test.ts` files with `node:fs` and passes explicit paths to `node --test` instead. Always go through `pnpm test` / `run-tests.mjs`, never a raw glob. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CLAUDE.md ---'
cat -n CLAUDE.md | sed -n '1,90p'
printf '%s\n' '--- run-tests.mjs references ---'
rg -n -C 5 -- 'run-tests|test-concurrency|single|file' scripts package.json pnpm-workspace.yaml 2>/dev/null || true
printf '%s\n' '--- test scripts ---'
python3 - <<'PY'
import json
from pathlib import Path
p = Path("package.json")
if p.exists():
data = json.loads(p.read_text())
print(json.dumps(data.get("scripts", {}), indent=2))
PY
printf '%s\n' '--- run-tests.mjs ---'
if [ -f scripts/run-tests.mjs ]; then
cat -n scripts/run-tests.mjs
else
printf '%s\n' 'scripts/run-tests.mjs not found'
fiRepository: nodesource/nsolid-plugin
Length of output: 43690
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
claude = Path("CLAUDE.md").read_text()
runner = Path("scripts/run-tests.mjs").read_text()
direct = re.search(r"^\s*node --experimental-test-module-mocks --import tsx/esm --test (.+)$", claude, re.M)
runner_args = re.search(r"const args = \[(.*?)\n\]", runner, re.S)
runner_scope = re.search(r"const onlyPackage = process\.argv\[2\]", runner)
runner_file_discovery = re.search(
r"findTestFiles\(testDir, files\).*?files\.sort\(\)",
runner,
re.S,
)
print("documented_direct_command:", direct.group(0).strip() if direct else "not found")
print("runner_has_test_concurrency:", bool(re.search(r"`--test-concurrency=\$\{concurrency\}`", runner)))
print("runner_has_custom_reporter:", "'--test-reporter', REPORTER" in runner_args.group(1) if runner_args else False)
print("runner_accepts_only_package_arg:", bool(runner_scope))
print("runner_discovers_files_recursively:", bool(runner_file_discovery))
print("runner_has_file_selection_option:",
bool(re.search(r"argv\[\d+\].*(file|test)|test-file|testFile|filePattern", runner, re.I)))
PYRepository: nodesource/nsolid-plugin
Length of output: 488
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("scripts/run-tests.mjs").read_text()
# Remove comments only for checking executable argument handling.
source = re.sub(r"//.*", "", source)
source = re.sub(r"/\*.*?\*/", "", source, flags=re.S)
argv_refs = re.findall(r"process\.argv\[(\d+)\]", source)
print("executable_process_argv_indexes:", argv_refs)
print("file_selection_argument:", any(i != "2" for i in argv_refs))
print("package_scope_argument:", "const onlyPackage = process.argv[2]" in source)
print("test_concurrency_argument:", "`--test-concurrency=${concurrency}`" in source)
print("reporter_argument:", "'--test-reporter', REPORTER" in source)
PYRepository: nodesource/nsolid-plugin
Length of output: 317
Use the supported test runner in this documentation.
The single-file command invokes node --test directly. It bypasses scripts/run-tests.mjs, including its test-concurrency limit and reporter. Add single-file selection to scripts/run-tests.mjs, or document the package-level runner instead.
🤖 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 `@CLAUDE.md` around lines 20 - 28, Update the test-running documentation so
single-file execution uses the supported scripts/run-tests.mjs runner rather
than invoking node --test directly. Either add single-file selection support to
run-tests.mjs and document that command, or replace the single-file example with
the existing package-level node scripts/run-tests.mjs core command; retain the
warning against raw shell globs.
Source: Coding guidelines
| - `src/harnesses/` — one adapter per harness (`claude-adapter.ts`, `codex-adapter.ts`, `opencode-adapter.ts`, `antigravity-adapter.ts`, `pi-adapter.ts`) implementing the `HarnessAdapter` interface (`harness-adapter.ts`): `getMcpConfigPath`, `getSkillsPath`, `readMcpConfig`/`writeMcpConfig`, and optional `detectNativePlugin()` for harnesses with a native plugin model (doctor treats it as N/A where absent, e.g. OpenCode). | ||
| - `src/mcp/` — MCP config merging (`mcp-config-merger.ts`, never clobbers non-NodeSource servers already in a harness config) and dedup tracking (`mcp-tracker.ts`). | ||
| - `src/skills/` — `skill-copier.ts` / `skill-linker.ts` install skills into a harness's skills dir; both validate the destination name and resolved source path stay within their base directories (path-traversal guards — see `*-security.test.ts` siblings) before touching disk. | ||
| - `src/cli.ts` → the `nsolid-plugin` bin, dispatching `setup | install | uninstall | doctor | restore`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include switch-org in the CLI dispatch list.
Line 66 omits switch-org, although this PR adds that command and the surrounding documentation describes it. Add the command to keep the architecture guidance complete.
Suggested update
-- `src/cli.ts` → the `nsolid-plugin` bin, dispatching `setup | install | uninstall | doctor | restore`.
+- `src/cli.ts` → the `nsolid-plugin` bin, dispatching `setup | install | switch-org | uninstall | doctor | restore`.📝 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.
| - `src/cli.ts` → the `nsolid-plugin` bin, dispatching `setup | install | uninstall | doctor | restore`. | |
| - `src/cli.ts` → the `nsolid-plugin` bin, dispatching `setup | install | switch-org | uninstall | doctor | restore`. |
🤖 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 `@CLAUDE.md` at line 66, Update the CLI dispatch list in the documentation to
include switch-org alongside setup, install, uninstall, doctor, and restore,
keeping the architecture guidance aligned with the commands supported by
src/cli.ts.
| command timeout of **at least 300000ms** (the CLI's own OAuth wait window is | ||
| 5 minutes): | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the code-fence language.
Line 36 opens a shell command fence without a language. markdownlint-cli2 reports MD040. Change the fence to sh.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 36-36: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SkillSpector (2.5.1)
[warning] 37: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 `@skills/ns-switch-org/SKILL.md` at line 36, Update the code fence at line 36
in SKILL.md to specify the shell language by changing the opening fence to use
sh, while leaving the enclosed command content unchanged.
Source: Linters/SAST tools
| 5 minutes): | ||
|
|
||
| ``` | ||
| npx -y nsolid-plugin switch-org --harness <harness> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the package version before execution.
Line 37 downloads and executes the current registry version of nsolid-plugin. An unexpected or compromised publish can execute code on the user host. Pin nsolid-plugin to the release version that ships this skill.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 37: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 `@skills/ns-switch-org/SKILL.md` at line 37, Update the nsolid-plugin
invocation in the switch-org command to pin an explicit package version matching
the release shipped with this skill, rather than resolving the current registry
version. Preserve the existing --harness argument and command behavior.
Source: Linters/SAST tools
|
Great work on the core switch-org flow. During follow-up review, I found a few edge cases worth addressing
I implemented the fixes and added coverage in PR #59 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/core/src/utils/format.ts (1)
200-204: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNormalize
nullandundefinedbefore comparing organizations.
orgon line 202 collapsesnullandundefinedto(unknown), but line 203 compares the raw values. The declared input type allows both. If a caller passespreviousOrg: nullandcurrentOrg: undefined,orgChangedbecomestrueandstateLineclaims "Now signed in to org: (unknown)" although the organization did not change. The current CLI caller passesundefinedfor both, so this path is not reachable today.♻️ Proposed normalization
const { success, authSucceeded, errors, previousOrg, currentOrg, harness, harnessLabel, isPluginOwned } = input - const org = currentOrg ?? '(unknown)' - const orgChanged = currentOrg !== previousOrg + const before = previousOrg ?? undefined + const after = currentOrg ?? undefined + const org = after ?? '(unknown)' + const orgChanged = after !== before🤖 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/core/src/utils/format.ts` around lines 200 - 204, Update buildSwitchOrgOutcome to normalize both previousOrg and currentOrg through the same nullish fallback before comparing them. Use the normalized values for orgChanged and the displayed organization so null and undefined are treated as equivalent and unchanged unknown organizations report the “Still signed in” state.packages/core/src/auth/auth-manager.ts (1)
165-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRoute the manual sign-in notice through the reporting abstraction.
ensureAuthenticatedwrites directly toprocess.stderr. Every other user-facing message in this flow goes throughloggeror the caller'sProgressReporter. A caller that uses--quietor--jsoncannot suppress these three lines, and library consumers get output they did not opt into.Keep the headless fallback, but emit it through an injected sink so the CLI stays in control of output.
♻️ Sketch of a caller-controlled notice
- process.stderr.write('\nNodeSource authentication started.\n') - process.stderr.write('If a browser did not open automatically, open this sign-in URL manually:\n') - process.stderr.write(`${signInUrl.toString()}\n\n`) + const notice = options.notify ?? ((text: string) => process.stderr.write(text)) + notice('\nNodeSource authentication started.\n') + notice('If a browser did not open automatically, open this sign-in URL manually:\n') + notice(`${signInUrl.toString()}\n\n`)🤖 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/core/src/auth/auth-manager.ts` around lines 165 - 173, Update ensureAuthenticated’s manual sign-in fallback to emit the three-line notice through the existing injected logger or ProgressReporter sink instead of process.stderr.write. Preserve the fallback content and always report the sign-in URL, while allowing --quiet, --json, and library callers to control or suppress the output.packages/core/src/cli.ts (1)
473-506: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
./utils/format.jsonce and useoutcome.exitCode.Line 475 and line 508 both dynamically import the same module. Load it once before the branch.
buildSwitchOrgOutcomealso returnsexitCode, but lines 490 and 505 hardcode1. Use the returned value so the exit contract lives in one place.♻️ Proposed consolidation
- const { buildSwitchOrgOutcome } = await import('./utils/format.js') + const { buildSwitchOrgOutcome, formatSwitchOrgGuidance } = await import('./utils/format.js') const outcome = buildSwitchOrgOutcome({ @@ console.error(paint.red(outcome.errorHeader ?? `✗ Switch organization failed for ${switchHarness}:`)) for (const line of outcome.detail) console.error(line) - process.exit(1) + process.exit(outcome.exitCode) @@ for (const line of outcome.commands) console.error(paint.dim(` ${line}`)) - process.exit(1) + process.exit(outcome.exitCode)Then drop the second import at line 508:
- const { formatSwitchOrgGuidance } = await import('./utils/format.js') let nativeInstalled = 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/core/src/cli.ts` around lines 473 - 506, Import ./utils/format.js once before the switch-org outcome branches and reuse the imported buildSwitchOrgOutcome reference, removing the later duplicate dynamic import. Replace the hardcoded process.exit(1) calls in the auth-failed and partial outcome paths with process.exit(outcome.exitCode).
🤖 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/core/src/cli.ts`:
- Line 85: Update the switch-org help text in the CLI to claim immediate MCP
config refresh only for direct-config harnesses outside PLUGIN_OWNED_HARNESSES;
state that claude, codex, and antigravity require native plugin reconnection
instead. Update the corresponding assertion in cli-help.test.ts to match the
revised wording.
In `@packages/core/test/integration/installer.test.ts`:
- Line 285: Correct the assertion message in the installer integration test to
reference opencode.jsonc instead of openocode.jsonc, keeping the assertion and
server validation unchanged.
---
Nitpick comments:
In `@packages/core/src/auth/auth-manager.ts`:
- Around line 165-173: Update ensureAuthenticated’s manual sign-in fallback to
emit the three-line notice through the existing injected logger or
ProgressReporter sink instead of process.stderr.write. Preserve the fallback
content and always report the sign-in URL, while allowing --quiet, --json, and
library callers to control or suppress the output.
In `@packages/core/src/cli.ts`:
- Around line 473-506: Import ./utils/format.js once before the switch-org
outcome branches and reuse the imported buildSwitchOrgOutcome reference,
removing the later duplicate dynamic import. Replace the hardcoded
process.exit(1) calls in the auth-failed and partial outcome paths with
process.exit(outcome.exitCode).
In `@packages/core/src/utils/format.ts`:
- Around line 200-204: Update buildSwitchOrgOutcome to normalize both
previousOrg and currentOrg through the same nullish fallback before comparing
them. Use the normalized values for orgChanged and the displayed organization so
null and undefined are treated as equivalent and unchanged unknown organizations
report the “Still signed in” state.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a120d7b2-2bf9-4f6f-a420-607140e638dd
📒 Files selected for processing (18)
README.mdpackages/core/src/auth/auth-manager.tspackages/core/src/auth/mcp-url.tspackages/core/src/cli.tspackages/core/src/index.tspackages/core/src/types.tspackages/core/src/utils/format.tspackages/core/test/integration/auth/auth-manager.test.tspackages/core/test/integration/cli-help.test.tspackages/core/test/integration/installer.test.tspackages/core/test/unit/auth/mcp-url.test.tspackages/core/test/unit/mcp/mcp-wrapper.test.tspackages/core/test/unit/utils/format.test.tspackages/core/test/unit/utils/test-concurrency.test.tsscripts/mcp-wrapper.jsscripts/plugin-generators.mjsscripts/run-tests.mjsscripts/test-concurrency.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/core/test/unit/auth/mcp-url.test.ts
- scripts/mcp-wrapper.js
- packages/core/src/auth/mcp-url.ts
- scripts/plugin-generators.mjs
- scripts/run-tests.mjs
| OpenCode: run setup --harness opencode for auth, then install --harness opencode for skills/MCP config. | ||
| Auth: only setup/login may open a browser.`) | ||
| OpenCode: setup --harness opencode authenticates AND writes its skills/MCP config; install --harness opencode re-runs that direct config. | ||
| After switch-org, the harness you pass to --harness has its direct MCP config refreshed on the spot; other direct-config harnesses (OpenCode, Pi, fallback CLI installs) need a later setup/install to re-bake the new org's token. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scope the post-switch refresh claim to direct-config harnesses.
Line 85 states that the harness passed to --harness has its direct MCP config refreshed on the spot. That is only true for harnesses outside PLUGIN_OWNED_HARNESSES. For claude, codex, and antigravity, setup is auth-only, so switch-org refreshes no on-disk MCP config for the selected harness; the user must reconnect the native plugin instead. Note that packages/core/test/integration/cli-help.test.ts line 41 asserts this wording, so update the assertion together with the text.
📝 Proposed wording
- After switch-org, the harness you pass to --harness has its direct MCP config refreshed on the spot; other direct-config harnesses (OpenCode, Pi, fallback CLI installs) need a later setup/install to re-bake the new org's token.
+ After switch-org, a direct-config harness passed to --harness (OpenCode, Pi, fallback CLI installs) has its MCP config refreshed on the spot; Claude/Codex/Antigravity native plugins must be reconnected, and other direct-config harnesses need a later setup/install to re-bake the new org's token.🤖 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/core/src/cli.ts` at line 85, Update the switch-org help text in the
CLI to claim immediate MCP config refresh only for direct-config harnesses
outside PLUGIN_OWNED_HARNESSES; state that claude, codex, and antigravity
require native plugin reconnection instead. Update the corresponding assertion
in cli-help.test.ts to match the revised wording.
| assert.strictEqual(loadCredentials()?.organizationId, 'org-456', 'shared credentials must be switched') | ||
| const cfg = readJsonFile<Record<string, any>>(join(tmpDir, '.config', 'opencode', 'opencode.jsonc')) | ||
| const server = (cfg?.mcp as Record<string, { url?: string; headers?: Record<string, string> }>)?.['nsolid-console'] | ||
| assert.ok(server, 'openocode.jsonc must contain an nsolid-console server') |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the filename in the assertion message.
The message reads openocode.jsonc. The file is opencode.jsonc. The wrong name appears in the failure output and slows down diagnosis.
📝 Proposed fix
- assert.ok(server, 'openocode.jsonc must contain an nsolid-console server')
+ assert.ok(server, 'opencode.jsonc must contain an nsolid-console server')📝 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.
| assert.ok(server, 'openocode.jsonc must contain an nsolid-console server') | |
| assert.ok(server, 'opencode.jsonc must contain an nsolid-console server') |
🤖 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/core/test/integration/installer.test.ts` at line 285, Correct the
assertion message in the installer integration test to reference opencode.jsonc
instead of openocode.jsonc, keeping the assertion and server validation
unchanged.
| variables.AUTH_TOKEN = credentials.serviceToken | ||
| variables.AUTH_ORG_ID = credentials.organizationId | ||
| const derivedMcpUrl = deriveMcpUrlFromConsoleUrl(credentials.consoleUrl) | ||
| const derivedMcpUrl = deriveMcpUrlFromConsoleUrl(credentials.consoleUrl, credentials.organizationId) |
There was a problem hiding this comment.
Also in scripts/mcp-wrapper.js:54
Credentials written by the previous release already contain a non-empty alias-based mcpUrl. Both paths continue to prefer that stored value, so updating and reinstalling preserves the dead alias endpoint for precisely the existing users this fix targets. Detect/migrate values matching the old derivation while preserving genuine custom overrides.
| // Pure, unit-tested orchestration of the switch-org output + exit code. | ||
| const { buildSwitchOrgOutcome } = await import('./utils/format.js') | ||
| const outcome = buildSwitchOrgOutcome({ | ||
| success: result.success, |
There was a problem hiding this comment.
BundleDescriptor.auth is optional, so a valid --bundle without auth makes setup() succeed without OAuth and leaves authSucceeded false. The outcome logic still exits 0 and prints “Still signed in to org: (unknown).” Require OAuth for switch-org, or treat authSucceeded: false as failure.
| // appear to hang indefinitely rather than just run slowly — this bit the | ||
| // husky pre-commit hook (`pnpm test`) in practice. Cap it, with an escape | ||
| // hatch for anyone who wants to override it (e.g. a beefier CI runner). | ||
| const DEFAULT_CONCURRENCY = Math.max(1, Math.floor(os.cpus().length / 2)) |
There was a problem hiding this comment.
os.cpus() can reflect host CPUs rather than a Docker/Kubernetes CPU quota. A container limited to 1–2 CPUs on a 32-core host will still run 16 test workers, recreating the resource starvation this cap is meant to avoid. Node 22 supports os.availableParallelism() for this purpose.
3 participants
Credentials are a single shared file with no way to re-authenticate into a different org once signed in. Adds
nsolid-plugin switch-org(plus anns-switch-orgskill) that forces a fresh OAuth round-trip, reusing NodeSource's existing multi-org sign-in picker, and reports harness-specific follow-up guidance (reconnect vs. re-run install) depending on whether a native plugin, a fallback install, or both are present for the target harness.Also fixes mcpUrl derivation to always use the org's UUID instead of consoleUrl's hostname label, which broke for aliased consoles — the alias's .mcp. subdomain was never a real ingress route.
Caps node --test concurrency in scripts/run-tests.mjs, which had no limit and could starve an already-busy machine badly enough to look hung rather than just slow — hit in practice via the pre-commit hook.
Fixes two skill-linker tests that hardcoded a regular Windows symlink for their fixtures, which needs elevated privileges Windows doesn't grant by default — unlike the junction links the production code actually creates on Windows, which they now match.
This addresses https://app.zenhub.com/workspaces/engineering-repos-5b366986b7758b104ffdb5e5/issues/zh/1629
Summary by CodeRabbit
switch-orgcommand and new skill.