From ea4318947b3bd0475171a05fb52190f1b91b4d26 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:04:01 +0800 Subject: [PATCH] fix(git): the interval fetches one run per folder, and the rest of the review (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scheduler is the part that runs with nobody watching, so it is decided per folder rather than per session: sessions sharing a folder are the same clone and share one run that both see, `inFlight`/`lastRunAt` are keyed by the resolved folder, the schedule is stamped when a folder is picked instead of when its run starts, and a tick is skipped while the previous one is still going. The click route joins a run already going for the same folder rather than starting a second fetch in one clone. The tick keeps ONE bound for the git processes of the whole run and hands it to every folder, and bounds its folders with a bound of its own: the same semaphore cannot serve both levels, because a folder holding a slot would then wait for a slot its own fetch needs. Both bounds are `FETCH_CONCURRENCY`, so the ceiling is unchanged. The bound claim in CLAUDE.md, the `Limiter` jsdoc and docs/api-reference.md now describes it as it is. Before anything is fetched the folder is probed with the bounded path probe, so a case on a mount that stopped answering is skipped and never touched. The rest of the review: the hooks are named as they really are (`reference-transaction`, `pre-auto-gc` through the maintenance a fetch triggers, config commands such as `credential.helper`) and `FETCH_CONFIG_ARGS` turns maintenance and gc off for an unattended run; the claim that an agent in the same repository is not disturbed is replaced by the ref-lock collision it actually is; the click route publishes its own run as `trigger: 'click'`; `autoSaveGitFetch` uses `_apiJson` so a 400 shows the error toast and applies the returned settings; and the two wording fixes. Tests: the scheduler pins one run per folder, the two bounds, the probe before anything runs, and that closing the session holding the override keeps the folder while another session is still open on it; the route test pins the click publish and the join, asserting the join before the held run is released — releasing first lets the run finish and clears the lane, so the joiner would be entitled to its own fetch and the test would pass for the wrong reason. --- CLAUDE.md | 2 +- README.md | 2 +- docs/api-reference.md | 4 +- docs/wiki/Settings-Reference.md | 12 +- docs/wiki/Working-With-Files.md | 14 +- src/git-fetch-scheduler.ts | 196 +++++++++++++++------- src/git-fetch.ts | 86 ++++++++-- src/git-workspace-status.ts | 49 +++++- src/types/session.ts | 10 +- src/web/public/i18n.js | 20 +-- src/web/public/index.html | 20 ++- src/web/public/session-ui.js | 17 +- src/web/routes/git-status-routes.ts | 40 +++-- src/web/routes/session-routes.ts | 12 +- src/web/schemas.ts | 8 +- src/web/server.ts | 14 +- test/git-fetch-scheduler.test.ts | 218 ++++++++++++++++++------- test/git-fetch.test.ts | 187 ++++++++++++++++++++- test/routes/git-status-routes.test.ts | 51 ++++++ test/session-options-structure.test.ts | 21 ++- 20 files changed, 774 insertions(+), 209 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b267b420..201306f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -236,7 +236,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Git status indicator** (`showGitStatus`, per-device, default OFF; `src/git-workspace-status.ts`, `routes/git-status-routes.ts`, `git-status-ui.js`): the bottom-bar indicator and its panel read `GET /api/sessions/:id/git-status` and `/git-diff`, read-only and offline on its own (it never fetches; fetching is the separate path below). ⚠️ git never runs on a repository at or inside a Docker case workspace (walk-up, scan and diff alike, and a cached repository list is re-checked against the CURRENT Docker roots), since a container could plant a clean filter that runs on the host; remote and Docker sessions answer `unsupported`. ⚠️ `git-diff` takes `repo`/`path`/`kind` only as keys matched against the workspace's own repository list (`findWorkspaceRepo()`) and that repository's current status, never as paths. ⚠️ Keep `--no-optional-locks`, `core.fsmonitor=false` and `log.showSignature=false` on every call and `--no-ext-diff --no-textconv` on diffs; clean filters still run, which is why the Docker rule exists. ⚠️ An enclosing repository at `$HOME` or above is ignored (`isUnrelatedAncestor()`), and is identified by one cached `rev-parse` before any full status runs. Everything git supplies renders via `textContent`. Tests: `test/git-workspace-status.test.ts`, `test/routes/git-status-routes.test.ts`, and `test/git-status.browser.test.ts` (browser suite, not in the gate). -**Git fetch** (`src/git-fetch.ts`, `src/git-fetch-scheduler.ts`, `routes/git-status-routes.ts`, the panel's **Fetch** button): `POST /api/sessions/:id/git-fetch` is the ONLY git command Codeman runs that WRITES, and the only one that runs a repository's own `fetch-post` hook — which is why it is opt-in and never runs on a timer until the user turns it on. ⚠️ Fetch updates remote-tracking refs only; it must never commit, check out, merge or touch the working tree, since an agent is often working in the same repository. ⚠️ The same Docker rule as above applies BEFORE any git runs (a repository at or inside a Docker case workspace is refused), and remote sessions answer `unsupported`. ⚠️ Keep it non-interactive (`gitNonInteractiveEnv()`: `GIT_TERMINAL_PROMPT=0` + BatchMode ssh) or a fetch hangs on an invisible login prompt, and bounded: `MAX_REPOS`/`MAX_REPOS_LIMIT` (12/50), `FETCH_CONCURRENCY` (4), `MAX_FETCH_OUTPUT_BYTES` (2 MB) and the git timeout per spawn. ⚠️ `buildFetchArgs()` is `['fetch', '--quiet']` and must stay verb-invariant (`test/git-workspace-status.test.ts`). ⚠️ Failure text reaching a client is control-stripped and credential-redacted (`sanitizeGitOutput`) and the browser writes it with `textContent`. Scheduler semantics: settings are read fresh each tick (`GIT_FETCH_TICK_INTERVAL_MS`, 15 s), a per-case override wins even while the app-wide switch is off, the interval is timed from the run START, runs never overlap, and remote/Docker cases are re-checked every tick. ⚠️ The two settings (`gitFetchEnabled`, `gitFetchIntervalSeconds`, off/300 by default, clamped 30–3600) are **synced**, not per-device: they must stay out of `displayKeys` and out of the strip destructure in settings-ui.js, or the scheduler reads nobody's value. Per-case state rides `SessionUpdated` (no new SSE event) as `gitFetch`. Tests: `test/git-fetch.test.ts`, `test/git-fetch-scheduler.test.ts`, `test/routes/session-routes-git-fetch-settings.test.ts`, `test/git-fetch-i18n.test.ts`, `test/app-settings-structure.test.ts`, `test/session-options-structure.test.ts`, and the Fetch block of `test/git-status.browser.test.ts` (browser suite, not in the gate). +**Git fetch** (`src/git-fetch.ts`, `src/git-fetch-scheduler.ts`, `routes/git-status-routes.ts`, the panel's **Fetch** button): `POST /api/sessions/:id/git-fetch` is the ONLY git command Codeman runs that WRITES, and the only one that can run a repository's own hooks (`reference-transaction`, `pre-auto-gc` through the automatic maintenance a fetch triggers, config commands such as `credential.helper`) — which is why it is opt-in and never runs on a timer until the user turns it on. ⚠️ Fetch updates remote-tracking refs only; it must never commit, check out, merge or touch the working tree. ⚠️ It is not as harmless as a read: a fetch holds git's ref lock, so an agent's own `git pull` in the same clone can hit the same `cannot lock ref` — hence one folder is fetched by at most one run at a time (`fetchWorkspaceOnce` joins a run already going), and `FETCH_CONFIG_ARGS` (`-c core.fsmonitor=false -c maintenance.auto=false -c gc.auto=0`) keeps an unattended fetch from starting a gc in a repository the agent is using. ⚠️ `buildFetchArgs()` is `['fetch', '--quiet']` and must stay verb-invariant (`test/git-workspace-status.test.ts`); those `-c` overrides are added by the runner, not by `buildFetchArgs()`. ⚠️ The same Docker rule as above applies BEFORE any git runs (a repository at or inside a Docker case workspace is refused), and remote sessions answer `unsupported`. ⚠️ Keep it non-interactive (`gitNonInteractiveEnv()`: `GIT_TERMINAL_PROMPT=0` + BatchMode ssh) or a fetch hangs on an invisible login prompt, and bounded: `MAX_REPOS`/`MAX_REPOS_LIMIT` (12/50), `MAX_FETCH_OUTPUT_BYTES` (2 MB), the git timeout per spawn, and `FETCH_CONCURRENCY` (4) as ONE `Limiter` shared by the fetches of the whole tick — `createLimiter` keeps its count in the closure, so a limiter that delegated to `mapLimited` per call bounds each call, and 4 folders × 4 repositories would start 16 git processes. ⚠️ One semaphore cannot bound both the folders and the fetches: a folder holding a slot would then wait for a slot its own fetch needs, so the tick bounds its folders with `mapLimited` and hands the shared bound to every run. ⚠️ Failure text reaching a client is control-stripped and credential-redacted (`sanitizeGitOutput`) and the browser writes it with `textContent`. Scheduler semantics: settings are read fresh each tick (`GIT_FETCH_TICK_INTERVAL_MS`, 15 s), a per-session override wins even while the app-wide switch is off, the interval is timed from the run START, state is keyed by FOLDER (sessions sharing one folder are the same clone, so one run serves them all, the schedule is stamped at selection, and every session on a folder is remembered — closing the one that holds the override must not stop the folder's fetching while another session is still open on it), a tick is skipped while the previous one is still going, and a user-chosen folder is probed BOUNDED with the default options before anything runs (rule #516 — `unknown` is never read as `absent`). ⚠️ The two settings (`gitFetchEnabled`, `gitFetchIntervalSeconds`, off/300 by default, clamped 30–3600) are **synced**, not per-device: they must stay out of `displayKeys` and out of the strip destructure in settings-ui.js, or the scheduler reads nobody's value. Per-session state rides `SessionUpdated` (no new SSE event) as `gitFetch`, and the click route publishes its own run with `trigger: 'click'` — the UI's "by click" text depends on it. Tests: `test/git-fetch.test.ts`, `test/git-fetch-scheduler.test.ts`, `test/routes/session-routes-git-fetch-settings.test.ts`, `test/git-fetch-i18n.test.ts`, `test/app-settings-structure.test.ts`, `test/session-options-structure.test.ts`, and the Fetch block of `test/git-status.browser.test.ts` (browser suite, not in the gate). **Raw file bodies are streamed and range-aware**: `file-raw`, the attachments `/raw` route, `GET /api/download` and the rendered-HTML route `GET /html-view//...` share `sendFileBody()`, advertise `Accept-Ranges: bytes` and answer `Range` with `206` + `Content-Range` (single-range, parser in `src/web/http-range.ts`); without it `