From 2974dc24b112c89ae9ee071762351d88aeb783f8 Mon Sep 17 00:00:00 2001 From: Jim-jimu <3230103159@zju.edu.cn> Date: Sun, 1 Nov 2026 00:19:41 +0800 Subject: [PATCH] fix(leases): reject directory and glob write-scope conflicts Match glob continuations beneath literal directory scopes while preserving character classes, Unicode folds and disjoint paths. Cover exclusive acquire and atomic Todo claim with File and SQLite CLI regressions, immutable retries and unchanged state after rejection. Signed-off-by: Jim-jimu <3230103159@zju.edu.cn> --- .../work_items/task_lease_acquire_decision.ts | 47 ++++++++-- .../test_canonical_lease_scopes.py | 57 ++++++++++++ .../task_lease_directory_scopes.test.ts | 89 +++++++++++++++++++ 3 files changed, 188 insertions(+), 5 deletions(-) create mode 100644 tests/control_plane_ts/task_lease_directory_scopes.test.ts diff --git a/loopx/control_plane/work_items/task_lease_acquire_decision.ts b/loopx/control_plane/work_items/task_lease_acquire_decision.ts index cf7ca25609..efef8e8f50 100644 --- a/loopx/control_plane/work_items/task_lease_acquire_decision.ts +++ b/loopx/control_plane/work_items/task_lease_acquire_decision.ts @@ -344,21 +344,55 @@ function singleCharacterMatchLengths(symbols: readonly string[], start: number): return lengths; } +/** Whether a class can supply a character after the known directory prefix. */ +function classHasMatch(body: string, negated: boolean): boolean { + const members = characterClass(body); + const intervals = [...members.ranges, ...members.literals.map(character => { + const code = character.codePointAt(0)!; + return [code, code] as const; + })].filter(([low, high]) => low <= high).sort(([left], [right]) => left - right); + if (!negated) return intervals.length > 0; + // Only inspect gaps, never enumerate a wide class. A gap character can still + // be excluded by a case-fold alias inside the class (for example A versus a). + let candidate = 0; + for (const [low, high] of [...intervals, [0x110000, 0x110000]]) { + for (; candidate < low; candidate += 1) { + if (!classMemberMatches(body, String.fromCodePoint(candidate))) return true; + } + candidate = Math.max(candidate, high + 1); + } + return false; +} + /** * Match against the compared path's folded code points. One original code point * may fold to several symbols (ß/SS), so token boundaries intentionally operate * on the folded stream rather than assuming one symbol per host character. + * A literal directory can accept a matching continuation after its final slash. + * No multi-symbol character fold crosses that slash, so the existing character + * transitions also decide prefix reachability without splitting a folded atom. */ -function fnmatchcase(value: string, pattern: string): boolean { +function fnmatchcase(value: string, pattern: string, descendants = false): boolean { const symbols = [...caseFold(value)]; const tokens = globTokens(pattern); + const suffixPossible: boolean[] = []; + if (descendants) { + suffixPossible[tokens.length] = true; + for (let index = tokens.length - 1; index >= 0; index -= 1) { + const token = tokens[index]; + suffixPossible[index] = suffixPossible[index + 1] && + (token.kind !== "class" || classHasMatch(token.body, token.negated)); + } + } const memo = new Map(); const match = (valueIndex: number, tokenIndex: number): boolean => { const key = `${valueIndex}:${tokenIndex}`; const cached = memo.get(key); if (cached !== undefined) return cached; let result: boolean; - if (tokenIndex === tokens.length) { + if (descendants && valueIndex === symbols.length) { + result = suffixPossible[tokenIndex]; + } else if (tokenIndex === tokens.length) { result = valueIndex === symbols.length; } else { const token = tokens[tokenIndex]; @@ -379,9 +413,10 @@ function fnmatchcase(value: string, pattern: string): boolean { } else { const literal = [...token.folded]; const end = valueIndex + literal.length; - result = end <= symbols.length && - literal.every((symbol, index) => symbols[valueIndex + index] === symbol) && - match(end, tokenIndex + 1); + const compared = literal.slice(0, symbols.length - valueIndex); + result = compared.every((symbol, index) => symbols[valueIndex + index] === symbol) && + (end <= symbols.length ? match(end, tokenIndex + 1) + : descendants && suffixPossible[tokenIndex + 1]); } } memo.set(key, result); @@ -407,11 +442,13 @@ function scopePairOverlaps(left: string, right: string): boolean { const leftGlob = ["*", "?", "["].some((token) => left.includes(token)); const rightGlob = ["*", "?", "["].some((token) => right.includes(token)); if (leftGlob && !rightGlob) { + if (right.endsWith("/")) return fnmatchcase(right, left, true); const prefix = scopeLiteralPrefix(left); return fnmatchcase(right, left) || (prefix.endsWith("/") && equal(right.replace(/\/$/u, ""), prefix.replace(/\/$/u, ""))); } if (rightGlob && !leftGlob) { + if (left.endsWith("/")) return fnmatchcase(left, right, true); const prefix = scopeLiteralPrefix(right); return fnmatchcase(left, right) || (prefix.endsWith("/") && equal(left.replace(/\/$/u, ""), prefix.replace(/\/$/u, ""))); diff --git a/tests/control_plane/test_canonical_lease_scopes.py b/tests/control_plane/test_canonical_lease_scopes.py index f56da30279..e4284234cb 100644 --- a/tests/control_plane/test_canonical_lease_scopes.py +++ b/tests/control_plane/test_canonical_lease_scopes.py @@ -265,3 +265,60 @@ def test_case_folded_negated_glob_class_excludes_unicode_alias(scope_goal, scope allowed = cli("task-lease", "acquire", "--todo-id", "todo_scope_b", "--owner", "agent-b", "--idempotency-key", "unicode-negated-b", "--write-scope", target, "--expected-version", "0") assert allowed["acquired"] + + +@pytest.mark.parametrize("first_scope,second_scope,conflict", [ + ("src/", "src/components/*.ts", True), + ("src/", "src/main.*", True), + ("src/", "**/*.ts", True), + ("src/components/", "src/*/*.ts", True), + ("src/", "src/components/?.ts", True), + ("src/SS/", "src/[ß]/*.ts", True), + ("src/z/", "src/[ab]/*.ts", False), + ("src/z/", "src/[!z]/*.ts", False), + ("src/long/", "src/?/main.ts", False), + ("src/a/", "src/ab/*.ts", False), + ("src/", "src/generated/[z-a].ts", False), +]) +@pytest.mark.parametrize("reverse", [False, True], ids=["forward", "reverse"]) +@pytest.mark.parametrize("entrypoint", ["acquire", "claim"]) +def test_directory_glob_scopes_at_real_entrypoints( + scope_goal, first_scope, second_scope, conflict, reverse, entrypoint, +): + cli, current = scope_goal + if reverse: + first_scope, second_scope = second_scope, first_scope + + def add_todo(label, scope): + return cli("todo", "add", "--role", "agent", "--priority", "P1", + "--text", f"Directory scope work {label}", + "--task-repository", "git:github.com/example/project", + "--required-write-scope", scope, + "--operation-id", f"directory-add-{label}")["todo_id"] + + def acquire(todo_id, label, scope, expected_exit=0): + if entrypoint == "acquire": + return cli("task-lease", "acquire", "--todo-id", todo_id, + "--owner", f"agent-{label}", "--idempotency-key", f"directory-{label}", + "--expected-version", "0", "--write-scope", scope, expected_exit=expected_exit) + return cli("todo", "claim", "--todo-id", todo_id, + "--claimed-by", f"agent-{label}", "--agent-id", f"agent-{label}", + "--claim-operation-id", f"directory-claim-{label}", + "--task-lease-idempotency-key", f"directory-{label}", + "--task-lease-expected-version", "0", expected_exit=expected_exit) + + first, second = add_todo("a", first_scope), add_todo("b", second_scope) + acquire(first, "a", first_scope) + # Exact retries retain the original scope spelling and receipt, including + # a literal trailing slash; conflict comparison must not rewrite identity. + before_retry = current() + acquire(first, "a", first_scope) + assert current() == before_retry + assert cli("task-lease", "inspect", "--todo-id", first)["lease"]["write_scopes"] == [first_scope] + before = current() + result = acquire(second, "b", second_scope, expected_exit=1 if conflict else 0) + if conflict: + assert result["error_code"] == "write_scope_conflict" + assert current() == before + else: + assert cli("task-lease", "inspect", "--todo-id", second)["lease"]["write_scopes"] == [second_scope] diff --git a/tests/control_plane_ts/task_lease_directory_scopes.test.ts b/tests/control_plane_ts/task_lease_directory_scopes.test.ts new file mode 100644 index 0000000000..18115904bc --- /dev/null +++ b/tests/control_plane_ts/task_lease_directory_scopes.test.ts @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { + comparisonWriteScopes, + evaluateTaskLeaseAcquireDecision, + evaluateTaskLeaseWriteScopesOverlap, +} from "../../loopx/control_plane/work_items/task_lease_acquire_decision.ts"; + +const overlaps = (left: string, right: string) => + evaluateTaskLeaseWriteScopesOverlap({left: [left], right: [right]}).overlap; + +// Each positive row names a concrete path in both sets, independently of how +// the matcher decides whether an arbitrary descendant can exist. +for (const [directory, pattern, witness] of [ + ["src/", "src/components/*.ts", "src/components/button.ts"], + ["src/", "src/main.*", "src/main.ts"], + ["src/", "**/*.ts", "src/components/button.ts"], + ["src/components/", "src/*/*.ts", "src/components/button.ts"], + ["src/", "src/components/?.ts", "src/components/a.ts"], + ["src/a/", "src/[ab]/*.ts", "src/a/button.ts"], + ["src/b/", "src/[!a]/*.ts", "src/b/button.ts"], + ["src/SS/", "src/[ß]/*.ts", "src/SS/button.ts"], + ["src/SS/", "src/S?/*.ts", "src/SS/button.ts"], + ["src/ς/", "src/[Σ]/*.ts", "src/ς/button.ts"], + ["src/😀/", "src/?/*.ts", "src/😀/button.ts"], + ["src/", "src/generated/[A-Z].ts", "src/generated/a.ts"], + ["src/", "src/generated/[!z-a].ts", "src/generated/a.ts"], + ["./src//", "src/./components/*.ts", "src/components/button.ts"], +]) { + test(`directory scope overlaps ${pattern} under ${directory}`, () => { + assert.equal(overlaps(directory, witness), true); + assert.equal(overlaps(pattern, witness), true); + assert.equal(overlaps(directory, pattern), true); + assert.equal(overlaps(pattern, directory), true); + }); +} + +for (const [directory, pattern] of [ + ["src/", "docs/*.ts"], + ["src/a/", "src/ab/*.ts"], + ["src/z/", "src/[ab]/*.ts"], + ["src/z/", "src/[!z]/*.ts"], + ["src/long/", "src/?/main.ts"], + ["src/Σ/", "src/[!ς]/*.ts"], + ["src/SS/", "src/[!ß]/*.ts"], + ["src/", "src/generated/[z-a].ts"], + ["src/", "src/generated/[!\u0000-\u{10ffff}].ts"], + // The only code point outside this class is A, whose fold alias a is inside. + ["src/", "src/generated/[!\u0000-@B-\u{10ffff}].ts"], + ["src/main.ts", "src/test.ts"], +]) { + test(`directory comparison preserves disjoint ${JSON.stringify([directory, pattern])}`, () => { + assert.equal(overlaps(directory, pattern), false); + assert.equal(overlaps(pattern, directory), false); + }); +} + +test("directory matching retains stored spelling and existing authority boundaries", () => { + const repository = "git:github.com/example/project"; + const workspace = {host: "1".repeat(64), common_directory: "2".repeat(64), + worktree: "3".repeat(64), repository}; + const request = { + handoff_mode: "hard_lease", registered_agents: ["agent-a", "agent-b"], + todo: {todo_id: "todo_b", status: "open", claimed_by: null, excluded_agents: [], + task_repository: repository}, + lease: null, + other_leases: [{todo_id: "todo_a", active: true, effective: true, + write_scopes: ["./src//"], write_repository: repository, write_workspace: workspace}], + command: {owner: "agent-b", idempotency_key: "second", expected_version: 0, + ttl_seconds: 60, write_scopes: ["src/components/*.ts"]}, + }; + const original = structuredClone(request); + assert.equal(evaluateTaskLeaseAcquireDecision(request).code, "write_scope_conflict"); + const differentRepository = structuredClone(request); + differentRepository.todo.task_repository = "git:github.com/example/other"; + assert.equal(evaluateTaskLeaseAcquireDecision(differentRepository).outcome, "apply"); + const inactive = structuredClone(request); + inactive.other_leases[0].active = false; + assert.equal(evaluateTaskLeaseAcquireDecision(inactive).outcome, "apply"); + const sameCheckout = {...request, command: {...request.command, write_workspace: workspace}}; + assert.equal(evaluateTaskLeaseAcquireDecision(sameCheckout).code, "write_scope_conflict"); + const independent = {...request, command: {...request.command, + write_workspace: {...workspace, worktree: "4".repeat(64)}}}; + const advisory = evaluateTaskLeaseAcquireDecision(independent); + assert.equal(advisory.outcome, "apply"); + assert.deepEqual(advisory.overlap_advisory_indexes, [0]); + assert.deepEqual(request, original); + assert.deepEqual(comparisonWriteScopes(["src/"]), ["src/"]); +});