dewey: row 63 (#1556) round 1 review packet, changes (test-only)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -0,0 +1,33 @@
|
||||
# Row 63 review mutants (Dewey): each swaps one exact string in the scratch copy, runs the tasks
|
||||
# verbs and tree tests, restores the file, and reports killed (tests fail) or SURVIVED.
|
||||
import subprocess, sys, os
|
||||
M = os.path.expanduser('~/dewey-scratch/r63mut')
|
||||
env = dict(os.environ, TMPDIR=os.path.expanduser('~/dewey-scratch/tmp'))
|
||||
V = 'packages/tasks/src/verbs.mjs'
|
||||
T = ['packages/tasks/tests/verbs.test.mjs', 'packages/tasks/tests/tree.test.mjs']
|
||||
ALL = ['packages/tasks/tests/*.test.mjs']
|
||||
muts = [
|
||||
('located: first relation only', V, " for (const r of list) {", " for (const r of list.slice(0, 1)) {", ALL),
|
||||
('located: a missing task passes', V, "const x = await ctx.sync('GET', `/tasks/${r.other}`);\n if (outcome(x) !== 'ok') fail(refusal(x));", "const x = await ctx.sync('GET', `/tasks/${r.other}`);\n if (outcome(x) !== 'ok') continue;", ALL),
|
||||
('relation: keeps the root project', V, "return { kind: x.kind, other: other.task, project: other.project };", "return { kind: x.kind, other: other.task, project: ctx.project };", ALL),
|
||||
('step: re-read on the root board', V, " after = await read(at);\n } catch {\n // the re-read failed too", " after = await read({ ...at, project: ctx.project, view: ctx.view });\n } catch {\n // the re-read failed too", ALL),
|
||||
('handle: final read on the root board', V, " after = await read(at);\n } catch {\n // handled below: a failed", " after = await read({ ...at, project: ctx.project, view: ctx.view });\n } catch {\n // handled below: a failed", ALL),
|
||||
('create: final read on the root board', V, " after = await read(at);\n } catch {\n // handled below, as in handle()", " after = await read({ ...at, project: ctx.project, view: ctx.view });\n } catch {\n // handled below, as in handle()", ALL),
|
||||
('create: fallback todo of the root', V, "taskFields(r.json, at.view.ids.todo)", "taskFields(r.json, ctx.view.ids.todo)", ALL),
|
||||
('create: authorize before located', V, " await located(plan.others);\n ctx.broker.authorize(cap, 'task.create', { decision: plan.decision, target: null });", " ctx.broker.authorize(cap, 'task.create', { decision: plan.decision, target: null });\n await located(plan.others);", ALL),
|
||||
('installed: outside is not-installed', V, " if (!ctx.tree.has(project)) fail('task-project');\n", "", ALL),
|
||||
('create: ref under the root', V, "const taskRef = ref(at.project, id);", "const taskRef = ref(ctx.project, id);", ALL),
|
||||
]
|
||||
only = sys.argv[1:]
|
||||
for name, f, old, new, tests in muts:
|
||||
if only and not any(o in name for o in only): continue
|
||||
p = os.path.join(M, f); src = open(p).read()
|
||||
n = src.count(old)
|
||||
if n != 1: print(f'BAD-ANCHOR ({n}) {name}', flush=True); continue
|
||||
open(p, 'w').write(src.replace(old, new))
|
||||
try:
|
||||
r = subprocess.run(['node', '--test', *tests], cwd=M, env=env, capture_output=True, text=True, timeout=600)
|
||||
finally:
|
||||
open(p, 'w').write(src)
|
||||
fails = [l for l in r.stdout.splitlines() if l.startswith('not ok')]
|
||||
print(('killed ' if r.returncode else 'SURVIVED') + f' {name}' + (f' ({len(fails)} failing)' if fails else ''), flush=True)
|
||||
@@ -0,0 +1,17 @@
|
||||
SURVIVED located: first relation only
|
||||
killed located: a missing task passes
|
||||
killed relation: keeps the root project
|
||||
SURVIVED step: re-read on the root board
|
||||
BAD-ANCHOR (2) handle: final read on the root board
|
||||
SURVIVED create: fallback todo of the root
|
||||
killed create: authorize before located
|
||||
killed installed: outside is not-installed
|
||||
killed create: ref under the root
|
||||
SURVIVED handle: final read on the root board
|
||||
SURVIVED create: final read on the root board
|
||||
-- with Dewey's four proof tests appended to verbs.test.mjs in the scratch copy --
|
||||
killed located: first relation only
|
||||
killed step: re-read on the root board
|
||||
killed handle: final read on the root board
|
||||
killed create: final read on the root board
|
||||
killed create: fallback todo of the root
|
||||
@@ -0,0 +1,59 @@
|
||||
// Dewey's four proof tests for row 63 round 1 (#1556). Appended to the end of
|
||||
// packages/tasks/tests/verbs.test.mjs in a scratch copy of candidate ec28d736, they pass
|
||||
// there and kill the five mutants that survived the packet's tests (mutants.txt). They use
|
||||
// that file's own tree(), task(), refusedWith, TREE, world, assert and FakeVikunja. Take,
|
||||
// rename or rewrite them as you like.
|
||||
const treeWith = async (t, extra) => {
|
||||
const w = await world(t, { ...TREE, ...extra });
|
||||
const child = w.fake.project('Child', w.owner, { parent: w.project });
|
||||
const board = w.fake.install(child);
|
||||
await w.adapter.start('demo');
|
||||
assert.equal(w.adapter.status()[0].state, 'ready', w.adapter.status()[0].refused);
|
||||
return { w, child, board };
|
||||
};
|
||||
test('DEWEY a child verb and a child create record their final read', async (t) => {
|
||||
const { w, child } = await tree(t);
|
||||
const pm = w.agent('pm');
|
||||
const made = await task(w, { project: child });
|
||||
assert.notEqual(w.snapshots(made.task_ref).at(-1).etag, null);
|
||||
await w.call(pm, 'task.assign', { task_ref: made.task_ref, role: 'coder' });
|
||||
assert.notEqual(w.snapshots(made.task_ref).at(-1).etag, null);
|
||||
});
|
||||
test('DEWEY a child create whose final read fails records the child todo bucket', async (t) => {
|
||||
const fake = new FakeVikunja();
|
||||
let armed = false;
|
||||
const fetch = async (url, init = {}) => {
|
||||
const res = await fake.fetch(url, init);
|
||||
if (armed && (init.method ?? 'GET') === 'POST' && /\/projects\/\d+\/tasks$/.test(new URL(url).pathname)) {
|
||||
armed = false;
|
||||
fake.fault('GET', /^\/tasks\/\d+$/, 0);
|
||||
}
|
||||
return res;
|
||||
};
|
||||
const { w, child, board } = await treeWith(t, { fake, fetch });
|
||||
w.agent('pm');
|
||||
armed = true;
|
||||
const r = await task(w, { project: child });
|
||||
assert.equal(w.snapshots(r.task_ref).at(-1).fields.bucket, board.buckets.todo);
|
||||
});
|
||||
test('DEWEY a lost answer in the child settles on the child board', async (t) => {
|
||||
const { w, child } = await tree(t);
|
||||
const pm = w.agent('pm');
|
||||
const coder = w.agent('coder');
|
||||
const { task_ref } = await task(w, { project: child });
|
||||
await w.call(pm, 'task.assign', { task_ref, role: 'coder' });
|
||||
w.fake.fault('PUT', /\/buckets\/\d+\/tasks$/, 500, { apply: true });
|
||||
await w.call(coder, 'task.update.assigned', { task_ref, state: 'in-progress' });
|
||||
assert.equal(w.events('task.state').length, 1);
|
||||
});
|
||||
test('DEWEY every related task is read, not only the first', async (t) => {
|
||||
const { w, child } = await tree(t);
|
||||
const pm = w.agent('pm');
|
||||
const root = await task(w);
|
||||
const other = await task(w);
|
||||
const id = Number(other.task_ref.split('/')[1]);
|
||||
await assert.rejects(
|
||||
w.call(pm, 'task.schedule', { task_ref: root.task_ref, relations: { add: [{ kind: 'related', task_ref: other.task_ref }, { kind: 'blocking', task_ref: `vikunja:${child}/${id}` }] } }),
|
||||
refusedWith('task-moved'),
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,40 @@
|
||||
**Dewey: changes, round 1, candidate manifest `ec28d73646c62a78c5f5af102193c867f4709f654520510e0f0db5527f4cc2bb`**
|
||||
|
||||
The code is right as far as I can find. One finding, test-only: five of the sites this row moved from the root to the task's own project have no test that fails if they go back to the root.
|
||||
|
||||
**What I checked**
|
||||
|
||||
- `build.patch` applied with `git apply --index` to a detached worktree of `08941d8b` checks 6/6. None of the six paths, and not `fake.mjs`, changed between `08941d8b` and `refactor` HEAD.
|
||||
- On the candidate, sequentially, 19:47:19-19:47:41Z, all rc=0: tasks 80, harness 59, business 60, bus 95. Same counts as your gate.
|
||||
- `ctx.tree` is always set before a verb runs. `startup` sets it, and `start()` runs reconcile (with `discover` when descendants is on) before `ready`. Verbs, `tick` and `reconcile` share the business's `serial()` queue, so the tree can't change in the middle of a verb.
|
||||
- No `ctx.view` is left in `read`, `step`, `handle` or `create`'s writes. The `ctx.project` and `ctx.tree` uses left are `installed()` and create's default project.
|
||||
- `project-not-installed` needs no registry entry. No other package lists task refusal codes.
|
||||
- The harness `project` property and its test: integer, minimum 1, optional, on `task_create` only.
|
||||
|
||||
**Finding (blocking): the child path's re-reads and fallback are untested**
|
||||
|
||||
My mutants (`mutants.py`, `mutants.txt`) ran in a scratch copy against every `packages/tasks` test. 5 of 10 survived. The `BAD-ANCHOR` line in `mutants.txt` is my first anchor for `handle`'s final read, which also matched create's. The two lines after it are the rerun with separate anchors.
|
||||
|
||||
| Mutant | What a child task would get |
|
||||
|---|---|
|
||||
| `handle`'s final read uses the root's project and view | The verb succeeds, but the snapshot carries the pre-write read's `updated` and a null etag. |
|
||||
| `create`'s final read uses the root's project and view | The create falls back to the POST answer, with a null etag. |
|
||||
| `create`'s fallback uses `ctx.view.ids.todo` | A child create whose final read fails records the root's todo bucket id as the child task's bucket. |
|
||||
| `step`'s re-read uses the root's project and view | A child write that landed but lost its answer refuses `write-uncertain` instead of settling. |
|
||||
| `located` reads only the first relation | With two or more relations, a later ref that names the wrong project gets written. This is the hole choice 2 closes. |
|
||||
|
||||
The first three fail silently, because a failed final read is tolerated by design. A regression there would leave wrong evidence and no failing test. The packet's tests always go through the root versions of these paths (lines 210-230 and 315-375).
|
||||
|
||||
Asked: tests in `verbs.test.mjs` that kill these five. `proof-tests.mjs` in this directory has four that do. Appended to `verbs.test.mjs` in the scratch copy, the file is 28/28, and all five mutants are killed (the last block of `mutants.txt`). The other five mutants were killed by your tests: a missing related task passing, `relation()` keeping the root project, create authorizing before `located`, an outside ref giving `project-not-installed`, and create's ref under the root. No code change is asked.
|
||||
|
||||
**Your five choices: I agree with all five.**
|
||||
|
||||
1. One code, `project-not-installed`, for create's watch-only, unknown and outside projects. A create's `project` names where to write, and a ref outside the tree keeps `task-project`. The distinction holds, and the README table says it.
|
||||
2. Relations read before authorize. This closes a real hole. The read tells a caller whether a task id exists in a project the sync token can see (`task-moved` or `task-not-found`). `read()` already did that for `task_ref` before this row, so nothing new is disclosed. The cost is at most 50 reads on create, or 100 on schedule (50 add plus 50 remove).
|
||||
3. `tree.test.mjs`. It is the one assertion whose behaviour the brief changes, and the comment says where the installed cases live.
|
||||
4. Create's decision binds no target. Agreed for this row. A create decision binds nothing about the task today, the title included, so the project is no worse off. A follow-up row would be the place if Sage wants decisions to name it.
|
||||
5. `fake.mjs` untouched. Nothing more was needed.
|
||||
|
||||
**Non-blocking**
|
||||
|
||||
- `packages/tasks/README.md` line 194 is 118 characters. The text added before "`task.created` is recorded" wasn't rewrapped. Rewrap it in the next round if you like.
|
||||
Reference in New Issue
Block a user