From 6294b4cca19ce3ce0ebb997a7a918201bcad519a Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:25:40 -0700 Subject: [PATCH 01/10] feat: owners.json records each task's Files list, with a shared fingerprint --- run.workflow.js | 28 +++++++ scripts/_owners.py | 62 ++++++++++++++- src/prompts-task.js | 28 +++++++ tests/fingerprint.test.mjs | 104 ++++++++++++++++++++++++++ tests/fixtures/fingerprint/cases.json | 12 +++ tests/ledger-carry.test.mjs | 2 +- tests/setup-script.test.mjs | 18 ++++- 7 files changed, 250 insertions(+), 4 deletions(-) create mode 100644 tests/fingerprint.test.mjs create mode 100644 tests/fixtures/fingerprint/cases.json diff --git a/run.workflow.js b/run.workflow.js index cdc6be2..9a275ec 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1742,6 +1742,34 @@ function declaredFiles(task) { return out; } +// The fingerprint of a declared list, N:HHHHHHHH: N the number of paths, H the +// 32-bit FNV-1a hash of the paths' UTF-8 bytes joined with one NUL byte, as +// _owners.fingerprint computes it (tests/fixtures/fingerprint/cases.json). +// The UTF-8 encoder is written out (the Workflow runtime need not have +// TextEncoder); a lone surrogate gets its 3-byte form, as Python's +// surrogatepass gives it. +function declaredFingerprint(paths) { + let h = 0x811c9dc5; + const add = (byte) => { h = Math.imul(h ^ byte, 0x01000193) >>> 0; }; + paths.forEach((p, i) => { + if (i > 0) add(0); + for (let j = 0; j < p.length; j += 1) { + const cp = p.codePointAt(j); + if (cp > 0xffff) j += 1; + if (cp < 0x80) { + add(cp); + } else if (cp < 0x800) { + add(0xc0 | (cp >> 6)); add(0x80 | (cp & 0x3f)); + } else if (cp < 0x10000) { + add(0xe0 | (cp >> 12)); add(0x80 | ((cp >> 6) & 0x3f)); add(0x80 | (cp & 0x3f)); + } else { + add(0xf0 | (cp >> 18)); add(0x80 | ((cp >> 12) & 0x3f)); add(0x80 | ((cp >> 6) & 0x3f)); add(0x80 | (cp & 0x3f)); + } + } + }); + return `${paths.length}:${h.toString(16).padStart(8, '0')}`; +} + // The start-task command a task agent runs first: the optional fast-forward // to the feature branch (opts.sync), every brief of the unit regenerated from // the current plan and printed, and with opts.pkg = {base, head} and diff --git a/scripts/_owners.py b/scripts/_owners.py index 36b1b9c..8609ed8 100644 --- a/scripts/_owners.py +++ b/scripts/_owners.py @@ -7,9 +7,16 @@ {"repo": {"mode", "root", "git_dir"}, "tasks": {id: {"where": "prelude" | | "join", "pos": n, - "wave": n (join tasks, when the join can run in waves)}}, + "wave": n (join tasks, when the join can run in waves), + "declared": [paths] (the task's Files list as given, 1.6.6)}}, "files": {: [task ids, in run order]}} +A unit's declared list (its tasks' `declared`, in order, each exact string +once) is checked against the engine's fingerprint of the same list +(declaredFingerprint in src/prompts-task.js): "N:HHHHHHHH", N the number of +paths and H the 32-bit FNV-1a hash of the paths' UTF-8 bytes joined with one +NUL byte (1.6.6). + A task changing another task's file is judged by when that task runs: "earlier" (an earlier task of the same list, the prelude seen from a lane or the join, a lane seen from the join), "alongside" (another lane, or another @@ -20,6 +27,7 @@ import json import os import posixpath +import re import sys sys.dont_write_bytecode = True @@ -58,7 +66,11 @@ def build(manifest): files = {} for where, items in lists: for pos, t in enumerate(items): - tasks[t["id"]] = {"where": where, "pos": pos} + declared = [] + for f in t.get("files") or []: + if f not in declared: + declared.append(f) + tasks[t["id"]] = {"where": where, "pos": pos, "declared": declared} for f in t.get("files") or []: key = path_key(f) if key is not None and t["id"] not in files.setdefault(key, []): @@ -81,6 +93,52 @@ def load(path): return data +FINGERPRINT = re.compile(r"^[1-9][0-9]*:[0-9a-f]{8}$") + + +def fingerprint(paths): + """N:HHHHHHHH for a declared list: N the number of paths, H the 32-bit + FNV-1a hash of the paths' UTF-8 bytes joined with one NUL byte (a lone + surrogate as its 3-byte form). Match it with FINGERPRINT.fullmatch: `$` + alone also matches before a trailing newline.""" + h = 0x811C9DC5 + for byte in b"\x00".join(p.encode("utf-8", "surrogatepass") for p in paths): + h = ((h ^ byte) * 0x01000193) & 0xFFFFFFFF + return f"{len(paths)}:{h:08x}" + + +def declared_for(owners, task_ids): + """The unit's declared list: each task's `declared` in the given order, + the first occurrence of each exact string kept.""" + tasks = owners.get("tasks") or {} + out = [] + for tid in task_ids: + entry = tasks.get(tid) + if not isinstance(entry, dict): + raise ValueError(f"task {tid} is not in it") + declared = entry.get("declared") + if not isinstance(declared, list) or not all(isinstance(p, str) for p in declared): + raise ValueError(f"task {tid} has no declared list") + for p in declared: + if p not in out: + out.append(p) + return out + + +def owners_declared(path, task_ids, expected): + """The unit's declared list from the owner map at path, checked against + the engine's fingerprint expected. Raises ValueError(reason).""" + try: + owners = load(path) + except (OSError, ValueError) as e: + raise ValueError(f"owners.json cannot be read: {e}") + declared = declared_for(owners, task_ids) + actual = fingerprint(declared) + if actual != expected: + raise ValueError(f"expected {expected}, found {actual}") + return declared + + def relation(owners, unit_ids, other): """earlier, alongside or later: when other runs, seen from the unit.""" tasks = owners["tasks"] diff --git a/src/prompts-task.js b/src/prompts-task.js index 5ce6762..d46229b 100644 --- a/src/prompts-task.js +++ b/src/prompts-task.js @@ -621,6 +621,34 @@ function declaredFiles(task) { return out; } +// The fingerprint of a declared list, N:HHHHHHHH: N the number of paths, H the +// 32-bit FNV-1a hash of the paths' UTF-8 bytes joined with one NUL byte, as +// _owners.fingerprint computes it (tests/fixtures/fingerprint/cases.json). +// The UTF-8 encoder is written out (the Workflow runtime need not have +// TextEncoder); a lone surrogate gets its 3-byte form, as Python's +// surrogatepass gives it. +function declaredFingerprint(paths) { + let h = 0x811c9dc5; + const add = (byte) => { h = Math.imul(h ^ byte, 0x01000193) >>> 0; }; + paths.forEach((p, i) => { + if (i > 0) add(0); + for (let j = 0; j < p.length; j += 1) { + const cp = p.codePointAt(j); + if (cp > 0xffff) j += 1; + if (cp < 0x80) { + add(cp); + } else if (cp < 0x800) { + add(0xc0 | (cp >> 6)); add(0x80 | (cp & 0x3f)); + } else if (cp < 0x10000) { + add(0xe0 | (cp >> 12)); add(0x80 | ((cp >> 6) & 0x3f)); add(0x80 | (cp & 0x3f)); + } else { + add(0xf0 | (cp >> 18)); add(0x80 | ((cp >> 12) & 0x3f)); add(0x80 | ((cp >> 6) & 0x3f)); add(0x80 | (cp & 0x3f)); + } + } + }); + return `${paths.length}:${h.toString(16).padStart(8, '0')}`; +} + // The start-task command a task agent runs first: the optional fast-forward // to the feature branch (opts.sync), every brief of the unit regenerated from // the current plan and printed, and with opts.pkg = {base, head} and diff --git a/tests/fingerprint.test.mjs b/tests/fingerprint.test.mjs new file mode 100644 index 0000000..9c2cbfe --- /dev/null +++ b/tests/fingerprint.test.mjs @@ -0,0 +1,104 @@ +// The declared-list fingerprint (1.6.6 design 4.2): the engine's +// declaredFingerprint (src/prompts-task.js) and the helpers' _owners.fingerprint +// give the same N:HHHHHHHH on the shared cases in +// tests/fixtures/fingerprint/cases.json; owners_declared rebuilds a unit's +// Files list from owners.json and refuses one that does not match. +import { test, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { readFileSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { loadHelpers, SKILL_DIR } from './harness.mjs'; +import { tempDir } from './platform.mjs'; + +const SCRIPTS = join(SKILL_DIR, 'scripts'); +const CASES = JSON.parse(readFileSync(join(SKILL_DIR, 'tests', 'fixtures', 'fingerprint', 'cases.json'), 'utf8')); +const TMP = tempDir('pl-fingerprint-'); +after(() => rmSync(TMP, { recursive: true, force: true })); + +const { declaredFingerprint } = await loadHelpers(['declaredFingerprint']); + +// Runs a Python snippet with scripts/ importable (no bytecode); stdin is JSON +// (read as UTF-8 bytes, whatever the console code page), stdout is JSON. +function python(lines, input) { + const code = [ + 'import json, sys', + 'sys.dont_write_bytecode = True', + `sys.path.insert(0, ${JSON.stringify(SCRIPTS)})`, + 'import _owners', + 'data = json.loads(sys.stdin.buffer.read().decode("utf-8"))', + ...lines, + ].join('\n'); + const r = spawnSync('python3', ['-B', '-c', code], { encoding: 'utf8', input: JSON.stringify(input) }); + assert.equal(r.status, 0, r.stderr); + return JSON.parse(r.stdout); +} + +// owners_declared(path, ids, expected) as {ok: list} or {error: text}. +function ownersDeclared(path, ids, expected) { + return python([ + 'try:', + ' print(json.dumps({"ok": _owners.owners_declared(data["path"], data["ids"], data["expected"])}))', + 'except ValueError as e:', + ' print(json.dumps({"error": str(e)}))', + ], { path, ids, expected }); +} + +test('the engine\'s fingerprint reproduces every shared case', () => { + assert.ok(CASES.length >= 10); + for (const c of CASES) assert.equal(declaredFingerprint(c.paths), c.fingerprint, c.name); +}); + +test('the helpers\' fingerprint reproduces every shared case', () => { + const got = python(['print(json.dumps([_owners.fingerprint(c["paths"]) for c in data]))'], CASES); + assert.deepEqual(got, CASES.map((c) => c.fingerprint)); +}); + +test('the fingerprint of no paths is 0 and the offset basis', () => { + assert.equal(declaredFingerprint([]), '0:811c9dc5'); + assert.deepEqual(python(['print(json.dumps(_owners.fingerprint([])))'], null), '0:811c9dc5'); +}); + +test('owners_declared rebuilds a unit\'s list and refuses a different one', () => { + const path = join(TMP, 'owners.json'); + writeFileSync(path, JSON.stringify({ + repo: {}, + tasks: { + T2: { where: 'a', pos: 0, declared: ['src/a.js', 'shared.css'] }, + T3: { where: 'a', pos: 1, declared: ['shared.css', 'src/b.js'] }, + T4: { where: 'a', pos: 2 }, + }, + files: {}, + })); + const list = ['src/a.js', 'shared.css', 'src/b.js']; + const fp = declaredFingerprint(list); + assert.deepEqual(ownersDeclared(path, ['T2', 'T3'], fp), { ok: list }); + const wrong = ownersDeclared(path, ['T2', 'T3'], '3:00000000'); + assert.ok(wrong.error.includes('expected 3:00000000') && wrong.error.includes(`, found ${fp}`), wrong.error); + assert.ok(wrong.error.includes('expected ') && wrong.error.includes(', found 3:'), wrong.error); + assert.equal(ownersDeclared(path, ['T9'], fp).error, 'task T9 is not in it'); + assert.equal(ownersDeclared(path, ['T4'], fp).error, 'task T4 has no declared list'); + const missing = ownersDeclared(join(TMP, 'no-such', 'owners.json'), ['T2'], fp); + assert.ok(missing.error.startsWith('owners.json cannot be read'), missing.error); +}); + +test('declared_for refuses a declared value that is not a list of strings', () => { + const owners = { tasks: { A: { declared: 'src/a.js' }, B: { declared: ['ok', 3] }, C: { declared: [] } } }; + const got = python([ + 'out = []', + 'for ids in (["A"], ["B"], ["C"]):', + ' try:', + ' out.append(_owners.declared_for(data, ids))', + ' except ValueError as e:', + ' out.append(str(e))', + 'print(json.dumps(out))', + ], owners); + assert.deepEqual(got, ['task A has no declared list', 'task B has no declared list', []]); +}); + +test('FINGERPRINT matches only the N:HHHHHHHH form with N of 1 or more', () => { + const got = python([ + 'print(json.dumps([bool(_owners.FINGERPRINT.fullmatch(s)) for s in data]))', + ], ['3:0a1b2c3d', '0:811c9dc5', '03:0a1b2c3d', '3:0A1B2C3D', '3:0a1b2c3', '3:0a1b2c3d\n', '12:0a1b2c3d']); + assert.deepEqual(got, [true, false, false, false, false, false, true]); +}); diff --git a/tests/fixtures/fingerprint/cases.json b/tests/fixtures/fingerprint/cases.json new file mode 100644 index 0000000..e629b90 --- /dev/null +++ b/tests/fixtures/fingerprint/cases.json @@ -0,0 +1,12 @@ +[ + {"name": "published FNV-1a vector a", "paths": ["a"], "fingerprint": "1:e40c292c"}, + {"name": "published FNV-1a vector foobar", "paths": ["foobar"], "fingerprint": "1:bf9cf968"}, + {"name": "two paths", "paths": ["src/a.js", "tests/a.test.mjs"], "fingerprint": "2:56442afd"}, + {"name": "case and dot variants are distinct", "paths": ["src/a.js", "./src/a.js", "SRC/A.js"], "fingerprint": "3:8920a8a1"}, + {"name": "order matters", "paths": ["SRC/A.js", "./src/a.js", "src/a.js"], "fingerprint": "3:74e2b2a1"}, + {"name": "the NUL separator counts (1 of 2)", "paths": ["a", "bc"], "fingerprint": "2:609747a3"}, + {"name": "the NUL separator counts (2 of 2)", "paths": ["ab", "c"], "fingerprint": "2:ef850b27"}, + {"name": "spaces and quotes", "paths": ["docs/it's here.md", "src/my file.js", "a$b`c\\d.txt"], "fingerprint": "3:a2a1486f"}, + {"name": "non-ASCII", "paths": ["src/café.js", "文档/说明.md", "emoji/😀.txt"], "fingerprint": "3:e9065fdf"}, + {"name": "a lone surrogate", "paths": ["bad\ud800name.txt"], "fingerprint": "1:bf305012"} +] diff --git a/tests/ledger-carry.test.mjs b/tests/ledger-carry.test.mjs index 2d13a72..cac1b14 100644 --- a/tests/ledger-carry.test.mjs +++ b/tests/ledger-carry.test.mjs @@ -271,7 +271,7 @@ test('write-owners maps every path to the tasks that list it, and owners annotat assert.equal(res.code, 0, res.stderr); const map = JSON.parse(readFileSync(join(dir, 'owners.json'), 'utf8')); assert.deepEqual(map.files['shared.css'], ['T2', 'T4']); - assert.deepEqual(map.tasks.T3, { where: 'alpha', pos: 1 }); + assert.deepEqual(map.tasks.T3, { where: 'alpha', pos: 1, declared: ['.\\SRC\\x2.js'] }); const out = ownersOf(dir, '--task', 'T2', 'src/y.js', 'a.txt', 'src/z.js', 'shared.css', 'new.js', 'src/x2.js', 'SRC/X.js'); assert.ok(out.includes('src/y.js (owned by T4, another lane, running alongside)'), out); assert.ok(out.includes('a.txt (owned by P1, earlier in the run)'), out); diff --git a/tests/setup-script.test.mjs b/tests/setup-script.test.mjs index 91595e2..59fa9ee 100644 --- a/tests/setup-script.test.mjs +++ b/tests/setup-script.test.mjs @@ -307,7 +307,23 @@ test('setup writes the owner map from the manifest', () => { setupOk(c, m); const owners = JSON.parse(readFileSync(join(c.ledgerDir, 'owners.json'), 'utf8')); assert.deepEqual(owners.files, { 'src/a.js': ['a1'], 'shared.css': ['a1', 'b1'] }); - assert.deepEqual(owners.tasks, { a1: { where: 'a', pos: 0 }, b1: { where: 'b', pos: 0 } }); + assert.deepEqual(owners.tasks, { + a1: { where: 'a', pos: 0, declared: ['src/a.js', 'shared.css'] }, + b1: { where: 'b', pos: 0, declared: ['shared.css'] }, + }); +}); + +test('setup records each task\'s Files list as given, each exact string once', () => { + const c = newCase(); + const m = manifest(c, { lanes: [ + { id: 'a', name: 'Lane A', tasks: [{ id: 'a1', files: ['src/A.js', './src/a.js', 'src/A.js'] }] }, + { id: 'b', name: 'Lane B', tasks: [{ id: 'b1' }] }, + ] }); + setupOk(c, m); + const owners = JSON.parse(readFileSync(join(c.ledgerDir, 'owners.json'), 'utf8')); + assert.deepEqual(owners.tasks.a1.declared, ['src/A.js', './src/a.js']); + assert.deepEqual(owners.tasks.b1.declared, []); + assert.deepEqual(owners.files, { 'src/a.js': ['a1'] }); }); test('setup: ledger records run_started and status reports the earliest start point', () => { From d0df968851612bfd42a59780f5d39a1e682671d8 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:27:41 -0700 Subject: [PATCH 02/10] fix: address review findings for Task 1 --- tests/fixtures/fingerprint/cases.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/fingerprint/cases.json b/tests/fixtures/fingerprint/cases.json index e629b90..81e52fb 100644 --- a/tests/fixtures/fingerprint/cases.json +++ b/tests/fixtures/fingerprint/cases.json @@ -7,6 +7,6 @@ {"name": "the NUL separator counts (1 of 2)", "paths": ["a", "bc"], "fingerprint": "2:609747a3"}, {"name": "the NUL separator counts (2 of 2)", "paths": ["ab", "c"], "fingerprint": "2:ef850b27"}, {"name": "spaces and quotes", "paths": ["docs/it's here.md", "src/my file.js", "a$b`c\\d.txt"], "fingerprint": "3:a2a1486f"}, - {"name": "non-ASCII", "paths": ["src/café.js", "文档/说明.md", "emoji/😀.txt"], "fingerprint": "3:e9065fdf"}, + {"name": "non-ASCII", "paths": ["src/caf\u00e9.js", "\u6587\u6863/\u8bf4\u660e.md", "emoji/\ud83d\ude00.txt"], "fingerprint": "3:e9065fdf"}, {"name": "a lone surrogate", "paths": ["bad\ud800name.txt"], "fingerprint": "1:bf305012"} ] From 9d67aee9f8748c777009c65fe68691957ad2cf01 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:30:39 -0700 Subject: [PATCH 03/10] feat: agents add to long files with Edit or Write, not heredocs --- run.workflow.js | 13 ++++++++----- src/prompts-task.js | 9 +++++---- src/rules.js | 4 +++- tests/check-policy-prompts.test.mjs | 21 +++++++++++++++++++++ 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/run.workflow.js b/run.workflow.js index 9a275ec..9a5cd91 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -60,7 +60,9 @@ function agentRules() { 'Combine independent shell commands into one call (with && or ;) when no command depends on reading the ' + "previous one's output: every separate call costs a full turn.", 'Change file contents with your file-editing tools (Edit, Write). Text written through shell heredocs, ' + - 'sed, or inline scripts can be corrupted by shell quoting (apostrophes, backslashes, $ and backticks). ' + + 'sed, or inline scripts can be corrupted by shell quoting (apostrophes, backslashes, $ and backticks), ' + + 'and the Bash tool fails on a command of about 7,000 characters or more (a long heredoc is one). ' + + 'To add to an existing file such as a report, Read it and then Edit it, or Write the whole file. ' + "Running the project's own tools that change files is fine: generators (for example scripts/build), " + 'formatters, codemods, and other maintenance commands.', 'Read a file right before you edit or overwrite it (and again after anything else may have changed it): ' + @@ -1697,8 +1699,9 @@ function fallbackFixer() { return [ 'Built-in instructions (superpowers not found):', 'Fix exactly the findings listed below, nothing more. Rerun the tests that cover the amended code, commit with', - 'the commit rules below, and append a fix report (what changed, covering tests, command, output) to the', - 'implementer report file. If you cannot fix a finding, report blocked with the specifics instead of guessing.', + 'the commit rules below, and add a fix report (what changed, covering tests, command, output) to the', + 'implementer report file with your file-editing tools. If you cannot fix a finding, report blocked with the', + 'specifics instead of guessing.', ].join('\n'); } @@ -2109,8 +2112,8 @@ function fixPrompt(m, task, where, findings, report, head, guidance = null, reop ].join('\n'), '', ...reopenLines(m, task, where, reopen), - `The branch is at ${head}. Fix these findings, commit on top of it, and append a fix report (what changed,`, - `covering tests, command, output) to ${files.report}.`, + `The branch is at ${head}. Fix these findings, commit on top of it, and add a fix report (what changed,`, + `covering tests, command, output) to ${files.report} with your file-editing tools.`, 'Rerun the tests that cover the amended code; do not run the task checks or the full suite: the re-review', 'judges the fix, and integration and verify run the full checks.', commitWithLine(fixCommitMessage(task)), diff --git a/src/prompts-task.js b/src/prompts-task.js index d46229b..e7c592f 100644 --- a/src/prompts-task.js +++ b/src/prompts-task.js @@ -576,8 +576,9 @@ function fallbackFixer() { return [ 'Built-in instructions (superpowers not found):', 'Fix exactly the findings listed below, nothing more. Rerun the tests that cover the amended code, commit with', - 'the commit rules below, and append a fix report (what changed, covering tests, command, output) to the', - 'implementer report file. If you cannot fix a finding, report blocked with the specifics instead of guessing.', + 'the commit rules below, and add a fix report (what changed, covering tests, command, output) to the', + 'implementer report file with your file-editing tools. If you cannot fix a finding, report blocked with the', + 'specifics instead of guessing.', ].join('\n'); } @@ -988,8 +989,8 @@ function fixPrompt(m, task, where, findings, report, head, guidance = null, reop ].join('\n'), '', ...reopenLines(m, task, where, reopen), - `The branch is at ${head}. Fix these findings, commit on top of it, and append a fix report (what changed,`, - `covering tests, command, output) to ${files.report}.`, + `The branch is at ${head}. Fix these findings, commit on top of it, and add a fix report (what changed,`, + `covering tests, command, output) to ${files.report} with your file-editing tools.`, 'Rerun the tests that cover the amended code; do not run the task checks or the full suite: the re-review', 'judges the fix, and integration and verify run the full checks.', commitWithLine(fixCommitMessage(task)), diff --git a/src/rules.js b/src/rules.js index ba16d3a..2078af2 100644 --- a/src/rules.js +++ b/src/rules.js @@ -45,7 +45,9 @@ function agentRules() { 'Combine independent shell commands into one call (with && or ;) when no command depends on reading the ' + "previous one's output: every separate call costs a full turn.", 'Change file contents with your file-editing tools (Edit, Write). Text written through shell heredocs, ' + - 'sed, or inline scripts can be corrupted by shell quoting (apostrophes, backslashes, $ and backticks). ' + + 'sed, or inline scripts can be corrupted by shell quoting (apostrophes, backslashes, $ and backticks), ' + + 'and the Bash tool fails on a command of about 7,000 characters or more (a long heredoc is one). ' + + 'To add to an existing file such as a report, Read it and then Edit it, or Write the whole file. ' + "Running the project's own tools that change files is fine: generators (for example scripts/build), " + 'formatters, codemods, and other maintenance commands.', 'Read a file right before you edit or overwrite it (and again after anything else may have changed it): ' + diff --git a/tests/check-policy-prompts.test.mjs b/tests/check-policy-prompts.test.mjs index 7499efc..d7e6c94 100644 --- a/tests/check-policy-prompts.test.mjs +++ b/tests/check-policy-prompts.test.mjs @@ -198,6 +198,27 @@ test('the fix fallback is not the implement fallback', () => { assert.ok(implementPrompt(m, t(m, 'T2'), where, H('b')).includes(impl)); }); +// ---- 1.6.6: agents write long files with Edit and Write (spec 4.6) + +test('agentRules names the Bash command limit and how to add to a file', () => { + const text = flat(agentRules()); + assert.ok(text.includes('7,000 characters'), text); + assert.ok(text.includes('heredoc'), text); + assert.ok(text.includes('Read it and then Edit it, or Write the whole file'), text); + assert.ok(text.includes('shell quoting'), text); +}); + +test('no fix prompt says append for the fix report', () => { + const finding = { id: 'F1', severity: 'important', file: 'src/T2.js', line: 1, issue: 'x', fix: 'y' }; + for (const m of [manifest(), manifest({ sp_dir: null })]) { + const where = laneWhere(m, lane(m, 'alpha')); + const p = fixPrompt(m, t(m, 'T2'), where, [finding], { status: 'done' }, H('b')); + assert.ok(p.includes('fix report'), p); + assert.ok(!/append a fix report/.test(flat(p)), p); + assert.ok(flat(p).includes('with your file-editing tools'), p); + } +}); + test('fixMessagesLines names the repair message', () => { assert.ok(flat(fixMessagesLines().join('\n')).includes(`\`${repairFixMessage()}\``)); }); From d0460b5009b62edb7918aab7155076f601b4d255 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:34:18 -0700 Subject: [PATCH 04/10] feat: start-task builds the scope list from owners.json --- scripts/start-task | 101 +++++++++++++++++++++++++--------- tests/task-helpers.test.mjs | 106 ++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 27 deletions(-) diff --git a/scripts/start-task b/scripts/start-task index 815d88d..4883127 100755 --- a/scripts/start-task +++ b/scripts/start-task @@ -3,7 +3,8 @@ usage: start-task DIR PLAN [--sync BRANCH] [--package SCRIPT BASE HEAD OUT] [--artifacts ROOT] [--record-start LEDGER_DIR LANE BASE] - [--scope BASE HEAD --declared PATH [--declared PATH ...] [--owners FILE]] + [--scope BASE HEAD (--declared PATH [--declared PATH ...] [--owners FILE] + | --declared-from-owners N:HHHHHHHH --owners FILE)] [--rulings LEDGER_DIR LANE] [--unit-branch NAME SHA --run-id RUN [--setup COMMAND ...]] [--restart] @@ -53,14 +54,26 @@ In order: (in DIR) changes that are not a --declared PATH (normalized and case-insensitive; a rename changes both paths), one per line after a "===== files changed outside the task's Files list =====" - line, or "(none)". --scope without --declared exits 2. With - --owners FILE (the ledger's owners.json, written by setup) each - path carries what it is, seen from the --brief tasks: "(owned by + line, or "(none)". --scope with neither --declared nor + --declared-from-owners exits 2. --declared-from-owners + N:HHHHHHHH (1.6.6) stands for the --declared list: the --brief + tasks' `declared` lists in FILE, in --brief order, each exact + string once, whose fingerprint (_owners.fingerprint) must be + N:HHHHHHHH. It needs --scope and --owners, does not go with + --declared, and a value not of that form exits 2. FILE is + read right after the --artifacts check, before any git work. + When it is missing or unreadable, or the fingerprint differs, + the section is the one line "(could not build the list: the + run's owner map does not match this unit's Files list; judge + scope from the brief)", everything else runs as before and the + exit stays 0. With --owners FILE (the ledger's owners.json, + written by setup) each path carries what it is, seen from the + --brief tasks: "(owned by X, earlier in the run)", "(owned by X, another lane, running alongside)", "(owned by X, later)" or "(no task owns it)"; a changed declared path that another task also lists follows as - "(in your Files list; also in X's Files)". A missing FILE prints - the plain list; an unreadable one exits 3. + "(in your Files list; also in X's Files)". Under --declared, a + missing FILE prints the plain list and an unreadable one exits 3. --rulings prints `ledger rulings LEDGER_DIR --lane LANE` (LANE "-": every ruling) under a "===== rulings in force =====" line, after the briefs; when that fails, one "(could not read the rulings: ...)" @@ -113,6 +126,10 @@ ENV = {k: v for k, v in os.environ.items() if k not in GIT_REDIRECTS} # The same forms as the ledger's UNIT_BRANCH and validate.js' runIdPattern. UNIT_BRANCH = re.compile(r"^pl-[a-z0-9-]+-u-[A-Za-z0-9_][A-Za-z0-9._-]*$") RUN_ID = re.compile(r"^[a-z0-9-]+$") +# The scope section under --declared-from-owners when the owner map does not +# give the unit's declared list. +UNMATCHED = ("(could not build the list: the run's owner map does not match this unit's Files list; " + "judge scope from the brief)") def fail(code, message): @@ -153,6 +170,7 @@ def parse(argv): parser.add_argument("--scope", nargs=2, metavar=("BASE", "HEAD")) parser.add_argument("--declared", action="append", default=[], metavar="PATH") parser.add_argument("--owners", metavar="FILE") + parser.add_argument("--declared-from-owners", metavar="N:HHHHHHHH") parser.add_argument("--rulings", nargs=2, metavar=("LEDGER_DIR", "LANE")) parser.add_argument("--unit-branch", nargs=2, metavar=("NAME", "SHA")) parser.add_argument("--run-id", metavar="RUN") @@ -170,10 +188,19 @@ def parse(argv): parser.error("--setup needs --unit-branch") if args.restart and args.record_start is None: parser.error("--restart needs --record-start") + if args.declared_from_owners is not None: + if args.scope is None: + parser.error("--declared-from-owners needs --scope") + if args.owners is None: + parser.error("--declared-from-owners needs --owners") + if args.declared: + parser.error("--declared-from-owners and --declared do not go together") + if not _owners.FINGERPRINT.fullmatch(args.declared_from_owners): + parser.error(f"--declared-from-owners {args.declared_from_owners!r} is not of the form N:HHHHHHHH") if args.owners is not None and args.scope is None: parser.error("--owners needs --scope") - if args.scope is not None and not args.declared: - parser.error("--scope needs at least one --declared PATH") + if args.scope is not None and not args.declared and args.declared_from_owners is None: + parser.error("--scope needs at least one --declared PATH, or --declared-from-owners") for task_id, _ in args.brief: if not _brief.is_safe_id(task_id): parser.error(f"unsafe task id {task_id!r}") @@ -258,6 +285,35 @@ def switch_unit_branch(directory, args): return name, lines +def scope_list(directory, args): + """The --scope section's lines: the changed paths outside args.declared, + annotated from the owner map when there is one.""" + res = git(directory, "-c", "core.quotePath=false", "diff", "--name-only", "--no-renames", + "--end-of-options", *args.scope) + if res.returncode != 0: + fail(1, f"git diff {' '.join(args.scope)} failed in {directory!r}: {res.stderr.strip()}") + declared = {path_key(p) for p in args.declared} + changed = [p for p in res.stdout.splitlines() if p] + outside = sorted(p for p in changed if path_key(p) not in declared) + lines = outside + if args.owners is not None and os.path.exists(args.owners): + # A ledger set up before 1.6.3 has no owner map: the plain list. + try: + owners = _owners.load(args.owners) + except (OSError, ValueError) as e: + if args.declared_from_owners is not None: + return UNMATCHED + fail(3, f"cannot read the owner map {args.owners!r}: {e}") + unit = [task_id for task_id, _ in args.brief] + shared = sorted(p for p in changed if path_key(p) in declared + and any(t not in unit for t in owners["files"].get(_owners.path_key(p), []))) + lines = [f"{p} ({_owners.annotate(owners, unit, p)})" for p in outside + shared] + elif args.declared_from_owners is not None: + # Read at the start, gone now: never the plain list. + return UNMATCHED + return "\n".join(lines) if lines else "(none)" + + def main(argv): _shell.setup_io() try: @@ -276,6 +332,15 @@ def run(args): for path in outs: if os.path.islink(path) or not contained(path, args.artifacts): fail(3, f"artifact path {path!r} is outside {args.artifacts!r}; nothing written") + # The scope list is advisory: an owner map that is missing, unreadable or + # does not match the engine's fingerprint never stops the start. + unmatched = False + if args.declared_from_owners is not None: + try: + args.declared = _owners.owners_declared( + args.owners, [task_id for task_id, _ in args.brief], args.declared_from_owners) + except ValueError: + unmatched = True branch = rev_parse(3, directory, "--abbrev-ref", "HEAD") start_base = None if args.record_start is not None: @@ -344,26 +409,8 @@ def run(args): out.append(f"===== review package =====\n{res.stdout}") if args.scope is not None: - res = git(directory, "-c", "core.quotePath=false", "diff", "--name-only", "--no-renames", - "--end-of-options", *args.scope) - if res.returncode != 0: - fail(1, f"git diff {' '.join(args.scope)} failed in {directory!r}: {res.stderr.strip()}") - declared = {path_key(p) for p in args.declared} - changed = [p for p in res.stdout.splitlines() if p] - outside = sorted(p for p in changed if path_key(p) not in declared) - lines = outside - if args.owners is not None and os.path.exists(args.owners): - # A ledger set up before 1.6.3 has no owner map: the plain list. - try: - owners = _owners.load(args.owners) - except (OSError, ValueError) as e: - fail(3, f"cannot read the owner map {args.owners!r}: {e}") - unit = [task_id for task_id, _ in args.brief] - shared = sorted(p for p in changed if path_key(p) in declared - and any(t not in unit for t in owners["files"].get(_owners.path_key(p), []))) - lines = [f"{p} ({_owners.annotate(owners, unit, p)})" for p in outside + shared] out.append("===== files changed outside the task's Files list =====\n" - + ("\n".join(lines) if lines else "(none)") + "\n") + + (UNMATCHED if unmatched else scope_list(directory, args)) + "\n") if start_base is not None: ledger_dir, lane, _ = args.record_start diff --git a/tests/task-helpers.test.mjs b/tests/task-helpers.test.mjs index 6dcc926..d7c0b4c 100644 --- a/tests/task-helpers.test.mjs +++ b/tests/task-helpers.test.mjs @@ -383,6 +383,112 @@ test('start-task --owners names the owner of each changed path, and shared decla + "shared.css (in your Files list; also in T5's Files)\n"), res.stdout); }); +// The engine's fingerprint of a declared list, as _owners.fingerprint computes +// it. Non-ASCII characters go to Python as JSON escapes. +function fp(paths) { + const json = JSON.stringify(paths).replace(/[^\x00-\x7f]/g, (ch) => `\\u${ch.charCodeAt(0).toString(16).padStart(4, '0')}`); + const res = sh('python3', ['-B', '-c', + 'import json, sys; sys.dont_write_bytecode = True; sys.path.insert(0, sys.argv[1]); ' + + 'import _owners; print(_owners.fingerprint(json.loads(sys.argv[2])))', SCRIPTS, json]); + assert.equal(res.code, 0, res.stderr); + return res.stdout.trim(); +} + +// The --owners test's repo and owner map, with T1's declared list added. +function ownersCase(declared = ['src/a.js', 'shared.css']) { + const c = newCase(); + const base = git(c.lane, 'rev-parse', 'HEAD'); + write(join(c.lane, 'src', 'a.js'), 'a\n'); + write(join(c.lane, 'src', 'b.js'), 'b\n'); + write(join(c.lane, 'shared.css'), 's\n'); + git(c.lane, 'add', '-A'); + git(c.lane, 'commit', '-q', '-m', 'work'); + const head = git(c.lane, 'rev-parse', 'HEAD'); + const owners = join(c.root, 'owners.json'); + write(owners, JSON.stringify({ + repo: {}, + tasks: { + T1: { where: 'alpha', pos: 0, declared }, + T2: { where: 'beta', pos: 0, declared: ['src/b.js'] }, + T5: { where: 'join', pos: 0, declared: ['shared.css'] }, + }, + files: { 'src/a.js': ['T1'], 'src/b.js': ['T2'], 'shared.css': ['T1', 'T5'] }, + })); + return { ...c, base, head, owners, out: join(c.root, 'briefs', 'T1.md') }; +} + +const COULD_NOT = "===== files changed outside the task's Files list =====\n" + + "(could not build the list: the run's owner map does not match this unit's Files list; judge scope from the brief)\n"; + +test('start-task --declared-from-owners prints the scope block --declared prints', () => { + const c = ownersCase(); + const listed = startTask(c.lane, c.plan, '--scope', c.base, c.head, '--declared', 'src/a.js', '--declared', 'shared.css', + '--owners', c.owners, '--brief', 'T1', c.out); + const fromOwners = startTask(c.lane, c.plan, '--scope', c.base, c.head, '--declared-from-owners', fp(['src/a.js', 'shared.css']), + '--owners', c.owners, '--brief', 'T1', c.out); + assert.equal(listed.code, 0, listed.stderr); + assert.equal(fromOwners.code, 0, fromOwners.stderr); + assert.equal(fromOwners.stdout, listed.stdout); + assert.ok(fromOwners.stdout.endsWith("===== files changed outside the task's Files list =====\n" + + "src/b.js (owned by T2, another lane, running alongside)\n" + + "shared.css (in your Files list; also in T5's Files)\n"), fromOwners.stdout); +}); + +test('start-task --declared-from-owners keeps a path with a quote, a space and non-ASCII', () => { + const quoted = "docs/it's here.md"; + const accented = 'caf\u{e9} menu.txt'; + const c = ownersCase([quoted]); + write(join(c.lane, 'docs', "it's here.md"), 'q\n'); + write(join(c.lane, accented), 'c\n'); + git(c.lane, 'add', '-A'); + git(c.lane, 'commit', '-q', '-m', 'more work'); + const head = git(c.lane, 'rev-parse', 'HEAD'); + const res = startTask(c.lane, c.plan, '--scope', c.head, head, '--declared-from-owners', fp([quoted]), + '--owners', c.owners, '--brief', 'T1', c.out); + assert.equal(res.code, 0, res.stderr); + assert.ok(res.stdout.endsWith("===== files changed outside the task's Files list =====\n" + + `${accented} (no task owns it)\n`), res.stdout); + assert.ok(!res.stdout.includes('here.md ('), res.stdout); +}); + +test('start-task goes on when the owner map does not match', () => { + const c = ownersCase(); + const torn = join(c.root, 'torn owners.json'); + write(torn, '{"tasks":'); + const cases = [ + ['a wrong fingerprint', c.owners, fp(['src/a.js'])], + ['a missing owners file', join(c.root, 'no-such-owners.json'), fp(['src/a.js', 'shared.css'])], + ['an unreadable owners file', torn, fp(['src/a.js', 'shared.css'])], + ]; + for (const [what, owners, value] of cases) { + rmSync(c.out, { force: true }); + const res = startTask(c.lane, c.plan, '--scope', c.base, c.head, '--declared-from-owners', value, + '--owners', owners, '--brief', 'T1', c.out); + assert.equal(res.code, 0, `${what}: ${res.stderr}`); + assert.ok(existsSync(c.out), `${what}: the brief is written`); + assert.ok(res.stdout.includes(`===== brief T1: ${shown(c.out)} =====\n`), `${what}: ${res.stdout}`); + assert.ok(res.stdout.endsWith(COULD_NOT), `${what}: ${res.stdout}`); + } +}); + +test('start-task --declared-from-owners usage errors exit 2', () => { + const c = ownersCase(); + const value = fp(['src/a.js', 'shared.css']); + const calls = [ + ['without --owners', '--scope', c.base, c.head, '--declared-from-owners', value], + ['with --declared', '--scope', c.base, c.head, '--declared-from-owners', value, '--declared', 'src/a.js', + '--owners', c.owners], + ['with a zero count', '--scope', c.base, c.head, '--declared-from-owners', '0:811c9dc5', '--owners', c.owners], + ['with a trailing newline', '--scope', c.base, c.head, '--declared-from-owners', `${value}\n`, '--owners', c.owners], + ['without --scope', '--declared-from-owners', value, '--owners', c.owners], + ]; + for (const [what, ...args] of calls) { + const res = startTask(c.lane, c.plan, ...args, '--brief', 'T1', c.out); + assert.equal(res.code, 2, `${what}: ${res.stderr}`); + assert.equal(res.stdout, '', what); + } +}); + test('start-task still starts without an owner map, or when the rulings print fails', () => { const c = newCase(); const base = git(c.lane, 'rev-parse', 'HEAD'); From a14e76dce47559982970eb15fda4c9e28d102b70 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:39:09 -0700 Subject: [PATCH 05/10] feat: task-checks reads the declared files from owners.json and takes --wide-as-lane --- scripts/task-checks | 97 ++++++++++++++++++++--- tests/task-checks.test.mjs | 158 +++++++++++++++++++++++++++++++++++-- 2 files changed, 237 insertions(+), 18 deletions(-) diff --git a/scripts/task-checks b/scripts/task-checks index 947dc52..b32294f 100755 --- a/scripts/task-checks +++ b/scripts/task-checks @@ -2,10 +2,11 @@ """Run a task's checks before it commits: targeted, the lane's, or widened. usage: task-checks DIR --base SHA --ledger LEDGER_DIR --unit UNIT_ID --lane LANE - --task ID [--task ID ...] [--declared PATH ...] [--widen PATH ...] - [--project-test CMD ...] + --task ID [--task ID ...] + [--declared PATH ... | --declared-from-owners N:HHHHHHHH] + [--widen PATH ...] [--project-test CMD ...] [--entry TASK_ID --targeted GROUP CMD [--targeted GROUP CMD ...]] - --lane-cmd GROUP CMD [...] --wide-cmd GROUP CMD [...] + --lane-cmd GROUP CMD [...] (--wide-cmd GROUP CMD [...] | --wide-as-lane) [--force-full REASON] [--evidence PATH] [--phase NAME] Options come in any order. The workflow builds the call (spec 4.2 of the @@ -13,6 +14,26 @@ Options come in any order. The workflow builds the call (spec 4.2 of the --phase NAME goes to run-checks, which sets PL_PHASE=NAME for the checks (without it PL_PHASE is removed); NAME is one of run-checks' phases. +--declared-from-owners N:HHHHHHHH (1.6.6) stands for the unit's --declared +list, which keeps the engine-built call short: the list is read from +LEDGER_DIR/owners.json, each --task's `declared` in the order given, the +first occurrence of each exact string kept (_owners.owners_declared), and +checked against the fingerprint the engine computed from its own list (N the +number of paths, H their 32-bit FNV-1a hash, as _owners.fingerprint). A +value that is not a fingerprint (N of 0 included), or one given with +--declared, is a usage error. When owners.json is missing or unreadable, a +--task is not in it, a task has no `declared`, a path in it is absolute or +leaves the project, or the fingerprint differs, the call selects and runs +nothing and exits 3 with the error "the run's owner map (owners.json) does +not match this unit's Files list (): report blocked with this +message; do not run setup and do not pass --declared yourself (the session +reruns setup)", written as the error record when FILE is known (with +--evidence; without it nothing is written). + +--wide-as-lane (1.6.6) stands for --wide-cmd pairs equal to the --lane-cmd +pairs: the lane list is used as the wide list. It cannot be given with +--wide-cmd; one of the two is required. + Selection (policy.selected): lane no --entry: the unit has no task_checks entry, so the --lane-cmd list runs (the lane's checks, as before 1.6.0); @@ -20,7 +41,8 @@ Selection (policy.selected): (the project's checks plus the lane's own additions); targeted an entry and no widening reason: the --targeted list runs. policy.reasons lists every reason that applies, as {code, detail}, in this -order: no_entry, no_files (no --declared), outside_files (a changed path the +order: no_entry, no_files (no --declared and no --declared-from-owners), +outside_files (a changed path the Files list does not cover, nor a ruling permits), widen_path (a changed path a --widen entry covers), forced (--force-full, its REASON as detail), git_failed (the changed paths could not be listed). @@ -86,7 +108,8 @@ Error records: once FILE is known (a valid --evidence PATH, or the reserved file), every later failure writes {"results": [], "ok": false, "error": , "policy": } to FILE before the exit: a usage error after a valid --evidence (exit 2), the lock not -taken within 5 s, or an unexpected error (exit 3). A background run's output +taken within 5 s, an owner-map mismatch, or an unexpected error (exit 3). A +background run's output is invisible to the agent that started it; the record says why it ended. This evidence covers a working tree before a commit. Nothing reads it to @@ -100,7 +123,8 @@ BASE must be a commit sha (7 to 40 lowercase hex digits). Exit: run-checks' exit when it printed JSON and exited 0 or 1 (0 every check passed, 1 a check failed); 2 on a usage error (nothing is written, except the error record after a valid --evidence); 3 when the reserved path is -outside LEDGER_DIR, FILE's lock cannot be taken, run-checks printed no JSON +outside LEDGER_DIR, FILE's lock cannot be taken, the owner map does not +match --declared-from-owners, run-checks printed no JSON or exited with another code, or any unexpected error happened (with the error record once FILE is known). Exit 1 always means a check failed. """ @@ -122,6 +146,7 @@ sys.dont_write_bytecode = True HERE = os.path.dirname(os.path.realpath(__file__)) sys.path.insert(0, HERE) import _await # noqa: E402 +import _owners # noqa: E402 import _shell # noqa: E402 GIT_REDIRECTS = ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", @@ -133,16 +158,21 @@ ERROR_TAIL = 2000 PRINT_PATHS = 50 PRINT_DETAIL = 2000 USAGE = ("usage: task-checks DIR --base SHA --ledger LEDGER_DIR --unit UNIT_ID --lane LANE " - "--task ID [...] [--declared PATH ...] [--widen PATH ...] [--project-test CMD ...] " + "--task ID [...] [--declared PATH ... | --declared-from-owners N:HHHHHHHH] " + "[--widen PATH ...] [--project-test CMD ...] " "[--entry TASK_ID --targeted GROUP CMD ...] --lane-cmd GROUP CMD [...] " - "--wide-cmd GROUP CMD [...] [--force-full REASON] [--evidence PATH] [--phase NAME]") + "(--wide-cmd GROUP CMD [...] | --wide-as-lane) [--force-full REASON] [--evidence PATH] " + "[--phase NAME]") # run-checks' phases (--phase sets PL_PHASE for the checks). PHASES = ("prelude", "lane", "join", "integrate", "post-integrate", "final-fix", "verify") SINGLE = {"--base": "base", "--ledger": "ledger", "--unit": "unit", "--lane": "lane", - "--entry": "entry", "--force-full": "forced", "--evidence": "evidence", "--phase": "phase"} + "--entry": "entry", "--force-full": "forced", "--evidence": "evidence", "--phase": "phase", + "--declared-from-owners": "owners_fp"} REPEATED = {"--task": "tasks", "--declared": "declared", "--widen": "widen", "--project-test": "project_tests"} PAIRS = {"--targeted": "targeted", "--lane-cmd": "lane_cmds", "--wide-cmd": "wide_cmds"} +# Options that take no value. +FLAGS = {"--wide-as-lane": "wide_as_lane"} # The result file once it is known (validated --evidence, or the reserved @@ -185,7 +215,8 @@ def parse(argv): first usage error the reading met (checked by validate), or None. A token that is not an option is skipped.""" args = {"dir": None, "base": None, "ledger": None, "unit": None, "lane": None, "entry": None, - "forced": None, "evidence": None, "phase": None, "tasks": [], "declared": [], "widen": [], "project_tests": [], + "forced": None, "evidence": None, "phase": None, "owners_fp": None, "wide_as_lane": False, + "tasks": [], "declared": [], "widen": [], "project_tests": [], "targeted": [], "lane_cmds": [], "wide_cmds": []} errors = [] rest = argv @@ -209,6 +240,11 @@ def parse(argv): elif flag in PAIRS and len(rest) >= 3 and rest[1] and rest[2]: args[PAIRS[flag]].append((rest[1], rest[2])) rest = rest[3:] + elif flag in FLAGS: + if args[FLAGS[flag]]: + errors.append(f"{flag} given twice") + args[FLAGS[flag]] = True + rest = rest[1:] else: errors.append(USAGE) rest = rest[1:] @@ -250,8 +286,15 @@ def validate(args): fail(2, "at least one --task is required") if args["phase"] is not None and args["phase"] not in PHASES: fail(2, f"--phase {args['phase']!r} is not one of {', '.join(PHASES)}") - if not args["lane_cmds"] or not args["wide_cmds"]: - fail(2, "--lane-cmd and --wide-cmd are required") + if not args["lane_cmds"] or not (args["wide_cmds"] or args["wide_as_lane"]): + fail(2, "--lane-cmd and --wide-cmd (or --wide-as-lane) are required") + if args["wide_as_lane"] and args["wide_cmds"]: + fail(2, "--wide-as-lane and --wide-cmd do not go together") + if args["owners_fp"] is not None: + if not _owners.FINGERPRINT.fullmatch(args["owners_fp"]): + fail(2, f"--declared-from-owners {args['owners_fp']!r} is not a fingerprint N:HHHHHHHH") + if args["declared"]: + fail(2, "--declared-from-owners and --declared do not go together") if (args["entry"] is None) != (not args["targeted"]): fail(2, "--entry and --targeted come together") if not SHA.match(args["base"]): @@ -273,6 +316,34 @@ def validate(args): # A --widen entry naming an existing directory covers it without the '/': # widening is the safe side. --declared keeps the strict rule. args["widen"] = [(e, d or os.path.isdir(os.path.join(args["dir"], e))) for e, d in args["widen"]] + if args["wide_as_lane"]: + args["wide_cmds"] = list(args["lane_cmds"]) + + +OWNERS_MISMATCH = ("the run's owner map (owners.json) does not match this unit's Files list ({}): " + "report blocked with this message; do not run setup and do not pass --declared " + "yourself (the session reruns setup)") + + +def resolve_declared(args): + """With --declared-from-owners: the unit's declared list from + LEDGER_DIR/owners.json, checked against the fingerprint, as declared_raw + and declared; exit 3 with the mismatch error otherwise.""" + if args["owners_fp"] is None: + return + path = os.path.join(args["ledger"], "owners.json") + try: + declared = _owners.owners_declared(path, args["tasks"], args["owners_fp"]) + except ValueError as e: + fail(3, OWNERS_MISMATCH.format(e)) + keys = [] + for value in declared: + k = entry_key(value) + if k is None: + fail(3, OWNERS_MISMATCH.format(f"owners.json lists {value!r}, which is not a project-relative path")) + keys.append(k) + args["declared_raw"] = declared + args["declared"] = keys def git_paths(directory, *git_args): @@ -442,11 +513,13 @@ def run(argv): def work(): validate(args) + resolve_declared(args) chosen, policy = plan(args, started) run_checks(args, chosen, policy, out) holding(out, work) else: validate(args) + resolve_declared(args) chosen, policy = plan(args, started) out = reserve(args["ledger"], args["unit"]) holding(out, lambda: run_checks(args, chosen, policy, out)) diff --git a/tests/task-checks.test.mjs b/tests/task-checks.test.mjs index b0787a3..dcc5ebc 100644 --- a/tests/task-checks.test.mjs +++ b/tests/task-checks.test.mjs @@ -57,12 +57,13 @@ const L = 'node t.mjs L'; const W = 'node t.mjs W'; // Runs task-checks with unit U1 (task U1), the given extra arguments, and the -// lane and wide lists; entry: true adds --entry U1 --targeted test T. -function run(r, { entry = true, args = [], unit = 'U1', tasks = [unit], lane = 'alpha', entryId = null } = {}) { +// lane and wide lists; entry: true adds --entry U1 --targeted test T; wide: +// false drops the --wide-cmd pair. +function run(r, { entry = true, args = [], unit = 'U1', tasks = [unit], lane = 'alpha', entryId = null, wide = true } = {}) { const argv = [SCRIPT, r.dir, '--base', r.base, '--ledger', r.ledger, '--unit', unit, '--lane', lane, ...tasks.flatMap((t) => ['--task', t]), ...(entry ? ['--entry', entryId || tasks[tasks.length - 1], '--targeted', 'test', T] : []), - '--lane-cmd', 'test', L, '--wide-cmd', 'test', W, '--project-test', W, ...args]; + '--lane-cmd', 'test', L, ...(wide ? ['--wide-cmd', 'test', W] : []), '--project-test', W, ...args]; const res = spawnSync('python3', argv, { encoding: 'utf8', env: { ...process.env, ...GIT_ENV } }); let json = null; try { json = JSON.parse(res.stdout); } catch { /* left null */ } @@ -142,9 +143,11 @@ test('a widen entry naming an existing directory without a trailing / covers fil function ledgerFile(r, stem, events) { writeFileSync(join(r.ledger, `${stem}.jsonl`), events.map((e) => JSON.stringify(e)).join('\n') + '\n'); } -function ownerMap(r, files) { - writeFileSync(join(r.ledger, 'owners.json'), JSON.stringify({ - tasks: { U1: { where: 'alpha', pos: 0 }, U2: { where: 'alpha', pos: 1 }, U9: { where: 'beta', pos: 0 } }, files })); +// declared: {task id: its Files list}, recorded as that task's `declared` (1.6.6). +function ownerMap(r, files, declared = {}) { + const tasks = { U1: { where: 'alpha', pos: 0 }, U2: { where: 'alpha', pos: 1 }, U9: { where: 'beta', pos: 0 } }; + for (const [id, entry] of Object.entries(tasks)) if (id in declared) entry.declared = declared[id]; + writeFileSync(join(r.ledger, 'owners.json'), JSON.stringify({ tasks, files })); } const permit = (task, files, extra = {}) => ({ event: 'ruling', task, by: 'adjudicator', text: 'Ruling: x - y - z', files, ...extra }); @@ -733,3 +736,146 @@ test('without --evidence the 1.6.0 reservation still works', () => { assert.equal(second.json.policy.selected, 'lane'); assert.deepEqual(sidecars(checks), [], 'no sidecar is left'); }); + +// ---- --declared-from-owners and --wide-as-lane (1.6.6, spec 4.3, 4.5) ---- + +const fwd = (p) => p.replace(/\\/g, '/'); +// The fingerprint of a declared list, from Python _owners.fingerprint. +function fp(paths) { + const code = [ + 'import json, sys', + 'sys.dont_write_bytecode = True', + `sys.path.insert(0, ${JSON.stringify(SCRIPTS)})`, + 'import _owners', + 'print(_owners.fingerprint(json.loads(sys.argv[1])))', + ].join('\n'); + const res = spawnSync('python3', ['-c', code, JSON.stringify(paths)], { encoding: 'utf8', env: ENV }); + assert.equal(res.status, 0, res.stderr); + return res.stdout.trim(); +} +// The policy without started, which differs between two calls. +const policyOf = (json) => { const { started, ...rest } = json.policy; return rest; }; + +test('--declared-from-owners gives the policy --declared gives', () => { + const r = repo({ 'src/a.js': '1\n', 'src/b.js': '1\n', 'shared.css': '1\n' }); + writeFileSync(join(r.dir, 'src/a.js'), '2\n'); + writeFileSync(join(r.dir, 'src/b.js'), '2\n'); + // One task: src/b.js is outside its Files list. + ownerMap(r, { 'src/a.js': ['U1'] }, { U1: ['src/a.js'] }); + const owners = run(r, { args: ['--declared-from-owners', fp(['src/a.js'])] }); + const explicit = run(r, { args: ['--declared', 'src/a.js'] }); + assert.equal(owners.status, 0, owners.stderr); + assert.deepEqual(codes(owners.json), ['outside_files']); + assert.match(owners.json.policy.reasons[0].detail, /src\/b\.js/); + assert.deepEqual(policyOf(owners.json), policyOf(explicit.json)); + // A batch whose tasks share a path: each exact string once, first occurrence kept. + ownerMap(r, { 'src/a.js': ['U1'], 'shared.css': ['U1', 'U2'], 'src/b.js': ['U2'] }, + { U1: ['src/a.js', 'shared.css'], U2: ['shared.css', 'src/b.js'] }); + writeFileSync(join(r.dir, 'shared.css'), '2\n'); + writeFileSync(join(r.dir, 'src/c.js'), 'new\n'); + const list = ['src/a.js', 'shared.css', 'src/b.js']; + const batch = { unit: 'U1-U2', tasks: ['U1', 'U2'] }; + const batchOwners = run(r, { ...batch, args: ['--declared-from-owners', fp(list)] }); + const batchExplicit = run(r, { ...batch, args: list.flatMap((p) => ['--declared', p]) }); + assert.equal(batchOwners.status, 0, batchOwners.stderr); + assert.deepEqual(codes(batchOwners.json), ['outside_files']); + assert.match(batchOwners.json.policy.reasons[0].detail, /^outside the Files list: src\/c\.js$/); + assert.deepEqual(policyOf(batchOwners.json), policyOf(batchExplicit.json)); +}); + +test('a path from owners.json with a space, a quote and non-ASCII reaches outside_files and permitted', () => { + const r = repo({ "docs/it's here.md": '1\n', 'src/a.js': '1\n' }); + writeFileSync(join(r.dir, "docs/it's here.md"), '2\n'); + writeFileSync(join(r.dir, 'src/a.js'), '2\n'); + writeFileSync(join(r.dir, 'caf\u00e9 menu.txt'), 'new\n'); + writeFileSync(join(r.dir, 'src/slot.js'), 'new\n'); + // A _run permit covers only a path in the folder of a declared file: src/a.js puts src/ there, + // so the permit shows the declared list from owners.json reaches the permits. + ledgerFile(r, '_run', [permit('_run', ['src/slot.js'])]); + const declared = ["docs/it's here.md", 'src/a.js']; + ownerMap(r, { "docs/it's here.md": ['U1'], 'src/a.js': ['U1'] }, { U1: declared }); + const { status, json, stderr } = run(r, { args: ['--declared-from-owners', fp(declared)] }); + assert.equal(status, 0, stderr); + assert.deepEqual(json.policy.changed, ['caf\u00e9 menu.txt', "docs/it's here.md", 'src/a.js', 'src/slot.js']); + assert.deepEqual(codes(json), ['outside_files']); + assert.equal(json.policy.reasons[0].detail, 'outside the Files list: caf\u00e9 menu.txt'); + assert.deepEqual(json.policy.permitted, [{ path: 'src/slot.js', ruling: '_run-R1' }]); +}); + +test('an owner-map mismatch writes the error record and exits 3', () => { + const r = repo({ 'src/a.js': '1\n' }); + writeFileSync(join(r.dir, 'src/a.js'), '2\n'); + const good = fp(['src/a.js']); + for (const [what, setUp, value, opts] of [ + ['a wrong fingerprint', () => ownerMap(r, {}, { U1: ['src/a.js'] }), fp(['src/b.js']), {}], + ['no owners.json', () => rmSync(join(r.ledger, 'owners.json'), { force: true }), good, {}], + ['an unknown --task', () => ownerMap(r, {}, { U1: ['src/a.js'] }), good, { tasks: ['U1', 'U7'] }], + ['a task without declared', () => ownerMap(r, {}, {}), good, {}], + ['a declared absolute path', () => ownerMap(r, {}, { U1: ['/abs/x.js'] }), fp(['/abs/x.js']), {}], + ]) { + setUp(); + const file = reserve(r); + const res = run(r, { ...opts, args: ['--declared-from-owners', value, '--evidence', file] }); + assert.equal(res.status, 3, `${what}: ${res.stderr}`); + const record = JSON.parse(readFileSync(file, 'utf8')); + assert.deepEqual(record.results, [], what); + assert.equal(record.ok, false, what); + assert.ok(record.error.includes('owners.json'), `${what}: ${record.error}`); + assert.ok(record.error.includes('report blocked'), `${what}: ${record.error}`); + assert.ok(record.error.startsWith("the run's owner map (owners.json) does not match this unit's Files list ("), + `${what}: ${record.error}`); + assert.ok(record.error.endsWith('do not run setup and do not pass --declared yourself (the session reruns setup)'), + `${what}: ${record.error}`); + assert.equal(record.policy, null, what); + const waited = spawnSync('python3', [AWAIT, 'wait', file, '--max', '30'], { encoding: 'utf8', env: ENV }); + assert.equal(waited.status, 3, `${what}: ${waited.stderr}`); + } +}); + +test('the ledger in forward-slash form still finds owners.json', () => { + const r = repo({ 'src/a.js': '1\n' }); + writeFileSync(join(r.dir, 'src/a.js'), '2\n'); + ownerMap(r, { 'src/a.js': ['U1'] }, { U1: ['src/a.js'] }); + const res = run({ ...r, ledger: fwd(r.ledger) }, { args: ['--declared-from-owners', fp(['src/a.js'])] }); + assert.equal(res.status, 0, res.stderr); + assert.equal(res.json.policy.selected, 'targeted'); +}); + +test('owner-map usage errors exit 2 with nothing written', () => { + const r = repo({ 'src/a.js': '1\n' }); + ownerMap(r, { 'src/a.js': ['U1'] }, { U1: ['src/a.js'] }); + for (const args of [ + ['--declared-from-owners', '0:811c9dc5'], + ['--declared-from-owners', '3:XYZ'], + ['--declared-from-owners', `${fp(['src/a.js'])}\n`], + ['--declared-from-owners', fp(['src/a.js']), '--declared', 'src/a.js'], + ]) { + const res = run(r, { args }); + assert.equal(res.status, 2, `${JSON.stringify(args)}: ${res.stderr}`); + assert.match(res.stderr, /^task-checks: --declared-from-owners /, JSON.stringify(args)); + } + const checks = join(r.ledger, 'checks'); + assert.deepEqual(existsSync(checks) ? readdirSync(checks) : [], []); +}); + +test('--wide-as-lane gives the policy the lane pairs as --wide-cmd give', () => { + const r = repo({ 'src/a.js': '1\n' }); + const asLane = run(r, { wide: false, args: ['--wide-as-lane', '--force-full', 'x'] }); + const pairs = run(r, { wide: false, args: ['--wide-cmd', 'test', L, '--force-full', 'x'] }); + assert.equal(asLane.status, 0, asLane.stderr); + assert.equal(asLane.json.policy.selected, 'widened'); + assert.deepEqual(ran(asLane.json), [L]); + assert.deepEqual(asLane.json.policy.offered.wide, [{ group: 'test', command: L }]); + assert.deepEqual(policyOf(asLane.json), policyOf(pairs.json)); +}); + +test('--wide-as-lane with --wide-cmd, or neither, exits 2', () => { + const r = repo(); + const both = run(r, { args: ['--wide-as-lane'] }); + assert.equal(both.status, 2, 'with --wide-cmd'); + assert.match(both.stderr, /--wide-as-lane and --wide-cmd do not go together/); + const neither = run(r, { wide: false }); + assert.equal(neither.status, 2, 'neither'); + assert.match(neither.stderr, /--lane-cmd and --wide-cmd \(or --wide-as-lane\) are required/); + assert.ok(!existsSync(join(r.ledger, 'checks')), 'nothing was written'); +}); From b6fb3b7f92ddff67a8fe829d241392b546439b48 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:50:24 -0700 Subject: [PATCH 06/10] docs: short check commands and the Bash command limit --- reference.md | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/reference.md b/reference.md index f66cf1c..69fa10d 100644 --- a/reference.md +++ b/reference.md @@ -157,8 +157,12 @@ The header shown with the dry-run table lists: mode, base and feature branch, wo `excluded` with reasons, batches, and whether deferral is allowed. Doctor's warnings go in it too. One line comes from the dry run's `checks` summary, for example "Checks: targeted entries for 9 of 12 units; delivery runs 3 checks (1 added by lane_commands); repairs up to 2", and its -`warnings` follow it (today one: "changes to shared files will not widen task checks", when -`task_checks` has entries and `widen_paths` is empty). One line comes from the dry run's +`warnings` follow it. There are two: "changes to shared files will not widen task checks", when +`task_checks` has entries and `widen_paths` is empty; and (1.6.6) " is about +characters; the Bash tool on Windows fails at about 7,000: shorten the project's check commands, +for example with a wrapper script", when the longest engine-built check command is over 6,000 +characters, with "'s task-checks command" or "the integration checks command" +and its length (see "Command length" in Check ownership). One line comes from the dry run's `join_waves`: "Join: waves 1-2-4 (DJ1 | DJ2, DJ4 | DJ3, DJ5, DJ6, DJ7)", or "Join: serial ()" (see Join waves). @@ -521,6 +525,19 @@ Files` (the reviewer checks the plan's split of that shared file), or `no task o `ledger undeclared ` (and `ledger status --manifest`) report `{task: [{path, owner, relation}]}`. +From 1.6.6 owners.json also gives each task its `declared`: its exact Files list as the manifest +gives it (no case change or normalising, manifest order, each exact string once, `[]` for a task +without one). The reviewer's start-task call builds its scope list from it: `--scope BASE HEAD +--declared-from-owners N:HHHHHHHH --owners /owners.json` takes the `--brief` tasks' +`declared` lists, in `--brief` order, each exact string once, and checks them against the +engine's fingerprint (see "task-checks" in Check ownership); the owner annotations print as +with `--declared`. When the owner map is missing or unreadable, or the fingerprint differs, the +scope section is the one line "(could not build the list: the run's owner map does not match +this unit's Files list; judge scope from the brief)", the briefs and package are written as +usual and start-task exits 0: the list is advice, and a failed start would spend a fix agent on +something no agent can repair. `--declared-from-owners` needs `--scope` and `--owners` and does +not go with `--declared`; `--declared PATH` still works by hand. + **Observed failures.** A check failure an implementer judges outside its task (it passed on a rerun, or the failing test covers code the task did not touch) is a run-wide ruling `Observed: failed - - `, so verify, a @@ -660,6 +677,46 @@ error record. The printed line lists at most 50 changed paths and cuts long reas note; the evidence file keeps everything. The evidence is a record and a count: nothing reads it to accept, skip or reuse checks. +**The engine's short forms** (1.6.6). The call the engine builds does not list the unit's Files: +- `--declared-from-owners N:HHHHHHHH` stands for the `--declared` list (passed when the unit + declares files; with none, neither is passed and `no_files` holds). task-checks reads + `/owners.json`, takes each `--task`'s `declared` in the order given, keeps the first + occurrence of each exact string (as the engine's `declaredFiles` does for a batch or join + unit), and compares the fingerprint: N the number of paths, H the 32-bit FNV-1a hash of their + UTF-8 bytes joined with one NUL byte, as 8 lowercase hex digits (the engine and + `scripts/_owners.py` compute it alike). A value that is not of that form (N of 0 included), + or one given with `--declared`, is a usage error (exit 2). When owners.json is missing or + unreadable, a `--task` is not in it, a task has no `declared`, a path in it is absolute or + leaves the project, or the fingerprint differs, task-checks selects and runs nothing, writes + its error record to the evidence file and exits 3 (`await-checks wait` then exits 3, not + `stopped`), with the error "the run's owner map (owners.json) does not match this unit's + Files list (): report blocked with this message; do not run setup and do not pass + --declared yourself (the session reruns setup)". The implementer's prompt says the same: "If + the call fails with an error that names owners.json, report blocked with its message; do not + run setup or pass --declared yourself." The session reruns setup (Relaunch or Resume), which + writes owners.json again from the manifest. Past the check nothing changes: the same + `no_files`, `outside_files`, permits and policy as with the list. +- `--wide-as-lane` stands for the `--wide-cmd` pairs when the wide list equals the lane list + (the same groups and commands in the same order, as with no `lane_commands`, or a lane whose + `lane_commands` setup differs from the project's): the lane list is used as the wide list, so + no recorded or printed value changes. It does not go with `--wide-cmd`; one of the two is + required. A lane with extra commands and the project's setup still gets the `--wide-cmd` pairs. +- `--declared PATH` and `--wide-cmd` still work by hand; a call without the new options behaves + as in 1.6.5. `--project-test` and `--targeted` stay as they are: they are bounded by the + project's commands. + +**Command length** (1.6.6). The Bash tool on Windows fails a command of about 7,000 characters +or more before bash runs it ("unexpected EOF while looking for matching"), however well it is +quoted. On one 2026-10-16 run, 7 of 40 task-checks calls (all of 6,907 characters or more), 1 +reviewer start and 6 heredocs written by agents failed this way; each had to be redone another +way, and the empty evidence files showed as unavailable calls and stopped waits. 1.6.6 keeps the +engine-built commands short: the declared files travel as a fingerprint and the wide list as +`--wide-as-lane`, so what remains grows only with the project's check commands. Agents write +and extend long files with Edit and Write, not heredocs (the rule every agent gets). The dry run +warns when a unit's task-checks command, or the integration's run-checks command, would be over +6,000 characters (see Confirmation header); the fix is to shorten the project's check commands, +for example with a wrapper script. The warning is advice and never refuses. + **Repair after a failing verify** (autonomous runs only; supervised runs reject as before): - Due only when verify ran at the head, with exactly the configured inventory, no `head_after`, a clean checkout, and a failure that is not made only of exits 75, 124, 128 or From 89777ce0b0bc4c5db167d5416a14666a9a05dc9c Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:54:56 -0700 Subject: [PATCH 07/10] feat: the engine passes the declared-files fingerprint and --wide-as-lane --- run.workflow.js | 18 ++- src/prompts-task.js | 18 ++- tests/check-policy-prompts.test.mjs | 207 ++++++++++++++++++++++++++-- tests/engine.test.mjs | 11 +- 4 files changed, 229 insertions(+), 25 deletions(-) diff --git a/run.workflow.js b/run.workflow.js index 9a5cd91..d921001 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1420,24 +1420,29 @@ function commandsText(m, laneId, dir = null, name = 'checks', phase = null) { // The scripts/task-checks call a task's implementer runs once before // committing (spec 4.2, 4.3), in where.dir for the unit's range from base, // writing to the result file the agent reserved (--evidence resultPath()); -// null when the ledger lane has no test, lint or build command. +// null when the ledger lane has no test, lint or build command. 1.6.6 (spec +// 4.5): the Files list goes as its fingerprint (task-checks reads the list +// from owners.json), and a wide list equal to the lane list as --wide-as-lane, +// so the command does not grow with the Files list. function taskChecksCommand(m, task, where, base) { const lane = laneChecks(m, where.lane); if (lane.length === 0) return null; const q = shellQuote; const pairs = (flag, checks) => checks.flatMap((c) => [flag, q(c.group), q(c.command)]); const entry = taskChecksEntry(m, task, where.lane); + const declared = declaredFiles(task); + const wide = wideChecks(m, where.lane); return [ `cd ${q(where.dir)} && ${pythonCommand(m)} ${q(`${m.skill_dir}/scripts/task-checks`)}`, q(where.dir), '--base', q(base), '--ledger', q(m.repo.ledger_dir), '--unit', q(task.id), '--evidence', resultPath(), '--lane', q(where.lane), '--phase', q(checksPhase(where.lane)), ...unitTasks(task).flatMap((t) => ['--task', q(t.id)]), - ...declaredFiles(task).flatMap((f) => ['--declared', q(f)]), + ...(declared.length === 0 ? [] : ['--declared-from-owners', q(declaredFingerprint(declared))]), ...(Array.isArray(m.widen_paths) ? m.widen_paths : []).flatMap((p) => ['--widen', q(p)]), ...commandGroup(m, null, 'test').flatMap((c) => ['--project-test', q(c)]), ...(entry === null ? [] : ['--entry', q(entry.id), ...pairs('--targeted', entry.checks)]), ...pairs('--lane-cmd', lane), - ...pairs('--wide-cmd', wideChecks(m, where.lane)), + ...(JSON.stringify(wide) === JSON.stringify(lane) ? ['--wide-as-lane'] : pairs('--wide-cmd', wide)), ].join(' '); } @@ -1460,6 +1465,8 @@ function taskChecksLines(m, task, where, base) { 'and the result file the wait read.', ], }), + 'If the call fails with an error that names owners.json, report blocked with its message; do not run setup or', + 'pass --declared yourself.', 'It picks the targeted checks, or the wider ones when your change reaches beyond them, runs them, and records', 'its choice. While iterating, run only the focused tests for what you change. When the call fails, fix and', 'rerun it; do not commit with failing checks (report blocked instead). If you know your change reaches code the', @@ -1778,7 +1785,8 @@ function declaredFingerprint(paths) { // the current plan and printed, and with opts.pkg = {base, head} and // superpowers present the review package for base..head. With opts.pkg and a // Files list it also lists the files base..head changes outside that list -// (--scope); with opts.start (the task base, implement only) it records that +// (--scope; 1.6.6: the list goes as its fingerprint, and start-task reads it +// from owners.json); with opts.start (the task base, implement only) it records that // base in the ledger once the briefs are written (--record-start). For a // join wave unit (where.unit_branch, 1.6.5) the implement start (opts.start) // also puts the worktree on the unit's branch from where.from and runs the @@ -1811,7 +1819,7 @@ function startCommand(m, task, where, opts = {}) { const declared = declaredFiles(task); if (pkg && declared.length > 0) { parts.push('--scope', shellQuote(pkg.base), shellQuote(pkg.head), - ...declared.flatMap((f) => ['--declared', shellQuote(f)]), + '--declared-from-owners', shellQuote(declaredFingerprint(declared)), '--owners', shellQuote(`${m.repo.ledger_dir}/owners.json`)); } // The rulings in force (1.6.3): a join task sees every lane's. diff --git a/src/prompts-task.js b/src/prompts-task.js index e7c592f..67741ae 100644 --- a/src/prompts-task.js +++ b/src/prompts-task.js @@ -297,24 +297,29 @@ function commandsText(m, laneId, dir = null, name = 'checks', phase = null) { // The scripts/task-checks call a task's implementer runs once before // committing (spec 4.2, 4.3), in where.dir for the unit's range from base, // writing to the result file the agent reserved (--evidence resultPath()); -// null when the ledger lane has no test, lint or build command. +// null when the ledger lane has no test, lint or build command. 1.6.6 (spec +// 4.5): the Files list goes as its fingerprint (task-checks reads the list +// from owners.json), and a wide list equal to the lane list as --wide-as-lane, +// so the command does not grow with the Files list. function taskChecksCommand(m, task, where, base) { const lane = laneChecks(m, where.lane); if (lane.length === 0) return null; const q = shellQuote; const pairs = (flag, checks) => checks.flatMap((c) => [flag, q(c.group), q(c.command)]); const entry = taskChecksEntry(m, task, where.lane); + const declared = declaredFiles(task); + const wide = wideChecks(m, where.lane); return [ `cd ${q(where.dir)} && ${pythonCommand(m)} ${q(`${m.skill_dir}/scripts/task-checks`)}`, q(where.dir), '--base', q(base), '--ledger', q(m.repo.ledger_dir), '--unit', q(task.id), '--evidence', resultPath(), '--lane', q(where.lane), '--phase', q(checksPhase(where.lane)), ...unitTasks(task).flatMap((t) => ['--task', q(t.id)]), - ...declaredFiles(task).flatMap((f) => ['--declared', q(f)]), + ...(declared.length === 0 ? [] : ['--declared-from-owners', q(declaredFingerprint(declared))]), ...(Array.isArray(m.widen_paths) ? m.widen_paths : []).flatMap((p) => ['--widen', q(p)]), ...commandGroup(m, null, 'test').flatMap((c) => ['--project-test', q(c)]), ...(entry === null ? [] : ['--entry', q(entry.id), ...pairs('--targeted', entry.checks)]), ...pairs('--lane-cmd', lane), - ...pairs('--wide-cmd', wideChecks(m, where.lane)), + ...(JSON.stringify(wide) === JSON.stringify(lane) ? ['--wide-as-lane'] : pairs('--wide-cmd', wide)), ].join(' '); } @@ -337,6 +342,8 @@ function taskChecksLines(m, task, where, base) { 'and the result file the wait read.', ], }), + 'If the call fails with an error that names owners.json, report blocked with its message; do not run setup or', + 'pass --declared yourself.', 'It picks the targeted checks, or the wider ones when your change reaches beyond them, runs them, and records', 'its choice. While iterating, run only the focused tests for what you change. When the call fails, fix and', 'rerun it; do not commit with failing checks (report blocked instead). If you know your change reaches code the', @@ -655,7 +662,8 @@ function declaredFingerprint(paths) { // the current plan and printed, and with opts.pkg = {base, head} and // superpowers present the review package for base..head. With opts.pkg and a // Files list it also lists the files base..head changes outside that list -// (--scope); with opts.start (the task base, implement only) it records that +// (--scope; 1.6.6: the list goes as its fingerprint, and start-task reads it +// from owners.json); with opts.start (the task base, implement only) it records that // base in the ledger once the briefs are written (--record-start). For a // join wave unit (where.unit_branch, 1.6.5) the implement start (opts.start) // also puts the worktree on the unit's branch from where.from and runs the @@ -688,7 +696,7 @@ function startCommand(m, task, where, opts = {}) { const declared = declaredFiles(task); if (pkg && declared.length > 0) { parts.push('--scope', shellQuote(pkg.base), shellQuote(pkg.head), - ...declared.flatMap((f) => ['--declared', shellQuote(f)]), + '--declared-from-owners', shellQuote(declaredFingerprint(declared)), '--owners', shellQuote(`${m.repo.ledger_dir}/owners.json`)); } // The rulings in force (1.6.3): a join task sees every lane's. diff --git a/tests/check-policy-prompts.test.mjs b/tests/check-policy-prompts.test.mjs index d7e6c94..7cb2f05 100644 --- a/tests/check-policy-prompts.test.mjs +++ b/tests/check-policy-prompts.test.mjs @@ -3,9 +3,13 @@ // reviewers see the unit's task-check evidence; no task agent gets the // every-check run-checks line. -import { test } from 'node:test'; +import { test, after } from 'node:test'; import assert from 'node:assert/strict'; -import { loadHelpers, H } from './harness.mjs'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { loadHelpers, H, SKILL_DIR } from './harness.mjs'; +import { BASH, tempDir } from './platform.mjs'; const { implementPrompt, reviewPrompt, reReviewPrompt, fixPrompt, laneChecks, wideChecks, taskChecksCommand, @@ -13,7 +17,8 @@ const { repairFixMessage, procedureLines, reserveCommand, waitCommand, longCheckRuleLines, resultPath, agentRules, preflightPrompt, integratePrompt, resolveConflictsPrompt, postIntegrateFixPrompt, postIntegrateReReviewPrompt, postIntegratePrompt, e2ePrompt, finalReviewPrompt, combinedFinalReviewPrompt, finalFixPrompt, finalReReviewPrompt, - verifyPrompt, verifyRepairPrompt, adjudicatorPrompt, + verifyPrompt, verifyRepairPrompt, adjudicatorPrompt, declaredFingerprint, declaredFiles, startCommand, taskChecksLines, + shellQuote, } = await loadHelpers([ 'implementPrompt', 'reviewPrompt', 'reReviewPrompt', 'fixPrompt', 'laneChecks', 'wideChecks', 'taskChecksCommand', 'taskEvidenceLine', 'fixMessagesLines', 'fallbackImplementer', 'fallbackFixer', 'laneWhere', 'featureWhere', @@ -21,7 +26,9 @@ const { 'resultPath', 'agentRules', 'preflightPrompt', 'integratePrompt', 'resolveConflictsPrompt', 'postIntegrateFixPrompt', 'postIntegrateReReviewPrompt', 'postIntegratePrompt', 'e2ePrompt', 'finalReviewPrompt', 'combinedFinalReviewPrompt', 'finalFixPrompt', 'finalReReviewPrompt', 'verifyPrompt', 'verifyRepairPrompt', 'adjudicatorPrompt', + 'declaredFingerprint', 'declaredFiles', 'startCommand', 'taskChecksLines', 'shellQuote', ]); +const fp = declaredFingerprint; function task(id, extra = {}) { return { id, title: `Task ${id}`, files: [`src/${id}.js`, `tests/${id}.test.mjs`], tier: 'standard', security: false, ...extra }; @@ -69,11 +76,14 @@ test('an implement prompt carries the task-checks call with the entry', () => { assert.match(p, /scripts\/task-checks' '\/work\/wt\/lane-alpha'/); for (const part of [ `--base '${H('base')}'`, "--ledger '/work/ledger'", "--unit 'T2'", "--lane 'alpha'", "--task 'T2'", - "--declared 'src/T2.js'", "--declared 'tests/T2.test.mjs'", "--widen 'package.json'", "--widen 'tests/harness.mjs'", - "--project-test 'npm test'", "--entry 'T2'", "--targeted 'test' 'node --test tests/t2.test.mjs'", - "--targeted 'lint' 'npm run lint'", "--lane-cmd 'test' 'npm test'", "--lane-cmd 'lint' 'npm run lint'", - "--wide-cmd 'test' 'npm test'", + `--declared-from-owners '${fp(['src/T2.js', 'tests/T2.test.mjs'])}'`, "--widen 'package.json'", + "--widen 'tests/harness.mjs'", "--project-test 'npm test'", "--entry 'T2'", + "--targeted 'test' 'node --test tests/t2.test.mjs'", "--targeted 'lint' 'npm run lint'", + "--lane-cmd 'test' 'npm test'", "--lane-cmd 'lint' 'npm run lint'", '--wide-as-lane', ]) assert.ok(p.includes(part), part); + // No --declared list (the prompt's owners.json sentence names the option, unquoted). + assert.ok(!/--declared '/.test(p), p); + assert.ok(!p.includes('--wide-cmd'), p); const text = flat(p); assert.ok(text.includes('Before you commit, run the task checks once, with the procedure below.')); assert.ok(text.includes('While iterating, run only the focused tests for what you change.')); @@ -81,21 +91,23 @@ test('an implement prompt carries the task-checks call with the entry', () => { assert.ok(!p.includes('- every check at once')); }); -test('a task without an entry gets no --entry and still gets --lane-cmd and --wide-cmd', () => { +test('a task without an entry gets no --entry and still gets --lane-cmd and --wide-as-lane', () => { const m = manifest(); const cmd = taskChecksCommand(m, t(m, 'T4'), laneWhere(m, lane(m, 'beta')), H('b')); assert.ok(!cmd.includes('--entry') && !cmd.includes('--targeted'), cmd); - assert.ok(cmd.includes("--lane-cmd 'test' 'npm test'") && cmd.includes("--wide-cmd 'test' 'npm test'"), cmd); + assert.ok(cmd.includes("--lane-cmd 'test' 'npm test'") && cmd.includes('--wide-as-lane'), cmd); }); -test('a batch unit uses its last task\'s entry and passes every --task and --declared', () => { +test('a batch unit uses its last task\'s entry and passes every --task and the fingerprint of every file', () => { const m = manifest(); const unit = batchUnit([t(m, 'T2'), t(m, 'T3')]); const cmd = taskChecksCommand(m, unit, laneWhere(m, lane(m, 'alpha')), H('b')); assert.ok(cmd.includes("--unit 'T2-T3'"), cmd); assert.ok(cmd.includes("--task 'T2' --task 'T3'"), cmd); assert.ok(cmd.includes("--entry 'T3'") && cmd.includes("'node --test tests/t3.test.mjs'"), cmd); - for (const f of ['src/T2.js', 'tests/T2.test.mjs', 'src/T3.js', 'tests/T3.test.mjs']) assert.ok(cmd.includes(`--declared '${f}'`), f); + const files = ['src/T2.js', 'tests/T2.test.mjs', 'src/T3.js', 'tests/T3.test.mjs']; + assert.ok(cmd.includes(`--declared-from-owners '${fp(files)}'`), cmd); + assert.ok(!cmd.includes('--declared '), cmd); }); test('prelude and join tasks get project-wide --wide-cmd lists', () => { @@ -483,3 +495,176 @@ test('the result placeholder is single-quoted in every start command', () => { const all = Object.values(everyPrompt()).join('\n'); assert.ok(all.includes("--evidence '

