From d8122cd85668e0d064022dde40c28ad9c92377f0 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Fri, 5 Jun 2026 16:07:41 +0200 Subject: [PATCH 1/4] feat(unic-spec-review): self-contained auth and preflight (/setup-confluence, /spec-doctor) The plugin had no way to set up credentials or verify prerequisites: both command stubs were unimplemented and the script layer was absent. A user who installed only unic-spec-review could not configure Confluence or preflight a review without first installing another plugin. Changes: - Add parseArgs to scripts/lib/args.mjs (CLI parser for the setup wizard) - Vendor scripts/setup-confluence.mjs by copying from unic-pr-review (no cross-import) - Add scripts/spec-doctor.mjs: Confluence credential + connectivity preflight (injectable deps) - Replace commands/setup-confluence.md stub with the full interactive wizard - Replace commands/spec-doctor.md stub with the orchestrator (script + Figma/Playwright MCP checks; absent MCPs are loud explicit failures with remediation) - Add unit tests for the wizard and the Confluence preflight logic; no live services - Add CHANGELOG bullets under [Unreleased] Refs #203 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../.claude-plugin/marketplace.json | 2 +- .../.claude-plugin/plugin.json | 2 +- .../claude-code/unic-spec-review/CHANGELOG.md | 15 ++ .../commands/setup-confluence.md | 52 ++++- .../unic-spec-review/commands/spec-doctor.md | 78 ++++++- .../claude-code/unic-spec-review/package.json | 2 +- .../unic-spec-review/scripts/lib/args.mjs | 42 ++++ .../scripts/setup-confluence.mjs | 118 ++++++++++ .../unic-spec-review/scripts/spec-doctor.mjs | 205 ++++++++++++++++++ .../tests/setup-confluence.test.mjs | 166 ++++++++++++++ .../tests/spec-doctor.test.mjs | 147 +++++++++++++ 11 files changed, 819 insertions(+), 10 deletions(-) create mode 100644 apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs create mode 100644 apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs create mode 100644 apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs create mode 100644 apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs diff --git a/apps/claude-code/unic-spec-review/.claude-plugin/marketplace.json b/apps/claude-code/unic-spec-review/.claude-plugin/marketplace.json index 88634c5e..c12de210 100644 --- a/apps/claude-code/unic-spec-review/.claude-plugin/marketplace.json +++ b/apps/claude-code/unic-spec-review/.claude-plugin/marketplace.json @@ -21,7 +21,7 @@ "name": "unic-spec-review", "source": "./", "tags": ["productivity", "quality"], - "version": "0.1.1" + "version": "0.1.2" } ] } diff --git a/apps/claude-code/unic-spec-review/.claude-plugin/plugin.json b/apps/claude-code/unic-spec-review/.claude-plugin/plugin.json index 03dbd62b..96e3a9c2 100644 --- a/apps/claude-code/unic-spec-review/.claude-plugin/plugin.json +++ b/apps/claude-code/unic-spec-review/.claude-plugin/plugin.json @@ -10,5 +10,5 @@ "keywords": ["spec-review", "confluence", "figma", "adversarial-review", "six-hats", "unic"], "license": "LGPL-3.0-or-later", "name": "unic-spec-review", - "version": "0.1.1" + "version": "0.1.2" } diff --git a/apps/claude-code/unic-spec-review/CHANGELOG.md b/apps/claude-code/unic-spec-review/CHANGELOG.md index cdf45147..0e4bfa17 100644 --- a/apps/claude-code/unic-spec-review/CHANGELOG.md +++ b/apps/claude-code/unic-spec-review/CHANGELOG.md @@ -10,8 +10,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Breaking - (none) +### Added +- (none) + +### Fixed +- (none) + +## [0.1.2] — 2026-06-05 + +### Breaking +- (none) + ### Added - Cover two previously untested paths flagged in PR #210 review: `loadAtlassianCreds` preferring env vars over a present credentials file, and `fetchConfluencePage` capping the stripped excerpt at 800 characters. +- Add `/setup-confluence` command: interactive credential wizard writing `~/.unic-confluence.json`, vendored by copying from `unic-pr-review` (ADR-0001 self-containment, no cross-import). +- Add `/spec-doctor` command: preflight checks for Confluence credentials and connectivity, Figma Dev Mode MCP, and Playwright MCP; absent MCPs are reported as explicit loud failures with remediation, never a silent skip. +- Add `parseArgs` to the `args` module (CLI parser shared by the setup wizard) alongside the existing `parseReviewSpecArgs`. +- Unit tests for the vendored `setup-confluence` wizard (`writeConfluenceCreds`, `isEnvConfigured`) and the Confluence preflight logic (`checkConfluence`, `runSpecDoctorCredentials`, `mapPingError`, `realPing`) with injected `homedir`/`platform`/`fetch`/`loadCreds`; no live services. ### Fixed - Reject non-http(s) URLs in arg parsing, link classification, and validate `pageTitle`/`pageUrl` in the report-renderer CLI entry, so ftp/file/mailto inputs no longer slip through and missing report fields no longer render as literal `undefined`. diff --git a/apps/claude-code/unic-spec-review/commands/setup-confluence.md b/apps/claude-code/unic-spec-review/commands/setup-confluence.md index a6b5d7c0..bd774b11 100644 --- a/apps/claude-code/unic-spec-review/commands/setup-confluence.md +++ b/apps/claude-code/unic-spec-review/commands/setup-confluence.md @@ -1,9 +1,53 @@ --- -description: Interactive setup wizard that writes Confluence credentials to ~/.unic-confluence.json for unic-spec-review. +allowed-tools: Bash(node *), Bash(test *), Bash(echo *) +argument-hint: (no arguments) +description: Interactive setup wizard — writes ~/.unic-confluence.json with Confluence credentials --- -# Setup Confluence +# unic-spec-review:setup-confluence -> ⚠️ Scaffold stub: not yet implemented. Behaviour is specified in the [PRD](../docs/issues/unic-spec-review/PRD.md) (#200); the command body lands with its implementation slice. +Guides you through creating `~/.unic-confluence.json` so the plugin can reach Confluence. -This plugin ships its own Confluence credential wizard so it is fully self-contained: it never depends on another plugin being installed or set up. It writes to the shared `~/.unic-confluence.json` convention (also honouring `CONFLUENCE_*` env vars), so a user with `unic-pr-review` already configured does not configure Confluence twice. See `AGENTS.md` for the locked design. +## Step 1 — Check for existing env-var configuration + +If `CONFLUENCE_URL`, `CONFLUENCE_USER`, and `CONFLUENCE_TOKEN` are all set, the file is not required. +Tell the user and offer to exit without writing. If they want to write the file anyway, continue. + +## Step 2 — Check for an existing credential file + +```sh +test -f ~/.unic-confluence.json && echo "exists" +``` + +If the file exists, show the current values (redact the token — print only `****`): + +```sh +node -e " +const c = JSON.parse(require('fs').readFileSync(process.env.HOME + '/.unic-confluence.json', 'utf8')); +console.log(JSON.stringify({ url: c.url, username: c.username, token: '****', jiraUrl: c.jiraUrl ?? null }, null, 2)); +" +``` + +Ask whether to overwrite. If the user chooses not to overwrite, stop. + +## Step 3 — Collect credentials + +Ask the user for: + +1. **Confluence base URL** — e.g. `https://your-org.atlassian.net` +2. **Username** — typically the Atlassian account email address +3. **API token** — generate one at + +## Step 4 — Write the credential file + +```sh +node "${CLAUDE_PLUGIN_ROOT}/scripts/setup-confluence.mjs" --url "" --username "" --token "" +``` + +## Step 5 — Confirm + +Tell the user: + +- `~/.unic-confluence.json` has been written with access restricted to the owner (chmod 600) on macOS/Linux +- On Windows: file permissions must be restricted manually (the wizard printed a reminder) +- They can run `/unic-spec-review:spec-doctor` to verify the full setup diff --git a/apps/claude-code/unic-spec-review/commands/spec-doctor.md b/apps/claude-code/unic-spec-review/commands/spec-doctor.md index 92c95dcc..6d2c73ab 100644 --- a/apps/claude-code/unic-spec-review/commands/spec-doctor.md +++ b/apps/claude-code/unic-spec-review/commands/spec-doctor.md @@ -1,9 +1,81 @@ --- +allowed-tools: Bash(node *) +argument-hint: (no arguments) description: Verify unic-spec-review prerequisites (Confluence credentials, Figma Dev Mode MCP, and Playwright MCP). --- -# Spec Doctor +# unic-spec-review:spec-doctor -> ⚠️ Scaffold stub: not yet implemented. Behaviour is specified in the [PRD](../docs/issues/unic-spec-review/PRD.md) (#200); the command body lands with its implementation slice. +Runs a preflight check for all unic-spec-review prerequisites so you can diagnose setup issues before running a review. -This preflight command will check that Confluence credentials (`~/.unic-confluence.json`), the Figma Dev Mode MCP, and the Playwright MCP are present and reachable before a review. See `AGENTS.md` for the locked design. +Checks performed: + +1. Atlassian credentials are present (`~/.unic-confluence.json` or `CONFLUENCE_*` env vars) +2. Confluence is reachable via HTTP (Basic auth) +3. Figma Dev Mode MCP is connected +4. Playwright MCP is connected + +## Step 1 — Print the spec-doctor header + +Print: + +``` +unic-spec-review spec-doctor +───────────────────────────────── +``` + +## Step 2 — Run the Confluence credential and connectivity check + +```sh +node "${CLAUDE_PLUGIN_ROOT}/scripts/spec-doctor.mjs" +``` + +Show the script output verbatim (one ✓/✗ line per check). + +Record whether the script exited 0 (all credential/connectivity checks passed) or 1 (failed). + +## Step 3 — Check for Figma Dev Mode MCP + +Determine whether a Figma Dev Mode MCP tool is available in the current Claude Code session +by checking the active tool set for tools whose names match `mcp__figma*` or any tool clearly +from a Figma Dev Mode MCP server. + +- If the Figma Dev Mode MCP is **available**: print `✓ Figma Dev Mode MCP — connected` +- If the Figma Dev Mode MCP is **NOT available**: print the following explicit failure (not a silent skip): + ``` + ✗ Figma Dev Mode MCP — not connected + Remediation: Enable the Figma Dev Mode MCP in your Claude Code MCP settings. + See https://help.figma.com/hc/en-us/articles/32132100888087 for setup instructions. + ``` + Record whether this check passed or failed. + +## Step 4 — Check for Playwright MCP + +Determine whether a Playwright MCP tool is available in the current Claude Code session +by checking the active tool set for tools whose names match `mcp__playwright*` or any tool +clearly from a Playwright MCP server. + +- If the Playwright MCP is **available**: print `✓ Playwright MCP — connected` +- If the Playwright MCP is **NOT available**: print the following explicit failure (not a silent skip): + ``` + ✗ Playwright MCP — not connected + Remediation: Enable the Playwright MCP in your Claude Code MCP settings. + Example config: https://github.com/microsoft/playwright-mcp + ``` + Record whether this check passed or failed. + +## Step 5 — Print overall result + +Print: + +``` +───────────────────────────────── +``` + +Then: + +- If **all checks passed**: print `All checks passed.` +- If **any check failed**: print `One or more checks failed — see lines marked ✗ above.` + +Missing MCPs are hard failures. Do not summarise them as warnings or caveats; +they must appear as ✗ lines. diff --git a/apps/claude-code/unic-spec-review/package.json b/apps/claude-code/unic-spec-review/package.json index 8e563da4..e05699e9 100644 --- a/apps/claude-code/unic-spec-review/package.json +++ b/apps/claude-code/unic-spec-review/package.json @@ -1,6 +1,6 @@ { "name": "unic-spec-review", - "version": "0.1.1", + "version": "0.1.2", "private": true, "license": "LGPL-3.0-or-later", "type": "module", diff --git a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs index f7058d79..60235039 100644 --- a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs +++ b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs @@ -38,3 +38,45 @@ export function parseReviewSpecArgs(input) { } return { urls, post } } + +/** + * Shared CLI argument parser for setup scripts. + * + * Accepts both `--key=value` and `--key value` forms. Bare positional args + * are ignored. A `--flag` with no following value (last arg, or followed by + * another `--flag`) throws — the previous silent-drop behaviour produced + * misleading "X is required" errors when the user actually did pass the flag. + * + * Boolean flags (presence-only, no value) must be declared in `options.booleanFlags`. + * They are recorded as `''` (empty string) when present so callers can use `'key' in result`. + * + * @param {string[]} args + * @param {{ booleanFlags?: Set }} [options] + * @returns {Record} + */ +export function parseArgs(args, options = {}) { + const booleans = options.booleanFlags ?? new Set() + /** @type {Record} */ + const result = {} + for (let i = 0; i < args.length; i++) { + const m = args[i].match(/^--([^=]+)=(.*)$/) + if (m) { + result[m[1]] = m[2] + continue + } + if (args[i].startsWith('--')) { + const key = args[i].slice(2) + if (booleans.has(key)) { + result[key] = '' + continue + } + const next = args[i + 1] + if (next === undefined || next.startsWith('--')) { + throw new Error(`${args[i]} requires a value`) + } + result[key] = next + i++ + } + } + return result +} diff --git a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs new file mode 100644 index 00000000..98b6c43d --- /dev/null +++ b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs @@ -0,0 +1,118 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: LGPL-3.0-or-later +// @ts-check +// Copyright © 2026 Unic + +/** + * setup-confluence.mjs — write ~/.unic-confluence.json with the user's + * Confluence credentials. Pure file-writer: values arrive as CLI args; the + * conversational prompting happens in commands/setup-confluence.md. + */ + +import { + chmodSync as realChmod, + existsSync as realExistsSync, + readFileSync as realReadFile, + renameSync as realRename, + writeFileSync as realWriteFile, +} from 'node:fs' +import os from 'node:os' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { parseArgs } from './lib/args.mjs' + +/** + * @typedef {Object} WriteDeps + * @property {string} [homedir] + * @property {string} [platform] + * @property {(path: string) => boolean} [exists] + * @property {(path: string, encoding: BufferEncoding) => string} [readFile] + * @property {(path: string, data: string, encoding: BufferEncoding) => void} [writeFile] + * @property {(oldPath: string, newPath: string) => void} [rename] + * @property {(path: string, mode: number) => void} [chmod] + * @property {(message: string) => void} [warn] + */ + +/** + * Write ~/.unic-confluence.json with the given credentials. Preserves any + * existing fields (e.g. `jiraUrl` from :setup-jira) so re-running to rotate + * a token does not silently drop data. If the existing file is unparseable + * the writer warns and overwrites; non-syntax read errors (EACCES, etc.) + * propagate so callers can surface them rather than silently losing data. + * + * @param {string} url + * @param {string} username + * @param {string} token + * @param {WriteDeps} [deps] + * @returns {{ path: string }} + */ +export function writeConfluenceCreds(url, username, token, deps = {}) { + const home = deps.homedir ?? os.homedir() + if (!home) throw new Error('could not determine home directory (HOME / USERPROFILE unset)') + const platform = deps.platform ?? process.platform + const exists = deps.exists ?? realExistsSync + const read = deps.readFile ?? realReadFile + const write = deps.writeFile ?? realWriteFile + const rename = deps.rename ?? realRename + const chmod = deps.chmod ?? realChmod + const warn = deps.warn ?? ((m) => process.stderr.write(`${m}\n`)) + + const path = join(home, '.unic-confluence.json') + let preserved = {} + if (exists(path)) { + const raw = read(path, 'utf8') + try { + const existing = JSON.parse(raw) + if (existing && typeof existing === 'object') preserved = existing + } catch (err) { + if (!(err instanceof SyntaxError)) throw err + warn(`${path} contains invalid JSON — overwriting (any prior jiraUrl will be lost).`) + } + } + const payload = { ...preserved, url, username, token } + const tmp = `${path}.tmp` + write(tmp, JSON.stringify(payload, null, 2), 'utf8') + if (platform === 'win32') { + warn( + `Windows detected — skipping chmod 600 on ${path}. Restrict file access manually, e.g.:\n icacls "${path}" /inheritance:r /grant:r "%USERNAME%:F"` + ) + } else { + chmod(tmp, 0o600) + } + rename(tmp, path) + return { path } +} + +/** + * True when CONFLUENCE_URL, CONFLUENCE_USER and CONFLUENCE_TOKEN are all set. + * + * @param {Record} env + * @returns {boolean} + */ +export function isEnvConfigured(env) { + return Boolean(env.CONFLUENCE_URL && env.CONFLUENCE_USER && env.CONFLUENCE_TOKEN) +} + +async function main() { + let args + try { + args = parseArgs(process.argv.slice(2)) + } catch (err) { + process.stderr.write(`setup-confluence: ${err instanceof Error ? err.message : String(err)}\n`) + process.exit(1) + } + const { url, username, token } = args + if (!url || !username || !token) { + process.stderr.write('setup-confluence: --url, --username and --token are all required\n') + process.exit(1) + } + const { path } = writeConfluenceCreds(url, username, token) + process.stdout.write(`Written: ${path}\n`) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((err) => { + process.stderr.write(`setup-confluence: unexpected error: ${err?.message ?? String(err)}\n`) + process.exit(1) + }) +} diff --git a/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs new file mode 100644 index 00000000..359674a9 --- /dev/null +++ b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs @@ -0,0 +1,205 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: LGPL-3.0-or-later +// @ts-check +// Copyright © 2026 Unic + +/** + * spec-doctor.mjs — Confluence credential + connectivity preflight for + * unic-spec-review. + * + * This script covers only the parts of the preflight that can run inside a + * Node.js subprocess: that Atlassian credentials load, and that Confluence is + * reachable via Basic auth. The Figma Dev Mode MCP and Playwright MCP checks + * cannot be observed from a subprocess and are performed by the command + * orchestrator (commands/spec-doctor.md). + * + * The Confluence predicate accepts an injectable fetcher (Ping) so unit tests + * can stub it without mocking global fetch. + */ + +import { Buffer } from 'node:buffer' +import { pathToFileURL } from 'node:url' +import { loadAtlassianCreds } from './lib/credentials.mjs' + +/** @import { AtlassianCreds } from './lib/credentials.mjs' */ + +/** + * Discriminated by `kind`: + * - 'http' — fetch resolved; `status` is the HTTP response code. + * - 'transport-error' — fetch rejected (invalid URL, wrong scheme, timeout, + * network error); `error` carries the failure message. + * + * @typedef {{ kind: 'http', status: number } | { kind: 'transport-error', error: string }} PingResult + */ + +/** + * @typedef {(url: string, headers: Record) => Promise} Ping + */ + +/** + * @param {PingResult} r + * @returns {boolean} + */ +function isPingOk(r) { + return r.kind === 'http' && r.status >= 200 && r.status < 300 +} + +/** + * @typedef {Object} CheckResult + * @property {boolean} ok + * @property {string} detail + * @property {boolean} [skipped] + */ + +/** + * Predicate: Confluence is reachable with the configured credentials. + * @param {AtlassianCreds} creds + * @param {Ping} ping + * @returns {Promise} + */ +export async function checkConfluence(creds, ping) { + const host = safeHost(creds.url) + const url = `${stripTrailingSlash(creds.url)}/wiki/rest/api/space?limit=1` + const headers = { Authorization: basicAuth(creds.username, creds.token), Accept: 'application/json' } + const r = await ping(url, headers) + if (r.kind === 'transport-error') { + return { ok: false, detail: `Confluence ${host} unreachable: ${r.error}` } + } + if (!isPingOk(r)) { + return { ok: false, detail: `Confluence ${host} returned HTTP ${r.status}` } + } + return { ok: true, detail: `Confluence reachable (${host})` } +} + +/** + * @param {string} u + */ +function stripTrailingSlash(u) { + return u.endsWith('/') ? u.slice(0, -1) : u +} + +/** + * @param {string} u + */ +function safeHost(u) { + try { + return new URL(u).host + } catch { + return u + } +} + +/** + * @param {string} user + * @param {string} token + */ +function basicAuth(user, token) { + return `Basic ${Buffer.from(`${user}:${token}`).toString('base64')}` +} + +export const PING_TIMEOUT_MS = 10_000 + +/** + * Map a fetch rejection to a human-readable error message. Recognises + * `AbortSignal.timeout`'s `TimeoutError` and emits a friendly fixed message + * so spec-doctor output stays consistent across Node versions. + * + * @param {unknown} err + * @returns {string} + */ +export function mapPingError(err) { + if (err instanceof Error && err.name === 'TimeoutError') { + return `Request timed out after ${PING_TIMEOUT_MS / 1000}s` + } + return err instanceof Error ? err.message : String(err) +} + +/** + * Default fetcher: GET via global fetch with a 10 s timeout. Handles both + * https:// and http:// URLs. Returns the discriminated PingResult — an HTTP + * result on resolution, a transport-error on any rejection (invalid URL, + * wrong scheme, timeout, network error). + * + * Exported for unit testing of error paths (e.g. malformed URL, timeout). + * @type {Ping} + */ +export function realPing(url, headers) { + return fetch(url, { + method: 'GET', + headers, + signal: AbortSignal.timeout(PING_TIMEOUT_MS), + }) + .then((res) => /** @type {PingResult} */ ({ kind: 'http', status: res.status })) + .catch((err) => /** @type {PingResult} */ ({ kind: 'transport-error', error: mapPingError(err) })) +} + +/** + * @param {CheckResult} result + * @param {string} label + * @returns {string} + */ +function formatLine(result, label) { + let glyph = '✗' + if (result.skipped) glyph = '○' + else if (result.ok) glyph = '✓' + return `${glyph} ${label} — ${result.detail}` +} + +/** + * @typedef {Object} RunSpecDoctorCredsDeps + * @property {Ping} [ping] + * @property {() => (AtlassianCreds|null)} [loadCreds] + */ + +/** + * Run the Confluence credential + connectivity checks and return the rendered + * output + overall status. Exported so unit tests can inject stubs without + * patching the module system. + * @internal + * @param {RunSpecDoctorCredsDeps} [deps] + * @returns {Promise<{ok: boolean, output: string }>} + */ +export async function runSpecDoctorCredentials(deps = {}) { + const ping = deps.ping ?? realPing + const loadCreds = deps.loadCreds ?? loadAtlassianCreds + + const lines = [] + let allOk = true + + let creds = null + let credsLoadError = null + try { + creds = loadCreds() + } catch (err) { + credsLoadError = err instanceof Error ? err.message : String(err) + } + + if (credsLoadError) { + lines.push(`✗ Atlassian credentials — credential file unreadable: ${credsLoadError}`) + allOk = false + } else if (!creds) { + lines.push('✗ Atlassian credentials — neither env vars nor ~/.unic-confluence.json found') + allOk = false + } + + if (creds) { + const conf = await checkConfluence(creds, ping) + lines.push(formatLine(conf, 'Confluence')) + if (!conf.ok) allOk = false + } + + return { ok: allOk, output: `${lines.join('\n')}\n` } +} + +async function main() { + const { ok, output } = await runSpecDoctorCredentials() + process.stdout.write(output) + process.exit(ok ? 0 : 1) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((err) => { + process.stderr.write(`spec-doctor: unexpected error: ${err?.stack ?? err?.message ?? String(err)}\n`) + process.exit(1) + }) +} diff --git a/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs new file mode 100644 index 00000000..d70abed4 --- /dev/null +++ b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs @@ -0,0 +1,166 @@ +// @ts-check +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright © 2026 Unic + +import assert from 'node:assert/strict' +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, it } from 'node:test' +import { isEnvConfigured, writeConfluenceCreds } from '../scripts/setup-confluence.mjs' + +let _seq = 0 +function tempDir() { + const p = join(tmpdir(), `setup-confluence-test-${Date.now()}-${++_seq}`) + mkdirSync(p, { recursive: true }) + return p +} + +describe('writeConfluenceCreds', () => { + it('happy path — writes correct JSON to temp home and chmods 600 on linux', () => { + const home = tempDir() + /** @type {{ p: string, m: number }[]} */ + const chmodCalls = [] + const { path } = writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', { + homedir: home, + platform: 'linux', + chmod: (p, m) => chmodCalls.push({ p, m }), + }) + const content = JSON.parse(readFileSync(path, 'utf8')) + assert.equal(content.url, 'https://x.atlassian.net') + assert.equal(content.username, 'u') + assert.equal(content.token, 'tok') + assert.equal(chmodCalls.length, 1) + assert.equal(chmodCalls[0].p, `${path}.tmp`) + assert.equal(chmodCalls[0].m, 0o600) + }) + + it('writes atomically via tmp + rename', () => { + const home = tempDir() + /** @type {string[]} */ + const writeOrder = [] + writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', { + homedir: home, + platform: 'linux', + writeFile: (p) => writeOrder.push(`write:${p}`), + rename: (from, to) => writeOrder.push(`rename:${from}->${to}`), + chmod: (p) => writeOrder.push(`chmod:${p}`), + }) + const target = join(home, '.unic-confluence.json') + assert.deepEqual(writeOrder, [`write:${target}.tmp`, `chmod:${target}.tmp`, `rename:${target}.tmp->${target}`]) + }) + + it('idempotent re-run — same file, no error on second call', () => { + const home = tempDir() + const opts = { homedir: home, platform: 'linux', chmod: () => {} } + writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', opts) + assert.doesNotThrow(() => writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', opts)) + const content = JSON.parse(readFileSync(join(home, '.unic-confluence.json'), 'utf8')) + assert.equal(content.url, 'https://x.atlassian.net') + }) + + it('preserves jiraUrl and other existing fields when re-running to rotate credentials', () => { + const home = tempDir() + const opts = { homedir: home, platform: 'linux', chmod: () => {} } + writeFileSync( + join(home, '.unic-confluence.json'), + JSON.stringify({ + url: 'https://x.atlassian.net', + username: 'u', + token: 'old', + jiraUrl: 'https://jira.atlassian.net', + bitbucketUrl: 'https://bitbucket.example.com', + }) + ) + writeConfluenceCreds('https://x.atlassian.net', 'u', 'new-tok', opts) + const content = JSON.parse(readFileSync(join(home, '.unic-confluence.json'), 'utf8')) + assert.equal(content.token, 'new-tok') + assert.equal(content.jiraUrl, 'https://jira.atlassian.net') + assert.equal(content.bitbucketUrl, 'https://bitbucket.example.com') + }) + + it('warns and overwrites when the existing file contains invalid JSON', () => { + const home = tempDir() + writeFileSync(join(home, '.unic-confluence.json'), 'not-valid-json') + /** @type {string[]} */ + const warns = [] + writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', { + homedir: home, + platform: 'linux', + chmod: () => {}, + warn: (m) => warns.push(m), + }) + assert.equal(warns.length, 1) + assert.match(warns[0], /invalid JSON/) + const content = JSON.parse(readFileSync(join(home, '.unic-confluence.json'), 'utf8')) + assert.equal(content.token, 'tok') + assert.equal(content.jiraUrl, undefined) + }) + + it('rethrows non-syntax read errors (e.g. EACCES) instead of silently dropping data', () => { + const home = tempDir() + assert.throws( + () => + writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', { + homedir: home, + platform: 'linux', + exists: () => true, + readFile: () => { + const err = /** @type {Error & { code?: string }} */ (new Error('EACCES: permission denied')) + err.code = 'EACCES' + throw err + }, + writeFile: () => { + throw new Error('write must not be called when read fails') + }, + chmod: () => {}, + }), + /EACCES/ + ) + }) + + it('Windows chmod warning branch — warn called with icacls hint, chmod skipped', () => { + const home = tempDir() + /** @type {string[]} */ + const warns = [] + /** @type {boolean[]} */ + const chmodCalled = [] + writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', { + homedir: home, + platform: 'win32', + warn: (msg) => warns.push(msg), + chmod: () => chmodCalled.push(true), + }) + assert.equal(chmodCalled.length, 0) + assert.equal(warns.length, 1) + assert.match(warns[0], /Windows/) + assert.match(warns[0], /icacls/) + }) + + it('throws when home cannot be determined', () => { + assert.throws( + () => writeConfluenceCreds('https://x', 'u', 't', { homedir: '', platform: 'linux', chmod: () => {} }), + /could not determine home directory/ + ) + }) +}) + +describe('isEnvConfigured', () => { + it('returns true when CONFLUENCE_URL, CONFLUENCE_USER and CONFLUENCE_TOKEN are all set', () => { + assert.equal( + isEnvConfigured({ + CONFLUENCE_URL: 'https://x.atlassian.net', + CONFLUENCE_USER: 'u', + CONFLUENCE_TOKEN: 't', + }), + true + ) + }) + + it('returns false when any of the three is missing', () => { + assert.equal(isEnvConfigured({ CONFLUENCE_URL: 'x', CONFLUENCE_USER: 'u' }), false) + assert.equal(isEnvConfigured({ CONFLUENCE_URL: 'x', CONFLUENCE_TOKEN: 't' }), false) + assert.equal(isEnvConfigured({ CONFLUENCE_USER: 'u', CONFLUENCE_TOKEN: 't' }), false) + assert.equal(isEnvConfigured({}), false) + }) +}) diff --git a/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs new file mode 100644 index 00000000..e4433392 --- /dev/null +++ b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs @@ -0,0 +1,147 @@ +// @ts-check +// SPDX-License-Identifier: LGPL-3.0-or-later +// Copyright © 2026 Unic + +import assert from 'node:assert/strict' +import { describe, it } from 'node:test' +import { + checkConfluence, + mapPingError, + PING_TIMEOUT_MS, + realPing, + runSpecDoctorCredentials, +} from '../scripts/spec-doctor.mjs' + +/** @import { Ping } from '../scripts/spec-doctor.mjs' */ + +/** @param {number} status @returns {Ping} */ +const pingHttp = (status) => async () => ({ kind: 'http', status }) + +/** @param {string} error @returns {Ping} */ +const pingError = (error) => async () => ({ kind: 'transport-error', error }) + +describe('checkConfluence', () => { + it('returns ok:true on 200', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(200)) + assert.equal(r.ok, true) + assert.match(r.detail, /example\.atlassian\.net/) + }) + + it('returns ok:false on 401', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 'bad', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(401)) + assert.equal(r.ok, false) + assert.match(r.detail, /401/) + }) + + it('returns ok:true on 299 (inclusive upper bound of 2xx)', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(299)) + assert.equal(r.ok, true) + }) + + it('returns ok:false on 199 (just below 2xx)', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(199)) + assert.equal(r.ok, false) + assert.match(r.detail, /199/) + }) + + it('returns ok:false on 300 (just above 2xx)', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(300)) + assert.equal(r.ok, false) + assert.match(r.detail, /300/) + }) + + it('returns ok:false when Confluence is unreachable (transport-error)', async () => { + const creds = { url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingError('ECONNREFUSED')) + assert.equal(r.ok, false) + assert.match(r.detail, /ECONNREFUSED/) + }) + + it('strips a trailing slash and pings the Confluence space endpoint with Basic auth', async () => { + const creds = { url: 'https://example.atlassian.net/', username: 'u', token: 't', jiraUrl: undefined } + /** @type {{ url?: string, headers?: Record }} */ + const captured = {} + /** @type {Ping} */ + const ping = async (url, headers) => { + captured.url = url + captured.headers = headers + return { kind: 'http', status: 200 } + } + await checkConfluence(creds, ping) + assert.equal(captured.url, 'https://example.atlassian.net/wiki/rest/api/space?limit=1') + assert.match(captured.headers?.Authorization ?? '', /^Basic /) + assert.equal(captured.headers?.Accept, 'application/json') + }) +}) + +describe('mapPingError', () => { + it('maps AbortSignal TimeoutError to a friendly fixed message', () => { + const err = new Error('aborted') + err.name = 'TimeoutError' + assert.equal(mapPingError(err), `Request timed out after ${PING_TIMEOUT_MS / 1000}s`) + }) + + it('uses the error message for non-timeout Errors', () => { + assert.equal(mapPingError(new Error('ECONNREFUSED')), 'ECONNREFUSED') + }) + + it('stringifies non-Error rejections', () => { + assert.equal(mapPingError('boom'), 'boom') + }) +}) + +describe('realPing', () => { + it('resolves to a transport-error when given a malformed URL', async () => { + const r = await realPing('not-a-valid-url', {}) + assert.equal(r.kind, 'transport-error') + if (r.kind === 'transport-error') assert.ok(r.error.length > 0) + }) +}) + +describe('runSpecDoctorCredentials', () => { + it('returns ok:true when credentials are present and Confluence is reachable (200)', async () => { + const { ok, output } = await runSpecDoctorCredentials({ + ping: pingHttp(200), + loadCreds: () => ({ url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined }), + }) + assert.equal(ok, true) + assert.match(output, /Confluence reachable/) + }) + + it('returns ok:false with a missing-credentials message when loadCreds returns null', async () => { + const { ok, output } = await runSpecDoctorCredentials({ + ping: pingHttp(200), + loadCreds: () => null, + }) + assert.equal(ok, false) + assert.match(output, /Atlassian credentials/) + assert.doesNotMatch(output, /Confluence/) + }) + + it('returns ok:false with an unreadable message when loadCreds throws', async () => { + const { ok, output } = await runSpecDoctorCredentials({ + ping: pingHttp(200), + loadCreds: () => { + throw new Error('~/.unic-confluence.json contains invalid JSON: Unexpected token') + }, + }) + assert.equal(ok, false) + assert.match(output, /credential file unreadable/) + assert.match(output, /invalid JSON/) + assert.doesNotMatch(output, /Confluence/) + }) + + it('returns ok:false when Confluence is unreachable (transport-error)', async () => { + const { ok, output } = await runSpecDoctorCredentials({ + ping: pingError('ECONNREFUSED'), + loadCreds: () => ({ url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined }), + }) + assert.equal(ok, false) + assert.match(output, /ECONNREFUSED/) + }) +}) From ce9223498ed1ac9267af55b32acb599268761feb Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Fri, 5 Jun 2026 16:21:35 +0200 Subject: [PATCH 2/4] fix(unic-spec-review): address PR #211 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add parseArgs test suite (7 cases) to args.test.mjs — covers all four behavioural branches including boolean flags and throw-on-missing-value - Add runSpecDoctorCredentials HTTP-error test (401 via pingHttp) - Remove dead CheckResult.skipped typedef property and formatLine skipped branch (YAGNI — nothing sets skipped: true in current code) - Fix setup-confluence.mjs top-level catch to include err?.stack for better debugging parity with spec-doctor.mjs - Fix process.env.HOME → require('os').homedir() in setup-confluence.md inline snippet for Windows-native portability - Broaden args.mjs module description to cover both exported parsers Tests: 101 pass / 0 fail (was 93) Co-Authored-By: Claude Sonnet 4.6 --- .../commands/setup-confluence.md | 2 +- .../unic-spec-review/scripts/lib/args.mjs | 9 ++-- .../scripts/setup-confluence.mjs | 2 +- .../unic-spec-review/scripts/spec-doctor.mjs | 5 +-- .../unic-spec-review/tests/args.test.mjs | 41 ++++++++++++++++++- .../tests/spec-doctor.test.mjs | 9 ++++ 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/apps/claude-code/unic-spec-review/commands/setup-confluence.md b/apps/claude-code/unic-spec-review/commands/setup-confluence.md index bd774b11..894bf978 100644 --- a/apps/claude-code/unic-spec-review/commands/setup-confluence.md +++ b/apps/claude-code/unic-spec-review/commands/setup-confluence.md @@ -23,7 +23,7 @@ If the file exists, show the current values (redact the token — print only `** ```sh node -e " -const c = JSON.parse(require('fs').readFileSync(process.env.HOME + '/.unic-confluence.json', 'utf8')); +const c = JSON.parse(require('fs').readFileSync(require('os').homedir() + '/.unic-confluence.json', 'utf8')); console.log(JSON.stringify({ url: c.url, username: c.username, token: '****', jiraUrl: c.jiraUrl ?? null }, null, 2)); " ``` diff --git a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs index 60235039..eb1c4bc3 100644 --- a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs +++ b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs @@ -3,11 +3,12 @@ // Copyright © 2026 Unic /** - * args.mjs - parse /review-spec command arguments. + * args.mjs — CLI argument parsers for unic-spec-review. * - * Accepts a raw argument string or a pre-split argv array. - * Tokens that parse as valid URLs go into `urls`; `--post` sets `post: true`; - * other flags and unrecognised tokens are silently ignored. + * - `parseReviewSpecArgs`: accepts a raw string or argv array; extracts http(s) + * URLs and the `--post` flag (for the /review-spec command). + * - `parseArgs`: key=value / key value flag parser for setup scripts; + * throws on a flag with no value rather than silently dropping it. */ /** diff --git a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs index 98b6c43d..5eab4cd1 100644 --- a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs +++ b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs @@ -112,7 +112,7 @@ async function main() { if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main().catch((err) => { - process.stderr.write(`setup-confluence: unexpected error: ${err?.message ?? String(err)}\n`) + process.stderr.write(`setup-confluence: unexpected error: ${err?.stack ?? err?.message ?? String(err)}\n`) process.exit(1) }) } diff --git a/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs index 359674a9..604dc481 100644 --- a/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs +++ b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs @@ -48,7 +48,6 @@ function isPingOk(r) { * @typedef {Object} CheckResult * @property {boolean} ok * @property {string} detail - * @property {boolean} [skipped] */ /** @@ -139,9 +138,7 @@ export function realPing(url, headers) { * @returns {string} */ function formatLine(result, label) { - let glyph = '✗' - if (result.skipped) glyph = '○' - else if (result.ok) glyph = '✓' + const glyph = result.ok ? '✓' : '✗' return `${glyph} ${label} — ${result.detail}` } diff --git a/apps/claude-code/unic-spec-review/tests/args.test.mjs b/apps/claude-code/unic-spec-review/tests/args.test.mjs index 108112b3..1cfcf101 100644 --- a/apps/claude-code/unic-spec-review/tests/args.test.mjs +++ b/apps/claude-code/unic-spec-review/tests/args.test.mjs @@ -4,7 +4,7 @@ import assert from 'node:assert/strict' import { describe, it } from 'node:test' -import { parseReviewSpecArgs } from '../scripts/lib/args.mjs' +import { parseArgs, parseReviewSpecArgs } from '../scripts/lib/args.mjs' describe('parseReviewSpecArgs', () => { it('parses a single URL from an array, post defaults to false', () => { @@ -76,3 +76,42 @@ describe('parseReviewSpecArgs', () => { }) }) }) + +describe('parseArgs', () => { + it('parses --key=value inline form', () => { + assert.deepEqual(parseArgs(['--url=https://x.atlassian.net', '--username=u']), { + url: 'https://x.atlassian.net', + username: 'u', + }) + }) + + it('parses --key value space-separated form', () => { + assert.deepEqual(parseArgs(['--url', 'https://x.atlassian.net', '--username', 'u']), { + url: 'https://x.atlassian.net', + username: 'u', + }) + }) + + it('records boolean flags as empty string (presence-only)', () => { + const result = parseArgs(['--dry-run', '--verbose'], { booleanFlags: new Set(['dry-run', 'verbose']) }) + assert.ok('dry-run' in result) + assert.ok('verbose' in result) + assert.equal(result['dry-run'], '') + }) + + it('ignores bare positional args (no leading --)', () => { + assert.deepEqual(parseArgs(['positional', '--url', 'https://x.example']), { url: 'https://x.example' }) + }) + + it('throws when --flag appears at end of args with no value', () => { + assert.throws(() => parseArgs(['--url']), /--url requires a value/) + }) + + it('throws when --flag is followed immediately by another --flag', () => { + assert.throws(() => parseArgs(['--url', '--username']), /--url requires a value/) + }) + + it('returns empty object for empty args array', () => { + assert.deepEqual(parseArgs([]), {}) + }) +}) diff --git a/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs index e4433392..69dcba03 100644 --- a/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs +++ b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs @@ -144,4 +144,13 @@ describe('runSpecDoctorCredentials', () => { assert.equal(ok, false) assert.match(output, /ECONNREFUSED/) }) + + it('returns ok:false when Confluence returns a non-2xx status (e.g. 401)', async () => { + const { ok, output } = await runSpecDoctorCredentials({ + ping: pingHttp(401), + loadCreds: () => ({ url: 'https://example.atlassian.net', username: 'u', token: 't', jiraUrl: undefined }), + }) + assert.equal(ok, false) + assert.match(output, /401/) + }) }) From 8c4673bfa4587668c0c5cce6aaab02f6b689f89f Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Fri, 5 Jun 2026 17:11:25 +0200 Subject: [PATCH 3/4] fix(unic-spec-review): remove em dashes from authored text (PR #211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #203's acceptance criterion forbids em dashes in authored text (except the mandated CHANGELOG version header). Copilot flagged the user-facing strings; this sweeps the rest too - JSDoc comments, command docs, spec-doctor/setup-confluence output, and test descriptions - so the whole slice complies. Replaced every " — " separator with " - ". The prior CHANGELOG note cited a now-removed org typography rule; reworded to ground the change in this slice's own acceptance criterion. Co-Authored-By: Claude Opus 4.8 --- .../claude-code/unic-spec-review/CHANGELOG.md | 2 +- .../commands/setup-confluence.md | 20 +++++++++---------- .../unic-spec-review/commands/spec-doctor.md | 20 +++++++++---------- .../unic-spec-review/scripts/lib/args.mjs | 4 ++-- .../scripts/setup-confluence.mjs | 6 +++--- .../unic-spec-review/scripts/spec-doctor.mjs | 14 ++++++------- .../tests/setup-confluence.test.mjs | 6 +++--- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/apps/claude-code/unic-spec-review/CHANGELOG.md b/apps/claude-code/unic-spec-review/CHANGELOG.md index 0e4bfa17..cd30461f 100644 --- a/apps/claude-code/unic-spec-review/CHANGELOG.md +++ b/apps/claude-code/unic-spec-review/CHANGELOG.md @@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Make `/review-spec` orchestration portable: write the scratch report JSON into the gitignored `.spec-review/` directory instead of the POSIX-only `/tmp` path (broke on Windows CI), and surface the structured `errors[].kind`/`errors[].message` from the fetch script so the real failure cause is shown. ### Documentation -- Correct stale cross-plugin references in code comments (drop the `render-summary.mjs`, `doctor.mjs`, and inaccurate `ADR-0001` citations), reword the `CONTEXT.md` status line so it no longer promises an unused `(S1)` per-term marking convention, and replace em dashes with hyphens in authored comments and messages per the org typography rule. +- Correct stale cross-plugin references in code comments (drop the `render-summary.mjs`, `doctor.mjs`, and inaccurate `ADR-0001` citations), reword the `CONTEXT.md` status line so it no longer promises an unused `(S1)` per-term marking convention, and replace em dashes with hyphens in authored comments, command docs, user-facing script output, and test descriptions, per this slice's acceptance criterion (no em dash in authored text except the mandated CHANGELOG version header). ## [0.1.1] — 2026-06-05 diff --git a/apps/claude-code/unic-spec-review/commands/setup-confluence.md b/apps/claude-code/unic-spec-review/commands/setup-confluence.md index 894bf978..6e645784 100644 --- a/apps/claude-code/unic-spec-review/commands/setup-confluence.md +++ b/apps/claude-code/unic-spec-review/commands/setup-confluence.md @@ -1,25 +1,25 @@ --- allowed-tools: Bash(node *), Bash(test *), Bash(echo *) argument-hint: (no arguments) -description: Interactive setup wizard — writes ~/.unic-confluence.json with Confluence credentials +description: Interactive setup wizard - writes ~/.unic-confluence.json with Confluence credentials --- # unic-spec-review:setup-confluence Guides you through creating `~/.unic-confluence.json` so the plugin can reach Confluence. -## Step 1 — Check for existing env-var configuration +## Step 1 - Check for existing env-var configuration If `CONFLUENCE_URL`, `CONFLUENCE_USER`, and `CONFLUENCE_TOKEN` are all set, the file is not required. Tell the user and offer to exit without writing. If they want to write the file anyway, continue. -## Step 2 — Check for an existing credential file +## Step 2 - Check for an existing credential file ```sh test -f ~/.unic-confluence.json && echo "exists" ``` -If the file exists, show the current values (redact the token — print only `****`): +If the file exists, show the current values (redact the token - print only `****`): ```sh node -e " @@ -30,21 +30,21 @@ console.log(JSON.stringify({ url: c.url, username: c.username, token: '****', ji Ask whether to overwrite. If the user chooses not to overwrite, stop. -## Step 3 — Collect credentials +## Step 3 - Collect credentials Ask the user for: -1. **Confluence base URL** — e.g. `https://your-org.atlassian.net` -2. **Username** — typically the Atlassian account email address -3. **API token** — generate one at +1. **Confluence base URL** - e.g. `https://your-org.atlassian.net` +2. **Username** - typically the Atlassian account email address +3. **API token** - generate one at -## Step 4 — Write the credential file +## Step 4 - Write the credential file ```sh node "${CLAUDE_PLUGIN_ROOT}/scripts/setup-confluence.mjs" --url "" --username "" --token "" ``` -## Step 5 — Confirm +## Step 5 - Confirm Tell the user: diff --git a/apps/claude-code/unic-spec-review/commands/spec-doctor.md b/apps/claude-code/unic-spec-review/commands/spec-doctor.md index 6d2c73ab..f4f43538 100644 --- a/apps/claude-code/unic-spec-review/commands/spec-doctor.md +++ b/apps/claude-code/unic-spec-review/commands/spec-doctor.md @@ -15,7 +15,7 @@ Checks performed: 3. Figma Dev Mode MCP is connected 4. Playwright MCP is connected -## Step 1 — Print the spec-doctor header +## Step 1 - Print the spec-doctor header Print: @@ -24,7 +24,7 @@ unic-spec-review spec-doctor ───────────────────────────────── ``` -## Step 2 — Run the Confluence credential and connectivity check +## Step 2 - Run the Confluence credential and connectivity check ```sh node "${CLAUDE_PLUGIN_ROOT}/scripts/spec-doctor.mjs" @@ -34,37 +34,37 @@ Show the script output verbatim (one ✓/✗ line per check). Record whether the script exited 0 (all credential/connectivity checks passed) or 1 (failed). -## Step 3 — Check for Figma Dev Mode MCP +## Step 3 - Check for Figma Dev Mode MCP Determine whether a Figma Dev Mode MCP tool is available in the current Claude Code session by checking the active tool set for tools whose names match `mcp__figma*` or any tool clearly from a Figma Dev Mode MCP server. -- If the Figma Dev Mode MCP is **available**: print `✓ Figma Dev Mode MCP — connected` +- If the Figma Dev Mode MCP is **available**: print `✓ Figma Dev Mode MCP - connected` - If the Figma Dev Mode MCP is **NOT available**: print the following explicit failure (not a silent skip): ``` - ✗ Figma Dev Mode MCP — not connected + ✗ Figma Dev Mode MCP - not connected Remediation: Enable the Figma Dev Mode MCP in your Claude Code MCP settings. See https://help.figma.com/hc/en-us/articles/32132100888087 for setup instructions. ``` Record whether this check passed or failed. -## Step 4 — Check for Playwright MCP +## Step 4 - Check for Playwright MCP Determine whether a Playwright MCP tool is available in the current Claude Code session by checking the active tool set for tools whose names match `mcp__playwright*` or any tool clearly from a Playwright MCP server. -- If the Playwright MCP is **available**: print `✓ Playwright MCP — connected` +- If the Playwright MCP is **available**: print `✓ Playwright MCP - connected` - If the Playwright MCP is **NOT available**: print the following explicit failure (not a silent skip): ``` - ✗ Playwright MCP — not connected + ✗ Playwright MCP - not connected Remediation: Enable the Playwright MCP in your Claude Code MCP settings. Example config: https://github.com/microsoft/playwright-mcp ``` Record whether this check passed or failed. -## Step 5 — Print overall result +## Step 5 - Print overall result Print: @@ -75,7 +75,7 @@ Print: Then: - If **all checks passed**: print `All checks passed.` -- If **any check failed**: print `One or more checks failed — see lines marked ✗ above.` +- If **any check failed**: print `One or more checks failed - see lines marked ✗ above.` Missing MCPs are hard failures. Do not summarise them as warnings or caveats; they must appear as ✗ lines. diff --git a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs index eb1c4bc3..04b4ddbc 100644 --- a/apps/claude-code/unic-spec-review/scripts/lib/args.mjs +++ b/apps/claude-code/unic-spec-review/scripts/lib/args.mjs @@ -3,7 +3,7 @@ // Copyright © 2026 Unic /** - * args.mjs — CLI argument parsers for unic-spec-review. + * args.mjs - CLI argument parsers for unic-spec-review. * * - `parseReviewSpecArgs`: accepts a raw string or argv array; extracts http(s) * URLs and the `--post` flag (for the /review-spec command). @@ -45,7 +45,7 @@ export function parseReviewSpecArgs(input) { * * Accepts both `--key=value` and `--key value` forms. Bare positional args * are ignored. A `--flag` with no following value (last arg, or followed by - * another `--flag`) throws — the previous silent-drop behaviour produced + * another `--flag`) throws - the previous silent-drop behaviour produced * misleading "X is required" errors when the user actually did pass the flag. * * Boolean flags (presence-only, no value) must be declared in `options.booleanFlags`. diff --git a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs index 5eab4cd1..1ac6e9fa 100644 --- a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs +++ b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs @@ -4,7 +4,7 @@ // Copyright © 2026 Unic /** - * setup-confluence.mjs — write ~/.unic-confluence.json with the user's + * setup-confluence.mjs - write ~/.unic-confluence.json with the user's * Confluence credentials. Pure file-writer: values arrive as CLI args; the * conversational prompting happens in commands/setup-confluence.md. */ @@ -66,7 +66,7 @@ export function writeConfluenceCreds(url, username, token, deps = {}) { if (existing && typeof existing === 'object') preserved = existing } catch (err) { if (!(err instanceof SyntaxError)) throw err - warn(`${path} contains invalid JSON — overwriting (any prior jiraUrl will be lost).`) + warn(`${path} contains invalid JSON - overwriting (any prior jiraUrl will be lost).`) } } const payload = { ...preserved, url, username, token } @@ -74,7 +74,7 @@ export function writeConfluenceCreds(url, username, token, deps = {}) { write(tmp, JSON.stringify(payload, null, 2), 'utf8') if (platform === 'win32') { warn( - `Windows detected — skipping chmod 600 on ${path}. Restrict file access manually, e.g.:\n icacls "${path}" /inheritance:r /grant:r "%USERNAME%:F"` + `Windows detected - skipping chmod 600 on ${path}. Restrict file access manually, e.g.:\n icacls "${path}" /inheritance:r /grant:r "%USERNAME%:F"` ) } else { chmod(tmp, 0o600) diff --git a/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs index 604dc481..7394112a 100644 --- a/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs +++ b/apps/claude-code/unic-spec-review/scripts/spec-doctor.mjs @@ -4,7 +4,7 @@ // Copyright © 2026 Unic /** - * spec-doctor.mjs — Confluence credential + connectivity preflight for + * spec-doctor.mjs - Confluence credential + connectivity preflight for * unic-spec-review. * * This script covers only the parts of the preflight that can run inside a @@ -25,8 +25,8 @@ import { loadAtlassianCreds } from './lib/credentials.mjs' /** * Discriminated by `kind`: - * - 'http' — fetch resolved; `status` is the HTTP response code. - * - 'transport-error' — fetch rejected (invalid URL, wrong scheme, timeout, + * - 'http' - fetch resolved; `status` is the HTTP response code. + * - 'transport-error' - fetch rejected (invalid URL, wrong scheme, timeout, * network error); `error` carries the failure message. * * @typedef {{ kind: 'http', status: number } | { kind: 'transport-error', error: string }} PingResult @@ -115,7 +115,7 @@ export function mapPingError(err) { /** * Default fetcher: GET via global fetch with a 10 s timeout. Handles both - * https:// and http:// URLs. Returns the discriminated PingResult — an HTTP + * https:// and http:// URLs. Returns the discriminated PingResult - an HTTP * result on resolution, a transport-error on any rejection (invalid URL, * wrong scheme, timeout, network error). * @@ -139,7 +139,7 @@ export function realPing(url, headers) { */ function formatLine(result, label) { const glyph = result.ok ? '✓' : '✗' - return `${glyph} ${label} — ${result.detail}` + return `${glyph} ${label} - ${result.detail}` } /** @@ -172,10 +172,10 @@ export async function runSpecDoctorCredentials(deps = {}) { } if (credsLoadError) { - lines.push(`✗ Atlassian credentials — credential file unreadable: ${credsLoadError}`) + lines.push(`✗ Atlassian credentials - credential file unreadable: ${credsLoadError}`) allOk = false } else if (!creds) { - lines.push('✗ Atlassian credentials — neither env vars nor ~/.unic-confluence.json found') + lines.push('✗ Atlassian credentials - neither env vars nor ~/.unic-confluence.json found') allOk = false } diff --git a/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs index d70abed4..1c609fd0 100644 --- a/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs +++ b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs @@ -17,7 +17,7 @@ function tempDir() { } describe('writeConfluenceCreds', () => { - it('happy path — writes correct JSON to temp home and chmods 600 on linux', () => { + it('happy path - writes correct JSON to temp home and chmods 600 on linux', () => { const home = tempDir() /** @type {{ p: string, m: number }[]} */ const chmodCalls = [] @@ -50,7 +50,7 @@ describe('writeConfluenceCreds', () => { assert.deepEqual(writeOrder, [`write:${target}.tmp`, `chmod:${target}.tmp`, `rename:${target}.tmp->${target}`]) }) - it('idempotent re-run — same file, no error on second call', () => { + it('idempotent re-run - same file, no error on second call', () => { const home = tempDir() const opts = { homedir: home, platform: 'linux', chmod: () => {} } writeConfluenceCreds('https://x.atlassian.net', 'u', 'tok', opts) @@ -119,7 +119,7 @@ describe('writeConfluenceCreds', () => { ) }) - it('Windows chmod warning branch — warn called with icacls hint, chmod skipped', () => { + it('Windows chmod warning branch - warn called with icacls hint, chmod skipped', () => { const home = tempDir() /** @type {string[]} */ const warns = [] From a8dcc5b838c0b9cedfce5f8ca00f1ffb2184bcc5 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Fri, 5 Jun 2026 17:16:24 +0200 Subject: [PATCH 4/4] fix(unic-spec-review): address PR #211 multi-agent review findings - Drop stale `:setup-jira` reference in writeConfluenceCreds JSDoc; that command exists in unic-pr-review (the vendoring source) but not in this plugin. Behavior unchanged. - Add tests for two untested in-scope pure-logic branches: isEnvConfigured rejecting a present-but-empty env var, and checkConfluence's raw-string fallback when the configured url is unparseable. Why: review surfaced a misleading vendored comment and two uncovered branches in the pure logic the slice's acceptance criteria require tested. No Critical/High findings; remaining suggestions are sub-threshold. Co-Authored-By: Claude Opus 4.8 --- apps/claude-code/unic-spec-review/CHANGELOG.md | 2 ++ .../unic-spec-review/scripts/setup-confluence.mjs | 5 +++-- .../unic-spec-review/tests/setup-confluence.test.mjs | 6 ++++++ .../unic-spec-review/tests/spec-doctor.test.mjs | 7 +++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/apps/claude-code/unic-spec-review/CHANGELOG.md b/apps/claude-code/unic-spec-review/CHANGELOG.md index cd30461f..5e5ac321 100644 --- a/apps/claude-code/unic-spec-review/CHANGELOG.md +++ b/apps/claude-code/unic-spec-review/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `/spec-doctor` command: preflight checks for Confluence credentials and connectivity, Figma Dev Mode MCP, and Playwright MCP; absent MCPs are reported as explicit loud failures with remediation, never a silent skip. - Add `parseArgs` to the `args` module (CLI parser shared by the setup wizard) alongside the existing `parseReviewSpecArgs`. - Unit tests for the vendored `setup-confluence` wizard (`writeConfluenceCreds`, `isEnvConfigured`) and the Confluence preflight logic (`checkConfluence`, `runSpecDoctorCredentials`, `mapPingError`, `realPing`) with injected `homedir`/`platform`/`fetch`/`loadCreds`; no live services. +- Cover two more pure-logic branches flagged in PR #211 review: `isEnvConfigured` rejecting an env var that is present but empty, and `checkConfluence` falling back to the raw url string when the configured url is unparseable. ### Fixed - Reject non-http(s) URLs in arg parsing, link classification, and validate `pageTitle`/`pageUrl` in the report-renderer CLI entry, so ftp/file/mailto inputs no longer slip through and missing report fields no longer render as literal `undefined`. @@ -34,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Documentation - Correct stale cross-plugin references in code comments (drop the `render-summary.mjs`, `doctor.mjs`, and inaccurate `ADR-0001` citations), reword the `CONTEXT.md` status line so it no longer promises an unused `(S1)` per-term marking convention, and replace em dashes with hyphens in authored comments, command docs, user-facing script output, and test descriptions, per this slice's acceptance criterion (no em dash in authored text except the mandated CHANGELOG version header). +- Reword the `writeConfluenceCreds` JSDoc to drop a stale `:setup-jira` reference (a command that exists in `unic-pr-review` but not in this plugin); the `jiraUrl` preservation behavior is unchanged. ## [0.1.1] — 2026-06-05 diff --git a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs index 1ac6e9fa..dd2ebcef 100644 --- a/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs +++ b/apps/claude-code/unic-spec-review/scripts/setup-confluence.mjs @@ -35,8 +35,9 @@ import { parseArgs } from './lib/args.mjs' /** * Write ~/.unic-confluence.json with the given credentials. Preserves any - * existing fields (e.g. `jiraUrl` from :setup-jira) so re-running to rotate - * a token does not silently drop data. If the existing file is unparseable + * existing fields (such as a `jiraUrl` written by another tool that shares + * ~/.unic-confluence.json) so re-running to rotate a token does not silently + * drop data. If the existing file is unparseable * the writer warns and overwrites; non-syntax read errors (EACCES, etc.) * propagate so callers can surface them rather than silently losing data. * diff --git a/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs index 1c609fd0..57b7cbe2 100644 --- a/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs +++ b/apps/claude-code/unic-spec-review/tests/setup-confluence.test.mjs @@ -163,4 +163,10 @@ describe('isEnvConfigured', () => { assert.equal(isEnvConfigured({ CONFLUENCE_USER: 'u', CONFLUENCE_TOKEN: 't' }), false) assert.equal(isEnvConfigured({}), false) }) + + it('returns false when a variable is present but empty (no silent empty-string pass-through)', () => { + assert.equal(isEnvConfigured({ CONFLUENCE_URL: '', CONFLUENCE_USER: 'u', CONFLUENCE_TOKEN: 't' }), false) + assert.equal(isEnvConfigured({ CONFLUENCE_URL: 'x', CONFLUENCE_USER: '', CONFLUENCE_TOKEN: 't' }), false) + assert.equal(isEnvConfigured({ CONFLUENCE_URL: 'x', CONFLUENCE_USER: 'u', CONFLUENCE_TOKEN: '' }), false) + }) }) diff --git a/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs index 69dcba03..66feb237 100644 --- a/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs +++ b/apps/claude-code/unic-spec-review/tests/spec-doctor.test.mjs @@ -62,6 +62,13 @@ describe('checkConfluence', () => { assert.match(r.detail, /ECONNREFUSED/) }) + it('falls back to the raw url string in the detail when the url is unparseable', async () => { + const creds = { url: 'not a url', username: 'u', token: 't', jiraUrl: undefined } + const r = await checkConfluence(creds, pingHttp(401)) + assert.equal(r.ok, false) + assert.match(r.detail, /not a url/) + }) + it('strips a trailing slash and pings the Confluence space endpoint with Basic auth', async () => { const creds = { url: 'https://example.atlassian.net/', username: 'u', token: 't', jiraUrl: undefined } /** @type {{ url?: string, headers?: Record }} */