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/"]); +});