diff --git a/.prettierignore b/.prettierignore index cdfaf71e..25f5ba46 100644 --- a/.prettierignore +++ b/.prettierignore @@ -24,7 +24,6 @@ src/web/public/settings-ui.js src/web/public/sw.js src/web/public/terminal-ui.js src/web/public/voice-input.js -src/web/public/upload.html scripts/remotion/ # Hand-maintained; Prettier escapes underscores in glob paths and corrupts paragraphs. diff --git a/CLAUDE.md b/CLAUDE.md index 0c3114c5..a87feccf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,7 +127,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Code style**: Prettier (`singleQuote: true`, `printWidth: 120`, `trailingComma: "es5"`) — config lives in the **`"prettier"` key of `package.json`**, not a `.prettierrc` (keeps the repo root short; editors read it natively). `.prettierignore` stays at the root because Prettier resolves it relative to cwd. ESLint flat config (`config/eslint.config.js`) allows `no-console`, warns on `@typescript-eslint/no-explicit-any`. Ignores: `app.js`, `scripts/**/*.mjs`, `src/web/public/vendor/**`, `scripts/remotion/**`. -**Prettier scope is deliberately narrow.** `npm run format` globs only `src/**/*.ts` and `src/web/public/**` (`lint` only `src/**/*.ts`), and `.prettierignore` then exempts most of `src/web/public/*.js` (app.js, styles.css, **mobile.css**, index.html, upload.html, and 15 hand-formatted modules) plus `CLAUDE.md`. Those files are hand-formatted by design; `npm run check:public-assets` and `check:frontend-syntax` are what guard them (NUL bytes + JS syntax), not Prettier. Do not "fix" a file by adding it back to Prettier's scope. +**Prettier scope is deliberately narrow.** `npm run format` globs only `src/**/*.ts` and `src/web/public/**` (`lint` only `src/**/*.ts`), and `.prettierignore` then exempts most of `src/web/public/*.js` (app.js, styles.css, **mobile.css**, index.html, and 15 hand-formatted modules) plus `CLAUDE.md`. Those files are hand-formatted by design; `npm run check:public-assets` and `check:frontend-syntax` are what guard them (NUL bytes + JS syntax), not Prettier. Do not "fix" a file by adding it back to Prettier's scope. ## Common Gotchas @@ -493,7 +493,7 @@ curl -sk https://localhost:3000/api/subagents | jq # Background agents cat ~/.codeman/state.json | jq # Persisted state ``` -Mobile screenshots: `~/.codeman/screenshots/`, accessed via `GET/POST /api/screenshots`. +Legacy screenshots (deprecated): `GET/POST /api/screenshots` still read and write `~/.codeman/screenshots/`, but the upload page that fed them is gone, they log a one-time deprecation warning, and they are removed in a later MAJOR, after at least one MINOR release that carries the warning (`docs/versioning-policy.md`). To hand a file to an agent use `POST /api/sessions/:id/paste-image`. ## Performance & Limits diff --git a/docs/api-reference.md b/docs/api-reference.md index 4b195b34..f46c06b1 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -45,6 +45,13 @@ payload return `{ "success": true, "data": {} }`. > `GET /api/sessions/:id/tail-file` (SSE), `GET /api/download`, > `GET /api/screenshots/:name`, `GET /q/:code` (QR redirect), and the > `GET /ws/sessions/:id/terminal` WebSocket upgrade. +> +> **Deprecated:** `POST /api/screenshots`, `GET /api/screenshots` and +> `GET /api/screenshots/:name` keep working but log a one-time warning on first +> use. They are removed in a later MAJOR, after at least one MINOR release that +> carries this warning (see `docs/versioning-policy.md`). To hand +> a file to an agent, use `POST /api/sessions/:id/paste-image`, which saves it into +> that session's workspace. > The [agent wait endpoints](#long-polling-agent-wait) use the normal envelope but > are the only JSON endpoints that deliberately **hold the connection open**, for up diff --git a/docs/multi-user-plan.md b/docs/multi-user-plan.md index a3438971..44055467 100644 --- a/docs/multi-user-plan.md +++ b/docs/multi-user-plan.md @@ -158,7 +158,7 @@ Codeman now ships a global **Startup Mode** picker (App Settings, Claude CLI tab | `GET /api/away-digest` | Aggregate only owned sessions/events | | `GET /api/subagents`, workflow runs | Filter by owning session (`claudeSessionId -> session -> owner`); agents not attributable to any session: admin-only | | Push (`push-routes.ts`) | Subscription records currently carry NO identity (keyed by endpoint only): `subscribe` stamps `username`. All 8 `PUSH_EVENT_MAP` events are session-scoped, so routing = resolve owner from `data.sessionId`, deliver to that owner's (plus admins') subscriptions. Legacy identity-less subscriptions: admin-only delivery | -| Screenshots `/api/screenshots` | Per-user subdir `~/.codeman/screenshots//` in multi-user mode. Note: `GET /:name` deliberately rejects `/` in names as traversal, so derive the subdir server-side from `req.authUser` and keep client-visible names flat | +| Screenshots `/api/screenshots` (deprecated) | Deprecated: removed in a later MAJOR, so no per-user subdir is planned; the replacement `POST /api/sessions/:id/paste-image` is already session-scoped. Former plan: per-user subdir `~/.codeman/screenshots//` in multi-user mode. Note: `GET /:name` deliberately rejects `/` in names as traversal, so derive the subdir server-side from `req.authUser` and keep client-visible names flat | | Attachments | Already session-scoped; inherits the session owner check. `attachmentConfineToWorkspace` is a global, default-OFF setting today: in multi-user mode it is FORCED ON for non-admins regardless of the setting (their attachments must resolve inside their own space); the setting keeps meaning what it means for admins | | File routes (browse/preview) | Path allowlist adds: non-admin paths must resolve (realpath) inside their own space or their own sessions' workingDirs | | Settings (`settings.json`) | Global, admin-only writes in multi-user mode; reads allowed (per-device display keys stay in localStorage as today). Per-user server settings: out of scope v1 | @@ -228,7 +228,7 @@ These operate directly on `users.json` via `user-store.ts` (no server needed), h | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Default (no flag) | No behavior change. No new file reads on the hot path. All new fields optional in state | | State round-trip | `SessionState.owner`, `MuxSession.owner`, `CronJob.owner`, registry `owner` fields are optional; old state loads clean; new state loaded by an old build ignores unknown fields (existing tolerant parsing) | -| Instance isolation | `users.json`, audit log, screenshots subdirs all via `dataPath()`; user spaces dir is shared across instances like `~/codeman-cases` is today (documented) | +| Instance isolation | `users.json` and the audit log via `dataPath()`; user spaces dir is shared across instances like `~/codeman-cases` is today (documented) | | API versioning | HTTP API is internal per `docs/versioning-policy.md`; still, all changes are additive. Ship as a **minor** version | | Hooks | Unchanged (instance-level hook secret; owner resolved from the session) | diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index ded505c8..4c899732 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -158,7 +158,7 @@ Rebinding for the shortcut registry. See [Keyboard Shortcuts](Keyboard-Shortcuts ### System `CLAUDE.md` template for new cases, default working directory, the image watcher, and -Cloudflare tunnel controls including the tunnel and upload URLs. The **Diagnostics** group runs +Cloudflare tunnel controls including the tunnel URL. The **Diagnostics** group runs `codeman doctor` on the server and lists the agent CLIs, tmux, Node and the optional office tools with their versions and install hints (admin only in multi-user mode). In multi-user mode, the **Users** administration entry is injected here. diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index ba17ca60..c6a52d9e 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -448,7 +448,6 @@ 'Remote Access': '远程访问', 'Cloudflare Tunnel': 'Cloudflare 隧道', 'Tunnel URL': '隧道地址', - 'Upload URL': '上传地址', Updates: '更新', 'Current Version': '当前版本', 'Check for Updates': '检查更新', diff --git a/src/web/public/index.html b/src/web/public/index.html index 962cab83..9739c5a7 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2992,10 +2992,6 @@ - diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 499b049d..332cff77 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -1760,17 +1760,16 @@ Object.assign(CodemanApp.prototype, { } }, - _updateTunnelUrlRow(rowId, displayId, url, suffix = '') { - const row = document.getElementById(rowId); - const display = document.getElementById(displayId); + _updateTunnelUrlDisplay(url) { + const row = document.getElementById('tunnelUrlRow'); + const display = document.getElementById('tunnelUrlDisplay'); if (!row || !display) return; if (url) { - const fullUrl = url + suffix; row.style.display = ''; - display.textContent = fullUrl; + display.textContent = url; display.onclick = () => { - navigator.clipboard.writeText(fullUrl).then(() => { - this.showToast(`${suffix ? 'Upload' : 'Tunnel'} URL copied`, 'success'); + navigator.clipboard.writeText(url).then(() => { + this.showToast('Tunnel URL copied', 'success'); }); }; } else { @@ -1780,11 +1779,6 @@ Object.assign(CodemanApp.prototype, { } }, - _updateTunnelUrlDisplay(url) { - this._updateTunnelUrlRow('tunnelUrlRow', 'tunnelUrlDisplay', url); - this._updateTunnelUrlRow('tunnelUploadUrlRow', 'tunnelUploadUrlDisplay', url, '/upload.html'); - }, - showTunnelQR() { // Close existing popup if open this.closeTunnelQR(); diff --git a/src/web/public/upload.html b/src/web/public/upload.html deleted file mode 100644 index cec405cb..00000000 --- a/src/web/public/upload.html +++ /dev/null @@ -1,155 +0,0 @@ - - - - -Upload Screenshot - Codeman - - -
- ← Back to Codeman -

