feat(sdk-cli): dotcms agent setup — one command to connect an IDE to dotCMS - #37416
feat(sdk-cli): dotcms agent setup — one command to connect an IDE to dotCMS#37416fmontes wants to merge 46 commits into
dotcms agent setup — one command to connect an IDE to dotCMS#37416Conversation
Spec-Kit PR 1 of 2 for #37390 — spec only, no implementation. Specifies a `dotcms agent setup` command that collapses the four manual steps needed to connect an AI coding agent to dotCMS (find the admin panel, mint a token, hand-edit an IDE config, install skills) into one command across seven agent targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…semantics Addresses two gaps found reviewing the spec. Setup proved the token was valid but never that the MCP server actually starts, so a stale package cache or unsupported runtime would produce a green summary and a broken agent. Confirmation now runs by default (FR-024a-e, SC-002a): it launches the server as configured, confirms it reports its tools, and reports a non-start distinctly from a credential failure without rolling back written configs. Writing spans up to seven files and nothing said what happens when one fails after others succeeded. Setup now continues, reports per-target outcomes, and exits non-zero on any failure (FR-020a-d, SC-006a). Also: FR-013 annotated as a structural constraint rather than a testable requirement, FR-023a covers project scope outside version control, concurrent writes documented as a known limitation, and SC-001/SC-002 labelled design intent rather than automated gates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records four /speckit-clarify decisions. Folder is now the default scope, not the user account: one dotcms entry per config file, multiple instances via different folders. Because that makes the credential-into-a-repo path the default, FR-023 is strengthened so assume-yes takes the safe answer on the gitignore offer rather than skipping it. status and remove are cut from this release; only agent setup ships. User Story 5 withdrawn, FR-029/030/031 and SC-007/008 retired, and everything that leaned on those commands reworded. The agent sub-command group stays as the seam for adding them later. The written entry references the latest published server rather than pinning a version (FR-020e). The instance address plus one auth mode are the only required inputs; supply both and setup completes without prompting, terminal or not (FR-003i-l). Targets default to every detected editor and scope to the folder, so neither blocks a run. assume-yes and force govern confirmation prompts only and can never suppress a prompt for a missing required input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No diagnostic mode ships — no verbose flag, no debug output, no log file. Recorded as a deliberate decision rather than an omission, with FR-032a requiring every failure message to be self-sufficient, since "re-run with more detail" is not available as a remedy. Terminology normalized to "token" for the thing minted, supplied, verified and written; a username and password are named as such rather than called "credentials". No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The /speckit-plan ADR gate found that ADR-0019 (accepted) requires SDK packages to compare the instance's dotCMS version against their own and warn, fail-open, when the instance is older. dotcms ships from libs/sdk/, is published by the SDK release pipeline, and is date-lockstep versioned, so the requirement applies. FR-005a reuses the response already fetched for the reachability check, so it costs no additional request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 0 + Phase 1 output of /speckit-plan for #37390. plan.md records the Constitution Check (PASS, three declared TDD exceptions needing sign-off) and the ADR Alignment gate. ADR-0019 is the one binding ADR and produced two conflicts: the unpinned @latest server reference is a justified deviation recorded in Complexity Tracking, and the missing CMS compatibility check is complied with, now FR-005a. research.md resolves ten unknowns. Three change the plan: the nx.json jest include entry is required rather than optional, @dotcms/mcp-server is outside date-lockstep with no release workflow in this repo, and chmod 0600 is a no-op on Windows so FR-021 is POSIX-only and must be reported honestly rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first layout put targets/, verify/, skills/ and asks.ts at the top level beside genuinely shared api/ and utils/. That reads fine at one command and degrades at two: create-app and the dotCLI port would spray files into the same directories with nothing marking which files serve which command. Restructured to shared/ plus commands/<group>/, mirroring what the user types. A second group is a new directory and zero edits to existing files. Adds an explicit one-way dependency rule — groups may import from shared/, never from each other, and shared/ never imports from a group — worth enforcing with ESLint rather than convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged main and re-checked create-app. Decision reversed: extract, not copy. http.ts sets Authorization headers and follows redirects -- the exact surface axios was removed from this workspace over, after semgrep found its Node adapter leaks Proxy-Authorization across a redirect to a non-proxied origin. Two copies of that code is where the next such fix lands in one package and not the other. That, not DRY, is the argument. http.ts, fetch-retry.ts, result.ts, the URL helpers and the endpoint constants move to a new internal lib core-web/libs/cli-shared -- no package.json, never published, inlined into both consumers by esbuild. Blast radius is 8 one-line import edits in create-app; its two spec files travel with their code. libs/cli-shared sits outside libs/sdk/ deliberately: the deploy action treats every direct child of libs/sdk/ as a publishable @dotcms/<dirname>. Token minting is NOT extracted -- create-app's getAuthToken returns Result values that are pre-formatted chalk strings, so presentation is entangled with the call. Only the endpoint constants are shared. Extraction lands as its own commit at the head of this PR, gated on create-app's 12 spec files passing with zero edits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
http.ts sets Authorization headers and follows redirects -- the surface axios was removed from this workspace over, after semgrep found its Node adapter leaks Proxy-Authorization across a redirect to a non-proxied origin. A second copy in the new dotcms CLI is where the next fix of that kind lands in one package and not the other. That, not DRY, is the case for extracting. Generated with nx g @nx/js:library. Named for what it is -- the HTTP layer for talking to a dotCMS instance -- not for the CLIs that are its only consumers today. It sits outside libs/sdk/ deliberately: the deploy action treats every direct child of libs/sdk/ as a publishable @dotcms/<dirname>. package.json is private, so publishing is refused; consumers inline it via esbuild. Moves http.ts, fetch-retry.ts, result.ts and their two spec files out of create-app, adds endpoints.ts so the two CLIs cannot drift on API paths, and repoints 8 imports. Three things this surfaced that the plan had wrong: - Two spec files import the moved code, so the gate is "no test logic changes", not "zero test-file edits". - jest.preset.js does not map tsconfig paths, so create-app needed a moduleNameMapper entry or the alias compiles but never resolves. - enforceBuildableLibDependency forbids a buildable lib importing a non-buildable one, so libs/http needed a tsc build target. Verified: sdk-create-app 132 tests/10 suites, http 34 tests/2 suites, both builds succeed, lint clean, @dotcms/http absent from the bundle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the Nx generator stub. Covers the four modules, the security reason the code is shared rather than copied, and the two wiring steps a consumer must not miss -- the jest moduleNameMapper entry and the buildable-lib constraint -- both of which fail confusingly when skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing the throwaway probe alias left a trailing comma after the @dotcms/http entry, making the file invalid JSON. TypeScript and esbuild parse tsconfig leniently, so every build and test still passed and the breakage was invisible -- a strict JSON reader would have failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Minting happens before configurations are written, so a run that mints and then fails leaves a real 365-day token on the instance that was never displayed and never recorded. Re-running mints another, and nothing identifies or revokes them. Accepted rather than mitigated. The alternative that actually recovers the credential is printing it, which contradicts FR-022 -- a rule User Story 3 rates P1. Orphans expire within a year and the failure requires an already-broken environment. Recorded in Assumptions rather than left silent, so a reviewer sees the trade-off instead of rediscovering it later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 1 and 2 of tasks.md -- scaffolding and types only. No behaviour, so nothing here precedes a TDD gate. Generated with nx g @nx/js:library, then given create-app's proven esbuild config: esm, bundled, runtime deps external, and the #!/usr/bin/env node banner that exists only in the production configuration. The generator named the project "cli". Renamed to "sdk-cli": the SDK release action builds --projects='sdk-*', so a project named "cli" would never be built or published. jest.config.ts carries the moduleNameMapper for @dotcms/http -- this workspace does not map tsconfig paths in jest.preset.js, so without it every suite touching the alias fails on module resolution instead of on the behaviour under test, and the Red gate could not be trusted. eslint.config.mjs enforces the one-way dependency rule: command groups may import shared/, shared/ may not import a command group, and groups may not import each other. Verified: production build emits the shebang, lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writes the User Story 1 tests: URL resolution and reachability, the ADR-0019 compatibility warning, token minting and verification, target registry and per-platform paths, fresh-file JSON writing, the ordering guarantee, the connection check, skills delegation, and the summary. Signature-only stubs accompany them so the failures are assertion failures rather than module-resolution errors -- T024 rejects the latter as an invalid Red. Running Red before requesting approval surfaced four vacuous tests that passed against a throwing stub, because rejects.toThrow() and not.toThrow(/../) are satisfied by any error including "not implemented". They asserted nothing and could never have gone Red. Tightened to assert positively: the message must name the address, the token rejection, or the conflicting options. Red: 50 failed / 0 passed across 8 suites, 0 module-resolution errors. HALTING at T023 -- developer approval of the test set, including explicit sign-off on the three test types plan.md declares cannot be implemented. No implementation code written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
connect.spec.ts: the child/spawn mock was rebuilt in all five it blocks. Hoisted to beforeEach. setup.spec.ts: process.chdir() was wrong, and not only stylistically. chdir is process-global and Jest reuses a worker across spec files, so one suite was moving the working directory out from under the others -- the run reported 50 tests where 55 exist. Five tests were never executing. Replaced with an injected working directory: RunOptions.cwd and configPath(scope, cwd). That removes the global mutation and improves the production design, since folder-scope resolution is now a parameter rather than ambient state. Kept real temp directories rather than switching to a mocked filesystem. "Nothing was written" is the assertion the ordering suite turns on, and only a real empty directory settles it; a mocked fs would prove one API was not called, which a write through any other path slips past. plan.md's Test Strategy said "mocked filesystem" and was wrong -- corrected. Red now: 55 failed / 0 passed across 8 suites, 0 module-resolution errors, 0 vacuous passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Approval (T023) and Red (T024) recorded, then implementation to Green. Adds the shared layer -- named errors, env reading, redaction, instance resolution with the ADR-0019 compatibility warning, token mint/verify over @dotcms/http, and generic merge/write/chmod -- plus the agent group: the seven-target registry, the JSON writer, the connection check, skills delegation, the summary, and the commander wiring. The ordering guarantee is the load-bearing part: nothing touches the filesystem until the token has been verified, and --yes/--force cannot disable that. Two mechanical fixes to the approved tests, changing no assertion. Under ts-jest the node:os and node:child_process namespace objects are non-configurable, so jest.spyOn on them throws "Cannot redefine property"; replaced with module-factory mocks. And the summary now writes via process.stdout rather than console.log, which the workspace lint rule reserves for warn/error. Verified: 55/55 tests, 8 suites; lint clean; production build emits the shebang with @dotcms/http inlined. Smoke-tested the real binary -- conflicting auth and unknown target exit 2, unreachable exits 1, and no file is written on any of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds 22 tests for User Story 2: JSON merge preserving other servers and unrelated settings verbatim, a similarly-named sibling left alone, replace-not-duplicate, malformed input as a named error with the file untouched, the Codex TOML target round-tripping comments and unrelated tables, overwrite confirmation, and partial-failure semantics. Installs smol-toml (1.8.0) and stubs the TOML target. Only 9 of the 22 are Red. The other 13 passed on arrival because US1's implementation already covered them -- config-file.ts was written with merge semantics and setup.ts with continue-on-error, both nominally US2 scope. Those are regression locks, not Red->Green tests, and the gate note in tasks.md says so rather than presenting a clean Red that is not there. RunOptions gains confirmOverwrite, injected so the FR-017 confirmation is testable without a terminal and setup.ts stays free of prompt mechanics. 77 tests total: 68 passing, 9 failing, 0 module-resolution errors. HALTING at T044. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ran targeted mutants against every behaviour that was implemented before its tests. Four were caught. Two survived with zero extra failures: disabling chmod, and making redact() return the raw secret. Neither was green-on-arrival -- both were entirely untested, and both are security requirements (FR-021/SC-004, FR-022/SC-005). Adds config-file.spec.ts and redact.spec.ts. The permission assertions are POSIX-guarded; the honesty assertion is not, and checks that permissionsApplied tracks the platform capability rather than being hard-coded true. Re-ran both mutants against the new tests: permissions now +3 failures, redaction +2, where both previously survived at +0. 85 tests: 76 passing, 9 failing (the genuine Phase 4 Red). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Implements the Codex writer with smol-toml, round-tripping comments and unrelated tables rather than regenerating the file -- replacing an existing [mcp_servers.dotcms] in place requires knowing where a table begins and ends, which is parsing by another name. Adds hasEntry() as a read-only check so FR-017's confirmation happens before anything is modified rather than as a rollback, and restrictFile() so the JSON and TOML writers share one permission path instead of drifting. setup.ts selects the writer from the registry's `format` field, so the flow still branches on nothing target-specific (FR-013). Mutation-checked the new code: dropping other servers and unrelated tables from the TOML writer takes failures 0 -> 3. 85/85 passing, lint clean, production build succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Folder scope is the default, so a token lands in a working directory on nearly every run. protectFromVersionControl() names every file it went into, offers exclusion, and warns where it cannot help. Three behaviours the tests pin down: - Not a git repository: the files are still named and warned as unprotected, rather than the step being silently skipped (FR-023a). Silence is how a token reaches a public repo. - A repo-root .mcp.json is conventionally COMMITTED, so excluding it is the unusual choice -- warned explicitly rather than quietly gitignored (FR-024). The one place the safe default is wrong for the workflow. - --yes supplies the SAFE answer (exclude) rather than bypassing the prompt, inverting the conventional meaning of -y (FR-023). Red first: all 9 new tests failed, 0 resolution errors. Then mutation-verified -- dropping the no-repo warning, the normally-committed warning, the decline check, or the dedup each takes exactly one test red. 94/94 passing, lint clean, production build succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…060-T072) Red first: 11 new tests, all failing, 0 resolution errors. Two rules that are easy to implement conventionally and wrongly, so both are pinned by tests and mutation-checked: - Prompting is driven by a MISSING REQUIRED INPUT, not by a mode. Supply the url and one auth mode and nothing is asked, terminal or not. A run does not become interactive merely because a terminal exists. - --yes governs CONFIRMATIONS ONLY. The usual reading, "assume defaults for everything", would silently skip a required input. Mutating resolveRequiredInputs to honour --yes that way takes one test red. shared/prompts.ts owns the rules; commands/agent/prompts.ts owns how to ask, behind a PromptPort. That split is what lets the rules be tested without a terminal. Uses inquirer's own prompt module rather than the five @inquirer/* sub-packages -- inquirer is already a declared dependency here and in create-app, and the sub-packages would add install weight for every npx user to no benefit. Fixed an unrealistic mock found during Red: the url prompt returned 'typed-url', which correctly failed validation, so the test died before its assertion. 105/105 passing, lint clean, production build succeeds. Binary verified on four non-interactive failure paths: all exit 2 in under 120ms with no file written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mutation-tested every behaviour implemented before its tests. Six mutants
were caught; two survived.
Hole 1 -- an assertion that could only fail on a platform we never run.
expect(permissionsApplied).toBe(CAN_RESTRICT) is true === true on POSIX,
so a hard-coded true satisfied it; the assertion could only bite on
Windows. Made the capability injectable (writeMerged({ canRestrict })) so
the claim is checked against a platform that cannot restrict, from any
platform.
Hole 2 -- nothing covered FR-003j/FR-010: omitting --agent must configure
every detected editor. Added three tests (detected set is used, empty
detection writes nothing and exits 0, default scope is the folder).
Both mutants now take a test red.
Also recorded a method note: hard-coding a literal scope produced a
COMPILE error, not a failing test -- TypeScript narrowed the type and
rejected the later comparison as provably false. The tell for an invalid
mutant is the collected test TOTAL changing, not the failure count. A
sweep must compare totals.
109/109 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… hole Fourteen more mutants across instance, auth, constants, json-target, connect, skills and ui. Thirteen caught, one survived. Removing the ^https?:// guard failed nothing: new URL() still rejects a bare host, so the existing test passed through that fallback. But new URL() parses ftp://, file:// and javascript: without complaint, so those would have reached fetch and been written into an editor's config with no test objecting. Added four tests -- three bad schemes plus plain http://localhost:8082, since local instances are the common case and must keep working. The mutant now takes 3 tests red. Also redid the compat-warning mutant in a valid form; the first attempt broke compilation. Everything else held, including the ones that are requirements rather than details: token in argv instead of the child environment, child not killed, 404 misclassified as a plain exit, skills invoked per-target, skills failure reported as success, summary saying "ready" after a failed connection, and unverified skills shown as installed. 113/113 passing, lint clean, production build succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
T081 error-message audit, written as a test rather than a one-off check. FR-032a makes "something failed" a defect, since there is no verbose mode to fall back on. Found two raw Error throws that named the target but offered no action -- replaced with NoConfigPathError. InvalidUrlError described the problem without the fix; now imperative. errors.spec.ts asserts every error names its subject, carries an action, is a sentence, leaks no secret, and is classified correctly for exit 1 vs 2. T082 proved no raw fetch error escapes: 12 cases across all three network call sites. T083 surfaced that ora and chalk were declared dependencies and externals that nothing imported -- every npx user installing them for nothing. That was a symptom: the connection check spawns npx and can run a minute on a cold cache with the CLI printing nothing. Added an onProgress port (mirroring promptPort) so setup.ts reports WHAT is happening and the command layer decides HOW -- a spinner on a terminal, plain lines in CI. Both dependencies now earn their place. Extended describeRequestFailure in @dotcms/http for ENOTFOUND, ECONNRESET and the TLS cases; "ENOTFOUND" is not a sentence. Kept them DIAGNOSTIC: the first attempt baked in advice and produced "Host not found - check the address... Check the address and that the instance is running." The caller owns the remedy, and a test now enforces that. T085 points the mcp-server README at the command while keeping the manual steps. Needed create-app's transformIgnorePatterns: [] -- chalk 5 and ora are ESM-only and Jest skips node_modules by default. sdk-cli 163, http 39, create-app 132. Lint clean, build succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cope
The publish step's idempotency guard was
`npm view "@dotcms/${sdk}@${VERSION}"`, deriving the package name from the
directory and hardcoding the scope. That held while every SDK was scoped.
It breaks on `dotcms`, the unscoped CLI: the guard could never match, so
the first publish succeeded and every re-run then tried to publish a
version that already existed and failed the release step -- the exact
stall the guard exists to prevent.
Reads `.name` from each package.json instead, which works for any naming
scheme. Same fix in the version-rewrite step, where sibling dependencies
were repointed under an assumed scope.
Considered replacing the loop with `nx release publish`, which knows each
project's real name. Rejected: it has no already-published semantics, so
it would 403 on a re-run and reintroduce the stall this guard prevents.
Left the examples/* loop alone -- examples import SDK libraries, never the
CLI binary, so the scoped assumption still holds there.
Verified: all five run blocks pass `bash -n`; simulated against a fake
dist containing both a scoped and an unscoped package, where the old
guard looked up a nonexistent @dotcms/cli and the new one finds dotcms;
and confirmed via `nx show projects --projects='sdk-*'` that sdk-cli is in
the set this action builds, with cli the only one of eleven whose npm
name differs from @dotcms/<dir>.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors create-app's verify-package pattern: nx:run-commands running a
bash script, dependsOn build, wired into test so it cannot be forgotten.
Asserts what unit tests structurally cannot see, since they run against
the source tree and this runs against the artifact npm would upload. Each
check is a defect that actually happened this session:
- shebang present, so `npx dotcms` has an interpreter
- bin points at a file that exists
- @dotcms/http is INLINED -- it is unpublished, so a surviving import
would be unresolvable for every user
- every declared dependency is actually imported (ora and chalk were
declared, externalised and unused: install weight for nothing)
- npm pack includes index.js and README.md
- the package name is still the unscoped `dotcms` the release guard
resolves by reading .name
Verified the verifier: declaring an unused dependency, renaming the
package, stripping the shebang, and leaving @dotcms/http as an import
each turn it red.
Also corrected research.md R1. The shebang trap is narrower than
documented -- defaultConfiguration is production, so a bare `nx build`
does carry it. The trap needs an explicit non-production configuration,
which is why the check asserts the artifact rather than the command.
Final gate: sdk-cli 163, http 39, create-app 132; lint clean on all three.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I stated in a commit message and in the README that @dotcms/http cannot
be published because package.json is private. That is wrong. npm 10's
check is:
if (workspace && manifest.private) throw EPRIVATE
It is gated on `workspace`. It stops `npm publish -w <pkg>`; it does not
stop a direct `cd libs/http && npm publish`, which exits 0 and packs the
tarball. Confirmed against npm's source and by dry-run.
What actually keeps the library unpublished is its LOCATION: the SDK
release action iterates the direct children of core-web/libs/sdk/, and
this sits outside that directory with no nx-release-publish target.
private: true stays as a declaration of intent, but it is not the
mechanism, and saying otherwise was a false assurance.
Since the real guarantee is a directory boundary rather than a flag,
verify-package.sh now asserts it: no package marked private may sit under
libs/sdk/. Planting one there turns the check red -- verified. That moves
the invariant out of my head and into the build.
README corrected. Recorded as R17.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wired Two defects, both in commands/agent/prompts.ts -- the one module the rules-vs-mechanics split leaves untested. 1. type: 'list' does not exist in inquirer 13; it was renamed to 'select' in v9/v10. An unregistered type does not throw, the prompt just hangs, which is why nothing surfaced until the CLI was run by hand and stopped at "How should we authenticate?". 2. chooseTargets was never called. esbuild tree-shook it -- checkbox appeared 0 times in the bundle. T070 claimed FR-010's interactive picker was implemented; the flow silently configured every detected editor instead, including ones the developer never chose. I had marked that task done. prompts.spec.ts now asserts every prompt type this adapter uses is registered by inquirer, with no TTY required, and fails if 'list' comes back. PromptPort gained multiSelect so target selection is a RULE in shared/ and testable; five tests cover asking, pre-checking the detected set, honouring a deselection, staying quiet when --agent is given, and the no-port fallback. While fixing it I reintroduced R18's bug -- re-deriving targets from the registry by id instead of using the objects detectTargets() returned. The FR-027 test caught it. Second occurrence, now recorded as a pattern. 178 tests, 15 suites, lint clean, verify-package 8/8. Bundle carries all five prompt types and zero 'list'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ilure Two defects found running the CLI by hand. The spinner never stopped on the error path. progress.done() ran only after a successful runSetup, so a thrown error printed underneath a spinner that kept spinning -- the run was over and the terminal still said it was working. The action now stops it in a catch, and marks the failed step with .fail() rather than silently clearing it. A rejected token ended the run after one attempt. FR-007 specified three attempts for a rejected username and password but said nothing about a supplied token, which is the same user error: one mistyped paste should not end the run. Authentication and verification now sit inside a three-attempt loop. Only a REJECTION is retried. An unreachable instance, a TLS failure or a 500 are not, because retyping a credential cannot fix them -- and neither is anything retried without a prompt port, since a script has no way to retype. Both are tested. Changed resolveRequiredInputs so `interactive` defaults to Boolean(port) instead of canPrompt(). The caller already decides whether a terminal exists; reading that global again second-guessed it and made the function untestable without a TTY. 183 tests, lint clean, verify-package 8/8. Verified by hand that a non-interactive bad token still fails fast: exit 1, no files, no hang. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FR-007 gave three attempts when a username and password fail to mint, but said nothing about a supplied token that fails to verify. Running the CLI by hand showed why that gap matters: one mistyped paste ended the run. The two are the same user error and now share the retry. FR-007a limits it to rejections -- an unreachable instance or a TLS error is not retried, because retyping a credential cannot fix it. FR-007b keeps a non-interactive run failing at the first attempt, since a script has nowhere to retype. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…version
Two bugs in checkReachable, one raised by the developer and one found
while fixing it.
1. Any answer counted as reachable. httpGet threw on non-2xx, so a host
that 404s that path surfaced as "Could not reach ... HTTP 404" --
wrong twice, since it WAS reached and the real problem is the address.
Worse, a host answering 200 with anything at all (proxy, CDN error
page, HTML site root) passed as healthy, and the run wrote a config
whose agent then failed every call with nothing to explain why.
Now acceptAnyStatus makes the status data, and the payload is
fingerprinted on entity.config carrying one of releaseInfo, license,
cluster, emailRegex or languages. NotADotCmsInstanceError is distinct
from unreachable.
2. FR-005a was a silent no-op. readVersion looked at entity.version,
entity.dotcmsVersion and entity.releaseVersion; none exist. The
version is at entity.config.releaseInfo.version ("26.09.03-01"). The
ADR-0019 warning could never fire, and its tests passed because they
called compatibilityWarning() directly while checkReachable fed it
null forever.
Root cause for both: every appconfiguration mock was { entity: {} }, a
body dotCMS cannot produce. A fixture the real system would never emit
tests nothing but itself. Added one shared fixture trimmed from the real
demo.dotcms.com response; adopting it turned six existing tests red,
which is how much the old mock was carrying.
190 tests. Verified against real hosts: example.com now says it is not a
dotCMS instance, demo.dotcms.com proceeds to token verification.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…alive FR-005 only required that something answer. A proxy, CDN error page or unrelated app answering 200 passed the check, and setup would write a configuration pointing at it -- producing an agent that fails every call later with nothing to explain why. FR-005b requires confirming the response is a dotCMS configuration, and reporting a non-dotCMS host distinctly from an unreachable one. The two have different remedies, so collapsing them costs the developer the one piece of information that helps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes, both from running the CLI by hand. Order. resolveRequiredInputs asked for the address AND the credential in one pass, then checkReachable ran afterwards -- so a developer typed a username and password against an address that was never going to work, and learned it only after the effort. Split resolveInstanceUrl out: the address is resolved and checked against the live instance first, and nothing asks for a credential until the instance is confirmed. Message. "answered (HTTP 404) but is not a dotCMS instance -- /api/v1/appconfiguration did not return a dotCMS configuration. Check the address points at the dotCMS server itself, not a proxy, CDN or site root." explained our method to someone who needs a verdict. Now: "https://example.com is not a valid dotCMS instance. Check the address." A test pins that: the message must not mention the endpoint, the payload shape, proxies or CDNs, and must stay under 120 characters. Three tests cover the ordering directly -- no password prompt when the address is not dotCMS, none when it is unreachable, and the address is always asked for first. 193 tests, lint clean. Verified by hand: --user and --password supplied against example.com never reach authentication, exit 1, nothing written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1. --skip-mcp silently killed the skills install and the summary. The early return skipped everything downstream, so --skip-mcp alone did nothing and said nothing. It now skips WRITING only: skills are chosen from the selected targets rather than from successful writes, each target is reported as skipped, and the connection check is skipped implicitly since there is no configuration to prove. 2. "ftp://x" suggested "https://ftp://x" -- the suggestion stripped only https?://. Now strips any scheme. 3. Every failed target claimed a permissions problem on macOS. permissionsApplied is false for a failed write because nothing was written, not because the platform refused. Gated on the outcome. 4. TOML comments were dropped. smol-toml does not round-trip them, and Codex's config.toml is hand-maintained. The writer now parses to VALIDATE and splices to WRITE: only our own tables are replaced textually, everything else survives byte-for-byte. A comment immediately before the next table belongs to that table -- the first attempt ate it, and there is a test for exactly that. 5. Pre-existing directories were re-chmodded to 0700, widening a .cursor the developer had set to 0500. mkdir(recursive) reports what it created; only that is restricted now. 6. Env-only auth conflict named flags the developer never typed. The error now names the source actually used. 7. Duplicate --agent inflated the count. Dedup now happens before the count, not inside the write loop. Fixing 1 exposed an eighth: with writing skipped, the version-control step still announced "these files now contain an access token" about files never created. Narrowed to written/replaced. Five of the eight are one shape -- a value computed for one purpose reused where its meaning differs. Recorded as R22. 214 tests. Each fix mutation-checked: reverting it turns its test red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tead
The comment-preservation rewrite hand-emitted command, args and env
behind a cast:
buildEntry(...) as { command: string; args: string[]; env: ... }
That cast asserts the entry has exactly those keys, so omitting `type`
was not an error -- the compiler had been told the field does not exist.
Every JSON target still wrote type: "stdio"; codex alone stopped. Any
field added to buildEntry later would have vanished the same way, with no
compile error and no failing test.
Fixed structurally rather than by adding the field back. renderEntry now
serializes whatever buildEntry returns via smol-toml's stringify.
Comments are irrelevant for that block because it is generated; the
splice still protects everything outside it, so the file keeps its
annotations and the entry keeps every field.
The guard is a structural assertion, not a field list:
expect(doc.mcp_servers.dotcms).toEqual(buildEntry(codex(), URL, TOKEN))
so a new field is covered the moment it exists. Reverting to the
hand-emitted version turns three tests red -- verified.
Second time today a cast hid a defect the type system had been tracking
(R20 was re-deriving an object from an id). Recorded as R23: where a value
must mirror another, compare it to that value, not to a description of it.
217 tests, lint clean. Verified on disk: comments preserved AND
type = "stdio" present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng or missing FR-003b said the conflict error must name the conflicting "options". The conflict can come from environment variables, and reporting --authToken to someone who set DOTCMS_AUTH_TOKEN sends them hunting for a flag they never typed. Now: name the inputs actually used. FR-003c1 is new. Nothing said the skip options are independent, and the implementation read --skip-mcp as "skip everything downstream", so it silently installed no skills and printed no summary. The only permitted implication is FR-024b's: with nothing written there is no configuration for the connection check to prove. FR-016a is new. "Preserves everything else exactly" was read as data only, so the TOML writer re-serialized a parsed document and deleted the developer's comments. For a hand-maintained format that is data loss even though every value survives. Not changed, because the spec was already right and the code was not: FR-021 says "any directory setup CREATES" (the implementation re-chmodded pre-existing ones), FR-023 says "every file it PLACED A TOKEN INTO" (it named files that were only skipped), and FR-024b already covered the connection check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @fmontes's task in 2m 54s —— View job Review:
|
| // real dotCMS is anyone asked for a credential: a password typed against a wrong | ||
| // address is wasted effort, and the failure would land after the work rather than | ||
| // before it. | ||
| const url = await resolveInstanceUrl(opts, opts.promptPort); |
There was a problem hiding this comment.
🔴 [P1] setup.ts:95 surface compatibility warning from checkReachable
Current code:
const url = await resolveInstanceUrl(opts, opts.promptPort);
step(`Checking ${url}`);
await checkReachable(url);Problem: Discards InstanceInfo.version; compatibilityWarning() has no production caller.
Fix:
const info = await checkReachable(url);
const warning = compatibilityWarning(info.version, toolVersion);
if (warning) step(warning);There was a problem hiding this comment.
Fixed in c3c2a1c. checkReachable's result is now kept and compared against TOOL_VERSION, surfaced via onWarning (live) and warnings[] (summary). TOOL_VERSION reads package.json at build time — the release pipeline rewrites .version before building, so a literal would report 0.2.0 from a package published as 26.9.x.
|
|
||
| opts.onAuthRetry?.((error as Error).message, attempt, MAX_AUTH_ATTEMPTS); | ||
| // Ask again from scratch: the url is settled, the credential is what was wrong. | ||
| inputs = await resolveRequiredInputs({ url, cwd: opts.cwd }, opts.promptPort); |
There was a problem hiding this comment.
🟡 [P2] setup.ts:135 re-prompt fresh credential on auth retry
Current code:
inputs = await resolveRequiredInputs({ url, cwd: opts.cwd }, opts.promptPort);Problem: Retry re-reads rejected env credential, exhausting attempts without fresh prompt.
Fix:
delete process.env[ENV_KEYS.authToken];
delete process.env[ENV_KEYS.password];
inputs = await resolveRequiredInputs({ url, cwd: opts.cwd }, opts.promptPort);There was a problem hiding this comment.
Fixed in c3c2a1c, though not with delete process.env[...] — mutating the environment would also affect anything else reading it. Instead extracted promptForAuth(port), which consults neither options nor env, and the retry calls that. For a credential the instance just rejected, the only useful source is the human.
| method: 'initialize', | ||
| params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'dotcms', version: '0' } } | ||
| })}\n` | ||
| ); |
There was a problem hiding this comment.
🔴 [P1] connect.ts:109 MCP handshake omits notifications/initialized before tools/list
Current code:
);
child.stdin?.write(`${JSON.stringify({ jsonrpc: '2.0', id: 2, method: 'tools/list' })}\n`);Problem: Missing notifications/initialized notification; tools/list may never be answered.
Fix:
);
child.stdin?.write(`${JSON.stringify({ jsonrpc: '2.0', method: 'notifications/initialized' })}\n`);
child.stdin?.write(`${JSON.stringify({ jsonrpc: '2.0', id: 2, method: 'tools/list' })}\n`);The MCP protocol requires the initialized notification before non-initialize requests; the server's own smoke test (apps/mcp-server/src/smoke/server-boot.spec.ts:104-112) sends initialize → notifications/initialized → tools/list. Without it the 60s timer can fire on every otherwise-successful setup, reporting connection: 'failed' and exit code 1 (FR-024a).
There was a problem hiding this comment.
Valid, but deliberately not in this commit — scoped to the two findings above. Tracking as follow-up.
| // before it. | ||
| const url = await resolveInstanceUrl(opts, opts.promptPort); | ||
| step(`Checking ${url}`); | ||
| await checkReachable(url); |
There was a problem hiding this comment.
🔴 [P1] setup.ts:95 discarded checkReachable result leaves FR-005a warning unimplemented
Current code:
const url = await resolveInstanceUrl(opts, opts.promptPort);
step(`Checking ${url}`);
await checkReachable(url);Problem: Return value discarded; compatibilityWarning() has no production caller.
Fix:
const info = await checkReachable(url);
const warning = compatibilityWarning(info.version, toolVersion);
if (warning) step(warning);checkReachable returns InstanceInfo { version } (instance.ts:12,71) and compatibilityWarning (instance.ts:117) is exported and unit-tested for the ADR-0019 / FR-005a warning, but grep finds no non-test caller — the version plumbing is dead code and the developer is never told the instance is older than the tool.
There was a problem hiding this comment.
Duplicate of the finding above — fixed in c3c2a1c.
| if (/Unsupported engine|requires Node|SyntaxError|Unexpected token/i.test(stderr)) { | ||
| return 'runtime-unsupported'; | ||
| } | ||
| return code === null ? 'exited' : 'exited'; |
There was a problem hiding this comment.
🟡 [P2] connect.ts:20 dead ternary returns 'exited' on both branches
Current code:
return code === null ? 'exited' : 'exited';Problem: Both branches return the same value; signal-killed case is unlabeled.
Fix:
return 'exited';Or give the code === null (killed-by-signal) case a distinct label per FR-024c's requirement to name distinguishable failure causes. No functional bug, but the branch is clearly unfinished.
There was a problem hiding this comment.
Valid, but deliberately not in this commit — scoped to the two findings above. Tracking as follow-up.
…-read env **1. The compatibility warning was dead code.** `compatibilityWarning()` existed and was unit-tested, `readVersion()` existed and was unit-tested, and nothing called one with the other. A developer on a dotCMS older than the tool was never told. Same shape as the `entity.version` defect earlier in this branch: the parts were tested, the join was not. `setup.ts` now keeps the `checkReachable` result and compares it against `TOOL_VERSION`, surfacing the notice through `onWarning` and the run's `warnings[]`. Both ends are wired: `agent/index.ts` prints it live, `ui.ts` repeats it in the summary. Writing the producer without the consumer is exactly the bug being fixed here, so the summary render is tested too. `TOOL_VERSION` reads `package.json` at build time rather than a literal — the SDK release pipeline rewrites `.version` to the release tag before building, so a hardcoded value would report `0.2.0` from a package published as `26.9.x`. The import is *named*, not default: a default import makes esbuild inline the entire manifest (dependency list, publishConfig) into the shipped bundle. **2. An auth retry re-read the credential the instance had just rejected.** The retry called `resolveRequiredInputs`, which consults options and the environment before prompting. So a token from `DOTCMS_AUTH_TOKEN` — or a password from `DOTCMS_PASSWORD` — was re-read unchanged and re-submitted until the three attempts ran out, with no prompt ever shown. For env users the retry feature did nothing at all. Extracted `promptForAuth(port)`, which consults neither options nor the environment, and the retry path calls that. For a credential the instance has just refused, the only useful source is the human. Version set to 0.2.0. Both fixes mutation-verified: reverting either turns its own tests red and no others, with the collected total unchanged. 225 tests, lint clean, verify-package green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`nx format:check` (Maven `format-test`) failed the Frontend Unit Tests job on 26 files across this branch. Whitespace only, no behaviour change: prettier collapses single-element JSON arrays onto one line and rewraps a number of long expressions. The pre-commit hook runs `nx format:write` against *staged* files only, so a file formatted at commit time can still drift when a later prettier-relevant edit lands elsewhere — the gate is repo-wide. Verified with a full `format:check`, then re-ran the three affected suites: sdk-cli 225, http 39, sdk-create-app 132. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
dotbot code review:
No new actionable bugs were found in the current changes, but 2 prior unresolved dotbot findings still apply, so the patch remains incorrect. Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads. reviewed by dotbot · meta/muse-spark-1.3 · medium |
|
dotbot code review:
No new actionable bugs were found in the current changes, but 2 prior unresolved dotbot findings still apply, so the patch remains incorrect. Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads. reviewed by dotbot · ~z-ai/glm-latest · medium |
Spec-Kit PR 2 of 2 — implementation. The spec is #37392; its commits are the shared ancestor here and disappear from this diff once it merges.
Fixes #37390
What it solves
Connecting an AI coding agent to dotCMS took four manual steps: find the admin panel, mint an API token, hand-edit whichever config file your editor reads, install the skills separately. Seven editors, no two storing it the same way. The
@dotcms/mcp-serverREADME covered two of them.Three questions — instance, credentials, editors — and the agent is connected.
How it works
Four properties do most of the work:
Nothing is written until the token verifies. A rejected token leaves no file, no directory, no skills install — so a bad credential can't produce seven configs that fail confusingly later.
--yes/--forcecannot disable it.The instance is validated before you're asked for anything.
/api/v1/appconfigurationis fingerprinted for a real dotCMS payload; a proxy or CDN answering 200 is rejected. Typing a password against a wrong address was wasted effort.Merge, never clobber. These files already hold other MCP servers. Only the
dotcmskey is touched; an unparseable file is a named error that leaves it untouched. For Codex the writer parses to validate and splices to write, so hand-written comments survive.The run proves the agent connects. After writing, it launches the configured server and confirms it lists tools. A valid token is not proof the server starts.
Command surface
--urlplus one auth mode are the only required inputs; supply both and it runs unprompted, terminal or not. Targets default to every detected editor, scope to the current folder. The two auth modes are mutually exclusive — passing both is a usage error, not a silent preference.--agent(repeatable) ·-g/--global·--skip-mcp·--skip-skills·--skip-verify·-y·--force. Exit0all succeeded,1a target or the connection check failed,2usage error.Targets: Claude Code, Cursor, VS Code/Copilot, Codex, Antigravity, Devin, OpenCode.
New library:
@dotcms/httpcreate-appand this CLI both authenticate against a dotCMS instance.http.tssetsAuthorizationheaders and follows redirects — the exact surface axios was removed from this workspace over (its Node adapter leakedProxy-Authorizationacross a redirect, #37264). Two copies of that code is where the next such fix lands in one package and not the other. That, not DRY, is the case for extracting it.core-web/libs/http/— internal, never published:http.tshttpGet/httpPostover nativefetch; bearer auth,AbortControllertimeout,HttpErrorcarrying an HTTP status or a transport codefetch-retry.tsdescribeRequestFailure()— turnsENOTFOUNDinto a sentence, kept diagnostic so callers own the remedyresult.tsResult<T,E>endpoints.ts/api/v1paths, so the two CLIs can't driftConsumers:
@dotcms/create-app(moved off its own copies — 8 import edits, its 132 tests pass unchanged) anddotcms. Both bundle it in at build time, so nothing new reaches npm.It sits outside
libs/sdk/deliberately: the SDK release action publishes every direct child of that directory.private: trueis intent, not the mechanism — npm only enforcesprivatefor workspace publishes — soverify-packageasserts the boundary instead.What to review for
shared/vscommands/agent/— the package is organised by command group, not technical layer, becausecreate-appand the dotCLI port fold in as siblings later. An ESLint rule enforces one-way imports.targets/registry.ts— every per-editor difference is data. Adding an eighth editor is one object literal.setup.ts, and its test. It's the thing most worth breaking on purpose to check.Also in here
npm view "@dotcms/${dir}@${version}"— scope hardcoded, name from the directory.dotcmsis unscoped, so the guard could never match: first publish succeeds, every re-run then fails the release. Now reads.name. Of eleven SDK packages, this is the only one that differs.apps/mcp-server/README.mdpoints at the command; the manual steps stay.libs/sdk/cli/**andlibs/http/**added tonx.json's jest include — without it a project gets notesttarget at all.Testing
sdk-cli217 ·http39 ·create-app132 — all green, lint clean, production build emits the shebang.nx run sdk-cli:verify-packageasserts eight packaging invariants against the artifact npm would upload, and runs as part ofnx test. Each check corresponds to a defect that actually happened: a missing shebang,@dotcms/httpsurviving as an unresolvable import, declared-but-unused dependencies, an internal library drifting underlibs/sdk/.Fixes were mutation-verified rather than assumed: reverting one turns its test red. That found three holes where a test existed but could not fail.
Known limitations
agent statusandagent removeare not in this release. Re-running setup replaces a stale entry.chmoddoesn't touch Windows ACLs, and the summary says so rather than implying protection.@latest). Justified deviation from ADR-0019 —@dotcms/mcp-serverisn't date-lockstep, so there's no matched version to pin to.Before merge
dotcms agent setup#37392 approved — this is gated on the spec, not on its mergedotcms@0.0.21download volume before first publish (381/month;^0.0.21pins exactly, so only barenpm i dotcmschanges)quickstart.mdrun, including real editors connecting🤖 Generated with Claude Code