diff --git a/VERSION b/VERSION index 15d45d4..1df3b82 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.9 +1.6.10 diff --git a/install.sh b/install.sh index 2b1c67c..8945477 100755 --- a/install.sh +++ b/install.sh @@ -5,7 +5,9 @@ # # Run it from a clone of this repo (or an unzipped copy that holds # parallel-lanes/). Copies the skill to ~/.claude/skills/parallel-lanes -# (without .git), copies its parallel-lanes-worker agent type to +# (from a git checkout its tracked files only, else everything but .git), +# staged and checked first so a failed copy keeps the old install; copies its +# parallel-lanes-worker agent type to # ~/.claude/agents/parallel-lanes-worker.md, and registers its two hooks in # ~/.claude/settings.json (backed up first): # SessionStart (startup|clear|compact) -> hooks/session-start.sh @@ -123,13 +125,48 @@ mkdir -p "$claude_dir/skills" if [ "$src" = "$(cd "$dest" 2>/dev/null && pwd)" ]; then echo "Skill already at $dest; installing the agent type and registering hooks only" else + # Stage the new copy, check it, then swap it in, so a failed copy never + # leaves the skill missing (1.6.10). The staged and old copies sit outside + # skills/, where Claude Code would see a second SKILL.md. From a git + # checkout only tracked files are copied (untracked notes and logs stay + # out); from an unzipped copy, everything but .git and tests/tmp. + stage="$claude_dir/.parallel-lanes-install.$$" + old="$claude_dir/.parallel-lanes-old.$$" + rm -rf "$stage" "$old" + mkdir -p "$stage" + copied=1 + if git -C "$src" ls-files --error-unmatch SKILL.md >/dev/null 2>&1; then + list="$(mktemp)" + git -C "$src" ls-files -z | while IFS= read -r -d '' f; do + # A tracked file deleted in the work tree is not copied. + [ -e "$src/$f" ] && printf '%s\0' "$f" + done > "$list" + tar -C "$src" --null -T "$list" -cf - | tar -C "$stage" -xf - || copied=0 + rm -f "$list" + else + tar -C "$src" --exclude=.git --exclude=tests/tmp -cf - . | tar -C "$stage" -xf - || copied=0 + fi + for f in SKILL.md VERSION run.workflow.js agents/parallel-lanes-worker.md; do + [ "$copied" = 1 ] && cmp -s "$src/$f" "$stage/$f" && continue + rm -rf "$stage" + die "the new copy is incomplete ($f did not copy); the installed skill is unchanged" + done + chmod +x "$stage"/hooks/*.sh "$stage"/scripts/* if [ -e "$dest" ]; then echo "Updating existing $dest" - rm -rf "$dest" + if ! mv "$dest" "$old"; then + rm -rf "$stage" + die "cannot move the installed skill aside (is a file in it open?); it is unchanged" + fi + if ! mv "$stage" "$dest"; then + mv "$old" "$dest" + rm -rf "$stage" + die "cannot put the new copy in place; the installed skill is unchanged" + fi + rm -rf "$old" + else + mv "$stage" "$dest" fi - mkdir -p "$dest" - tar -C "$src" --exclude=.git --exclude=tests/tmp -cf - . | tar -C "$dest" -xf - - chmod +x "$dest"/hooks/*.sh "$dest"/scripts/* echo "Installed skill to $dest" fi diff --git a/reference.md b/reference.md index 394c152..1179966 100644 --- a/reference.md +++ b/reference.md @@ -78,11 +78,11 @@ PATH, else `C:/Program Files/Git`), never the WSL `bash.exe` in System32. | `repo.ledger_dir` | `/..ledger`, where `` is the plan's directory when the plan is outside the project and its repo, else `/parallel-lanes/runs//`, by default `~/.claude/parallel-lanes/runs//` (plan inside the project, e.g. `docs/superpowers/plans/`). It holds briefs, reports, and review packages too, so it must never be inside the project; the manifest goes in the same ``. | | `commands` | `setup`, `test`, `lint`, `build`: lists of shell commands run from a checkout. Take them from the plan's conventions, then the project (package.json scripts, pyproject, Makefile). Use `[]` for a group the project lacks. | | `lane_commands` | Optional `{: {setup?, test?, lint?, build?}}` overrides for lanes that need only a subset (e.g. backend lanes skip the frontend suite). They are the lane's own checks: integration and verify run every distinct one, so a narrower run of a project check belongs in `task_checks`, not here. | -| `task_checks` | Optional `{: {test, lint?, build?}}`: the targeted checks a task runs once before committing (see Check ownership). `test` is required and names at least one command; an absent `lint` or `build` inherits the lane's. A batch uses its last task's entry. Every command must be a narrower run of a check the delivery inventory already runs, in the same checkout and environment; must run correctly at the task that uses it (list the lane's test files created or changed by that task and the lane's earlier tasks, cumulatively, so no entry names a file a later task creates); must fail when it selects no tests (name existing test files; a filter that passes on zero matches needs a wrapper that fails); and keeps an explicit worker limit like the project's test command (for example `node --test --test-concurrency=4 ...`). Derive the entries from the briefs' test files and commands; a task you cannot map gets no entry and runs its lane's checks. Never part of the delivery inventory. | +| `task_checks` | Optional `{: {test, lint?, build?}}`: the targeted checks a task runs once before committing (see Check ownership). `test` is required and names at least one command; an absent `lint` or `build` inherits the lane's. A batch uses its last task's entry. Every command must be a narrower run of a check the delivery inventory already runs, in the same checkout and environment; must run correctly at the task that uses it (list the lane's test files created or changed by that task and the lane's earlier tasks, cumulatively, so no entry names a file a later task creates); must fail when it selects no tests (name existing test files; a filter that passes on zero matches needs a wrapper that fails); and keeps an explicit worker limit like the project's test command (for example `node --test --test-concurrency=4 ...`). Derive the entries from the briefs' test files and commands; a task you cannot map gets no entry and runs its lane's checks. Never part of the delivery inventory. An entry for a task whose lane (or, for a prelude or join task, the project) has no test, lint or build command is a validation error from 1.6.10: the run builds no task-checks command there, so the entry would never run. | | `widen_paths` | Optional list of project-relative paths (a trailing `/` marks a directory; an existing directory named without it counts as one too) of shared infrastructure: test harness and helpers, package and lock files, build and CI config, modules many lanes import. A task whose change touches one runs project-wide checks instead of its targeted ones. Name the files every check runs through (wrappers, shared fixtures, settings), not a folder that also holds tools and tests; name a folder only when everything under it is shared (`Packages/`, `ProjectSettings/`). For example, run f2a listed `Tools/Unity/` for the Unity batch wrappers in it: units RP2 and RB4 declared other files under it (`Tools/Unity/tests/__init__.py`) and widened to every Unity suite, 72 minutes on the critical path (32% of the run) that found nothing; the wrapper files themselves were the entries it needed. The dry run lists every unit that will widen (`checks.widening`, see Confirmation header). Take them from the plan's shared files; no absolute path or `..`. | | `repairs_spent` | Optional integer, default 0: repairs after a failing verify that earlier launches of this run made (the sum of `final.repairs` lengths in their reports; a report without `final` made none). Set it on a transient relaunch or a resume, like the budget carry-over; a run allows 2 in all (see Check ownership). | | `prelude` | Tasks run first on the feature branch, before lanes. | -| `lanes` | `[{id, name, setup_note?, tasks}]`. `id` matches `^[A-Za-z0-9_][A-Za-z0-9._-]*$` (it names the ledger file); `name` is the progress phase label. Ids `prelude` and `join` are reserved. No file may appear in two lanes unless an `overlaps` entry records it; files are compared normalized (`./a`, `a//b`, `a/../b`) and case-insensitively. | +| `lanes` | `[{id, name, setup_note?, tasks}]`. `id` matches `^[A-Za-z0-9_][A-Za-z0-9._-]*$` (it names the ledger file); `name` is the progress phase label. Ids `prelude` and `join` are reserved. No file may appear in two lanes unless an `overlaps` entry records it; files are compared normalized (`./a`, `a//b`, `a/../b`) and case-insensitively. A folder entry (`src/`, or `src` holding a path another task lists) covers the paths under it, as task-checks reads it, so a folder in one lane and a path under it in another are a claim by both lanes too (from 1.6.10); an `overlaps` entry on the file, listing both tasks, records it. Join waves use the same rule: a unit that declares a folder never shares a wave with a unit declaring a path under it. | | `join` | Tasks run in order on the merged branch after integration. | | task | `{id, title, files, tier, security, batch?, depends_on?}`. `id` exactly as in the plan heading (`T13a`, `7`) and a safe file name (`^[A-Za-z0-9_][A-Za-z0-9._-]*$`: it names brief, report, and review files); `title` and `files` from derive-lanes (project-relative: no absolute path, nothing that leaves the project; each review is shown the files its range changes outside `files`, from git, and judges each); `tier` `standard`, `sonnet`, or `light` (see Tiers); a sonnet or light task cannot have `security: true`; `batch` only on light tasks (see Batching); `depends_on` `[{id, kind: "code" or "contract"}]` (see Building lanes). | | `overlaps` | Optional `[{file, tasks, reason, merge_owner, validation?}]`: a file tasks in different lanes both change on purpose. Every listed task lists the file; `merge_owner` is one of them (its version wins where both changes cannot stand); `validation` says how the merged file is checked (a command, or what to look at). The integration agent is told about each and runs the validation after the merge. | @@ -733,7 +733,11 @@ raised it). since the final re-review must follow it), or the agents unavailable, the run makes no call and records `final.start_check` as `unavailable` with `why: 'budget'`, and nothing else changes; `repairReserve` is unchanged, since the start check runs before repair - admission and its agent is already counted. + admission and its agent is already counted. From 1.6.10 it is an advisory call: it runs once, + with no retry and no availability probe, and when no agent is free it is not run and sets no + refusal latch. A start check that returns nothing is `unavailable` (`why: 'no result'`), and + the run goes on to its acceptance; before 1.6.10 its refused retry could stop the run on + `budget`. - Agents unavailable (see "Usage limit") uses the same refusal path: once the availability probe found no agent answering, every later spawn is refused, `verify` included (a refused verify is never stored as evidence), and the run stops with reason `agents_unavailable` @@ -2071,7 +2075,7 @@ Hard failures: |---|---| | `git_identity` | `git var GIT_COMMITTER_IDENT` or (from 1.6.7) `git var GIT_AUTHOR_IDENT` fails, after any `-c user.name=... -c user.email=...` pair `commit_rules` passes | | `base_not_current` | the local `base_ref` does not contain its upstream tip (details below) | -| `python` | the manifest's `python` does not run Python 3.8 or newer | +| `python` | the manifest's `python` does not run Python 3.8 or newer, run as one path the way the run uses it (a path with spaces is fine), or it is wrapped in quotes (from 1.6.10; doctor used to split it into shell words) | | `dirty_checkout` | git mode: the checkout has uncommitted changes or untracked, non-ignored files; its fix says to commit any file a check needs and gitignore only known generated output, and never to ignore source a check needs | | `shadow_repo` | shadow mode: the shadow repo is missing or broken | diff --git a/run.workflow.js b/run.workflow.js index 0884445..35ff3c5 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1,6 +1,6 @@ export const meta = { name: 'parallel-lanes', - description: 'parallel-lanes v1.6.9: Execute an implementation plan as parallel lanes of tasks with per-task review, then integrate.', + description: 'parallel-lanes v1.6.10: Execute an implementation plan as parallel lanes of tasks with per-task review, then integrate.', phases: [ { title: 'Pre-flight', detail: 'plan and spec conflicts' }, { title: 'Prelude', detail: 'shared tasks on the feature branch' }, @@ -432,19 +432,22 @@ function validateManifest(m) { if (isText(t.id)) laneOfTask.set(t.id, lane.id); if (isTextList(t.files)) t.files.forEach((f) => files.set(fileKey(f), { f, task: t.id })); } + // A folder entry covers the paths under it (task-checks reads it so), + // so a folder in one lane and a path under it in another are a claim + // by both lanes too (external review F2): keys are equal, or one is a + // folder holding the other. + const holds = (a, b) => a === b || b.startsWith(`${a}/`) || a.startsWith(`${b}/`); for (const [key, { f, task }] of files) { - const owner = fileOwner.get(key); - if (owner === undefined) { - fileOwner.set(key, { lane: lane.id, task, f }); - continue; - } - if (owner.lane === lane.id) continue; - const o = overlapFor.get(key); - if (!o || !o.tasks.includes(task) || !o.tasks.includes(owner.task)) { - err(`file ${f}: claimed by lanes ${owner.lane} and ${lane.id}` + - (owner.f !== f ? ` (as ${owner.f} and ${f})` : '') + - '; record a deliberate overlap in overlaps or keep it in one lane'); + const others = [...fileOwner].filter(([k, o]) => o.lane !== lane.id && holds(k, key)); + for (const [ownerKey, owner] of others) { + const o = overlapFor.get(key) || overlapFor.get(ownerKey); + if (!o || !o.tasks.includes(task) || !o.tasks.includes(owner.task)) { + err(`file ${f}: claimed by lanes ${owner.lane} and ${lane.id}` + + (owner.f !== f ? ` (as ${owner.f} and ${f})` : '') + + '; record a deliberate overlap in overlaps or keep it in one lane'); + } } + if (!fileOwner.has(key)) fileOwner.set(key, { lane: lane.id, task, f }); } }); } @@ -452,15 +455,55 @@ function validateManifest(m) { if ('join' in m) checkTaskList('join', m.join); - // Every overlaps entry names known tasks that list its file. + // Batch unit ids (batchUnit: '-') name agent labels, briefs, + // reports and branches as task ids do, so none may equal a task id or + // another batch's, in any letter case (external review F1, 2026-10-17). A + // resume can run part of a batch (batchGroup), so every first-last stretch + // of consecutive tasks with one batch key counts. + { + const taskOf = new Map(); + for (const t of allTasks) if (isObject(t) && isText(t.id)) taskOf.set(t.id.toLowerCase(), t.id); + const unitOf = new Map(); + const lists = [m.prelude, ...(Array.isArray(m.lanes) ? m.lanes.map((l) => (isObject(l) ? l.tasks : null)) : []), m.join]; + for (const list of lists) { + if (!Array.isArray(list)) continue; + for (let i = 0; i < list.length;) { + const t = list[i]; + let j = i + 1; + if (isObject(t) && isText(t.batch)) { + while (j < list.length && isObject(list[j]) && list[j].batch === t.batch) j += 1; + } + const run = list.slice(i, j).filter((x) => isObject(x) && isText(x.id)); + for (let a = 0; a < run.length; a += 1) { + for (let b = a + 1; b < run.length; b += 1) { + const id = `${run[a].id}-${run[b].id}`; + const key = id.toLowerCase(); + const what = `batch ${run[a].id}..${run[b].id}`; + if (taskOf.has(key)) { + err(`${what}: runs as unit ${id}, which task ${taskOf.get(key)} also names (ids name files); rename one`); + } else if (unitOf.has(key)) { + err(`${what}: runs as unit ${id}, as ${unitOf.get(key)} does (ids name files); rename a task`); + } else { + unitOf.set(key, what); + } + } + } + i = j; + } + } + } + + // Every overlaps entry names known tasks that list its file, or a folder + // holding it (a folder entry covers the paths under it). if (Array.isArray(m.overlaps)) { const files = new Map(); for (const t of allTasks) if (isObject(t) && isText(t.id) && isTextList(t.files)) files.set(t.id, t.files.map(fileKey)); m.overlaps.forEach((o, i) => { if (!isObject(o) || !isTextList(o.tasks)) return; for (const id of o.tasks) { + const key = isText(o.file) ? fileKey(o.file) : null; if (!files.has(id)) err(`overlaps[${i}]: unknown task id ${id}`); - else if (isText(o.file) && !files.get(id).includes(fileKey(o.file))) { + else if (key !== null && !files.get(id).some((k) => k === key || key.startsWith(`${k}/`))) { err(`overlaps[${i}]: task ${id} does not list ${o.file} in its files`); } } @@ -563,6 +606,22 @@ function validateManifest(m) { } if (!('test' in entry)) err(`${where}.test: missing`); else if (Array.isArray(entry.test) && entry.test.length === 0) err(`${where}.test: must name at least one command`); + // The run builds the task-checks command only when the task's lane + // has checks (taskChecksCommand), so an entry there would never run + // (external review F3). Malformed commands are reported above. + if (taskIds.has(id) && isObject(m.commands)) { + const lane = laneOfTask.has(id) ? laneOfTask.get(id) : null; + let none = false; + try { + none = laneChecks(m, lane).length === 0; + } catch (e) { + none = false; + } + if (none) { + err(`${where}: ${lane === null ? 'the project' : `the task's lane ${lane}`} has no test, lint or build command, ` + + 'so the run would never run these task checks; add those commands, or remove the entry'); + } + } } } } @@ -4257,9 +4316,28 @@ function makeIo(m, baseIo, state) { } return again; }; + // An advisory call (opts.advisory: the start check) runs once: no retry and + // no availability probe, and with no agent free it is not run and sets no + // refusal latch, so it can never change how the run ends (external review + // F5). Its answer, a refusal sentinel, or null on an error, goes back. + const advisoryCall = async (prompt, opts) => { + const { advisory, ...rest } = opts; + if (state.unavailable || state.refused.length > 0 || state.agents >= limits.max_agents) { + baseIo.log(`parallel-lanes: ${rest.label} was not run: no agent free (it is advisory; the run goes on)`); + return state.unavailable ? { __budget: true, __unavailable: true } : { __budget: true }; + } + const agentType = state.untyped ? null : agentTypeFor(m, rest.label); + try { + return await spawn(prompt, agentType === null ? rest : { ...rest, agentType }); + } catch (e) { + baseIo.log(`parallel-lanes: ${rest.label} failed (${e && e.message ? e.message : String(e)}); it is advisory`); + return null; + } + }; return { ...baseIo, agent: async (prompt, opts) => { + if (opts && opts.advisory) return advisoryCall(prompt, opts); // Every null result has been retried once: a null here is a dead pair. const r = await call(prompt, opts); if (r !== null && r !== undefined) return r; @@ -4333,6 +4411,16 @@ function waveFileKey(path) { return String(path).replace(/\\/g, '/').replace(/\/+/g, '/').replace(/^(\.\/)+/, '').toLowerCase(); } +// Whether two declared paths (waveFileKey) can name one file: they are equal, +// or one is a folder holding the other (a file holds no paths, so the +// shorter one is a folder, named with or without a trailing '/'). task-checks +// reads a folder entry as covering the paths under it (external review F2). +function filesOverlap(a, b) { + const x = a.replace(/\/+$/, ''); + const y = b.replace(/\/+$/, ''); + return x === y || x === '' || y === '' || y.startsWith(`${x}/`) || x.startsWith(`${y}/`); +} + // The waves of the join's units, in plan order: [[unit id, ...], ...]. // units: [{id, tasks?, depends_on?, files}] (a batch unit lists its tasks; // its tasks' ids, plain or as objects, are what other units name). edges: @@ -4341,7 +4429,8 @@ function waveFileKey(path) { // it names, and on each edge's producer that is earlier (later producers and // later ids are ignored: validation refuses a code dependency on a later join // task). Its wave is 1 + the highest wave it depends on; then, while an -// earlier unit of that wave declares one of its files (waveFileKey), the next. +// earlier unit of that wave declares a path that overlaps one of its own +// (filesOverlap on waveFileKey), the next. function joinWaves(units, edges = []) { const ids = (u) => (Array.isArray(u.tasks) ? u.tasks.map((t) => (typeof t === 'string' ? t : t.id)) : [u.id]); const wave = []; @@ -4358,8 +4447,9 @@ function joinWaves(units, edges = []) { if (j >= 0) on.add(j); } let w = 1 + Math.max(0, ...[...on].map((j) => wave[j])); - const mine = new Set((u.files || []).map(waveFileKey)); - const clash = (n) => earlier.some((e, j) => wave[j] === n && (e.files || []).some((f) => mine.has(waveFileKey(f)))); + const mine = (u.files || []).map(waveFileKey); + const clash = (n) => earlier.some((e, j) => wave[j] === n + && (e.files || []).some((f) => mine.some((k) => filesOverlap(k, waveFileKey(f))))); while (clash(w)) w += 1; wave.push(w); }); @@ -6652,7 +6742,8 @@ async function runAll(manifest, io) { return { ...classifyStartCheck(m, v, sha, null), why: 'budget' }; } const r = await callM('start check', 'Verify', startCheckPrompt(m, v, sha), startCheckSchema(), - { model: 'haiku', effort: 'high' }); + { model: 'haiku', effort: 'high', advisory: true }); + if (r && r.__budget) return { ...classifyStartCheck(m, v, sha, null), why: 'budget' }; return classifyStartCheck(m, v, sha, r); }; // A budget stop once every task is in: the checks still run at the @@ -6806,7 +6897,7 @@ function noHead(r, label) { // The skill version this script was built from (scripts/build stamps VERSION // here and in the meta description): the version actually running. Logged // first and returned with every result, dry runs included. -const VERSION = '1.6.9'; +const VERSION = '1.6.10'; log(`parallel-lanes v${VERSION} loaded`); if (args !== null && typeof args === 'object' && args.dry_run === true) { diff --git a/scripts/_doctor.py b/scripts/_doctor.py index 56f859b..1f1a8ce 100644 --- a/scripts/_doctor.py +++ b/scripts/_doctor.py @@ -93,7 +93,6 @@ import json import os import re -import shlex import shutil import signal import subprocess @@ -636,24 +635,26 @@ def check_base_not_current(manifest, probes, prefix, report): def _python_argv(manifest): - """(text, argv): the manifest's python command (python3 when unset or - blank) and its shell words ([] when its quoting does not parse).""" + """(text, argv): the manifest's python (python3 when unset or blank) + and the argv that runs it: the value as one path, as the engine runs it + (pythonCommand shell-quotes the whole value), so a path with a space + works; [] when the value is wrapped in quotes, which the engine would + take as part of the path.""" python = manifest.get("python") python = python if isinstance(python, str) and python.strip() else "python3" - try: - argv = shlex.split(python) - except ValueError: - argv = [] - return python, argv + if len(python) >= 2 and python[0] == python[-1] and python[0] in "'\"": + return python, [] + return python, [python] def check_python(manifest, probes, report): check = "python" - fix = ("set the manifest's python to a Python 3.8 or newer (scripts/find-python " - "prints one), shell-quoted.") + fix = ("set the manifest's python to the path of a Python 3.8 or newer as " + "scripts/find-python prints it, not quoted.") python, argv = _python_argv(manifest) if not argv: - report.fail(check, "python is not a shell-quoted command", fix) + report.fail(check, "python is quoted: the run uses the value as one path, quotes " + "included: " + python, fix) return if probes.platform == "win32": argv[0] = _shell._drive_form(argv[0]) or argv[0] @@ -823,7 +824,6 @@ def check_python_alias(manifest, probes, report): program = argv[0] if argv else python if "/" not in program and "\\" not in program: program = _read(probes.which, program) or program - # The unsplit text too: shlex drops the backslashes of an unquoted C:\... path. for candidate in (program, python): if isinstance(candidate, str) and _WINDOWS_APPS.search(candidate): report.warn(check, "the manifest's python is the WindowsApps alias, which is " diff --git a/scripts/_owners.py b/scripts/_owners.py index 8609ed8..479b47d 100644 --- a/scripts/_owners.py +++ b/scripts/_owners.py @@ -173,8 +173,13 @@ def annotate(owners, unit_ids, path): def first_owner(owners, unit_ids, path): - """(owner, relation) of the first other task listing path, or (None, None).""" - others = [t for t in owners["files"].get(path_key(path), []) if t not in unit_ids] - if not others: - return None, None - return others[0], relation(owners, unit_ids, others[0]) + """(owner, relation) of the first other task listing path, or else a + folder holding it, nearest folder first (a folder entry covers the paths + under it, as in task-checks); (None, None) when no other task does.""" + key = path_key(path) + while key and key != ".": + others = [t for t in owners["files"].get(key, []) if t not in unit_ids] + if others: + return others[0], relation(owners, unit_ids, others[0]) + key = posixpath.dirname(key) + return None, None diff --git a/scripts/_waves.py b/scripts/_waves.py index 097f909..5e10f8f 100644 --- a/scripts/_waves.py +++ b/scripts/_waves.py @@ -51,6 +51,15 @@ def file_key(path): return key.lower() +def overlap(a, b): + """Whether two declared paths (file_key) can name one file: equal, or + one is a folder holding the other (a file holds no paths, so the shorter + is a folder, named with or without a trailing '/'), as filesOverlap in + src/waves.js.""" + x, y = a.rstrip("/"), b.rstrip("/") + return x == y or x == "" or y == "" or y.startswith(x + "/") or x.startswith(y + "/") + + def waves(units, edges=()): """The waves of units in plan order, as lists of unit ids. edges: (unit_id, producer_id) pairs; a producer that is not earlier is @@ -71,10 +80,10 @@ def waves(units, edges=()): if e["id"] == producer: on.add(j) w = 1 + max([wave[j] for j in on] or [0]) - files = set(file_key(f) for f in unit.get("files") or []) + files = [file_key(f) for f in unit.get("files") or []] def clash(n): - return any(wave[j] == n and any(file_key(f) in files for f in e.get("files") or []) + return any(wave[j] == n and any(overlap(file_key(f), k) for f in e.get("files") or [] for k in files) for j, e in enumerate(earlier)) while clash(w): diff --git a/scripts/derive-lanes b/scripts/derive-lanes index f8f7505..4fe35f5 100755 --- a/scripts/derive-lanes +++ b/scripts/derive-lanes @@ -38,8 +38,9 @@ Per task: Prints JSON on stdout: tasks [{id, title, files, deps}] in plan order - groups connected components over shared files, each in plan - order, sorted by first task position + groups connected components over shared files (a folder links + the paths under it), each in plan order, sorted by + first task position bridge_files [{file, splits_into}] for each file whose edges alone hold its group together: without them the group falls into the listed parts (each in plan order) @@ -275,12 +276,24 @@ class UnionFind: def components(ids, file_tasks, skip=None): - """Connected components of ids over shared files, in plan order.""" + """Connected components of ids over shared files, in plan order. A path + also links the tasks listing a folder above it: a folder entry covers + the paths under it, and validation refuses a folder in one lane and a + path under it in another (1.6.10). skip leaves one path's links out.""" uf = UnionFind(ids) + folders = {} + for path in file_tasks: + folders.setdefault(path.lower(), []).append(path) for path, touching in file_tasks.items(): if path == skip: continue members = [t for t in touching if t in uf.parent] + up = posixpath.dirname(path) + while up: + for folder in folders.get(up.lower(), []): + if folder != skip: + members += [t for t in file_tasks[folder] if t in uf.parent] + up = posixpath.dirname(up) for other in members[1:]: uf.union(members[0], other) parts = {} diff --git a/src/budget.js b/src/budget.js index 5bc1b97..212215b 100644 --- a/src/budget.js +++ b/src/budget.js @@ -153,9 +153,28 @@ function makeIo(m, baseIo, state) { } return again; }; + // An advisory call (opts.advisory: the start check) runs once: no retry and + // no availability probe, and with no agent free it is not run and sets no + // refusal latch, so it can never change how the run ends (external review + // F5). Its answer, a refusal sentinel, or null on an error, goes back. + const advisoryCall = async (prompt, opts) => { + const { advisory, ...rest } = opts; + if (state.unavailable || state.refused.length > 0 || state.agents >= limits.max_agents) { + baseIo.log(`parallel-lanes: ${rest.label} was not run: no agent free (it is advisory; the run goes on)`); + return state.unavailable ? { __budget: true, __unavailable: true } : { __budget: true }; + } + const agentType = state.untyped ? null : agentTypeFor(m, rest.label); + try { + return await spawn(prompt, agentType === null ? rest : { ...rest, agentType }); + } catch (e) { + baseIo.log(`parallel-lanes: ${rest.label} failed (${e && e.message ? e.message : String(e)}); it is advisory`); + return null; + } + }; return { ...baseIo, agent: async (prompt, opts) => { + if (opts && opts.advisory) return advisoryCall(prompt, opts); // Every null result has been retried once: a null here is a dead pair. const r = await call(prompt, opts); if (r !== null && r !== undefined) return r; diff --git a/src/phases.js b/src/phases.js index e825e54..bd618ef 100644 --- a/src/phases.js +++ b/src/phases.js @@ -1292,7 +1292,8 @@ async function runAll(manifest, io) { return { ...classifyStartCheck(m, v, sha, null), why: 'budget' }; } const r = await callM('start check', 'Verify', startCheckPrompt(m, v, sha), startCheckSchema(), - { model: 'haiku', effort: 'high' }); + { model: 'haiku', effort: 'high', advisory: true }); + if (r && r.__budget) return { ...classifyStartCheck(m, v, sha, null), why: 'budget' }; return classifyStartCheck(m, v, sha, r); }; // A budget stop once every task is in: the checks still run at the diff --git a/src/validate.js b/src/validate.js index b7998c7..988a0a4 100644 --- a/src/validate.js +++ b/src/validate.js @@ -331,19 +331,22 @@ function validateManifest(m) { if (isText(t.id)) laneOfTask.set(t.id, lane.id); if (isTextList(t.files)) t.files.forEach((f) => files.set(fileKey(f), { f, task: t.id })); } + // A folder entry covers the paths under it (task-checks reads it so), + // so a folder in one lane and a path under it in another are a claim + // by both lanes too (external review F2): keys are equal, or one is a + // folder holding the other. + const holds = (a, b) => a === b || b.startsWith(`${a}/`) || a.startsWith(`${b}/`); for (const [key, { f, task }] of files) { - const owner = fileOwner.get(key); - if (owner === undefined) { - fileOwner.set(key, { lane: lane.id, task, f }); - continue; - } - if (owner.lane === lane.id) continue; - const o = overlapFor.get(key); - if (!o || !o.tasks.includes(task) || !o.tasks.includes(owner.task)) { - err(`file ${f}: claimed by lanes ${owner.lane} and ${lane.id}` + - (owner.f !== f ? ` (as ${owner.f} and ${f})` : '') + - '; record a deliberate overlap in overlaps or keep it in one lane'); + const others = [...fileOwner].filter(([k, o]) => o.lane !== lane.id && holds(k, key)); + for (const [ownerKey, owner] of others) { + const o = overlapFor.get(key) || overlapFor.get(ownerKey); + if (!o || !o.tasks.includes(task) || !o.tasks.includes(owner.task)) { + err(`file ${f}: claimed by lanes ${owner.lane} and ${lane.id}` + + (owner.f !== f ? ` (as ${owner.f} and ${f})` : '') + + '; record a deliberate overlap in overlaps or keep it in one lane'); + } } + if (!fileOwner.has(key)) fileOwner.set(key, { lane: lane.id, task, f }); } }); } @@ -351,15 +354,55 @@ function validateManifest(m) { if ('join' in m) checkTaskList('join', m.join); - // Every overlaps entry names known tasks that list its file. + // Batch unit ids (batchUnit: '-') name agent labels, briefs, + // reports and branches as task ids do, so none may equal a task id or + // another batch's, in any letter case (external review F1, 2026-10-17). A + // resume can run part of a batch (batchGroup), so every first-last stretch + // of consecutive tasks with one batch key counts. + { + const taskOf = new Map(); + for (const t of allTasks) if (isObject(t) && isText(t.id)) taskOf.set(t.id.toLowerCase(), t.id); + const unitOf = new Map(); + const lists = [m.prelude, ...(Array.isArray(m.lanes) ? m.lanes.map((l) => (isObject(l) ? l.tasks : null)) : []), m.join]; + for (const list of lists) { + if (!Array.isArray(list)) continue; + for (let i = 0; i < list.length;) { + const t = list[i]; + let j = i + 1; + if (isObject(t) && isText(t.batch)) { + while (j < list.length && isObject(list[j]) && list[j].batch === t.batch) j += 1; + } + const run = list.slice(i, j).filter((x) => isObject(x) && isText(x.id)); + for (let a = 0; a < run.length; a += 1) { + for (let b = a + 1; b < run.length; b += 1) { + const id = `${run[a].id}-${run[b].id}`; + const key = id.toLowerCase(); + const what = `batch ${run[a].id}..${run[b].id}`; + if (taskOf.has(key)) { + err(`${what}: runs as unit ${id}, which task ${taskOf.get(key)} also names (ids name files); rename one`); + } else if (unitOf.has(key)) { + err(`${what}: runs as unit ${id}, as ${unitOf.get(key)} does (ids name files); rename a task`); + } else { + unitOf.set(key, what); + } + } + } + i = j; + } + } + } + + // Every overlaps entry names known tasks that list its file, or a folder + // holding it (a folder entry covers the paths under it). if (Array.isArray(m.overlaps)) { const files = new Map(); for (const t of allTasks) if (isObject(t) && isText(t.id) && isTextList(t.files)) files.set(t.id, t.files.map(fileKey)); m.overlaps.forEach((o, i) => { if (!isObject(o) || !isTextList(o.tasks)) return; for (const id of o.tasks) { + const key = isText(o.file) ? fileKey(o.file) : null; if (!files.has(id)) err(`overlaps[${i}]: unknown task id ${id}`); - else if (isText(o.file) && !files.get(id).includes(fileKey(o.file))) { + else if (key !== null && !files.get(id).some((k) => k === key || key.startsWith(`${k}/`))) { err(`overlaps[${i}]: task ${id} does not list ${o.file} in its files`); } } @@ -462,6 +505,22 @@ function validateManifest(m) { } if (!('test' in entry)) err(`${where}.test: missing`); else if (Array.isArray(entry.test) && entry.test.length === 0) err(`${where}.test: must name at least one command`); + // The run builds the task-checks command only when the task's lane + // has checks (taskChecksCommand), so an entry there would never run + // (external review F3). Malformed commands are reported above. + if (taskIds.has(id) && isObject(m.commands)) { + const lane = laneOfTask.has(id) ? laneOfTask.get(id) : null; + let none = false; + try { + none = laneChecks(m, lane).length === 0; + } catch (e) { + none = false; + } + if (none) { + err(`${where}: ${lane === null ? 'the project' : `the task's lane ${lane}`} has no test, lint or build command, ` + + 'so the run would never run these task checks; add those commands, or remove the entry'); + } + } } } } diff --git a/src/waves.js b/src/waves.js index 2b0ae4a..94b02d9 100644 --- a/src/waves.js +++ b/src/waves.js @@ -39,6 +39,16 @@ function waveFileKey(path) { return String(path).replace(/\\/g, '/').replace(/\/+/g, '/').replace(/^(\.\/)+/, '').toLowerCase(); } +// Whether two declared paths (waveFileKey) can name one file: they are equal, +// or one is a folder holding the other (a file holds no paths, so the +// shorter one is a folder, named with or without a trailing '/'). task-checks +// reads a folder entry as covering the paths under it (external review F2). +function filesOverlap(a, b) { + const x = a.replace(/\/+$/, ''); + const y = b.replace(/\/+$/, ''); + return x === y || x === '' || y === '' || y.startsWith(`${x}/`) || x.startsWith(`${y}/`); +} + // The waves of the join's units, in plan order: [[unit id, ...], ...]. // units: [{id, tasks?, depends_on?, files}] (a batch unit lists its tasks; // its tasks' ids, plain or as objects, are what other units name). edges: @@ -47,7 +57,8 @@ function waveFileKey(path) { // it names, and on each edge's producer that is earlier (later producers and // later ids are ignored: validation refuses a code dependency on a later join // task). Its wave is 1 + the highest wave it depends on; then, while an -// earlier unit of that wave declares one of its files (waveFileKey), the next. +// earlier unit of that wave declares a path that overlaps one of its own +// (filesOverlap on waveFileKey), the next. function joinWaves(units, edges = []) { const ids = (u) => (Array.isArray(u.tasks) ? u.tasks.map((t) => (typeof t === 'string' ? t : t.id)) : [u.id]); const wave = []; @@ -64,8 +75,9 @@ function joinWaves(units, edges = []) { if (j >= 0) on.add(j); } let w = 1 + Math.max(0, ...[...on].map((j) => wave[j])); - const mine = new Set((u.files || []).map(waveFileKey)); - const clash = (n) => earlier.some((e, j) => wave[j] === n && (e.files || []).some((f) => mine.has(waveFileKey(f)))); + const mine = (u.files || []).map(waveFileKey); + const clash = (n) => earlier.some((e, j) => wave[j] === n + && (e.files || []).some((f) => mine.some((k) => filesOverlap(k, waveFileKey(f))))); while (clash(w)) w += 1; wave.push(w); }); diff --git a/tests/derive.test.mjs b/tests/derive.test.mjs index a1c481d..b430caa 100644 --- a/tests/derive.test.mjs +++ b/tests/derive.test.mjs @@ -208,6 +208,33 @@ test('derive-lanes warns about tasks without a Files block', () => { // --- other plan shapes ------------------------------------------------------ +// 1.6.10 (external review F2): a folder in a task's Files covers the paths +// under it, and validation refuses a folder in one lane and a path under it +// in another, so derive-lanes groups those tasks together. +test('derive-lanes groups a task listing a folder with the tasks listing paths under it', () => { + const plan = planFile( + [ + '# Plan', + '', + '### Task 1: Folder', + '**Files:**', + '- Modify: `src/`', + '', + '### Task 2: Deep file', + '**Files:**', + '- Modify: `src/a/x.js`', + '', + '### Task 3: Sibling', + '**Files:**', + '- Modify: `src2/y.js`', + '', + ].join('\n'), + ); + const { out } = deriveJson(plan); + assert.deepEqual(out.tasks.map((t) => t.files), [['src'], ['src/a/x.js'], ['src2/y.js']]); + assert.deepEqual(out.groups, [['1', '2'], ['3']]); +}); + test('derive-lanes gives one group per task and a warning when no task has Files', () => { const plan = planFile( [ diff --git a/tests/doctor.cases.mjs b/tests/doctor.cases.mjs index 4c23ce9..58ef21a 100644 --- a/tests/doctor.cases.mjs +++ b/tests/doctor.cases.mjs @@ -5,10 +5,10 @@ import { after } from 'node:test'; import { test } from './shard.mjs'; import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { join } from 'node:path'; +import { chmodSync, existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { basename, dirname, join } from 'node:path'; import { pathToFileURL } from 'node:url'; -import { SKILL_DIR } from './harness.mjs'; +import { SKILL_DIR, loadHelpers } from './harness.mjs'; import { BASH, IS_WINDOWS, tempDir } from './platform.mjs'; const SCRIPT = join(SKILL_DIR, 'scripts', 'doctor'); @@ -37,7 +37,8 @@ writeFileSync(IDENTITY, [ const ENV = cleanEnv({ GIT_CONFIG_GLOBAL: IDENTITY, GIT_CONFIG_SYSTEM: EMPTY, GIT_CONFIG_NOSYSTEM: '1' }); const NO_IDENTITY = cleanEnv({ GIT_CONFIG_GLOBAL: EMPTY, GIT_CONFIG_SYSTEM: EMPTY, GIT_CONFIG_NOSYSTEM: '1' }); -// The manifest's python: this Python, shell-quoted, with '/' separators. +// The manifest's python: this Python's path as scripts/find-python prints it +// (bare, not quoted), with '/' separators. const PYTHON = spawnSync('python3', ['-c', 'import sys; print(sys.executable)'], { encoding: 'utf8' }) .stdout.trim().replace(/\\/g, '/'); @@ -86,7 +87,7 @@ function fixture() { mode: 'git', root, git_dir: null, base_ref: 'main', branch: 'pl-feature', worktree_root: join(dir, 'wt'), ledger_dir: join(dir, 'ledger'), ...repo, }, - python: `'${PYTHON}'`, + python: PYTHON, commit_rules: 'Follow the repository style.', commands: {}, lanes: [{ id: 'a' }], @@ -238,16 +239,44 @@ test('an unreachable remote or no upstream does not block', () => { test('a python that is not 3.8+ fails', () => { const f = fixture(); const missing = join(f.dir, 'no-such-python').replace(/\\/g, '/'); - const r = doctor(f.manifest({}, { python: `'${missing}'` })); + const r = doctor(f.manifest({}, { python: missing })); assert.equal(r.code, 3, r.stderr); assert.ok(named(r.out.failures, 'python'), JSON.stringify(r.out)); - // A Python that runs but reports an old version. - const old = doctor(f.manifest({}, { python: `'${PYTHON}' -c "print('3.7.9')" --` })); + // An interpreter that runs but reports an old version: a file that prints 3.7.9. + const fake = join(f.dir, IS_WINDOWS ? 'old-python.bat' : 'old-python').replace(/\\/g, '/'); + writeFileSync(fake, IS_WINDOWS ? '@echo 3.7.9\r\n' : '#!/bin/sh\necho 3.7.9\n'); + if (!IS_WINDOWS) chmodSync(fake, 0o755); + const old = doctor(f.manifest({}, { python: fake })); assert.equal(old.code, 3, old.stderr); assert.match(named(old.out.failures, 'python').detail, /3\.7/); }); +// External review F4 (2026-10-17): doctor split python with shlex, so a +// bare path with a space (a Windows user folder such as C:/Users/John +// Smith) failed doctor, while the engine shell-quotes the whole path. +test('a python path with a space and an apostrophe passes doctor and runs a helper command', async () => { + const f = fixture(); + const link = join(f.dir, "it's my python"); + symlinkSync(dirname(PYTHON), link, IS_WINDOWS ? 'junction' : 'dir'); + const python = `${link.replace(/\\/g, '/')}/${basename(PYTHON)}`; + const r = doctor(f.manifest({}, { python })); + assert.equal(named(r.out.failures, 'python'), undefined, JSON.stringify(r.out)); + const { pythonCommand } = await loadHelpers(['pythonCommand']); + const ran = spawnSync(BASH, ['-c', `${pythonCommand({ python })} -c 'print(6 * 7)'`], { encoding: 'utf8' }); + assert.equal(ran.status, 0, ran.stderr); + assert.equal(ran.stdout.trim(), '42'); +}); + +test('a quoted python fails with the reason: the engine runs the value as one path', () => { + const f = fixture(); + for (const python of [`'${PYTHON}'`, `"${PYTHON}"`]) { + const r = doctor(f.manifest({}, { python })); + assert.equal(r.code, 3, r.stderr); + assert.match(named(r.out.failures, 'python').detail, /quoted/, JSON.stringify(r.out)); + } +}); + test('a dirty checkout fails and the fix never says to ignore everything', () => { const f = fixture(); writeFileSync(join(f.root, 'untracked.txt'), 'new\n'); @@ -694,10 +723,10 @@ test('Windows-only checks are not_applicable elsewhere', () => { test('the WindowsApps python warns', () => { const f = fixture(); const alias = 'C:/Users/u/AppData/Local/Microsoft/WindowsApps/python3.exe'; - const r = fakeDoctor(f.manifest({}, { python: `'${alias}'` }), { platform: 'win32' }); + const r = fakeDoctor(f.manifest({}, { python: alias }), { platform: 'win32' }); assert.match(named(r.warnings, 'python_alias').detail, /WindowsApps/); - const plain = fakeDoctor(f.manifest({}, { python: "'C:/Python312/python.exe'" }), { platform: 'win32' }); + const plain = fakeDoctor(f.manifest({}, { python: 'C:/Python312/python.exe' }), { platform: 'win32' }); assert.equal(named(plain.warnings, 'python_alias'), undefined, JSON.stringify(plain)); assert.equal(named(plain.skipped, 'python_alias'), undefined, JSON.stringify(plain)); }); diff --git a/tests/fixtures/waves/cases.json b/tests/fixtures/waves/cases.json index a524c41..af88941 100644 --- a/tests/fixtures/waves/cases.json +++ b/tests/fixtures/waves/cases.json @@ -887,5 +887,115 @@ "b1-b2" ] ] + }, + { + "name": "folder-and-file-under-it", + "units": [ + { + "id": "J1", + "files": [ + "src/" + ], + "depends_on": [] + }, + { + "id": "J2", + "files": [ + "src/x.js" + ], + "depends_on": [] + } + ], + "edges": [], + "expected": [ + [ + "J1" + ], + [ + "J2" + ] + ] + }, + { + "name": "folder-named-without-slash", + "units": [ + { + "id": "J1", + "files": [ + "src/x.js" + ], + "depends_on": [] + }, + { + "id": "J2", + "files": [ + "src" + ], + "depends_on": [] + } + ], + "edges": [], + "expected": [ + [ + "J1" + ], + [ + "J2" + ] + ] + }, + { + "name": "folder-with-dot-case-and-backslash", + "units": [ + { + "id": "J1", + "files": [ + "./SRC/" + ], + "depends_on": [] + }, + { + "id": "J2", + "files": [ + "src\\deep\\x.js" + ], + "depends_on": [] + } + ], + "edges": [], + "expected": [ + [ + "J1" + ], + [ + "J2" + ] + ] + }, + { + "name": "folder-sibling-with-same-prefix", + "units": [ + { + "id": "J1", + "files": [ + "src/" + ], + "depends_on": [] + }, + { + "id": "J2", + "files": [ + "src2/x.js" + ], + "depends_on": [] + } + ], + "edges": [], + "expected": [ + [ + "J1", + "J2" + ] + ] } ] diff --git a/tests/install.test.mjs b/tests/install.test.mjs index 551c177..25eba56 100644 --- a/tests/install.test.mjs +++ b/tests/install.test.mjs @@ -45,7 +45,7 @@ function install(configDir, args = [], env = {}) { } // The tools install.sh and find-python start, found on the real PATH. -const TOOLS = ['bash', 'cat', 'chmod', 'cp', 'date', 'dirname', 'git', 'head', 'jq', 'ls', 'mkdir', +const TOOLS = ['bash', 'cat', 'chmod', 'cmp', 'cp', 'date', 'dirname', 'git', 'head', 'jq', 'ls', 'mkdir', 'mktemp', 'mv', 'rm', 'sed', 'tar', 'tr', 'uname']; const which = (name) => spawnSync(BASH, ['-c', `command -v ${name}`], { encoding: 'utf8' }).stdout.trim(); @@ -254,6 +254,68 @@ test('install.sh with invalid settings keeps the old install on an update', { sk assert.ok(existsSync(marker), 'the old skill directory is untouched'); }); +// 1.6.10 (external review, Installation): the installer removed the old +// skill before copying, so a failed copy left none, and it copied untracked +// files (a review's notes and logs) into the installed skill. + +// A git checkout of the working tree's tracked files (this install.sh +// included, edits and all), plus the named untracked files. +function checkoutWith(untracked) { + counter += 1; + const dir = join(TMP, `checkout ${counter}`); + const ls = spawnSync('git', ['-C', SKILL_DIR, 'ls-files', '-z'], { encoding: 'utf8' }); + assert.equal(ls.status, 0, ls.stderr); + for (const rel of ls.stdout.split('\0').filter(Boolean)) { + if (!existsSync(join(SKILL_DIR, rel))) continue; + mkdirSync(join(dir, rel, '..'), { recursive: true }); + writeFileSync(join(dir, rel), readFileSync(join(SKILL_DIR, rel))); + } + const git = (...args) => { + const r = spawnSync('git', ['-C', dir, ...args], { encoding: 'utf8' }); + assert.equal(r.status, 0, r.stderr); + }; + git('init', '-q'); + git('add', '-A'); + for (const [rel, text] of Object.entries(untracked)) { + mkdirSync(join(dir, rel, '..'), { recursive: true }); + writeFileSync(join(dir, rel), text); + } + return dir; +} + +test('install.sh from a git checkout installs only tracked files', { skip: SKIP }, () => { + const src = checkoutWith({ 'notes.md': 'mine\n', 'reports/review/tests.log': 'log\n' }); + const dir = freshConfig(); + const res = spawnSync(BASH, [join(src, 'install.sh')], { + cwd: src, encoding: 'utf8', env: mergeEnv(process.env, { CLAUDE_CONFIG_DIR: dir }), + }); + assert.equal(res.status, 0, res.stderr); + assert.ok(existsSync(join(skillDir(dir), 'SKILL.md'))); + assert.ok(existsSync(join(skillDir(dir), 'scripts', 'setup'))); + assert.ok(!existsSync(join(skillDir(dir), 'notes.md')), 'an untracked file was installed'); + assert.ok(!existsSync(join(skillDir(dir), 'reports')), 'an untracked folder was installed'); + assert.deepEqual(readdirSync(dir).filter((n) => n.startsWith('.parallel-lanes-')), []); +}); + +test('install.sh keeps the old install when the new copy fails', { skip: SKIP }, () => { + const dir = freshConfig(); + install(dir); + const marker = join(skillDir(dir), 'marker.txt'); + writeFileSync(marker, 'old install\n'); + // A copy that comes out incomplete: a tracked file the check needs is gone. + const src = checkoutWith({}); + rmSync(join(src, 'run.workflow.js')); + const res = spawnSync(BASH, [join(src, 'install.sh')], { + cwd: src, encoding: 'utf8', env: mergeEnv(process.env, { CLAUDE_CONFIG_DIR: dir }), + }); + assert.notEqual(res.status, 0); + assert.match(res.stderr, /unchanged/); + assert.ok(existsSync(marker), 'the old skill directory is untouched'); + assert.ok(existsSync(join(skillDir(dir), 'SKILL.md'))); + assert.deepEqual(readdirSync(dir).filter((n) => n.startsWith('.parallel-lanes-')), []); + assert.deepEqual(readdirSync(join(dir, 'skills')), ['parallel-lanes']); +}); + // A stock Windows Python install has python but no python3. test('install.sh needs a working Python, not a python3 command', { skip: SKIP || NO_SYMLINKS }, (t) => { const python3 = which('python3'); diff --git a/tests/repair.test.mjs b/tests/repair.test.mjs index 80fb625..e6d53b6 100644 --- a/tests/repair.test.mjs +++ b/tests/repair.test.mjs @@ -820,6 +820,49 @@ test('one agent left after a final fix: the re-review gets it, not the start che assert.deepEqual(kinds(result), ['checks_failed']); }); +// External review F5 (2026-10-17): the start check took the last free agent, +// returned nothing, its retry was refused, and the refusal latch turned the +// run into a budget stop with no acceptance. The start check is advisory: one +// call, no retry, no availability probe, and a refusal never sets the latch. +test('the last free agent: a start check with no result is unavailable and the run still completes', async () => { + // 13 agents up to the lenses, then verify (counted, past the cap): one left. + const m = started({ limits: { review_rounds: 5, max_parallel_lanes: 3, max_agents: 15 } }); + const { result, calls } = await run(m, { + ...noFindings({ verify: [verified(TIP, 1)] }), 'start check': [null], ...taskScript(ALL), + }); + assert.equal(result.status, 'complete', JSON.stringify(result.reason)); + assert.deepEqual(labels(calls).filter((l) => l.startsWith('start check')), ['start check']); + assert.equal(result.final.start_check.status, 'unavailable'); + assert.deepEqual(kinds(result), ['checks_failed']); +}); + +test('the start check gets no retry and no availability probe when it returns nothing', async () => { + // Supervised: no repairs follow, but the start check still runs. + const m = started({ autonomy: 'supervised' }); + const { result, calls } = await run(m, { + ...noFindings({ verify: [verified(TIP, 1)] }), 'start check': [null], ...taskScript(ALL), + }); + assert.equal(result.status, 'complete', JSON.stringify(result.reason)); + assert.ok(!labels(calls).includes('start check retry'), `${labels(calls)}`); + assert.ok(!labels(calls).includes('availability probe'), `${labels(calls)}`); + assert.equal(result.final.start_check.status, 'unavailable'); + assert.equal(result.final.start_check.why, 'no result'); +}); + +test('two agents left after a final fix: a start check with no result leaves the re-review its agent', async () => { + // 13 agents up to the lenses, the final fix and its verify: two left. + const m = started({ limits: { review_rounds: 5, max_parallel_lanes: 3, max_agents: 17 } }); + const { result, calls } = await run(m, { + ...withFix({ verify: [verified(H('f1'), 1)], 'final re-review': [resolved(H('f1'))] }), + 'start check': [null], ...taskScript(ALL), + }); + assert.equal(result.status, 'complete', JSON.stringify(result.reason)); + assert.ok(labels(calls).includes('start check'), `${labels(calls)}`); + assert.ok(labels(calls).includes('final re-review'), `${labels(calls)}`); + assert.equal(result.final.start_check.status, 'unavailable'); + assert.deepEqual(kinds(result), ['checks_failed']); +}); + // The engine's guards (spec 4.4 "The engine classifies"). test('classifyStartCheck: each guard gives the expected at_start', () => { const m = started(); diff --git a/tests/setup-script.cases.mjs b/tests/setup-script.cases.mjs index 30a1c89..123cfbb 100644 --- a/tests/setup-script.cases.mjs +++ b/tests/setup-script.cases.mjs @@ -261,8 +261,15 @@ test('setup: a dirty main checkout exits 3 and creates nothing', () => { }); test('setup: a SHA-256 repository exits 3 with a clear message and creates nothing', () => { - const c = newCase(); - rmSync(join(c.project, '.git'), { recursive: true, force: true }); + // The SHA-256 repository goes in a fresh folder: deleting the case's .git + // and initializing again raced, on Linux CI, with a git process the case's + // own commit had left running ("attempt to reinitialize repository with + // different hash", 2026-10-17). + const c = { ...newCase() }; + c.project = join(c.root, 'sha256 project'); + mkdirSync(c.project, { recursive: true }); + write(c.project, 'README.md', 'hello\n'); + write(c.project, '.gitignore', 'scratch/\n'); git(c.project, 'init', '-q', '-b', 'main', '--object-format=sha256'); git(c.project, 'add', '-A'); git(c.project, 'commit', '-q', '-m', 'init'); diff --git a/tests/validate.test.mjs b/tests/validate.test.mjs index 6c8bede..f5948bb 100644 --- a/tests/validate.test.mjs +++ b/tests/validate.test.mjs @@ -478,6 +478,32 @@ test('batch: a non-empty string on a light task is valid', () => { assert.deepEqual(validateManifest(m), []); }); +// External review F1 (2026-10-17): batch A, B runs as the unit A-B, which +// a task named A-B also is; the units then share agent labels, briefs, +// reports and branches, and the join wave map keeps one of them. A resume +// can run part of a batch (B-C of A, B, C), so every first-last stretch of a +// batch is a possible unit id. +test('a batch unit id that equals a task id or another batch unit id is reported', () => { + const light = (id, file, batch) => task(id, [file], { tier: 'light', batch }); + const cases = [ + ['a task named like the batch', [light('A', 'a.md', 'x'), light('B', 'b.md', 'x'), task('A-B', ['c.js'])], 'A-B'], + ['in another letter case', [light('A', 'a.md', 'x'), light('B', 'b.md', 'x'), task('a-b', ['c.js'])], 'a-b'], + ['part of a batch', [light('A', 'a.md', 'x'), light('B', 'b.md', 'x'), light('C', 'c.md', 'x'), task('B-C', ['d.js'])], 'B-C'], + ['two batches', [light('A', 'a.md', 'x'), light('B-C', 'b.md', 'x'), light('A-B', 'c.md', 'y'), light('C', 'd.md', 'y')], 'A-B-C'], + ]; + for (const [name, tasks, id] of cases) { + const m = validManifest(); + m.lanes[0].tasks = tasks; + assertError(validateManifest(m), id, 'batch'); + assert.ok(validateManifest(m).length >= 1, name); + } + // Across groups: the lane's batch T2-T6 and a prelude task T2-T6. + const m = validManifest(); + m.lanes[0].tasks = [light('T2', 'src/a.js', 'docs'), light('T6', 'src/a2.js', 'docs')]; + m.prelude = [task('T2-T6', ['src/p.js'])]; + assertError(validateManifest(m), 'T2-T6', 'batch'); +}); + test('batch on a standard or sonnet task is reported', () => { for (const tier of ['standard', 'sonnet']) { const m = validManifest(); @@ -727,11 +753,58 @@ test('startCommand ignores inherited consumes_extra entries', async () => { // ---- 1.6.0 check policy fields (spec 4.1, 4.5, 4.8) ---- +// External review F2 (2026-10-17): task-checks treats a folder entry as +// covering the paths under it, so two lanes claiming src/ and src/x.js may +// both change src/x.js; validation compared exact strings. +test('a folder in one lane and a path under it in another is a claim by two lanes', () => { + for (const [folder, file] of [['src/', 'src/a/x.js'], ['src', 'src/x.js'], ['./SRC/', 'src/x.js'], ['src/x.js', 'src/']]) { + const m = validManifest(); + m.lanes[0].tasks = [task('T2', [folder])]; + m.lanes[1].tasks = [task('T3', [file])]; + assertError(validateManifest(m), 'claimed by lanes alpha and beta', `${folder} and ${file}`); + } + // A sibling that only shares a prefix, and a folder and file in one lane, are fine. + const sibling = validManifest(); + sibling.lanes[0].tasks = [task('T2', ['src/'])]; + sibling.lanes[1].tasks = [task('T3', ['src2/x.js'])]; + assert.deepEqual(validateManifest(sibling), []); + const one = validManifest(); + one.lanes[0].tasks = [task('T2', ['src/']), task('T5', ['src/x.js'])]; + one.lanes[1].tasks = [task('T3', ['lib/y.js'])]; + assert.deepEqual(validateManifest(one), []); + // A deliberate overlap on the file covers the pair. + const declared = validManifest(); + declared.lanes[0].tasks = [task('T2', ['src/'])]; + declared.lanes[1].tasks = [task('T3', ['src/x.js'])]; + declared.overlaps = [{ file: 'src/x.js', tasks: ['T2', 'T3'], reason: 'both edit it', merge_owner: 'T3' }]; + assert.deepEqual(validateManifest(declared), []); +}); + test('task_checks accepts task ids with test and inherits lint and build', () => { const m = { ...validManifest(), task_checks: { T2: { test: ['node --test tests/a.test.mjs'] } } }; assert.deepEqual(validateManifest(m), []); }); +// External review F3 (2026-10-17): with no test, lint or build command for +// the task's lane, the run builds no task-checks command, so the entry was +// silently never run. +test('a task_checks entry whose lane has no test, lint or build command is reported', () => { + const none = { setup: [], test: [], lint: [], build: [] }; + for (const [id, extra] of [ + ['T2', { commands: none }], + ['T1', { commands: none }], + ['T4', { commands: none }], + ['T2', { lane_commands: { alpha: { test: [], lint: [], build: [] } } }], + ]) { + const m = { ...validManifest(), ...extra, task_checks: { [id]: { test: ['npm test -- one'] } } }; + assertError(validateManifest(m), `task_checks.${id}`, 'no test, lint or build command'); + } + // Another lane's empty override does not matter. + const ok = { ...validManifest(), lane_commands: { beta: { test: [], lint: [], build: [] } }, + task_checks: { T2: { test: ['npm test -- one'] } } }; + assert.deepEqual(validateManifest(ok), []); +}); + test('task_checks rejects lane ids, prelude and join as keys', () => { for (const key of ['alpha', 'prelude', 'join']) { const errors = validateManifest({ ...validManifest(), task_checks: { [key]: { test: ['t'] } } }); diff --git a/tests/waves.test.mjs b/tests/waves.test.mjs index 7e0f022..8479c3f 100644 --- a/tests/waves.test.mjs +++ b/tests/waves.test.mjs @@ -134,6 +134,19 @@ test('unitBranch names the unit branch', () => { assert.equal(unitBranch(manifest(), { id: 'J2-J3' }), 'pl-run-1-u-J2-J3'); }); +// External review F2: the owner map names the task whose folder holds a +// path, as it names the task listing the path itself. +test('_owners.first_owner finds the task whose folder holds the path', () => { + const m = { lanes: [{ id: 'a', tasks: [{ id: 'T1', files: ['src/'] }] }, { id: 'b', tasks: [{ id: 'T2', files: ['docs/x.md'] }] }], + join: [] }; + const got = python([ + 'import _owners', + 'o = _owners.build(json.load(sys.stdin))', + 'print(json.dumps([_owners.first_owner(o, ["T2"], p)[0] for p in ["src/a/b.js", "SRC/c.js", "src2/d.js", "docs/x.md"]]))', + ], m); + assert.deepEqual(got, ['T1', 'T1', null, null]); +}); + test('_waves.task_waves and join_units', () => { const d1a = CASES.find((c) => c.name === 'd1a'); const got = python(['tasks = json.load(sys.stdin)', 'print(json.dumps(_waves.task_waves(tasks)))'], d1a.units);