'") && all.includes("--out '

'")); }); + +// ---- The declared-files fingerprint and --wide-as-lane (1.6.6, spec 4.5) ---- + +const SCRIPTS = join(SKILL_DIR, 'scripts'); +const GIT_ENV = { + GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_NOSYSTEM: '1', + GIT_AUTHOR_NAME: 'T', GIT_AUTHOR_EMAIL: 't@example.invalid', + GIT_COMMITTER_NAME: 'T', GIT_COMMITTER_EMAIL: 't@example.invalid', +}; +const ENV = { ...process.env, ...GIT_ENV }; +const fwd = (p) => p.replace(/\\/g, '/'); +const fpOf = (cmd) => (cmd.match(/--declared-from-owners '([^']+)'/) || [])[1]; + +// The fingerprint of a declared list, from Python _owners.fingerprint. The +// list goes over as ASCII JSON (\u escapes), so argv encoding plays no part. +function pyFingerprint(paths) { + const code = [ + 'import json, sys', + 'sys.dont_write_bytecode = True', + `sys.path.insert(0, ${JSON.stringify(SCRIPTS)})`, + 'import _owners', + 'print(_owners.fingerprint(json.loads(sys.argv[1])))', + ].join('\n'); + const json = JSON.stringify(paths).replace(/[\u0080-\uffff]/g, (c) => `\\u${c.charCodeAt(0).toString(16).padStart(4, '0')}`); + const res = spawnSync('python3', ['-B', '-c', code, json], { encoding: 'utf8', env: ENV }); + assert.equal(res.status, 0, res.stderr); + return res.stdout.trim(); +} + +test('the task-checks and start-task commands do not grow with the Files list', () => { + const m = manifest(); + const where = laneWhere(m, lane(m, 'alpha')); + const one = task('T2', { files: ['src/f0.js'] }); + const many = task('T2', { files: Array.from({ length: 200 }, (_, i) => `src/f${i}.js`) }); + const checks = [one, many].map((x) => taskChecksCommand(m, x, where, H('b'))); + assert.ok(Math.abs(checks[0].length - checks[1].length) <= 2, `${checks[0].length} vs ${checks[1].length}`); + const pkg = { pkg: { base: H('b'), head: H('h') } }; + const starts = [one, many].map((x) => startCommand(m, x, where, pkg)); + assert.ok(starts[1].includes('--declared-from-owners'), starts[1]); + assert.ok(Math.abs(starts[0].length - starts[1].length) <= 2, `${starts[0].length} vs ${starts[1].length}`); +}); + +test('an e1a-sized unit\'s task-checks command is under 4,000 characters', () => { + // A command of exactly n characters: its group, its number, then padding. + const sized = (group, i, n) => { const head = `${group}-runner --step ${i} `; return head + 'x'.repeat(n - head.length); }; + const groups = ['test', 'lint', 'build']; + const commands = { setup: [] }; + for (const g of groups) commands[g] = [0, 1, 2].map((i) => sized(g, i, 133)); + // PF-6: the entry sets test, lint and build itself, about 500 characters in all. + const entry = {}; + for (const g of groups) entry[g] = [sized(`${g}-t`, 0, 166)]; + const files = Array.from({ length: 60 }, (_, i) => `src/Feature/Area${String(i).padStart(2, '0')}/` + 'y'.repeat(40) + '.cs'); + const m = manifest({ commands, task_checks: { T2: entry } }); + const cmd = taskChecksCommand(m, task('T2', { files }), laneWhere(m, lane(m, 'alpha')), H('b')); + assert.ok(Object.values(commands).flat().join('').length >= 1150, 'about 1,200 characters of project commands'); + assert.ok(cmd.length < 4000, String(cmd.length)); + assert.ok(cmd.includes('--wide-as-lane'), cmd); + assert.ok(cmd.includes(`--declared-from-owners '${fp(files)}'`), cmd); +}); + +test('--wide-as-lane only when the wide list equals the lane list', () => { + const where = (m) => laneWhere(m, lane(m, 'alpha')); + const plain = manifest(); + assert.ok(taskChecksCommand(plain, t(plain, 'T2'), where(plain), H('b')).includes('--wide-as-lane')); + const own = manifest({ lane_commands: { alpha: { test: ['npm test -- alpha'] } } }); + const ownCmd = taskChecksCommand(own, t(own, 'T2'), where(own), H('b')); + assert.ok(!ownCmd.includes('--wide-as-lane'), ownCmd); + assert.ok(ownCmd.includes("--wide-cmd 'test' 'npm test' --wide-cmd 'lint' 'npm run lint' --wide-cmd 'test' 'npm test -- alpha'"), + ownCmd); + // A lane with its own setup widens to its own checks: the wide list is the lane list. + const setup = manifest({ lane_commands: { alpha: { setup: ['uv sync'], test: ['pytest'] } } }); + const setupCmd = taskChecksCommand(setup, t(setup, 'T2'), where(setup), H('b')); + assert.ok(setupCmd.includes('--wide-as-lane') && !setupCmd.includes('--wide-cmd'), setupCmd); +}); + +test('the engine\'s fingerprint matches the helpers\' for a unit', () => { + const m = manifest(); + const unit = batchUnit([ + task('T2', { files: ['src/a.js', 'docs/caf\u00e9 menu.md'] }), + task('T3', { files: ['docs/caf\u00e9 menu.md', "notes/it's \u{1d11e}.txt", 'src/a.js'] }), + ]); + const cmd = taskChecksCommand(m, unit, laneWhere(m, lane(m, 'alpha')), H('b')); + assert.deepEqual(declaredFiles(unit), ['src/a.js', 'docs/caf\u00e9 menu.md', "notes/it's \u{1d11e}.txt"]); + assert.equal(fpOf(cmd), pyFingerprint(declaredFiles(unit))); + assert.equal(fpOf(cmd), fp(declaredFiles(unit))); +}); + +test('taskChecksLines tells the implementer to report blocked on an owners.json error', () => { + const m = manifest(); + const text = flat(taskChecksLines(m, t(m, 'T2'), laneWhere(m, lane(m, 'alpha')), H('b')).join('\n')); + assert.ok(text.includes('If the call fails with an error that names owners.json, report blocked with its message; ' + + 'do not run setup or pass --declared yourself.'), text); +}); + +const TMP = tempDir('pl-check-policy-'); +after(() => rmSync(TMP, { recursive: true, force: true })); + +function git(dir, ...args) { + const res = spawnSync('git', ['-C', dir, ...args], { encoding: 'utf8', env: ENV }); + assert.equal(res.status, 0, res.stderr); + return res.stdout.trim(); +} + +function python(script, ...args) { + const res = spawnSync('python3', ['-B', join(SCRIPTS, script), ...args], { encoding: 'utf8', env: ENV }); + assert.equal(res.status, 0, `${script}: ${res.stderr}`); + return res.stdout.trim(); +} + +const policyOf = (record) => { const { started, ...rest } = record.policy; return rest; }; + +test('the engine-built task-checks command gives the same policy on real git', () => { + const dir = join(TMP, 'e2e', 'repo'); + const ledger = join(TMP, 'e2e', 'ledger'); + mkdirSync(ledger, { recursive: true }); + for (const f of ['src/a.js', 'src/b.js', 'docs/other.md']) { + mkdirSync(join(dir, f, '..'), { recursive: true }); + writeFileSync(join(dir, f), '1\n'); + } + git(dir, 'init', '-q', '-b', 'main'); + git(dir, 'add', '-A'); + git(dir, 'commit', '-q', '-m', 'base'); + const base = git(dir, 'rev-parse', 'HEAD'); + const unit = task('U1', { files: ['src/a.js', 'src/b.js'] }); + const m = manifest({ + repo: { + mode: 'git', root: fwd(dir), git_dir: null, base_ref: 'main', branch: 'pl/run-1', + worktree_root: fwd(join(TMP, 'e2e')), ledger_dir: fwd(ledger), + }, + commands: { setup: [], test: ['node --version'], lint: ['node -v'], build: ['git --version'] }, + prelude: [], + lanes: [{ id: 'alpha', name: 'Lane alpha', tasks: [unit] }], + join: [], + task_checks: { U1: { test: ['node --version'] } }, + widen_paths: [], + skill_dir: fwd(SKILL_DIR), + }); + // owners.json as setup writes it. + const manifestFile = join(TMP, 'e2e', 'manifest.json'); + writeFileSync(manifestFile, JSON.stringify(m, null, 2)); + python('ledger', 'write-owners', fwd(ledger), manifestFile); + // One declared file and one undeclared file change. + writeFileSync(join(dir, 'src/a.js'), '2\n'); + writeFileSync(join(dir, 'docs/other.md'), '2\n'); + git(dir, 'commit', '-q', '-am', 'change'); + const head = git(dir, 'rev-parse', 'HEAD'); + const where = { dir: fwd(dir), branch: 'pl-run-1-alpha', lane: 'alpha' }; + const cmd = taskChecksCommand(m, unit, where, base); + assert.ok(cmd.includes('--declared-from-owners') && cmd.includes('--wide-as-lane'), cmd); + // The same call in the 1.6.5 form: the explicit list and the lane pairs. + const old = cmd + .split(`--declared-from-owners '${fpOf(cmd)}'`).join(unit.files.map((f) => `--declared ${shellQuote(f)}`).join(' ')) + .split('--wide-as-lane').join(laneChecks(m, 'alpha') + .map((c) => `--wide-cmd ${shellQuote(c.group)} ${shellQuote(c.command)}`).join(' ')); + assert.ok(!old.includes('--declared-from-owners') && !old.includes('--wide-as-lane'), old); + const records = [cmd, old].map((call) => { + const file = python('await-checks', 'reserve', '--ledger', fwd(ledger), '--name', 'task-U1'); + const res = spawnSync(BASH, ['-c', call.split(resultPath()).join(shellQuote(file))], { encoding: 'utf8', env: ENV }); + assert.equal(res.status, 0, `${res.stdout}\n${res.stderr}`); + return JSON.parse(readFileSync(file, 'utf8')); + }); + assert.ok(!records[0].error, records[0].error); + const outside = records.map((r) => r.policy.reasons.find((x) => x.code === 'outside_files')); + assert.ok(outside[0], JSON.stringify(records[0].policy)); + assert.match(outside[0].detail, /docs\/other\.md/); + assert.deepEqual(outside[0], outside[1]); + assert.deepEqual(policyOf(records[0]), policyOf(records[1])); + // The review's scope check uses the same fingerprint. + const start = startCommand(m, unit, where, { pkg: { base, head } }); + const scope = start.match(/--scope '[^']+' '[^']+' --declared-from-owners '([^']+)' --owners /); + assert.ok(scope, start); + assert.equal(scope[1], fpOf(cmd)); +}); diff --git a/tests/engine.test.mjs b/tests/engine.test.mjs index 11ab8f4..1547e81 100644 --- a/tests/engine.test.mjs +++ b/tests/engine.test.mjs @@ -6,13 +6,14 @@ const { runTask, runLane, runLanes, implementPrompt, reviewPrompt, fixPrompt, reReviewPrompt, ledgerCommand, implementSchema, implementResultText, finalFixSchema, reviewSettings, - finalFixPrompt, agentRules, adjudicatorPrompt, finalChecks, finalSetup, + finalFixPrompt, agentRules, adjudicatorPrompt, finalChecks, finalSetup, declaredFingerprint, } = await loadHelpers([ 'runTask', 'runLane', 'runLanes', 'implementPrompt', 'reviewPrompt', 'fixPrompt', 'reReviewPrompt', 'ledgerCommand', 'implementSchema', 'implementResultText', 'finalFixSchema', 'reviewSettings', - 'finalFixPrompt', 'agentRules', 'adjudicatorPrompt', 'finalChecks', 'finalSetup', + 'finalFixPrompt', 'agentRules', 'adjudicatorPrompt', 'finalChecks', 'finalSetup', 'declaredFingerprint', ]); +const fp = declaredFingerprint; function task(id, extra = {}) { return { id, title: `Task ${id}`, files: [`src/${id}.js`], tier: 'standard', security: false, ...extra }; @@ -533,6 +534,8 @@ test('lane_commands override the project commands for that lane', () => { // test command is still offered as the widened list (spec 4.2). assert.ok(text.includes("--lane-cmd 'test' 'uv run pytest -q'")); assert.ok(text.includes("--wide-cmd 'test' 'npm test'")); + // 1.6.6: the wide list is longer than the lane list, so the pairs stay. + assert.ok(!text.includes('--wide-as-lane')); }); // ---- The final check inventory (1.4.0, F3) ---- @@ -1472,12 +1475,12 @@ test('reviews get the files their range changes outside the Files list, and judg [reviewPrompt(m, task('T2'), WHERE, H('b0'), H('h1')), H('b0'), H('h1')], [reReviewPrompt(m, task('T2'), WHERE, H('h1'), H('h2'), [finding('x')]), H('h1'), H('h2')], ]) { - assert.ok(text.includes(`--scope '${base}' '${head}' --declared 'src/T2.js'`), text); + assert.ok(text.includes(`--scope '${base}' '${head}' --declared-from-owners '${fp(['src/T2.js'])}' --owners `), text); assert.ok(text.includes("files changed outside the task's Files list"), text); assert.ok(/a change the task did not need[^.]*is an important finding/.test(text.replace(/\n/g, ' ')), text); } const batch = reviewPrompt(m, unit, WHERE, H('b0'), H('h1')); - assert.ok(batch.includes("--declared 'src/T2.js' --declared 'docs/a.md' --declared 'src/T3.js'"), batch); + assert.ok(batch.includes(`--declared-from-owners '${fp(['src/T2.js', 'docs/a.md', 'src/T3.js'])}'`), batch); // Without superpowers the scope list still comes from start-task. assert.ok(reviewPrompt(manifest({ sp_dir: null }), task('T2'), WHERE, H('b0'), H('h1')).includes(`--scope '${H('b0')}' '${H('h1')}'`)); // A task with no Files list has nothing to compare against. From 3db372d367b559588e87da823483fdcf65bc6f93 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:01:17 -0700 Subject: [PATCH 08/10] feat: the dry run warns when a check command nears the Bash limit --- run.workflow.js | 68 ++++++++++++++++++++++++++++++++++++----- src/plan.js | 68 ++++++++++++++++++++++++++++++++++++----- tests/dryrun.test.mjs | 71 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 16 deletions(-) diff --git a/run.workflow.js b/run.workflow.js index d921001..69fcbda 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1086,21 +1086,70 @@ function repairReserve(m) { return 7 + (hooks.e2e ? 4 : 0) + (hooks.post_integrate && m.profile !== 'lite' ? 2 : 0); } -// The dry run's checks summary (spec 4.9). Units are the units with work, -// built as planAgents builds them. -function checksSummary(m) { - const withEntry = []; - const withoutEntry = []; +// The units with work, built as planAgents builds them, each with its ledger +// lane (a lane id, prelude or join), in run order: [{unit, laneId}]. +function unitsWithWork(m) { + const out = []; const groups = [['prelude', m.prelude], ...m.lanes.map((l) => [l.id, l.tasks]), ['join', m.join]]; for (const [laneId, tasks] of groups) { for (let i = 0; i < tasks.length;) { const group = batchGroup(m, tasks, i); i += group.length; if (taskState(m, group[0].id) === 'skip') continue; - const unit = group.length > 1 ? batchUnit(group) : group[0]; - (taskChecksEntry(m, unit, laneId) === null ? withoutEntry : withEntry).push(unit.id); + out.push({ unit: group.length > 1 ? batchUnit(group) : group[0], laneId }); } } + return out; +} + +// The length of each check command the run would hand an agent (1.6.6 spec +// 4.7), as [{what, length}]: each unit's task-checks command, then the +// integration's run-checks call; a unit or integration with no check +// command is left out. Each is measured with the result file it would get +// (/checks/task-.1.json, integrate.1.json) in place of +// resultPath() and a 40-character base; a join unit, whose wave may place it +// in a lane worktree, is measured in the longest of the feature checkout and +// the lane worktree dirs. +function checkCommandLengths(m) { + const out = []; + const placed = (cmd, name) => cmd.replace(resultPath(), shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); + const base = '0'.repeat(40); + const joinDir = [featureDir(m), ...m.lanes.map((l) => laneWhere(m, l).dir)] + .reduce((a, b) => (b.length > a.length ? b : a)); + for (const { unit, laneId } of unitsWithWork(m)) { + let where; + if (laneId === 'prelude') where = featureWhere(m, 'prelude'); + else if (laneId === 'join') where = { dir: joinDir, lane: 'join' }; + else where = laneWhere(m, m.lanes.find((l) => l.id === laneId)); + const cmd = taskChecksCommand(m, unit, where, base); + if (cmd !== null) out.push({ what: `${unit.id}'s task-checks command`, length: placed(cmd, `task-${unit.id}`).length }); + } + const integration = runChecksCommand(m, featureDir(m), finalChecks(m), resultPath(), 'integrate'); + if (integration !== null) { + out.push({ what: 'the integration checks command', length: placed(integration, 'integrate').length }); + } + return out; +} + +// The dry-run warning for a check command near the Bash tool's limit on +// Windows (about 7,000 characters; 1.6.6 spec 4.7): [] when every length is +// at most 6,000, else one warning, for the longest. +function longCommandWarnings(lengths) { + const over = lengths.filter((l) => l.length > 6000); + if (over.length === 0) return []; + const longest = over.reduce((a, b) => (b.length > a.length ? b : a)); + return [`${longest.what} is about ${longest.length} characters; the Bash tool on Windows fails at about 7,000:` + + " shorten the project's check commands, for example with a wrapper script"]; +} + +// The dry run's checks summary (spec 4.9). Units are the units with work, +// built as planAgents builds them. +function checksSummary(m) { + const withEntry = []; + const withoutEntry = []; + for (const { unit, laneId } of unitsWithWork(m)) { + (taskChecksEntry(m, unit, laneId) === null ? withoutEntry : withEntry).push(unit.id); + } const project = (group, command) => commandGroup(m, null, group).includes(command); const added = finalChecks(m).filter((c) => !project(c.group, c.command)).map((c) => ({ lane: m.lanes.find((l) => commandGroup(m, l.id, c.group).includes(c.command)).id, @@ -1117,7 +1166,10 @@ function checksSummary(m) { widen_paths_count: widen.length, repair_limit: repairLimit(m), repairs_spent: m.repairs_spent || 0, - warnings: hasEntries && widen.length === 0 ? ['changes to shared files will not widen task checks'] : [], + warnings: [ + ...(hasEntries && widen.length === 0 ? ['changes to shared files will not widen task checks'] : []), + ...longCommandWarnings(checkCommandLengths(m)), + ], }; } diff --git a/src/plan.js b/src/plan.js index 26eb3aa..85421fa 100644 --- a/src/plan.js +++ b/src/plan.js @@ -273,21 +273,70 @@ function repairReserve(m) { return 7 + (hooks.e2e ? 4 : 0) + (hooks.post_integrate && m.profile !== 'lite' ? 2 : 0); } -// The dry run's checks summary (spec 4.9). Units are the units with work, -// built as planAgents builds them. -function checksSummary(m) { - const withEntry = []; - const withoutEntry = []; +// The units with work, built as planAgents builds them, each with its ledger +// lane (a lane id, prelude or join), in run order: [{unit, laneId}]. +function unitsWithWork(m) { + const out = []; const groups = [['prelude', m.prelude], ...m.lanes.map((l) => [l.id, l.tasks]), ['join', m.join]]; for (const [laneId, tasks] of groups) { for (let i = 0; i < tasks.length;) { const group = batchGroup(m, tasks, i); i += group.length; if (taskState(m, group[0].id) === 'skip') continue; - const unit = group.length > 1 ? batchUnit(group) : group[0]; - (taskChecksEntry(m, unit, laneId) === null ? withoutEntry : withEntry).push(unit.id); + out.push({ unit: group.length > 1 ? batchUnit(group) : group[0], laneId }); } } + return out; +} + +// The length of each check command the run would hand an agent (1.6.6 spec +// 4.7), as [{what, length}]: each unit's task-checks command, then the +// integration's run-checks call; a unit or integration with no check +// command is left out. Each is measured with the result file it would get +// (/checks/task-.1.json, integrate.1.json) in place of +// resultPath() and a 40-character base; a join unit, whose wave may place it +// in a lane worktree, is measured in the longest of the feature checkout and +// the lane worktree dirs. +function checkCommandLengths(m) { + const out = []; + const placed = (cmd, name) => cmd.replace(resultPath(), shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); + const base = '0'.repeat(40); + const joinDir = [featureDir(m), ...m.lanes.map((l) => laneWhere(m, l).dir)] + .reduce((a, b) => (b.length > a.length ? b : a)); + for (const { unit, laneId } of unitsWithWork(m)) { + let where; + if (laneId === 'prelude') where = featureWhere(m, 'prelude'); + else if (laneId === 'join') where = { dir: joinDir, lane: 'join' }; + else where = laneWhere(m, m.lanes.find((l) => l.id === laneId)); + const cmd = taskChecksCommand(m, unit, where, base); + if (cmd !== null) out.push({ what: `${unit.id}'s task-checks command`, length: placed(cmd, `task-${unit.id}`).length }); + } + const integration = runChecksCommand(m, featureDir(m), finalChecks(m), resultPath(), 'integrate'); + if (integration !== null) { + out.push({ what: 'the integration checks command', length: placed(integration, 'integrate').length }); + } + return out; +} + +// The dry-run warning for a check command near the Bash tool's limit on +// Windows (about 7,000 characters; 1.6.6 spec 4.7): [] when every length is +// at most 6,000, else one warning, for the longest. +function longCommandWarnings(lengths) { + const over = lengths.filter((l) => l.length > 6000); + if (over.length === 0) return []; + const longest = over.reduce((a, b) => (b.length > a.length ? b : a)); + return [`${longest.what} is about ${longest.length} characters; the Bash tool on Windows fails at about 7,000:` + + " shorten the project's check commands, for example with a wrapper script"]; +} + +// The dry run's checks summary (spec 4.9). Units are the units with work, +// built as planAgents builds them. +function checksSummary(m) { + const withEntry = []; + const withoutEntry = []; + for (const { unit, laneId } of unitsWithWork(m)) { + (taskChecksEntry(m, unit, laneId) === null ? withoutEntry : withEntry).push(unit.id); + } const project = (group, command) => commandGroup(m, null, group).includes(command); const added = finalChecks(m).filter((c) => !project(c.group, c.command)).map((c) => ({ lane: m.lanes.find((l) => commandGroup(m, l.id, c.group).includes(c.command)).id, @@ -304,6 +353,9 @@ function checksSummary(m) { widen_paths_count: widen.length, repair_limit: repairLimit(m), repairs_spent: m.repairs_spent || 0, - warnings: hasEntries && widen.length === 0 ? ['changes to shared files will not widen task checks'] : [], + warnings: [ + ...(hasEntries && widen.length === 0 ? ['changes to shared files will not widen task checks'] : []), + ...longCommandWarnings(checkCommandLengths(m)), + ], }; } diff --git a/tests/dryrun.test.mjs b/tests/dryrun.test.mjs index 8b6e7eb..c466f53 100644 --- a/tests/dryrun.test.mjs +++ b/tests/dryrun.test.mjs @@ -433,6 +433,77 @@ test('the summary warns when task_checks has entries and widen_paths is empty', assert.deepEqual(plain.result.checks.units_with_entry, []); }); +// ---- 1.6.6 long check commands (spec 4.7) ---- + +const { longCommandWarnings, checkCommandLengths } = await loadHelpers(['longCommandWarnings', 'checkCommandLengths']); + +test('longCommandWarnings warns only over 6,000', () => { + assert.deepEqual(longCommandWarnings([]), []); + assert.deepEqual(longCommandWarnings([{ what: 'X', length: 6000 }]), []); + assert.deepEqual(longCommandWarnings([{ what: 'X', length: 6001 }, { what: 'Y', length: 100 }]), [ + "X is about 6001 characters; the Bash tool on Windows fails at about 7,000: shorten the project's check commands, for example with a wrapper script", + ]); + // One warning, for the longest. + assert.deepEqual(longCommandWarnings([{ what: 'X', length: 6500 }, { what: 'Y', length: 7100 }]), [ + "Y is about 7100 characters; the Bash tool on Windows fails at about 7,000: shorten the project's check commands, for example with a wrapper script", + ]); +}); + +test('the dry run warns about a long task-checks command', async () => { + const pad = `npm test ${'x'.repeat(7000 - 9)}`; + assert.equal(pad.length, 7000); + const m = policyManifest({ commands: { setup: [], test: [pad], lint: [], build: [] } }); + const { result } = await dryRun(m); + assert.deepEqual(result.errors, []); + const { warnings } = result.checks; + assert.equal(warnings.length, 1, JSON.stringify(warnings)); + assert.match(warnings[0], /^(T1|T2-T3|T4|T5)'s task-checks command is about \d+ characters; the Bash tool on Windows/); + const lengths = checkCommandLengths(m); + const longest = Math.max(...lengths.map((l) => l.length)); + assert.ok(longest > 7000, String(longest)); + assert.ok(warnings[0].includes(`is about ${longest} characters`)); + assert.deepEqual(lengths.map((l) => l.what), [ + "T1's task-checks command", "T2-T3's task-checks command", "T4's task-checks command", + "T5's task-checks command", 'the integration checks command', + ]); +}); + +test('the dry run warns about a long integration command', async () => { + const long = (tag) => `${tag} ${'x'.repeat(2400 - tag.length - 1)}`; + const m = manifest({ + lanes: [ + { id: 'alpha', name: 'Lane alpha', tasks: [task('T2', ['src/a.js']), task('T3', ['src/a2.js'])] }, + { id: 'beta', name: 'Lane beta', tasks: [task('T4', ['src/b.js'])] }, + { id: 'gamma', name: 'Lane gamma', tasks: [task('T6', ['src/c.js'])] }, + ], + lane_commands: { + alpha: { test: [long('test-alpha')] }, + beta: { test: [long('test-beta')] }, + gamma: { test: [long('test-gamma')] }, + }, + }); + assert.equal(long('test-alpha').length, 2400); + const { result } = await dryRun(m); + assert.deepEqual(result.errors, []); + const { warnings } = result.checks; + assert.equal(warnings.length, 1, JSON.stringify(warnings)); + assert.match(warnings[0], /^the integration checks command is about \d+ characters; /); + const lengths = checkCommandLengths(m); + const tasks = lengths.filter((l) => l.what !== 'the integration checks command'); + assert.deepEqual(tasks.map((l) => l.what), ['T1', 'T2', 'T3', 'T4', 'T6', 'T5'].map((id) => `${id}'s task-checks command`)); + for (const l of tasks) assert.ok(l.length <= 6000, `${l.what}: ${l.length}`); + const integration = lengths.find((l) => l.what === 'the integration checks command'); + assert.ok(integration.length > 6000, String(integration.length)); +}); + +test('no check commands, no warning and no throw', async () => { + const m = manifest({ commands: { setup: [], test: [], lint: [], build: [] } }); + const { result } = await dryRun(m); + assert.deepEqual(result.errors, []); + assert.deepEqual(result.checks.warnings, []); + assert.deepEqual(checkCommandLengths(m), []); +}); + test('an invalid manifest has no checks summary', async () => { const { result } = await dryRun(manifest({ task_checks: 'x' })); assert.ok(result.errors.length > 0); From efcd61001f39cf715978542a3c3b23d8ef5f49b5 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:14:03 -0700 Subject: [PATCH 09/10] fix: address the final review findings --- reference.md | 6 ++++-- run.workflow.js | 2 +- scripts/start-task | 3 ++- src/plan.js | 2 +- tests/task-checks.test.mjs | 5 +++-- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/reference.md b/reference.md index 69fa10d..7bbe0a0 100644 --- a/reference.md +++ b/reference.md @@ -531,7 +531,8 @@ without one). The reviewer's start-task call builds its scope list from it: `--s --declared-from-owners N:HHHHHHHH --owners /owners.json` takes the `--brief` tasks' `declared` lists, in `--brief` order, each exact string once, and checks them against the engine's fingerprint (see "task-checks" in Check ownership); the owner annotations print as -with `--declared`. When the owner map is missing or unreadable, or the fingerprint differs, the +with `--declared`. When the owner map is missing or unreadable, a `--brief` task is not in it or has no +declared list, or the fingerprint differs, the scope section is the one line "(could not build the list: the run's owner map does not match this unit's Files list; judge scope from the brief)", the briefs and package are written as usual and start-task exits 0: the list is advice, and a failed start would spend a fix agent on @@ -688,7 +689,8 @@ checks. or one given with `--declared`, is a usage error (exit 2). When owners.json is missing or unreadable, a `--task` is not in it, a task has no `declared`, a path in it is absolute or leaves the project, or the fingerprint differs, task-checks selects and runs nothing, writes - its error record to the evidence file and exits 3 (`await-checks wait` then exits 3, not + its error record to the evidence file and exits 3 (without `--evidence` it exits 3 and writes + nothing; `await-checks wait` then exits 3, not `stopped`), with the error "the run's owner map (owners.json) does not match this unit's Files list (): report blocked with this message; do not run setup and do not pass --declared yourself (the session reruns setup)". The implementer's prompt says the same: "If diff --git a/run.workflow.js b/run.workflow.js index 69fcbda..dd23220 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1112,7 +1112,7 @@ function unitsWithWork(m) { // the lane worktree dirs. function checkCommandLengths(m) { const out = []; - const placed = (cmd, name) => cmd.replace(resultPath(), shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); + const placed = (cmd, name) => cmd.replace(resultPath(), () => shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); const base = '0'.repeat(40); const joinDir = [featureDir(m), ...m.lanes.map((l) => laneWhere(m, l).dir)] .reduce((a, b) => (b.length > a.length ? b : a)); diff --git a/scripts/start-task b/scripts/start-task index 4883127..075afc4 100755 --- a/scripts/start-task +++ b/scripts/start-task @@ -62,7 +62,8 @@ In order: N:HHHHHHHH. It needs --scope and --owners, does not go with --declared, and a value not of that form exits 2. FILE is read right after the --artifacts check, before any git work. - When it is missing or unreadable, or the fingerprint differs, + When it is missing or unreadable, a --brief task is not in it + or has no declared list, or the fingerprint differs, the section is the one line "(could not build the list: the run's owner map does not match this unit's Files list; judge scope from the brief)", everything else runs as before and the diff --git a/src/plan.js b/src/plan.js index 85421fa..0ad3539 100644 --- a/src/plan.js +++ b/src/plan.js @@ -299,7 +299,7 @@ function unitsWithWork(m) { // the lane worktree dirs. function checkCommandLengths(m) { const out = []; - const placed = (cmd, name) => cmd.replace(resultPath(), shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); + const placed = (cmd, name) => cmd.replace(resultPath(), () => shellQuote(`${m.repo.ledger_dir}/checks/${name}.1.json`)); const base = '0'.repeat(40); const joinDir = [featureDir(m), ...m.lanes.map((l) => laneWhere(m, l).dir)] .reduce((a, b) => (b.length > a.length ? b : a)); diff --git a/tests/task-checks.test.mjs b/tests/task-checks.test.mjs index dcc5ebc..3465c09 100644 --- a/tests/task-checks.test.mjs +++ b/tests/task-checks.test.mjs @@ -592,7 +592,7 @@ function writerAlive(file) { 'import _await', 'print(json.dumps(_await.writer_alive(sys.argv[1])))', ].join('\n'); - const res = spawnSync('python3', ['-c', code, file], { encoding: 'utf8', env: ENV }); + const res = spawnSync('python3', ['-B', '-c', code, file], { encoding: 'utf8', env: ENV }); assert.equal(res.status, 0, res.stderr); return JSON.parse(res.stdout); } @@ -749,7 +749,7 @@ function fp(paths) { 'import _owners', 'print(_owners.fingerprint(json.loads(sys.argv[1])))', ].join('\n'); - const res = spawnSync('python3', ['-c', code, JSON.stringify(paths)], { encoding: 'utf8', env: ENV }); + const res = spawnSync('python3', ['-B', '-c', code, JSON.stringify(paths)], { encoding: 'utf8', env: ENV }); assert.equal(res.status, 0, res.stderr); return res.stdout.trim(); } @@ -812,6 +812,7 @@ test('an owner-map mismatch writes the error record and exits 3', () => { ['an unknown --task', () => ownerMap(r, {}, { U1: ['src/a.js'] }), good, { tasks: ['U1', 'U7'] }], ['a task without declared', () => ownerMap(r, {}, {}), good, {}], ['a declared absolute path', () => ownerMap(r, {}, { U1: ['/abs/x.js'] }), fp(['/abs/x.js']), {}], + ['a declared path leaving the project', () => ownerMap(r, {}, { U1: ['../x.js'] }), fp(['../x.js']), {}], ]) { setUp(); const file = reserve(r); From 4b34721e040f2725468233c84f1ca1d5a9f40009 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:29:54 -0700 Subject: [PATCH 10/10] chore: release 1.6.6 --- VERSION | 2 +- run.workflow.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index 9f05f9f..ec70f75 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.5 +1.6.6 diff --git a/run.workflow.js b/run.workflow.js index dd23220..c85f7b3 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1,6 +1,6 @@ export const meta = { name: 'parallel-lanes', - description: 'parallel-lanes v1.6.5: Execute an implementation plan as parallel lanes of tasks with per-task review, then integrate.', + description: 'parallel-lanes v1.6.6: 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' }, @@ -6185,7 +6185,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.5'; +const VERSION = '1.6.6'; log(`parallel-lanes v${VERSION} loaded`); if (args !== null && typeof args === 'object' && args.dry_run === true) {