Upload Screenshot

-
-

Tap to select image

- PNG, JPG, WebP — up to 10 MB -
- -
- Preview -
-
- -
-
-
- - diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 53275f5e..e2251c24 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -1295,8 +1295,20 @@ export function registerSystemRoutes( // ═══════════════════════════════════════════════════════════════ // ========== Screenshots ========== + // Deprecated (the upload page is gone): removed in a later MAJOR per + // docs/versioning-policy.md. Warns once per process on first use. + let screenshotsDeprecationWarned = false; + const warnScreenshotsDeprecated = (): void => { + if (screenshotsDeprecationWarned) return; + screenshotsDeprecationWarned = true; + console.warn( + '[deprecated] /api/screenshots is deprecated and will be removed in a future major release. ' + + 'Use POST /api/sessions/:id/paste-image to hand a file to a session.' + ); + }; app.post('/api/screenshots', async (req, reply) => { + warnScreenshotsDeprecated(); const contentType = req.headers['content-type'] ?? ''; if (!contentType.includes('multipart/form-data')) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Expected multipart/form-data'); @@ -1383,6 +1395,7 @@ export function registerSystemRoutes( }); app.get('/api/screenshots', async () => { + warnScreenshotsDeprecated(); if (!existsSync(SCREENSHOTS_DIR)) { return { files: [] }; } @@ -1396,6 +1409,7 @@ export function registerSystemRoutes( }); app.get('/api/screenshots/:name', async (req, reply) => { + warnScreenshotsDeprecated(); const { name } = req.params as { name: string }; // Prevent path traversal if (name.includes('/') || name.includes('\\') || name.includes('..')) { diff --git a/src/web/server.ts b/src/web/server.ts index c3dda5ad..01f51dcc 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -932,7 +932,9 @@ export class WebServer extends EventEmitter { }); // Serve static files — content-hashed assets (e.g. app.a3f8c2e1.js) are immutable, cache aggressively. - // HTML must revalidate every time so browsers pick up new hashed filenames after deploys. + // HTML must revalidate every time so browsers pick up new hashed filenames after deploys, and every + // HTML page has its own route that says so (/, /index.html, /session/:id). ⚠️ A new static .html + // needs such a route too: this plugin would hand it a year of `immutable`. // cacheControl disabled so setHeaders owns Cache-Control for plain static assets. // preCompressed: serve pre-built .br/.gz files (from build step) to avoid per-request CPU compression await this.app.register(fastifyStatic, { @@ -944,7 +946,7 @@ export class WebServer extends EventEmitter { // `ServerResponse` to a `FastifyReply`, so it is `reply.header()` here and // NOT `res.setHeader()`. A v9-style body throws TypeError on every static // request, which is every page load. See the v10.0.0 release notes. - setHeaders: (reply, path) => { + setHeaders: (reply) => { // ⚠️ That same change ALSO flipped precedence, and silently. Under v9 this // callback wrote to the raw response and Fastify's staged reply headers then // overwrote it, so a route that set its own Cache-Control before .sendFile() @@ -953,12 +955,7 @@ export class WebServer extends EventEmitter { // no-store` its route asks for — a service worker that can never update. // So: a route that already decided keeps its answer. if (reply.getHeader('Cache-Control') !== undefined) return; - // Use .includes() not .endsWith() — preCompressed serves .html.br/.html.gz - if (path.includes('.html')) { - reply.header('Cache-Control', 'no-cache'); - } else { - reply.header('Cache-Control', 'public, max-age=31536000, immutable'); - } + reply.header('Cache-Control', 'public, max-age=31536000, immutable'); }, }); diff --git a/test/routes/system-routes.test.ts b/test/routes/system-routes.test.ts index aa1fae13..6087e7c4 100644 --- a/test/routes/system-routes.test.ts +++ b/test/routes/system-routes.test.ts @@ -680,6 +680,24 @@ describe('system-routes', () => { }); }); + describe('/api/screenshots deprecation', () => { + it('warns once across requests and leaves the response unchanged', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + mockedExistsSync.mockReturnValue(false); + const first = await harness.app.inject({ method: 'GET', url: '/api/screenshots' }); + const second = await harness.app.inject({ method: 'GET', url: '/api/screenshots' }); + expect(JSON.parse(first.body)).toEqual({ files: [] }); + expect(JSON.parse(second.body)).toEqual({ files: [] }); + const deprecations = warn.mock.calls.filter((c) => String(c[0]).includes('/api/screenshots')); + expect(deprecations).toHaveLength(1); + expect(String(deprecations[0][0])).toContain('POST /api/sessions/:id/paste-image'); + } finally { + warn.mockRestore(); + } + }); + }); + // ========== GET /api/screenshots/:name ========== describe('GET /api/screenshots/:name', () => { diff --git a/test/static-cache-headers.test.ts b/test/static-cache-headers.test.ts index 9bad8785..99ca6f6d 100644 --- a/test/static-cache-headers.test.ts +++ b/test/static-cache-headers.test.ts @@ -11,7 +11,8 @@ * * That contract is load-bearing: assets are served `immutable` for a year, and * `index.html` must revalidate every time or a deploy leaves browsers on stale - * markup (see `cacheBustAssets` in server.ts). + * markup (see `cacheBustAssets` in server.ts). No HTML is left behind the static + * plugin (every page has its own route), so the HTML half is asserted on the route. * * These tests drive a REAL WebServer on purpose. Asserting against an inline * re-registration of the plugin would keep passing after a revert in server.ts. @@ -61,18 +62,6 @@ describe('static asset Cache-Control headers', () => { expect(res.headers.get('cache-control')).toBe('public, max-age=31536000, immutable'); }); - it('makes static HTML revalidate so deploys are picked up', async () => { - // ⚠️ upload.html, NOT index.html. `/index.html` has its own explicit route that - // answers from renderIndexHtml() and never reaches @fastify/static, so asserting - // on it passes even with setHeaders fully broken (verified: reverting server.ts - // to the v9 form fails the two /app.js tests and leaves an index.html assertion - // green). upload.html has no route of its own, so it is the only HTML that - // actually exercises the `.html` branch of setHeaders. - const res = await get(`${baseUrl}/upload.html`); - expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('no-cache'); - }); - it('lets a route keep the Cache-Control it set, so sw.js stays uncached', async () => { // Regression guard for the OTHER half of the v10 change. setHeaders runs for // sendFile() too, and v10 moved it from the raw response onto the reply, which