From 8da4a07a602482c7fb478dfade602d51de2484fb Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:11:52 +0800 Subject: [PATCH 01/76] fix(viewer): make in-document links in rendered markdown scroll to their heading marked emits no heading ids and, with , a bare #section href points at the dashboard root, so [Install](#installation) in the File Viewer did nothing. The shared click delegate now resolves fragment links against the rendered document: GitHub-style slugs in data-md-anchor (never ids, so a heading cannot capture an app element), case-insensitive and percent-decoded, repeats numbered -1/-2, # = top, explicit ids supported, an unmatched fragment ignored instead of navigating. Tests: slug/assign/find unit tests (CI gate) and a real-browser test clicking links in a File Viewer document, which fails without the change. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS --- .changeset/markdown-anchor-links.md | 5 + config/test-suites.ts | 1 + docs/wiki/Working-With-Files.md | 2 +- src/web/public/app.js | 17 +++ src/web/public/constants.js | 62 ++++++++ test/markdown-anchor-links.browser.test.ts | 167 +++++++++++++++++++++ test/markdown-anchors.test.ts | 110 ++++++++++++++ 7 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 .changeset/markdown-anchor-links.md create mode 100644 test/markdown-anchor-links.browser.test.ts create mode 100644 test/markdown-anchors.test.ts diff --git a/.changeset/markdown-anchor-links.md b/.changeset/markdown-anchor-links.md new file mode 100644 index 00000000..1c9b279e --- /dev/null +++ b/.changeset/markdown-anchor-links.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +A link to another heading of the same file in the markdown File Viewer (`[Install](#installation)`) now scrolls to that heading. It did nothing: marked emits no heading ids, so there was nothing to jump to, and with `` a bare `#installation` href points at the dashboard's root, not at the page, so letting the browser follow it was never an in-page jump either. The shared link handler (File Viewer and Response Viewer) now resolves fragment links itself against the rendered document: headings get GitHub-style slugs (lower-case, punctuation dropped, a repeated title numbered `-1`, `-2`), matched case-insensitively and percent-decoded, `#` goes to the top, an explicit `` in the document works, and a fragment that matches nothing is ignored instead of navigating the app. The anchors are `data-md-anchor` attributes looked up inside the document, never `id`s, so a heading called "Settings" cannot collide with an element of the app itself. diff --git a/config/test-suites.ts b/config/test-suites.ts index 6283d361..ee36a4ad 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -45,6 +45,7 @@ export const BROWSER_TEST_GLOBS = [ 'test/spreadsheet-preview.browser.test.ts', 'test/mobile-ime-preview.browser.test.ts', 'test/run-mode-menu-scroll.browser.test.ts', + 'test/markdown-anchor-links.browser.test.ts', ]; /** diff --git a/docs/wiki/Working-With-Files.md b/docs/wiki/Working-With-Files.md index bfa4a6ef..8e7b5afa 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -14,7 +14,7 @@ It renders what it can: | Kind | Behaviour | | ------------------------ | ------------------------------------------------------------------------- | | Text and code | Plain preview with Lines (line numbers) and Wrap toggles in the header. Long files are truncated in plain preview. | -| Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. | +| Markdown | Rendered by default: headings, tables, code blocks with copy buttons, images and links relative to the file (root-relative ones resolve from the workspace root, as on GitHub). Links to another heading of the same file (`[Install](#installation)`) scroll to it, with GitHub's heading names (lower-case, punctuation dropped, repeats numbered `-1`, `-2`), and never leave the page. Opened from an attachment card, where the file's folder is unknown, relative images show their alt text and relative links show as plain text. The MD pill in the header flips to source. | | Images | Inline. | | Audio and video | Inline with a working scrub bar, because range requests are supported. | | Spreadsheets (`.xlsx`) | Read-only grid, parsed in your browser (never on the server), up to 10 MB. `.xls` and `.ods` are download only. | diff --git a/src/web/public/app.js b/src/web/public/app.js index 447a959e..c7aefe7e 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2732,6 +2732,23 @@ class CodemanApp { return; } + // An in-document link (`[Install](#installation)`). The browser must not follow it: with + // `` a bare fragment points at the dashboard's root and would navigate the + // app away. Resolve it inside this rendered document and scroll there (constants.js). + // A fragment that matches nothing is simply ignored, never a navigation. + const fragmentLink = ev.target.closest('a[href^="#"]'); + if (fragmentLink && body.contains(fragmentLink)) { + ev.preventDefault(); + ev.stopPropagation(); + const root = fragmentLink.closest('.rv-text') || body; + const target = window.CodemanMarkdownAnchors?.find(root, fragmentLink.getAttribute('href')); + if (target) { + const calm = window.matchMedia?.('(prefers-reduced-motion: reduce)')?.matches; + target.scrollIntoView({ block: 'start', behavior: calm ? 'auto' : 'smooth' }); + } + return; + } + // A `localhost` URL in the agent's answer: from another device that can // only load through the server, so hand it to a proxied web tab // (webview-tabs.js). Every other link keeps its new-tab default. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 2272378d..239fb5d5 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -1298,6 +1298,63 @@ function cleanCopiedSelection(text, options) { return lines.join('\n'); } +// ── Markdown heading anchors ──────────────────────────────────────────────── +// marked emits no `id` on headings, so a rendered document's own `[Install](#installation)` links had +// nothing to jump to. And with `` a bare `#installation` href points at the dashboard's +// root, not at the page, so letting the browser follow it navigates the app away. The click delegate +// (`_bindResponseViewerInteractions`) therefore resolves in-document links itself, with the helpers +// below. Anchors are `data-md-anchor` attributes, NOT `id`s: a heading titled "Settings" must not claim +// the id of an element in the app's own DOM, and the lookup is scoped to the rendered document. + +/** + * GitHub's heading slug: lower-cased, anything that is not a letter, mark, number, `_`, `-` or space + * dropped, each space a hyphen (`Why `codeman`? → `why-codeman`, `Über uns` → `über-uns`). + */ +function markdownHeadingSlug(text) { + return String(text ?? '') + .trim() + .toLowerCase() + .replace(/[^\p{L}\p{M}\p{N}_\- ]/gu, '') + .replace(/ /g, '-'); +} + +/** Give every h1..h6 under `root` its slug in `data-md-anchor`; a repeat gets `-1`, `-2`, ... as on GitHub. Idempotent. */ +function assignMarkdownHeadingAnchors(root) { + const used = new Set(); + for (const heading of root.querySelectorAll('h1, h2, h3, h4, h5, h6')) { + const base = markdownHeadingSlug(heading.textContent); + let slug = base; + for (let n = 1; used.has(slug); n += 1) slug = `${base}-${n}`; + used.add(slug); + heading.dataset.mdAnchor = slug; + } +} + +/** + * The element inside `root` that an in-document link (`#installation`, `#Installation`, `#my%20title`) + * points at, or null. An empty fragment (`#`) means the top of the document. Headings are matched by + * slug, then a heading the author wrote an explicit ``/`id` for, looked up INSIDE `root` + * only (never `document.getElementById`, which could find an app element of the same name). + */ +function findMarkdownAnchorTarget(root, href) { + let fragment = String(href ?? '').replace(/^#/, ''); + try { + fragment = decodeURIComponent(fragment); + } catch { + /* a malformed escape: use it as written */ + } + if (!fragment) return root; + assignMarkdownHeadingAnchors(root); + const wanted = [fragment.toLowerCase(), markdownHeadingSlug(fragment)]; + for (const heading of root.querySelectorAll('[data-md-anchor]')) { + if (wanted.includes(heading.dataset.mdAnchor)) return heading; + } + for (const el of root.querySelectorAll('[id]')) { + if (el.id === fragment) return el; + } + return null; +} + if (typeof window !== 'undefined') { window.WEBGL_FALLBACK = WEBGL_FALLBACK; window.evaluateWebGLLongTaskTrip = evaluateWebGLLongTaskTrip; @@ -1367,6 +1424,11 @@ if (typeof window !== 'undefined') { window.CodemanCopySelection = { clean: cleanCopiedSelection, }; + window.CodemanMarkdownAnchors = { + slug: markdownHeadingSlug, + assign: assignMarkdownHeadingAnchors, + find: findMarkdownAnchorTarget, + }; window.CodemanTerminalFont = { DEFAULT_STACK: TERMINAL_FONT_DEFAULT_STACK, resolve: resolveTerminalFontFamily, diff --git a/test/markdown-anchor-links.browser.test.ts b/test/markdown-anchor-links.browser.test.ts new file mode 100644 index 00000000..843c5574 --- /dev/null +++ b/test/markdown-anchor-links.browser.test.ts @@ -0,0 +1,167 @@ +/** + * Clicking a link to another heading of the SAME markdown file in the File Viewer must scroll to + * that heading. It did nothing: marked emits no heading ids, and with `` a bare + * `#section` href is not an in-page link anyway. Real xterm-free browser, real preview overlay, + * real click. + * + * Browser-driven, so excluded from `npm run test:ci` (config/test-suites.ts). Run locally: + * npm run test:browser -- test/markdown-anchor-links.browser.test.ts + * + * Port: 3294 + */ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { chromium, type Browser, type Page } from 'playwright'; +import { WebServer } from '../src/web/server.js'; + +const PORT = 3294; +const filler = (n: number) => + Array.from({ length: n }, (_, i) => `Paragraph ${i} of filler text so the document is taller than the viewer.`).join( + '\n\n' + ); + +describe('in-document links in the markdown File Viewer', () => { + let server: WebServer; + let browser: Browser; + let page: Page; + let root: string; + + beforeAll(async () => { + root = mkdtempSync(join(homedir(), 'md-anchors-')); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, 'README.md'), + [ + '# Title', + '', + '[Usage](#usage) · [Second install](#install-1) · [Mixed case](#My-Title) · [Encoded](#my%20title) · [Nowhere](#nope) · [Top](#)', + '', + '## Install', + filler(30), + '## Usage', + filler(30), + '## Install', + filler(30), + '## My Title', + filler(30), + '## Settings', + filler(10), + ].join('\n') + ); + server = new WebServer(PORT, false, true); + await server.start(); + browser = await chromium.launch({ headless: true }); + page = await (await browser.newContext({ viewport: { width: 1100, height: 700 } })).newPage(); + // Reduced motion makes the scroll instant (the code uses behavior 'auto' for it), so the + // position can be read right after the click instead of racing a smooth-scroll animation. + await page.emulateMedia({ reducedMotion: 'reduce' }); + await page.goto(`http://localhost:${PORT}`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 }); + const sessionId = await page.evaluate(async (workingDir) => { + const res = await fetch('/api/sessions', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workingDir, mode: 'shell' }), + }); + const id = (await res.json()).data.session.id; + await fetch(`/api/sessions/${id}/shell`, { method: 'POST' }); + return id as string; + }, root); + await page.evaluate(async (sid) => { + const app = (window as any).app; + for (let i = 0; i < 100 && !app.sessions.has(sid); i++) await new Promise((r) => setTimeout(r, 100)); + await app.selectSession(sid); + }, sessionId); + await page.evaluate(({ sid, path }) => (window as any).app.openFilePreview(path, sid), { + sid: sessionId, + path: join(root, 'README.md'), + }); + await page.waitForSelector('#filePreviewBody .file-preview-md h2'); + // A marker that a full navigation would wipe. + await page.evaluate(() => ((window as any).__stillHere = true)); + }, 90000); + + afterAll(async () => { + if (browser) await browser.close(); + if (server) await server.stop(); + rmSync(root, { recursive: true, force: true }); + }, 60000); + + const state = () => + page.evaluate(() => { + const body = document.getElementById('filePreviewBody')!; + return { + scrollTop: body.scrollTop, + stillHere: (window as any).__stillHere === true, + url: location.href, + bodyTop: body.getBoundingClientRect().top, + }; + }); + /** How far the nth heading's top is from the top of the scrolling viewer, in px. */ + const headingOffset = (text: string, nth = 0) => + page.evaluate( + ({ text, nth }) => { + const body = document.getElementById('filePreviewBody')!; + const hs = [...body.querySelectorAll('.file-preview-md h1, .file-preview-md h2')].filter( + (h) => h.textContent === text + ); + return Math.round(hs[nth].getBoundingClientRect().top - body.getBoundingClientRect().top); + }, + { text, nth } + ); + const click = async (name: string) => { + // A DOM click: Playwright's own click scrolls the link into view first, which would move the + // very scroll position these tests measure. + await page.locator(`#filePreviewBody a:text-is("${name}")`).evaluate((a) => (a as HTMLElement).click()); + await page.waitForTimeout(100); + }; + + it('scrolls to the heading a link names, and stays on the page', async () => { + const before = await state(); + expect(before.scrollTop).toBe(0); + await click('Usage'); + const after = await state(); + expect(after.scrollTop).toBeGreaterThan(500); + expect(Math.abs(await headingOffset('Usage'))).toBeLessThan(60); // the heading is at the top of the viewer + expect(after.stillHere).toBe(true); // no navigation + expect(after.url).toBe(before.url); + }); + + it('a repeated heading is reachable as slug-1, as on GitHub', async () => { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click('Second install'); + expect(Math.abs(await headingOffset('Install', 1))).toBeLessThan(60); + }); + + it('matches case-insensitively and with an encoded space', async () => { + for (const name of ['Mixed case', 'Encoded']) { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click(name); + expect(Math.abs(await headingOffset('My Title')), name).toBeLessThan(60); + } + }); + + it('a link to nowhere does nothing (no scroll, no navigation), and # goes back to the top', async () => { + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 0)); + await click('Nowhere'); + const stay = await state(); + expect(stay.scrollTop).toBe(0); + expect(stay.stillHere).toBe(true); + + await click('Usage'); + expect((await state()).scrollTop).toBeGreaterThan(500); + await page.evaluate(() => (document.getElementById('filePreviewBody')!.scrollTop = 99999)); + await page.evaluate(() => document.querySelector('#filePreviewBody a:not([data-path])')!.scrollIntoView()); + await click('Top'); + expect((await state()).scrollTop).toBeLessThan(5); + }); + + it('a heading cannot capture an element of the app by id (Settings heading, no id given)', async () => { + const ids = await page.evaluate(() => + [...document.querySelectorAll('#filePreviewBody h1, #filePreviewBody h2')].map((h) => h.id) + ); + expect(ids.every((id) => id === '')).toBe(true); + }); +}); diff --git a/test/markdown-anchors.test.ts b/test/markdown-anchors.test.ts new file mode 100644 index 00000000..1dfe5b72 --- /dev/null +++ b/test/markdown-anchors.test.ts @@ -0,0 +1,110 @@ +/** + * In-document links in rendered markdown (`[Install](#installation)`). + * + * marked emits no heading ids, and with `` a bare `#installation` href points at the + * dashboard's root, so the File Viewer's links neither found a target nor stayed on the page. The + * helpers in constants.js give headings GitHub-style slugs (as `data-md-anchor`, never `id`) and + * resolve a fragment to a heading inside the rendered document only. + * + * Builds a JSDOM window in-test under the default node env (do NOT declare a per-file jsdom + * environment: it externalizes node:fs under vite and the readFileSync below stops working). + */ +import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; +import { describe, expect, it } from 'vitest'; + +const CONSTANTS = readFileSync(new URL('../src/web/public/constants.js', import.meta.url), 'utf-8'); +const APP = readFileSync(new URL('../src/web/public/app.js', import.meta.url), 'utf-8'); + +interface Anchors { + slug(text: string): string; + assign(root: Element): void; + find(root: Element, href: string): Element | null; +} + +function boot(html: string) { + const dom = new JSDOM( + `
app element
${html}
`, + { + url: 'http://localhost/', + runScripts: 'outside-only', + } + ); + const win = dom.window as unknown as Window & { eval(code: string): unknown; CodemanMarkdownAnchors: Anchors }; + win.eval(CONSTANTS); + const doc = win.document.querySelector('.doc') as Element; + return { win, doc, anchors: win.CodemanMarkdownAnchors }; +} + +describe('markdownHeadingSlug (GitHub rules)', () => { + const { anchors } = boot(''); + it.each([ + ['Installation', 'installation'], + ['Why `codeman`?', 'why-codeman'], + ['Über uns', 'über-uns'], + ['A B', 'a--b'], + [' Trim me ', 'trim-me'], + ['Q&A: what, why?', 'qa-what-why'], + ['snake_case and kebab-case', 'snake_case-and-kebab-case'], + ['日本語 の 見出し', '日本語-の-見出し'], + ['🚀 Launch', '-launch'], + ['', ''], + ])('%j -> %j', (text, slug) => expect(anchors.slug(text)).toBe(slug)); +}); + +describe('assign + find', () => { + const html = ` +

