Fixes from the second contributor report

- Static gate: a file main already holds may grow 128 KB beyond its size on
  main (Unit_C.cpp is 796 KB, Player_C.cpp 491 KB: no edit to them could
  pass the 256 KB limit); only what a submission adds counts toward the total.
- A peer-confirmed function in another unit may move closer to the original
  (a header layout fix); only moving away from it is a regression.
- A function main no longer annotates anywhere (fn_07162, removed from stpl.h
  in 0f9ebdb4 while its record still said exact) is not "removed" by every
  later submission of the unit it was recorded at.
- A library function main already annotates in a file (fn_25851 in
  BSNEncodeInterface.h) no longer refuses every edit of that file; adding
  such an annotation still is refused.
- td_withdraw / td withdraw <attemptId>: the author takes back a function
  waiting for review; it returns to their claim.
- td_submit says when held functions have no changed code in the working
  tree, instead of "you hold no tasks".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Saty 2026-10-22 12:37:10 +02:00
commit f542ad5cb2
14 changed files with 165 additions and 14 deletions

View file

@ -40,6 +40,7 @@ Work (a region: a source file or class, done as a whole)
td submit [taskId...] [--notes T] submit everything you hold (or these tasks) as one batch
td batch <batchId> state of a submitted batch, per function
td result <attemptId> show a verification result
td withdraw <attemptId> take back a function waiting for review, to fix it
td heartbeat [taskId] | td release <taskId> extend your leases (all without an id)
td config [ghidraHome|server <value>] show or change workspace settings
td symbol <id|name|0xaddr> look up a symbol
@ -171,6 +172,8 @@ connect your AI with: td connect --workspace "${ws.dir}"`);
return print(await client.get(`/api/batches/${args[0]}`));
case 'release':
return print(await client.post(`/api/tasks/${args[0]}/release`));
case 'withdraw':
return print(await client.post(`/api/attempts/${args[0]}/withdraw`));
case 'toolchain':
if (args[0] === 'install') return print(await ops.toolchainInstall(ws));
throw new Error('usage: td toolchain install');

View file

@ -68,6 +68,12 @@ const TOOLS = [
inputSchema: { type: 'object', properties: { taskId: int('task id') }, required: ['taskId'] },
run: async (ws, a) => ops.api(ws).post(`/api/tasks/${a.taskId}/release`),
},
{
name: 'td_withdraw',
description: 'Take back one of your submitted functions that is waiting for peer or human review (you found a mistake): it leaves the review queue and returns to your claim, to fix and submit again. The attempt id is in td_result and in the batch status.',
inputSchema: { type: 'object', properties: { attemptId: int('attempt id') }, required: ['attemptId'] },
run: async (ws, a) => ops.api(ws).post(`/api/attempts/${a.attemptId}/withdraw`),
},
{
name: 'td_decompile',
description: 'Ghidra decompiler output for a function in the original executable (by symbol ID, name or hex address). A starting point only; the result must be rewritten into matching C++.',

View file

@ -252,12 +252,20 @@ export async function submit(ws, taskIds = [], { files, notes, agentMeta, confir
allowed = [...new Set([...allowed, ...(me.regionAllowedPaths ?? [])])];
}
}
if (!ids.length && me.leases.length) {
// Held, but none of them differs from HEAD in the working tree (committed locally, edited in
// another clone, or written in a file outside the held ones' paths).
throw new Error(
`none of the ${me.leases.length} functions you hold has changed code in this working tree compared with HEAD (${(me.leasedAllowedPaths ?? []).slice(0, 5).join(', ')}). ` +
'Leave your work uncommitted in this clone, or name the tasks: td_submit taskIds: [...]',
);
}
if (!ids.length) throw new Error('you hold no tasks; claim a region first (td_regions, then td_claim_region)');
const candidates = files?.length ? files : ws.changedFiles().filter((f) => matchesAny(f, allowed));
if (!candidates.length) throw new Error(`no changed files match the allowed paths of your tasks (${allowed.join(', ')})`);
const contents = {};
for (const f of candidates) contents[f.replace(/\\/g, '/')] = fs.readFileSync(path.join(ws.dir, f), 'utf8');
const errors = lintSubmission(contents, { allowedPaths: allowed, limits: ws.project.lintLimits, allowedPragmas: ws.project.allowedPragmas });
const errors = lintSubmission(contents, { allowedPaths: allowed, limits: ws.project.lintLimits, allowedPragmas: ws.project.allowedPragmas, baseSize: (f) => ws.size(f) });
if (errors.length) {
const e = new Error('submission fails the static gate:\n' + errors.map((x) => ` ${x.file ?? ''}${x.line ? ':' + x.line : ''} ${x.message}`).join('\n'));
e.details = errors;

View file

@ -97,6 +97,15 @@ export class Workspace {
return this.git('rev-parse', 'HEAD');
}
/** Size in bytes of a file as HEAD has it, or null if HEAD does not have it. */
size(file) {
try {
return Number(this.git('cat-file', '-s', `HEAD:${file}`));
} catch {
return null;
}
}
/** Files changed relative to HEAD (modified, added, untracked). */
changedFiles() {
// Untrimmed: each porcelain line starts with a two-column status that may begin with a space.

View file

@ -713,6 +713,11 @@ export function createApp(ctx) {
return null;
}
router.on('POST', '/api/attempts/:id/withdraw', ({ req, params }) => {
const user = auth(req);
return tasks.withdraw(user, intParam(params.id));
});
router.on('GET', '/api/attempts/:id', ({ req, params }) => {
const user = auth(req);
const a = store.get('SELECT a.*, u.name AS author FROM attempts a JOIN users u ON u.id = a.user_id WHERE a.id = ?', intParam(params.id));
@ -812,7 +817,7 @@ export function createApp(ctx) {
if (!body.files || typeof body.files !== 'object' || Array.isArray(body.files)) throw new HttpError(400, 'files must be an object of path -> content');
body.files = lf(body.files);
const allowed = [...project.sourceDirs.map((d) => `${d}/**`)];
const lintErrors = lintSubmission(body.files, { allowedPaths: allowed, limits: project.lintLimits, allowedPragmas: project.allowedPragmas });
const lintErrors = lintSubmission(body.files, { allowedPaths: allowed, limits: project.lintLimits, allowedPragmas: project.allowedPragmas, baseSize: (f) => repo.size('HEAD', f) });
if (lintErrors.length) return { gate: 'rejected', lintErrors };
if (!repo.isAncestor(body.baseCommit)) throw new HttpError(409, 'your base commit is not part of main; run td sync');
const textErrors = ctx.verifier.freeTextGate(body.files, body.baseCommit);

View file

@ -44,6 +44,15 @@ export class Repo {
}
}
/** Size in bytes of a file at a commit, or null if it does not exist there. */
size(commit, file) {
try {
return Number(this.git(['cat-file', '-s', `${commit}:${file}`]).trim());
} catch {
return null;
}
}
isClean() {
return this.git(['status', '--porcelain']).trim() === '';
}