Overview

x

+

Install

Usage

Install

Install

+

My Title

`; + + it('slugs every heading, and a repeated title gets -1, -2 like GitHub', () => { + const { doc, anchors } = boot(html); + anchors.assign(doc); + const slugs = [...doc.querySelectorAll('h1, h2, h3')].map((h) => (h as HTMLElement).dataset.mdAnchor); + expect(slugs).toEqual(['overview', 'install', 'usage', 'install-1', 'install-2', 'my-title']); + anchors.assign(doc); // idempotent + expect([...doc.querySelectorAll('h1, h2, h3')].map((h) => (h as HTMLElement).dataset.mdAnchor)).toEqual(slugs); + }); + + it('finds a heading by its slug, case-insensitively, percent-encoded or with a space', () => { + const { doc, anchors } = boot(html); + const title = doc.querySelectorAll('h2')[2]; + for (const href of ['#my-title', '#My-Title', '#my%20title', '#My Title']) { + expect(anchors.find(doc, href), href).toBe(title); + } + expect(anchors.find(doc, '#install-2')?.tagName).toBe('H3'); + }); + + it('finds an explicit id the author wrote, and treats # as the top of the document', () => { + const { doc, anchors } = boot(html); + expect(anchors.find(doc, '#custom-spot')?.id).toBe('custom-spot'); + expect(anchors.find(doc, '#')).toBe(doc); + expect(anchors.find(doc, '')).toBe(doc); + }); + + it('returns null for a fragment that matches nothing, and a malformed escape is used as written', () => { + const { doc, anchors } = boot(html); + expect(anchors.find(doc, '#nope')).toBeNull(); + expect(() => anchors.find(doc, '#%E0%A4%A')).not.toThrow(); + }); + + it('never resolves against the app: a heading cannot claim an element id outside the document', () => { + const { doc, anchors } = boot('

Settings

'); + const found = anchors.find(doc, '#settings'); + expect(found?.tagName).toBe('H1'); // the heading, not
in the app + expect(found?.id).toBe(''); // and the heading was not given an id + }); +}); + +describe('the click delegate', () => { + it('handles fragment links before anything that would let the browser follow them', () => { + const delegate = APP.slice(APP.indexOf('_bindResponseViewerInteractions(body) {')); + const fragment = delegate.indexOf('a[href^="#"]'); + expect(fragment).toBeGreaterThan(-1); + // After file-path links (their href is '#', they open the preview) and before the loopback/new-tab handling. + expect(fragment).toBeGreaterThan(delegate.indexOf("closest('a.rv-path')")); + expect(fragment).toBeLessThan(delegate.indexOf('openLinkThroughWebTabIfLoopback')); + expect(delegate.slice(fragment, fragment + 400)).toContain('preventDefault'); + }); +}); From 26487f0ec57df3b6f8c2f8cb41d7cce4e56de918 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:41:03 +0800 Subject: [PATCH 02/76] fix(cli-install): install npm CLIs to ~/.local when the npm global prefix is not writable installEnv() only redirected NPM_CONFIG_PREFIX inside the Docker container (CODEMAN_IN_CONTAINER=1). On a native install with a root-owned system node (prefix /usr) `npm install -g @deepseek-ai/dsh` run as the server user died with EACCES (exit 243), so DeepSeek, pi and other npm-based CLIs could not be installed from Settings. Ask npm for its global prefix and, when the server user cannot write it, redirect to $HOME/.local like the container path does. An operator-set NPM_CONFIG_PREFIX and a writable prefix are left alone. --- .changeset/install-npm-prefix-writable.md | 5 +++ src/web/routes/cli-registry-routes.ts | 54 ++++++++++++++++++++--- test/routes/cli-registry-routes.test.ts | 35 ++++++++++++++- 3 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 .changeset/install-npm-prefix-writable.md diff --git a/.changeset/install-npm-prefix-writable.md b/.changeset/install-npm-prefix-writable.md new file mode 100644 index 00000000..75143f4c --- /dev/null +++ b/.changeset/install-npm-prefix-writable.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Installing an npm-based CLI from Settings (DeepSeek's `dsh`, pi, ...) no longer fails with EACCES on a native install whose system node keeps its global prefix under `/usr`. When the npm global prefix is not writable by the Codeman user, the install now goes to `~/.local` (where Codeman already looks for CLIs), as the Docker deployment already did. A prefix you set yourself, or one that is writable, is left alone. diff --git a/src/web/routes/cli-registry-routes.ts b/src/web/routes/cli-registry-routes.ts index 3b75ffda..88f076ee 100644 --- a/src/web/routes/cli-registry-routes.ts +++ b/src/web/routes/cli-registry-routes.ts @@ -29,7 +29,9 @@ * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`). */ -import { spawn } from 'node:child_process'; +import { execFileSync, spawn } from 'node:child_process'; +import { accessSync, constants as fsConstants } from 'node:fs'; +import { join } from 'node:path'; import type { FastifyInstance, FastifyRequest } from 'fastify'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; import { getAuthUser, isAdmin, parseBody, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js'; @@ -207,14 +209,54 @@ const CLI_INSTALL_TIMEOUT_MS = 300_000; */ const installsInFlight = new Set(); +/** + * True when `npm install -g` can write to this process's npm global prefix, or when that cannot + * be determined (then nothing is redirected: a wrong guess would move installs somewhere the + * user did not choose). `npm config get prefix` is asked rather than guessed from `process.execPath` + * because a user `.npmrc` / `NPM_CONFIG_PREFIX` can point it anywhere. + */ +export function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): boolean { + try { + const prefix = execFileSync('npm', ['config', 'get', 'prefix'], { + env: source, + encoding: 'utf8', + timeout: 5_000, + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + if (!prefix) return true; + // npm creates lib/node_modules under the prefix; check that directory when it exists, else the prefix. + for (const dir of [join(prefix, 'lib', 'node_modules'), prefix]) { + try { + accessSync(dir, fsConstants.W_OK); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false; + } + } + return false; + } catch { + return true; + } +} + /** * The server's environment minus every `CODEMAN_*` variable. An install script is third-party * code, and those variables carry Codeman's own secrets and wiring (`CODEMAN_PASSWORD`, the - * data dir, the tmux socket), none of which an installer needs. Inside the Docker Compose - * container (`CODEMAN_IN_CONTAINER=1`) it also points `NPM_CONFIG_PREFIX` at `$HOME/.local`, - * so an `npm install -g` lands on the persistent home mount instead of the image. + * data dir, the tmux socket), none of which an installer needs. + * + * It also points `NPM_CONFIG_PREFIX` at `$HOME/.local` so an `npm install -g` lands somewhere the + * server user can write and Codeman's resolvers already search (`~/.local/bin`): + * - inside the Docker Compose container (`CODEMAN_IN_CONTAINER=1`), so installs survive an image + * update (the image's own prefix is image content); + * - on a native install whose npm global prefix is not writable by the server user (a system node + * under `/usr`, installed by root). Without this `npm install -g` died with EACCES (exit 243), + * e.g. DeepSeek's `npm install -g @deepseek-ai/dsh`. An explicit `NPM_CONFIG_PREFIX` the + * operator set is respected, and so is a prefix that is writable (nvm, `~/.npm-global`, ...). */ -export function installEnv(source: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { +export function installEnv( + source: NodeJS.ProcessEnv = process.env, + prefixWritable: (env: NodeJS.ProcessEnv) => boolean = npmGlobalPrefixWritable +): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {}; for (const [key, value] of Object.entries(source)) { if (!key.startsWith('CODEMAN_')) env[key] = value; @@ -225,6 +267,8 @@ export function installEnv(source: NodeJS.ProcessEnv = process.env): NodeJS.Proc // list, so npm-based installs are redirected there. curl|bash installers already target HOME. if (source.CODEMAN_IN_CONTAINER === '1' && source.HOME) { env.NPM_CONFIG_PREFIX = `${source.HOME}/.local`; + } else if (process.platform !== 'win32' && source.HOME && !source.NPM_CONFIG_PREFIX && !prefixWritable(env)) { + env.NPM_CONFIG_PREFIX = `${source.HOME}/.local`; } return env; } diff --git a/test/routes/cli-registry-routes.test.ts b/test/routes/cli-registry-routes.test.ts index bc33b1f8..03fe2650 100644 --- a/test/routes/cli-registry-routes.test.ts +++ b/test/routes/cli-registry-routes.test.ts @@ -15,7 +15,12 @@ import { EventEmitter } from 'node:events'; import { chmodSync, mkdirSync, readFileSync, rmSync, writeFileSync, statSync } from 'node:fs'; import { dirname } from 'node:path'; import { createRouteTestHarness } from './_route-test-utils.js'; -import { installEnv, registerCliRegistryRoutes, type CliListItem } from '../../src/web/routes/cli-registry-routes.js'; +import { + installEnv, + npmGlobalPrefixWritable, + registerCliRegistryRoutes, + type CliListItem, +} from '../../src/web/routes/cli-registry-routes.js'; import { SETTINGS_PATH } from '../../src/web/route-helpers.js'; import { CreateSessionSchema } from '../../src/web/schemas.js'; import { buildSpawnCommandFromRegistry } from '../../src/session-cli-registry-bridge.js'; @@ -711,7 +716,7 @@ describe('registry writes are serialized and never clobber a file the reader wou } finally { delete process.env.CODEMAN_TEST_SECRET; } - expect(installEnv({ CODEMAN_PASSWORD: 'x', HOME: '/h' })).toEqual({ HOME: '/h' }); + expect(installEnv({ CODEMAN_PASSWORD: 'x', HOME: '/h' }, () => true)).toEqual({ HOME: '/h' }); }); it('redirects npm installs to the persistent HOME inside the Compose container', () => { @@ -719,6 +724,32 @@ describe('registry writes are serialized and never clobber a file the reader wou installEnv({ CODEMAN_IN_CONTAINER: '1', HOME: '/home/codeman', NPM_CONFIG_PREFIX: '/opt/codeman-cli' }) ).toEqual({ HOME: '/home/codeman', NPM_CONFIG_PREFIX: '/home/codeman/.local' }); }); + + it('redirects npm installs to ~/.local on a native install whose global prefix is not writable', () => { + // A system node under /usr: `npm install -g` as the server user dies with EACCES (exit 243). + expect(installEnv({ HOME: '/home/dev', CODEMAN_PASSWORD: 'x' }, () => false)).toEqual({ + HOME: '/home/dev', + NPM_CONFIG_PREFIX: '/home/dev/.local', + }); + }); + + it('leaves a writable, explicit or undeterminable npm prefix alone on a native install', () => { + expect(installEnv({ HOME: '/home/dev' }, () => true)).toEqual({ HOME: '/home/dev' }); + // An operator-set prefix wins even if it is not writable: it is theirs to fix. + expect(installEnv({ HOME: '/home/dev', NPM_CONFIG_PREFIX: '/opt/npm' }, () => false)).toEqual({ + HOME: '/home/dev', + NPM_CONFIG_PREFIX: '/opt/npm', + }); + // No HOME means nowhere to redirect to. + expect(installEnv({ PATH: '/usr/bin' }, () => false)).toEqual({ PATH: '/usr/bin' }); + }); + + it('npmGlobalPrefixWritable follows npm config and treats a probe failure as writable', () => { + // Real probe against this machine: must return a boolean and never throw. + expect(typeof npmGlobalPrefixWritable({ PATH: process.env.PATH })).toBe('boolean'); + // npm not found on PATH: cannot tell, so do not redirect. + expect(npmGlobalPrefixWritable({ PATH: '/nonexistent' })).toBe(true); + }); }); /** From 9c63c41283b3b30cba7f7e60e49b8dfaf96b3524 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 7 Oct 2026 13:53:06 +0800 Subject: [PATCH 03/76] feat(settings): add an Apply button that saves without closing Switches such as MCP server sync only unlock their controls once saved, and Save closed the modal, so the user had to reopen Settings to continue. Apply runs the same save, keeps the modal open and refreshes the dependent groups (MCP sync, custom model endpoints, CLI management) in place. --- .changeset/settings-apply-button.md | 5 ++ src/web/public/index.html | 2 + src/web/public/settings-ui.js | 45 +++++++++++++- src/web/public/styles.css | 6 ++ test/settings-apply-button.test.ts | 93 +++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 .changeset/settings-apply-button.md create mode 100644 test/settings-apply-button.test.ts diff --git a/.changeset/settings-apply-button.md b/.changeset/settings-apply-button.md new file mode 100644 index 00000000..950394df --- /dev/null +++ b/.changeset/settings-apply-button.md @@ -0,0 +1,5 @@ +--- +'aicodeman': minor +--- + +Settings has an Apply button next to Save. It saves the same way but keeps Settings open and refreshes the groups that depend on a saved value, so switching on MCP server sync (or custom model endpoints, or CLI management) brings up its controls straight away instead of needing a close-and-reopen. The MCP sync hint now says to Apply or Save first. diff --git a/src/web/public/index.html b/src/web/public/index.html index 335d3785..d28db067 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1635,6 +1635,7 @@ Save; row-reverse keeps Save to the left of it on phones. -->
+
@@ -3037,6 +3038,7 @@
+
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index a419121b..a7c2f77a 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1225,7 +1225,7 @@ Object.assign(CodemanApp.prototype, { }; // Switched on in this modal but not saved yet: the routes would only answer "disabled". if (!this._mcpSyncSavedOn) { - show('Save settings to turn MCP sync on first, then reopen Settings to preview or sync.'); + show('Apply or Save settings to turn MCP sync on first, then preview or sync.'); return; } if (apply && !confirm('Add missing MCP servers to every installed, enabled CLI\'s config file? Env values and headers on those servers are copied too.')) return; @@ -2506,7 +2506,25 @@ Object.assign(CodemanApp.prototype, { claudeEl.className = 'voice-provider-status' + (status?.available ? ' active' : ''); }, + /** + * Apply button: the same save as Save, but the modal stays open and the groups that depend on + * a saved value (MCP sync, custom model endpoints, CLI management) are refreshed in place, so + * a switch that unlocks more settings needs no close-and-reopen. It is a wrapper rather than + * an option on saveAppSettings() so that function's signature (which tests locate by text) + * stays as it was. + */ + async applyAppSettings() { + if (this._applyingSettings) return; + this._applyingSettings = true; + try { + await this.saveAppSettings(); + } finally { + this._applyingSettings = false; + } + }, + async saveAppSettings() { + const keepOpen = this._applyingSettings === true; // Gesture overlay is injected at page render (server-side), so a change to it // only takes effect on reload — remember the prior value to decide below. const _prev = this.loadAppSettingsFromStorage(); @@ -2866,7 +2884,7 @@ Object.assign(CodemanApp.prototype, { this.saveAppSettingsToStorage(settings); const cb = document.getElementById('appSettingsTunnelEnabled'); if (cb) cb.checked = false; - this.closeAppSettings(); + if (!keepOpen) this.closeAppSettings(); return; } @@ -2881,7 +2899,7 @@ Object.assign(CodemanApp.prototype, { if (webhookError) { this.showToast(`Settings saved, but not the webhook: ${webhookError}`, 'warning'); } else { - this.showToast('Settings saved', 'success'); + this.showToast(keepOpen ? 'Settings applied' : 'Settings saved', 'success'); } // Show tunnel-specific feedback if toggled on @@ -2895,6 +2913,8 @@ Object.assign(CodemanApp.prototype, { if (webhookError) { document.getElementById('webhookGroup')?.scrollIntoView({ block: 'center' }); + } else if (keepOpen) { + this._refreshSettingsAfterApply(settings); } else { this.closeAppSettings(); } @@ -2916,6 +2936,25 @@ Object.assign(CodemanApp.prototype, { } }, + /** + * After Apply: bring the groups whose contents depend on a SAVED value up to date without + * reopening the modal. openAppSettings does the same on open; this is the part of it that + * a save can change, without touching what the user is editing or the scroll position. + */ + _refreshSettingsAfterApply(settings) { + // The MCP routes read the saved flag, so switching it on is only usable from now. + this._mcpSyncSavedOn = settings.mcpSyncEnabled === true; + const out = this.$('mcpSyncResult'); + if (this._mcpSyncSavedOn && out && out.textContent.startsWith('Apply or Save settings')) { + out.style.display = 'none'; + out.innerHTML = ''; + } + this.applyMcpSyncVisibility(); + this.applyCustomModelEndpointsVisibility(); + this.applyCliManagementVisibility(); + this._applyDoctorAdminGate(); + }, + // Load model configuration from server for the settings modal async loadModelConfigForSettings() { try { diff --git a/src/web/public/styles.css b/src/web/public/styles.css index df4d47bf..b5e7ead1 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -16974,6 +16974,12 @@ html[data-tab-orientation='vertical'] .home-sessions { transform 0.1s ease; } +:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save.set-head-apply { + background: transparent; + color: var(--text); + box-shadow: inset 0 0 0 1px var(--border); +} + :is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save:hover { filter: brightness(1.08); } diff --git a/test/settings-apply-button.test.ts b/test/settings-apply-button.test.ts new file mode 100644 index 00000000..c8e989e1 --- /dev/null +++ b/test/settings-apply-button.test.ts @@ -0,0 +1,93 @@ +/** + * @fileoverview Settings "Apply": saves like Save but keeps the modal open and refreshes the + * groups that depend on a saved value (MCP sync, custom model endpoints, CLI management). + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +const publicDir = resolve(import.meta.dirname, '../src/web/public'); +const html = readFileSync(resolve(publicDir, 'index.html'), 'utf8'); +const settingsUi = readFileSync(resolve(publicDir, 'settings-ui.js'), 'utf8'); + +function loadApp() { + const CodemanApp = function CodemanApp() {} as unknown as { prototype: Record }; + const context = vm.createContext({ + CodemanApp, + localStorage: { getItem: () => null, setItem: () => {} }, + document: { getElementById: () => null, querySelectorAll: () => [], addEventListener: () => {} }, + window: {}, + console, + }); + vm.runInContext(settingsUi, context, { filename: 'settings-ui.js' }); + return new (CodemanApp as unknown as new () => Record)(); +} + +describe('Settings Apply button', () => { + it('sits next to Save in the footer and the phone header, wired to applyAppSettings', () => { + const start = html.indexOf('