View file

@ -5,6 +5,7 @@ import { now } from './db.js';
import { hex } from '../shared/annotations.js';
import { groupFor } from './grouping.js';
import { normalizeRepoPath } from '../shared/lint.js';
import { settleBatch } from './batches.js';
// Why a function needs no decompilation (a "no decompile" claim).
export const SKIP_CATEGORIES = {
@ -149,6 +150,32 @@ export class Tasks {
});
}
/**
* The author takes back a function waiting for review (a mistake found after submitting): it
* leaves the review queue and returns to the author's claim to fix and submit again. The rest of
* its batch goes on; a batch with nothing left on its way is returned.
*/
withdraw(user, attemptId) {
return this.store.tx(() => {
const a = this.store.get('SELECT * FROM attempts WHERE id = ?', attemptId);
if (!a || a.user_id !== user.id) throw new HttpError(404, 'no such attempt of yours');
if (!['peer_review', 'in_review'].includes(a.state)) throw new HttpError(409, `attempt ${a.id} is ${a.state}; only functions waiting for review can be withdrawn`);
this.store.run(`UPDATE attempts SET state = 'withdrawn', finished_at = ?, result_json = json_set(COALESCE(result_json, '{}'), '$.withdrawnByAuthor', json('true')) WHERE id = ?`, now(), a.id);
this.store.run('DELETE FROM peer_leases WHERE attempt_id = ?', a.id);
if (a.competing) {
// A candidate for someone else's function: that function goes on as it was.
const live = this.store.get(`SELECT 1 FROM attempts WHERE task_id = ? AND id != ? AND state IN ('queued', 'verifying', 'peer_review', 'in_review', 'approved', 'merging')`, a.task_id, a.id);
if (!live) this.store.run(`UPDATE tasks SET state = 'open', updated_at = ? WHERE id = ? AND state IN ('peer_review', 'in_review')`, now(), a.task_id);
} else {
const expires = now() + (this.config.leaseMinutes ?? 120) * 60_000;
this.store.run(`UPDATE tasks SET state = 'leased', lease_user = ?, lease_expires = ?, updated_at = ? WHERE id = ?`, user.id, expires, now(), a.task_id);
}
settleBatch(this.store, a.batch_id, `the author withdrew attempt ${a.id}`);
this.store.event('withdrawn', user.id, { attempt: a.id, task: a.task_id });
return { attempt: a.id, task: a.task_id, state: 'withdrawn', note: a.competing ? 'withdrawn' : 'withdrawn; the function is back in your claim: fix it and submit again' };
});
}
/**
* "No decompile" claims for held functions: each becomes an attempt that peer review judges
* like code (3 agreeing verdicts decide, 5 accept without a human). Accepted ones are skipped.

View file

@ -366,8 +366,10 @@ export class Verifier {
// on which other units this check compiled, not on the change.
const rank = (s) => (s === STATUS.UNRESOLVED ? RANK[STATUS.EXACT] : RANK[s]);
if (!now_ || rank(now_.status) < rank(row.status)) out.push({ id: row.symbol_id, file: row.file, was: row.status, now: now_?.status ?? 'removed' });
// A peer-confirmed function whose compiled code changes was not seen by its reviewers.
else if (row.confirmed && now_.status === STATUS.MISMATCH && Math.abs(now_.score - row.score) > 1e-4) {
// A peer-confirmed function whose compiled code moves away from the original was not seen by
// its reviewers. Closer to it is fine (a header layout fix in another unit): it keeps its
// confirmation, and the merge records the better score.
else if (row.confirmed && now_.status === STATUS.MISMATCH && now_.score < row.score - 1e-4) {
out.push({ id: row.symbol_id, file: row.file, was: 'peer-confirmed', now: `changed (score ${(row.score * 100).toFixed(1)}% -> ${(now_.score * 100).toFixed(1)}%)` });
}
}
@ -390,13 +392,17 @@ export class Verifier {
const m = /^HEAD:(.+?):FUNCTION: (\S+)$/.exec(line.trim());
if (m && !defined.has(m[2])) defined.set(m[2], m[1]);
}
} catch {
// no annotation on main for any of them (git grep exits 1): judged as before
} catch (e) {
// Exit 1: no annotation on main for any of them. Anything else: main unknown, judged strictly.
if (e.status !== 1) return found;
}
}
return found.filter((r) => {
const file = defined.get(r.id);
if (!file || isUnit(file)) return true;
// Not annotated anywhere on main (an older merge removed it; its record is stale): the
// submission cannot be what removes it.
if (!file) return r.now !== 'removed';
if (isUnit(file)) return true;
if (typeof files[file] !== 'string') return false;
return r.now !== 'removed' || !new RegExp(`FUNCTION: ${r.id}\\b`).test(files[file]);
});
@ -477,6 +483,7 @@ export class Verifier {
allowedPaths: this.tasks.allowedPathsForTasks(attempts.map((a) => a.task_id)),
limits: this.project.lintLimits,
allowedPragmas: this.project.allowedPragmas,
baseSize: (f) => this.repo.size('HEAD', f),
});
if (lintErrors.length) return finishAll('rejected', { summary: 'rejected by the static gate', lintErrors });
if (!this.repo.isAncestor(batch.base_commit)) return finishAll('rejected', { summary: `base commit ${batch.base_commit} is not part of main; sync and resubmit` });
@ -593,7 +600,14 @@ export class Verifier {
return { state: 'rejected', score: 0, feedback };
}
const annErrors = check.annotationErrors.filter((e) => fileSet.has(e.file) && e.severity !== 'warning');
// A library function main already annotates in that file (folded with project code) is not
// the submission's doing: only adding such an annotation is refused.
const onMain = new Map();
const mainHas = (e) => {
if (!onMain.has(e.file)) onMain.set(e.file, this.repo.show('HEAD', e.file) ?? '');
return new RegExp(`:\\s*${e.id}\\b`).test(onMain.get(e.file));
};
const annErrors = check.annotationErrors.filter((e) => fileSet.has(e.file) && e.severity !== 'warning' && !(e.rule === 'library' && mainHas(e)));
const annWarnings = check.annotationErrors.filter((e) => fileSet.has(e.file) && e.severity === 'warning');
for (const w of annWarnings) feedback.warnings.push(`${w.file}:${w.line} ${w.message}`);
const failed = check.units.filter((u) => !u.compiled);

View file

@ -135,7 +135,7 @@ export function lintAnnotations(annotations, symbols) {
if (symbols) {
const sym = symbols.get(a.id);
if (!sym) errors.push({ file: a.file, line: a.line, message: `${a.id} does not exist in the symbol database (request a new ID with \`td symbol new\`)` });
else if (sym.library) errors.push({ file: a.file, line: a.line, message: `${a.id} is marked as library code and must not be decompiled` });
else if (sym.library) errors.push({ file: a.file, line: a.line, id: a.id, rule: 'library', message: `${a.id} is marked as library code and must not be decompiled` });
}
}
return errors;

View file

@ -6,6 +6,8 @@ import path from 'node:path';
export const DEFAULT_LIMITS = {
maxFiles: 24,
maxFileBytes: 256 * 1024,
// A file main already holds may grow this much beyond its size there (Unit_C.cpp is 800 KB).
maxFileGrowth: 128 * 1024,
maxTotalBytes: 1024 * 1024,
maxNotesBytes: 4000,
maxInitializerNumbers: 256,
@ -137,9 +139,12 @@ export function lintSource(file, text, { allowedPragmas = DEFAULT_PRAGMAS, maxIn
/**
* Checks a submission's shape and content.
* @param {Record<string,string>} files path -> content
* @param {{allowedPaths: string[], limits?: object, allowedPragmas?: string[]}} opts
* @param {{allowedPaths: string[], limits?: object, allowedPragmas?: string[], baseSize?: (path: string) => number | null}} opts
* baseSize: the file's size on main (null when new). The size limits count what a submission
* adds: a file main already holds may be larger than maxFileBytes by maxFileGrowth over its size
* there, and only the part beyond the main size counts toward maxTotalBytes.
*/
export function lintSubmission(files, { allowedPaths, limits = {}, allowedPragmas } = {}) {
export function lintSubmission(files, { allowedPaths, limits = {}, allowedPragmas, baseSize = () => null } = {}) {
const L = { ...DEFAULT_LIMITS, ...limits };
const errors = [];
const entries = Object.entries(files ?? {});
@ -160,12 +165,14 @@ export function lintSubmission(files, { allowedPaths, limits = {}, allowedPragma
continue;
}
const bytes = Buffer.byteLength(content, 'utf8');
total += bytes;
if (bytes > L.maxFileBytes) errors.push({ file: n, message: `file too large (${bytes} bytes)` });
const base = baseSize(n) ?? 0;
total += Math.max(0, bytes - base);
const max = Math.max(L.maxFileBytes, base ? base + L.maxFileGrowth : 0);
if (bytes > max) errors.push({ file: n, message: base > L.maxFileBytes ? `file too large (${bytes} bytes; it may grow by ${L.maxFileGrowth} bytes over its ${base} on main)` : `file too large (${bytes} bytes)` });
if (content.includes('\0')) errors.push({ file: n, message: 'file contains NUL bytes' });
errors.push(...lintSource(n, content, { allowedPragmas, maxInitializerNumbers: L.maxInitializerNumbers }));
}
if (total > L.maxTotalBytes) errors.push({ file: null, message: `submission too large (${total} bytes)` });
if (total > L.maxTotalBytes) errors.push({ file: null, message: `submission too large (${total} bytes beyond what main holds)` });
return errors;
}

View file

@ -60,6 +60,9 @@ region right away; do not wait for its result.
three "different" send it back;
- compile errors, regressions or wrong references come back to you, per
function; the rest of the batch goes on.
Found a mistake in a function that waits for review? `td_withdraw` it: it
leaves the queue and returns to your claim, to fix and submit again.
7. **If something breaks a neighbour** (your change, or someone else's, makes a
function of the file stop compiling or matching), fix it in the same
submission; your region claim covers the file's functions.

View file

@ -94,6 +94,15 @@ test('large numeric initializers are rejected', () => {
assert.ok(lintSubmission({ 'src/player.cpp': blob }, allowed).some((e) => e.message.includes('initializer')));
});
test('a file main already holds may be larger than the limit, and grow a little', () => {
const big = (kb) => 'int x;\n'.repeat(Math.ceil((kb * 1024) / 7));
const tooLarge = (files, sizes) => lintSubmission(files, { ...allowed, baseSize: (f) => sizes[f] ?? null }).filter((e) => /too large/.test(e.message));
assert.equal(tooLarge({ 'src/unit.cpp': big(300) }, {}).length, 1, 'a new 300 KB file');
assert.equal(tooLarge({ 'src/unit.cpp': big(820) }, { 'src/unit.cpp': 800 * 1024 }).length, 0, 'Unit_C.cpp with an edit');
assert.equal(tooLarge({ 'src/unit.cpp': big(1000) }, { 'src/unit.cpp': 800 * 1024 }).length, 1, 'grows by 200 KB');
assert.equal(tooLarge({ 'src/unit.cpp': big(800), 'src/player.cpp': big(500) }, { 'src/unit.cpp': 800 * 1024, 'src/player.cpp': 500 * 1024 }).length, 0, 'two big files count only what they add');
});
test('preprocessed output catches asm hidden behind macros, ignoring compiler headers', () => {
const pre = ['#line 1 "C:\\\\vc\\\\include\\\\intrin.h"', 'void sys() { __asm int 3 }', '#line 1 "z:\\\\job\\\\src\\\\player.cpp"', 'void f() { __asm { nop } }'].join('\n');
const errors = lintPreprocessed(pre, (f) => f.includes('/src/'));

View file

@ -58,3 +58,31 @@ test('a stale baseline is measured again on main before it counts as a regressio
assert.equal(compiles, 1);
assert.equal(events.length, 1);
});
// A header layout fix that moves a peer-confirmed function in another unit closer to the original
// is progress; moving it away is a regression its reviewers never saw.
test('a peer-confirmed function may move closer to the original, not away from it', () => {
const v = Object.create(Verifier.prototype);
v.repo = { git: () => '' };
v.store = { all: () => [{ symbol_id: 'fn_19918', file: 'WoW/Missile_C.cpp', status: 'mismatch', score: 0.9, confirmed: 1 }] };
const at = (score) => ({ units: [{ file: 'WoW/Missile_C.cpp', compiled: true, functions: [{ id: 'fn_19918', status: 'mismatch', score }] }] });
assert.deepEqual(v.regressions(at(0.905)), []);
assert.deepEqual(v.regressions(at(0.9)), []);
assert.deepEqual(v.regressions(at(0.895)).map((r) => r.was), ['peer-confirmed']);
});
// A function whose annotation an older merge removed from main (its record still says exact)
// is not "removed" by every later submission of the unit it was recorded at.
test('a function main no longer annotates is no regression', () => {
const v = Object.create(Verifier.prototype);
const found = [
{ id: 'fn_07162', file: 'Engine/SE3.cpp', was: 'exact', now: 'removed' },
{ id: 'fn_00009', file: 'Engine/SE3.cpp', was: 'exact', now: 'removed' },
];
v.repo = { git: () => 'HEAD:Engine/SE3.cpp:FUNCTION: fn_00009\n' };
assert.deepEqual(v.withoutHeaderCopies(found, new Map(), {}).map((r) => r.id), ['fn_00009'], 'still on main: removing it counts');
v.repo = { git: () => { throw Object.assign(new Error('git grep'), { status: 1 }); } };
assert.deepEqual(v.withoutHeaderCopies(found, new Map(), {}), []);
v.repo = { git: () => { throw Object.assign(new Error('git broke'), { status: 128 }); } };
assert.equal(v.withoutHeaderCopies(found, new Map(), {}).length, 2, 'main unknown: strict');
});

View file

@ -1415,3 +1415,26 @@ test('four "same" accept a function nobody objected to; five once someone did',
assert.equal(ctx.peers.submit(revs[4], disputed, 'same', why, null).state, 'peer_review', 'four "same" against one "different": not yet');
ctx.store.run(`UPDATE attempts SET state = 'superseded' WHERE id IN (?, ?)`, clean, disputed);
});
test('an author withdraws a function waiting for review: it leaves the queue and is theirs again', async () => {
const token = await activeUser('withdrawer');
const author = ctx.accounts.userByApiToken(token);
const other = ctx.accounts.userByApiToken(await activeUser('withdrawpeer'));
const saved = ctx.store.get(`SELECT id, state, lease_user, lease_expires FROM tasks WHERE state != 'done' ORDER BY id DESC LIMIT 1`);
const task = saved.id;
const head = ctx.repo.head();
const batch = Number(ctx.store.run(`INSERT INTO batches (user_id, base_commit, files_json, state, created_at) VALUES (?, ?, '{}', 'open', ?)`, author.id, head, now()).lastInsertRowid);
const attempt = Number(ctx.store.run(`INSERT INTO attempts (task_id, user_id, base_commit, files_json, state, batch_id, created_at) VALUES (?, ?, ?, '{}', 'peer_review', ?, ?)`, task, author.id, head, batch, now()).lastInsertRowid);
ctx.store.run(`UPDATE tasks SET state = 'peer_review' WHERE id = ?`, task);
ctx.store.run('INSERT INTO peer_leases (attempt_id, reviewer_id, expires_at) VALUES (?, ?, ?)', attempt, other.id, now() + 60_000);
assert.equal((await call('POST', `/api/attempts/${attempt}/withdraw`, {}, await activeUser('notauthor'))).status, 404, 'only the author');
const r = await call('POST', `/api/attempts/${attempt}/withdraw`, {}, token);
assert.equal(r.status, 200, JSON.stringify(r.body));
assert.equal(ctx.store.get('SELECT state FROM attempts WHERE id = ?', attempt).state, 'withdrawn');
assert.deepEqual({ ...ctx.store.get('SELECT state, lease_user FROM tasks WHERE id = ?', task) }, { state: 'leased', lease_user: author.id });
assert.equal(ctx.store.get('SELECT COUNT(*) AS n FROM peer_leases WHERE attempt_id = ?', attempt).n, 0);
assert.equal(ctx.store.get('SELECT state FROM batches WHERE id = ?', batch).state, 'returned', 'nothing else on its way');
assert.equal((await call('POST', `/api/attempts/${attempt}/withdraw`, {}, token)).status, 409, 'once');
ctx.store.run('UPDATE tasks SET state = ?, lease_user = ?, lease_expires = ? WHERE id = ?', saved.state, saved.lease_user, saved.lease_expires, task);
});