diff --git a/AGENTS.md b/AGENTS.md index a9601d7..db1a22f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -104,6 +104,13 @@ Use `knowledgeReleaseReport()` before promotion. It folds the candidate and base - Research state is durable state. A driver that accumulates belief across rounds takes a store and a `ledgerId` (`createPersistentResearchDrivingDriver`) so corroboration counts, contradiction edges, and open questions survive the process. `runVerifiedResearchLoop` durably announces a fold before its synchronous question generation, calls `driver.checkpoint()` before publishing the round event, and reconstructs an interrupted fold on resume. - `TrackedClaim` remains the live Set-based driver API; `ResearchClaimRecord` is its sorted-array durable form. Convert at the persistence boundary rather than changing the published live shape. - More than one writer per ledger means `mergeClaimLedger(id, merge)`, never `putClaimLedger`. `putClaimLedger` writes the whole record, so two writers accumulating into one ledger each write what they built from a stale read and the later write erases the earlier writer's claims. `mergeClaimLedger` holds the store's lock across read, merge, and write; `mergeClaimLedgers` is the combining rule and is commutative, associative, and idempotent, so replay and arrival order cannot change the result. +- One lock per store root, and a consumer with its own lock wrapper joins it rather than building a second one. +`withKnowledgeMutation` is reentrant per async context, `isKnowledgeMutationHeld(root)` reports whether this context already holds the root, and `runInKnowledgeMutationScope(root, hold, body)` enters the scope on a lock the caller took by its own path. +Inside either, every lock-taking function in this package runs inline instead of blocking against a lock the caller already holds. +A second lock over the same root is a second writer, whatever lockfile it uses. +- Grade a claim's re-executed check with `gradeFor`, and a whole pass with `gradeClaims`. +The verdict lattice is calibrated: a check that carries its own expected value, one killed at its deadline, and one whose output says it never reached its input are all refusals or environment verdicts, never a refutation of the claim. +`gradeClaims` adds the one judgment a single claim cannot make — it flags a later claim that repeats an earlier claim's check, expectation and title at the same verdict, so one verification counted N times is visible rather than silent. - Use `writeFileDurable` / `writeJsonDurableWithinRoot` from the entrypoint for any file that must survive a crash. They are atomic, fsynced, and symlink-safe; a hand-rolled `writeFile` is none of those. - Use `KnowledgeDiscoveryDispatcher` for research workers. Applications should connect it to their own runtime. - Do not bypass `lint` or `validate` before using generated knowledge in an agent. diff --git a/CHANGELOG.md b/CHANGELOG.md index acee661..b3ef80c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,55 @@ # Changelog +## 12.0.0 — 2026-09-01 + +### Fixed + +- A check whose output says it never reached its input is graded `unrunnable` whatever its exit status. +The signature list was consulted only after a nonzero exit. +A shell pipeline exits with the status of its last stage, so `sha256sum | cut -d' ' -f1` prints "No such file or directory" and exits 0, and `gradeFor` then compared that error text to the expectation and returned `contradicted` — a refutation of research that was never run. +Swept over 272 grade files, 29 of 61 recorded refutations were this shape: 7 a malformed check body, 7 a shell that could not parse the check, 6 a solver that could not open its problem file, 5 `Permission denied`, 4 `cannot create directory`. +The list now also matches case-insensitively, because a tool that prints its own diagnostic chooses its own case, and it covers `Permission denied`, `EACCES`, `cannot create directory`, `cannot open`, `SyntaxError` and the two bash parse errors. +- The list only ever downgrades `contradicted` to `unrunnable`, so it can turn a measurement that did not happen into an honest "not measured" and can never turn a real failure into a pass. +It consults the expectation first: a claim that predicted the error is genuinely contradicted when the error does not appear, so an expectation naming a signature switches the guard off for that claim. +- A check killed at its deadline is graded `unrunnable`, not `contradicted`. +A deadline is a budget, not a verdict, and the rule fires before the expectation is compared, so a slow solver that printed nothing is no longer recorded as refuted. +- A passing check is no longer refused for naming several values at once. +`expectationRefusalNote` returned `uncheckable` when the expectation carried three or more `key=value` tokens, after the check had exited 0 and immediately above the comparison that would have passed. +The rule inverted its own intent: `expect: OK` is one token and is satisfied by any output containing those two letters, while `GRID OK cells=8 WIN=1 PARETO=6` needs four independent numbers to coincide before it can pass falsely. +Measured, 173 of 1,001 claims carrying an expectation at rung 4 or above — 17.3% — were discarded this way after their check exited 0, including one whose output matched its expectation byte for byte. +Multiline expectations are still refused, because the comparison is one contiguous substring. + +### Added + +- `gradeFor` and `assertGradeableEvidence` refuse a check whose text contains its own expected value. +Such a check prints a value the claim's subject never had to produce, so the comparison tests the author's typing rather than the artifact. +The test is the whole trimmed expectation appearing verbatim in the command, which is narrow on purpose: it costs an author a rewrite of the command, and it catches the shape `isConstantEmitter` cannot, where a real command's arguments already spell the answer. +- `gradeClaims(claims)` grades one pass and flags the repeats inside it, with `claimCheckKey` for a grader that keeps its own map. +`gradeFor` judges one claim and cannot see a second claim carrying the same check, so a run that recorded one verification under several titles reported as several independent verifications. +A later claim sharing check, expectation, normalized title and verdict now carries `duplicateOf` naming the first. +The verdict and the count do not change: the flag is for a reader deciding how much evidence a run really produced. +Only a full match is a duplicate, so one check shared by claims that say different things is one instrument used several times. +- `DEADLINE_EXIT_CODE` and `CheckExecution.timedOut`, the two ways an executor reports a deadline kill. +A grader that knows it killed the process sets `timedOut`; a grader holding only an exit status reports 124, which is `timeout(1)`'s status. +`verifyGradeableEvidence` now sets `timedOut` when its own budget killed the check, so the deadline rule fires on this package's own execution path rather than only on a caller's. +A killed process reports no exit status of its own, so it read as 127 and was compared to the expectation like any other failure. +- `isKnowledgeMutationHeld(root)` and `runInKnowledgeMutationScope(root, hold, body)`, with the `KnowledgeMutationHold` type. +`withKnowledgeMutation` is reentrant per async context, but the `AsyncLocalStorage` behind that was module-private, so a consumer holding the store lock through its own wrapper could not enter or observe the scope and calling any lock-taking function from this package inside its wrapper self-blocked against a lock it already held. +The scope is now joinable: inside `runInKnowledgeMutationScope` every lock-taking function in this package sees the root as held and runs inline. +The caller owns acquiring and releasing its lock and supplies the hold, which is asked `assertOwned()` at the same points a lock this package acquired is asked, so an externally held lock carries the same guarantee and not a weaker one. +- `KnowledgeMutationOptions.staleMs` documents what the window costs. +The holder heartbeats at a third of it, so the window also bounds how long a crashed holder wedges every other writer. +The 15-minute default is sized for a knowledge-improvement run that holds the lock across agent turns; measured holds are 9 ms for one page, 586 ms for a 50-page batch, and about 390 ms for the longest promotion in the largest store. + +### Changed + +- **This is a major bump because two exported shapes moved.** +`CheckExecution` gains optional `timedOut` and `ClaimGrade` gains optional `duplicateOf`. +Both are optional, so a consumer that constructs neither field compiles unchanged, and a consumer that switches exhaustively over `ClaimGrade`'s fields does not. +- **A claim whose check contains its expected value is now refused where it was recorded before.** +`assertGradeableEvidence` throws `UncheckableClaimError` for it, and `gradeFor` returns `uncheckable`. +Record time and grade time read one detector and one set of notes, so a shape the recorder admits is never one the grader refuses. + ## 11.0.0 — 2026-08-31 ### Changed diff --git a/api-surface.json b/api-surface.json index 77a4723..39cfafe 100644 --- a/api-surface.json +++ b/api-surface.json @@ -78,10 +78,10 @@ "BuildRetrievalEvalDispatchOptions": "value 2a1fe2b2da73", "CHECKABLE_RUNG_THRESHOLD": "value 534bcde62c80", "CITES_INVALIDATED_FIELD": "value 3fff28ee6d1f", - "CheckExecution": "value 079d2aa6b6cd", + "CheckExecution": "value f8a48e381773", "ChunkingOptions": "value 00fb66d7d155", "ClaimEvidence": "value f780da49a3ef", - "ClaimGrade": "value c38c514e14fc", + "ClaimGrade": "value a9e6edbc7877", "ClaimGroundingDriverOptions": "value 4c5ca74e7255", "ClaimLedgerGoalConflictError": "value c96a14249e23", "ClaimLedgerMigrationRequiredError": "value 2dc627c3fa60", @@ -98,6 +98,7 @@ "CreateKnowledgeToolsOptions": "value 8cf079704a21", "CreateKnowledgeUseReceiptInput": "value eb120a93f4c7", "D1Adapter": "value fd8669db1f2e", + "DEADLINE_EXIT_CODE": "value 16590df13e07", "DEFAULT_KNOWLEDGE_BRIEF_LIMIT": "value a16e63bcd9c7", "DEFAULT_MEMORY_CLEANUP_TIMEOUT_MS": "value 10d1ef3e7731", "DEFAULT_PAGES_DIRECTORY": "value 5aa47f8e3dde", @@ -133,6 +134,8 @@ "FreshnessMark": "value 93bc4803d9b3", "FreshnessRecord": "value 65bdf72051de", "FreshnessTtl": "value e3c26a4f6c3e", + "GradeableClaim": "value bf465a1848d5", + "GradedClaim": "value acfba8ff752d", "GraphitiMcpClientLike": "value a6c48bcfcdd6", "GraphitiMemoryAdapterOptions": "value 6226003a629c", "GraphitiToolNames": "value 65e079641c44", @@ -258,6 +261,7 @@ "KnowledgeMemoryBenchmarkTaskKind": "type 33dd3b696a1d", "KnowledgeMemoryEvent": "type b5b297e3f9b1", "KnowledgeMemoryFactMatcher": "type 095c9075c340", + "KnowledgeMutationHold": "type 353c820b01ff", "KnowledgeMutationLock": "type 974f26ecfc42", "KnowledgeMutationOptions": "type 1ab92dd5d3de", "KnowledgePage": "value 46af63a0ac13", @@ -553,6 +557,7 @@ "chunkMarkdown": "value 3f930d4ba32f", "citedClaimKey": "value 810f529bf94c", "citedClaimOf": "value 1b37f8f9af86", + "claimCheckKey": "value f7b71c731a17", "claimEvidenceId": "value 10ea894124db", "claimId": "value 4c9eca896a37", "claimSourceHost": "value a8a18cde9fcc", @@ -612,6 +617,7 @@ "formatKnowledgeCitationReference": "value 8c9301778549", "formatKnowledgeInvalidationProposal": "value 0facdbad3e8e", "fromAgentCandidateKnowledgeRef": "value 247b544b764e", + "gradeClaims": "value 5a3c1c162258", "gradeCompanyAgainstText": "value 7470a3d96d31", "gradeFactAgainstText": "value 9818c38e4ce2", "gradeFor": "value 79db4a5137a3", @@ -628,6 +634,7 @@ "investmentThesisSet": "value 373728f5643d", "isKernelAnchoredPath": "value 2819fa14db65", "isKnowledgeMemoryBenchmarkCase": "value 8634e2c95fec", + "isKnowledgeMutationHeld": "value e8f0de09e802", "isKnowledgePagePath": "value 2819fa14db65", "isMissingFile": "value 73ca49f2b72a", "isReachable": "value 980b56e89641", @@ -718,6 +725,7 @@ "runAgentMemoryLearningExperiment": "value f11e54703bb5", "runBoundedMemoryLifecycle": "value 75f3e3b274a0", "runDiscoveryLoop": "value addb8a464a58", + "runInKnowledgeMutationScope": "value 83a5698000d7", "runInvestmentThesisTask": "value 2eac673e45ab", "runKnowledgeBenchmarkSuite": "value ce72ffe18eef", "runKnowledgeResearchLoop": "value 4d00a0a2e19d", diff --git a/package.json b/package.json index ecaaf1f..6a96ce7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "11.0.0", + "version": "12.0.0", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { diff --git a/src/claim-evidence-intake.test.ts b/src/claim-evidence-intake.test.ts index 845bfe3..f668535 100644 --- a/src/claim-evidence-intake.test.ts +++ b/src/claim-evidence-intake.test.ts @@ -41,14 +41,16 @@ describe('calibrated static intake refusals', () => { ) }) - it('refuses a brittle multi-value expectation', () => { - expect(() => + it('records an expectation that names several values at once', () => { + // The strongest expectation available: a false pass needs three independent values to + // coincide, where a one-token expectation is met by any output containing that token. + expect( assertGradeableEvidence({ rung: 4, check: 'python3 check.py', expect: 'rank=12 size=40 verified=true', - }), - ).toThrow(/one decisive value per claim/) + }).expect, + ).toBe('rank=12 size=40 verified=true') }) it('refuses the escaped-newline long-first-word shape', () => { diff --git a/src/claim-evidence.test.ts b/src/claim-evidence.test.ts index 4cf023e..686b1b8 100644 --- a/src/claim-evidence.test.ts +++ b/src/claim-evidence.test.ts @@ -1,9 +1,14 @@ +import { tmpdir } from 'node:os' import { describe, expect, it } from 'vitest' import { assertGradeableEvidence, + claimCheckKey, + DEADLINE_EXIT_CODE, + gradeClaims, gradeFor, UncheckableClaimError, verdictFor, + verifyGradeableEvidence, } from './claim-evidence' describe('assertGradeableEvidence', () => { @@ -263,3 +268,338 @@ describe('gradeFor — the refusal says which shape it refused', () => { expect(grade).toEqual({ verdict: 'verified' }) }) }) + +describe('gradeFor — a check that carries its own expectation cannot certify itself', () => { + it('refuses the expectation authored into the check text, however the check exits', () => { + const evidence = { + rung: 4 as const, + check: 'grep -c "PARETO=6" results.txt', + expect: 'PARETO=6', + } + expect(gradeFor(evidence, { ...PASSED, stdout: 'PARETO=6' })).toEqual({ + verdict: 'uncheckable', + note: expect.stringContaining('contains the expected value'), + }) + expect(verdictFor(evidence, { exitCode: 1, stdout: '', stderr: 'boom' })).toBe('uncheckable') + expect(verdictFor(evidence, null)).toBe('uncheckable') + }) + + it('refuses a script invocation whose arguments spell the answer', () => { + expect( + verdictFor( + { rung: 4, check: './solve.sh --assert cells=8', expect: 'cells=8' }, + { ...PASSED, stdout: 'cells=8' }, + ), + ).toBe('uncheckable') + }) + + it('refuses it at record time too, in the same words', () => { + const evidence = { + rung: 4 as const, + check: 'grep -c "PARETO=6" results.txt', + expect: 'PARETO=6', + } + let raised: UncheckableClaimError | undefined + try { + assertGradeableEvidence(evidence) + } catch (error) { + raised = error as UncheckableClaimError + } + expect(raised).toBeInstanceOf(UncheckableClaimError) + expect(raised?.note).toBe(gradeFor(evidence, { ...PASSED, stdout: 'PARETO=6' }).note) + }) + + it('leaves a check that reads the value it prints alone', () => { + expect( + verdictFor( + { rung: 4, check: 'grep -c PARETO results.txt', expect: '6' }, + { ...PASSED, stdout: '6' }, + ), + ).toBe('verified') + expect( + verdictFor( + { rung: 4, check: 'echo "cells=$(wc -l < grid.txt)"', expect: 'cells=8' }, + { ...PASSED, stdout: 'cells=8' }, + ), + ).toBe('verified') + }) + + it('below the checkable rungs nothing is refused', () => { + expect( + verdictFor( + { rung: 3, check: 'echo ok | grep ok', expect: 'ok' }, + { ...PASSED, stdout: 'ok' }, + ), + ).toBe('verified') + }) +}) + +describe('gradeFor — a check that never reached its input is not a refutation', () => { + it('downgrades a zero-exit pipeline whose output says the file is missing', () => { + // `sha256sum missing.json | cut -d' ' -f1` exits with cut's status, which is 0, and prints + // the error text. Comparing that to the expectation records a refutation of research that + // was never run: 29 of 61 recorded contradictions were this shape. + const grade = gradeFor( + { rung: 4, check: "sha256sum out/k3.json | cut -d' ' -f1", expect: 'a91f3c' }, + { exitCode: 0, stdout: '', stderr: 'sha256sum: out/k3.json: No such file or directory' }, + ) + expect(grade.verdict).toBe('unrunnable') + expect(grade.note).toContain('never reached what the claim is about') + }) + + it('reads the signature whatever case the tool printed it in', () => { + for (const stderr of [ + 'python: no such file or directory', + 'bash: line 1: ./solve: Permission denied', + 'mkdir: cannot create directory ‘out’: Read-only file system', + "bash: -c: line 3: syntax error near unexpected token `fi'", + 'SyntaxError: invalid syntax', + ]) { + expect( + verdictFor({ rung: 4, check: 'x', expect: 'a91f3c' }, { exitCode: 0, stdout: '', stderr }), + ).toBe('unrunnable') + } + }) + + it('still refutes a claim that PREDICTED the error', () => { + // The expectation names the signature, so the guard is off for this claim and its absence + // is a real contradiction. + expect( + verdictFor( + { rung: 4, check: 'cat missing.json', expect: 'No such file or directory' }, + { exitCode: 0, stdout: 'contents', stderr: '' }, + ), + ).toBe('contradicted') + expect( + verdictFor( + { rung: 4, check: 'cat missing.json', expect: 'No such file or directory' }, + { exitCode: 1, stdout: '', stderr: 'cat: missing.json: No such file or directory' }, + ), + ).toBe('contradicted') + }) + + it('never turns a real failure into a pass', () => { + // The guard only ever moves `contradicted` to `unrunnable`. + expect( + verdictFor( + { rung: 4, check: 'pytest -q', expect: '12 passed' }, + { exitCode: 1, stdout: 'ENOENT: conftest.py', stderr: '' }, + ), + ).toBe('unrunnable') + expect( + verdictFor( + { rung: 4, check: 'pytest -q', expect: '12 passed' }, + { ...PASSED, stdout: '12 passed\nNo such file or directory' }, + ), + ).toBe('verified') + }) +}) + +describe('gradeFor — a deadline is a budget, not a verdict', () => { + it('grades a killed check unrunnable and names the deadline', () => { + const grade = gradeFor( + { rung: 4, check: 'cadical problem.cnf', expect: 'UNSAT' }, + { exitCode: DEADLINE_EXIT_CODE, stdout: '', stderr: '' }, + ) + expect(grade.verdict).toBe('unrunnable') + expect(grade.note).toContain('killed at its deadline') + }) + + it('reads the executor that says it killed the check, whatever status it reports', () => { + expect( + verdictFor( + { rung: 4, check: 'cadical problem.cnf', expect: 'UNSAT' }, + { exitCode: 0, stdout: 'SAT', stderr: '', timedOut: true }, + ), + ).toBe('unrunnable') + }) + + it('decides the deadline before the expectation, so a silent solver is not refuted', () => { + expect( + verdictFor( + { rung: 4, check: 'cadical problem.cnf', expect: 'UNSAT' }, + { exitCode: DEADLINE_EXIT_CODE, stdout: 'c partial search\n', stderr: '' }, + ), + ).toBe('unrunnable') + }) +}) + +describe('gradeFor — a precise expectation is the strongest one, not a refused one', () => { + it('verifies a check whose output carries a four-token expectation', () => { + expect( + gradeFor( + { rung: 4, check: 'node report.mjs', expect: 'GRID OK cells=8 WIN=1 PARETO=6 NEGATIVE=1' }, + { ...PASSED, stdout: 'GRID OK cells=8 WIN=1 PARETO=6 NEGATIVE=1\n' }, + ), + ).toEqual({ verdict: 'verified' }) + }) + + it('records it at rung 4 as well', () => { + expect( + assertGradeableEvidence({ + rung: 4, + check: 'node report.mjs', + expect: 'cells=8 WIN=1 PARETO=6', + }).rung, + ).toBe(4) + }) + + it('still refuses an expectation the comparison cannot use', () => { + expect(() => + assertGradeableEvidence({ rung: 4, check: 'node report.mjs', expect: 'a\nb' }), + ).toThrow(/multiple lines/) + }) +}) + +describe('gradeClaims — one verification counted twice is flagged, not dropped', () => { + const shared = { check: 'node report.mjs', expect: 'PARETO=6' } + const ran = { ...PASSED, stdout: 'PARETO=6' } + + it('flags the later claim and keeps both verdicts', () => { + const graded = gradeClaims([ + { + id: 'page-a', + title: 'The grid is Pareto-optimal', + evidence: { rung: 4, ...shared }, + execution: ran, + }, + { + id: 'page-b', + title: 'the grid is pareto-optimal ', + evidence: { rung: 4, ...shared }, + execution: ran, + }, + ]) + expect(graded.map((row) => row.verdict)).toEqual(['verified', 'verified']) + expect(graded[0]?.duplicateOf).toBeUndefined() + expect(graded[1]?.duplicateOf).toBe('page-a') + }) + + it('does not flag one check shared by claims that say different things', () => { + const graded = gradeClaims([ + { + id: 'page-a', + title: 'The grid is Pareto-optimal', + evidence: { rung: 4, ...shared }, + execution: ran, + }, + { + id: 'page-b', + title: 'The grid has one negative cell', + evidence: { rung: 4, ...shared }, + execution: ran, + }, + ]) + expect(graded.every((row) => row.duplicateOf === undefined)).toBe(true) + }) + + it('does not flag the same claim at two different verdicts', () => { + const graded = gradeClaims([ + { + id: 'page-a', + title: 'The grid is Pareto-optimal', + evidence: { rung: 4, ...shared }, + execution: ran, + }, + { + id: 'page-b', + title: 'The grid is Pareto-optimal', + evidence: { rung: 4, ...shared }, + execution: { ...PASSED, stdout: 'PARETO=5' }, + }, + ]) + expect(graded.map((row) => row.verdict)).toEqual(['verified', 'contradicted']) + expect(graded.every((row) => row.duplicateOf === undefined)).toBe(true) + }) + + it('grades every claim exactly as gradeFor does', () => { + const claims = [ + { id: 'a', title: 'one', evidence: { rung: 4 as const, ...shared }, execution: ran }, + { id: 'b', title: 'two', evidence: { rung: 4 as const, check: 'x' }, execution: null }, + { id: 'c', title: 'three', evidence: { rung: 4 as const, ...shared }, execution: null }, + ] + for (const [index, graded] of gradeClaims(claims).entries()) { + const { id, duplicateOf, ...grade } = graded + expect(id).toBe(claims[index]?.id) + expect(duplicateOf).toBeUndefined() + expect(grade).toEqual(gradeFor(claims[index]!.evidence, claims[index]!.execution)) + } + }) + + it('keys nothing it cannot compare, so two unkeyable claims are not each other', () => { + expect(claimCheckKey({ check: 'x' })).toBeUndefined() + expect(claimCheckKey({ title: 'a claim' })).toBeUndefined() + expect(claimCheckKey({ check: ' node x.mjs ', expect: ' 6 ', title: ' A Claim.md ' })).toBe( + claimCheckKey({ check: 'node x.mjs', expect: '6', title: 'a claim' }), + ) + const graded = gradeClaims([ + { id: 'a', evidence: { rung: 4, ...shared }, execution: ran }, + { id: 'b', evidence: { rung: 4, ...shared }, execution: ran }, + ]) + expect(graded.every((row) => row.duplicateOf === undefined)).toBe(true) + }) + + it('separates two claims whose fields concatenate to the same text', () => { + expect(claimCheckKey({ check: 'ab', expect: 'c', title: 'd' })).not.toBe( + claimCheckKey({ check: 'a', expect: 'bc', title: 'd' }), + ) + }) +}) + +describe('gradeFor — the self-certification guard does not refuse a correct claim', () => { + it('ignores a short expectation that only appears inside a longer word', () => { + // `expect: 6` appears inside `report6.mjs`. Refusing that would cost a true claim, which is + // the failure this lattice is calibrated against. + expect( + verdictFor( + { rung: 4, check: 'node report6.mjs --count', expect: '6' }, + { ...PASSED, stdout: '6' }, + ), + ).toBe('verified') + expect( + verdictFor({ rung: 4, check: 'wc -l < grid8.txt', expect: '8' }, { ...PASSED, stdout: '8' }), + ).toBe('verified') + }) + + it('still refuses the same value when the check spells it on its own', () => { + expect( + verdictFor( + { rung: 4, check: 'node report6.mjs --assert 6', expect: '6' }, + { ...PASSED, stdout: '6' }, + ), + ).toBe('uncheckable') + }) + + it('refuses an expectation that ends at the end of the check text', () => { + expect( + verdictFor( + { rung: 4, check: 'grep -c PARETO=6', expect: 'PARETO=6' }, + { ...PASSED, stdout: 'PARETO=6' }, + ), + ).toBe('uncheckable') + }) +}) + +describe('verifyGradeableEvidence — the executor reports its own deadline', () => { + it.skipIf(process.platform === 'win32')( + 'grades a check that outran its budget unrunnable, not contradicted', + async () => { + const verified = await verifyGradeableEvidence( + { rung: 4, check: 'sleep 5; printf a | wc -c', expect: '1' }, + { cwd: tmpdir(), env: { PATH: process.env.PATH ?? '' }, timeoutMs: 100 }, + ) + expect(verified.execution.timedOut).toBe(true) + expect(verified.grade.verdict).toBe('unrunnable') + expect(verified.grade.note).toContain('killed at its deadline') + }, + ) + + it.skipIf(process.platform === 'win32')('grades a check that finishes in budget', async () => { + const verified = await verifyGradeableEvidence( + { rung: 4, check: 'echo "count=$(printf a | wc -c)"', expect: 'count=1' }, + { cwd: tmpdir(), env: { PATH: process.env.PATH ?? '' }, timeoutMs: 10_000 }, + ) + expect(verified.execution.timedOut).toBeUndefined() + expect(verified.grade).toEqual({ verdict: 'verified' }) + }) +}) diff --git a/src/claim-evidence.ts b/src/claim-evidence.ts index 904a0f0..5a5169f 100644 --- a/src/claim-evidence.ts +++ b/src/claim-evidence.ts @@ -40,6 +40,10 @@ export interface ClaimEvidence { * A substring the check's stdout must contain — the decisive value, printed. A check that * exits zero but prints nothing cannot confirm a value; authors should print the number. * Required at rung 4 and above, where an exit code alone reproduces no value. + * + * An expectation naming several values at once is admitted, and is the strongest shape + * available: a false pass on `GRID OK cells=8 WIN=1 PARETO=6` needs three independent numbers + * to coincide, where a one-token `OK` is satisfied by any output containing those two letters. */ expect?: string /** Where the artifact backing the claim lives, for humans following the trail. */ @@ -106,9 +110,10 @@ const MULTILINE_EXPECTATION_NOTE = 'the expected value spans multiple lines, but the grader performs one contiguous-substring ' + 'comparison — record one decisive single-line value per claim' -const MULTI_VALUE_EXPECTATION_NOTE = - 'the expected value carries three or more key=value tokens, so any extra token in the check ' + - 'output breaks an otherwise correct claim — record one decisive value per claim' +const SELF_CERTIFYING_NOTE = + 'the check text contains the expected value, so the check carries the answer it is graded ' + + 'against and prints a value the claim did not have to produce — record a command that reads ' + + 'the artifact and prints the value it finds there' const LONG_FIRST_WORD_NOTE = 'the check begins with a shell word longer than 200 characters; this is the escaped-newline ' + @@ -148,15 +153,18 @@ export class UncheckableClaimError extends Error { * `gradeFor` grades `uncheckable`: no check, a check that cannot fail, an unusable expectation, * or the escaped-newline command shape. Below the threshold nothing is refused. * - * Four additional rules are calibrated on downstream losses, not taste: four correct claims died - * from multiline expectations, two from brittle multi-value expectations, one campaign recorded - * an enormous first word made of literal `\\n` sequences, and a 299-run fleet autopsy reduced ten - * reported grader failures to checks that were never executed before their authors disappeared. - * The dynamic half of that last rule is `verifyGradeableEvidence` below. + * Three additional rules are calibrated on downstream losses, not taste: four correct claims + * died from multiline expectations, one campaign recorded an enormous first word made of literal + * `\\n` sequences, and a 299-run fleet autopsy reduced ten reported grader failures to checks that + * were never executed before their authors disappeared. The dynamic half of that last rule is + * `verifyGradeableEvidence` below. */ export function assertGradeableEvidence(evidence: ClaimEvidence): ClaimEvidence { if (evidence.rung < CHECKABLE_RUNG_THRESHOLD) return evidence - const note = checkRefusalNote(evidence.check) ?? expectationRefusalNote(evidence.expect) + const note = + checkRefusalNote(evidence.check) ?? + selfCertifyingNote(evidence) ?? + expectationRefusalNote(evidence.expect) if (note) throw new UncheckableClaimError(evidence.rung, note) return evidence } @@ -169,8 +177,8 @@ export function assertGradeableEvidence(evidence: ClaimEvidence): ClaimEvidence * about the expected value; the author should make the check PRINT it * contradicted the check ran and refuted the claim: nonzero exit, or non-empty output that * lacks the expectation - * unrunnable the check itself could not execute (missing input, missing module) — an - * environment verdict, never a claim verdict + * unrunnable the check itself could not execute (missing input, missing module), or its + * deadline killed it — an environment verdict, never a claim verdict * uncheckable the recorded evidence cannot decide the claim at this rung */ export type ClaimVerdict = @@ -180,14 +188,45 @@ export type ClaimVerdict = | 'unrunnable' | 'uncheckable' -/** The error signatures that mean the check could not run, as opposed to ran and failed. */ +/** + * The output signatures that mean the check never reached what the claim is about. + * + * Matched case-insensitively, because a tool that prints its own diagnostic chooses its own case, + * and read from the OUTPUT rather than from the exit status. A shell pipeline exits with the + * status of its last stage, so `sha256sum | cut -d' ' -f1` prints + * "No such file or directory" and exits 0; reading only the status calls that a refutation of + * research that was never run. Swept over 272 grade files, 29 of 61 recorded refutations were + * this shape: 7 a malformed check body, 7 a shell that could not parse the check, 6 a solver that + * could not open its problem file, 5 `Permission denied`, 4 `cannot create directory`. + */ const UNRUNNABLE_SIGNATURES = - /No such file|FileNotFoundError|ModuleNotFoundError|command not found|ENOENT|AbortError|timed out|ERR_CHILD_PROCESS_STDIO_MAXBUFFER/ + /No such file|FileNotFoundError|ModuleNotFoundError|command not found|ENOENT|AbortError|timed out|ERR_CHILD_PROCESS_STDIO_MAXBUFFER|Permission denied|EACCES|cannot create directory|cannot open|SyntaxError|unexpected EOF while looking for matching|syntax error near unexpected token/i + +/** + * The exit status a deadline kill reports: `timeout(1)`'s status, and the one a bounded process + * runner forces so a killed child that closes with 0 cannot read as a pass. A grader that has + * only an exit status reports this one; a grader that knows it killed the process sets + * `CheckExecution.timedOut` instead, which is unambiguous. + */ +export const DEADLINE_EXIT_CODE = 124 + +const DEADLINE_NOTE = + 'the check was killed at its deadline, so it never tested the claim — a deadline is a budget, ' + + 'not a verdict; raise the budget or record a check that decides within it' + +const UNREACHED_INPUT_NOTE = + 'the check never reached what the claim is about, so this is a verdict on the environment ' + + 'and not on the claim' export interface CheckExecution { exitCode: number stdout: string stderr: string + /** + * True when the executor killed the check at its deadline. The deadline is a budget, not a + * verdict: a check that ran out of time never tested the claim. + */ + timedOut?: boolean } /** A verdict with the reason a grader may report to the claim's author. */ @@ -195,11 +234,27 @@ export interface ClaimGrade { verdict: ClaimVerdict /** Present when the verdict is a refusal the author can fix, absent otherwise. */ note?: string + /** + * The id of the earlier claim in the same grading pass that carries this claim's check, + * expectation, title and verdict. Present only on the later claim, and it does not change the + * verdict: N claims sharing one check are one verification counted N times, which is a + * check-quality defect for a reader to see rather than a result to drop. + */ + duplicateOf?: string } /** * The calibrated verdict function, pure so every grader shares one semantics. Callers execute the * check however their environment requires and pass the observation; this function only judges. + * + * Three rules decide a verdict before the expectation is compared, and each one exists because + * comparing first blamed the claim for something the claim did not do: + * + * - a check that carries its own expected value is refused, because its output is authored + * rather than measured; + * - a check killed at its deadline is `unrunnable`, because a budget is not a verdict; + * - a check whose output says it never reached its input is `unrunnable` whatever its exit + * status, because a pipeline hides the failing stage's status behind its last stage. */ export function gradeFor( evidence: Pick, @@ -207,24 +262,104 @@ export function gradeFor( ): ClaimGrade { const mustBeCheckable = evidence.rung >= CHECKABLE_RUNG_THRESHOLD if (mustBeCheckable) { - const note = checkRefusalNote(evidence.check) + const note = checkRefusalNote(evidence.check) ?? selfCertifyingNote(evidence) if (note) return { verdict: 'uncheckable', note } } if (!execution) return { verdict: 'unrunnable' } const output = `${execution.stdout}\n${execution.stderr}`.trim() - if (execution.exitCode !== 0) { - return { verdict: UNRUNNABLE_SIGNATURES.test(output) ? 'unrunnable' : 'contradicted' } + if (execution.timedOut || execution.exitCode === DEADLINE_EXIT_CODE) { + return { verdict: 'unrunnable', note: DEADLINE_NOTE } } + if (execution.exitCode !== 0) return refutation(output, evidence.expect) if (mustBeCheckable) { const note = expectationRefusalNote(evidence.expect) if (note) return { verdict: 'uncheckable', note } } if (evidence.expect && !output.includes(evidence.expect)) { - return { verdict: output === '' ? 'silent-check' : 'contradicted' } + if (output === '') return { verdict: 'silent-check' } + return refutation(output, evidence.expect) } return { verdict: 'verified' } } +/** + * Decide between a claim verdict and an environment verdict for output that failed to establish + * the claim. The signature list only ever downgrades a refutation to `unrunnable`: it can turn a + * measurement that did not happen into an honest "not measured", and it can never turn a real + * failure into a pass. + * + * The expectation is consulted first. A claim that PREDICTED the error is genuinely contradicted + * when the error does not appear, so an expectation naming a signature switches this guard off + * for that claim rather than letting it hide the one case it would grade backwards. + */ +function refutation(output: string, expect: string | undefined): ClaimGrade { + if (expect && UNRUNNABLE_SIGNATURES.test(expect)) return { verdict: 'contradicted' } + const hit = UNRUNNABLE_SIGNATURES.exec(output) + if (!hit) return { verdict: 'contradicted' } + return { verdict: 'unrunnable', note: `${UNREACHED_INPUT_NOTE} (${hit[0]})` } +} + +/** A claim and the observation a grader made of it, as one grading pass sees them. */ +export interface GradeableClaim { + /** Stable identity a duplicate flag can point back to, such as the page or claim id. */ + id: string + /** The claim's own sentence. Two claims that share a check but say different things differ. */ + title?: string + evidence: Pick + execution: CheckExecution | null +} + +/** A grade with the claim it belongs to, in the order the claims were supplied. */ +export interface GradedClaim extends ClaimGrade { + id: string +} + +/** + * Grade one pass of claims and flag the repeats within it. + * + * `gradeFor` judges one claim and cannot see a second claim carrying the same check, so a run + * that recorded one verification under several titles reports as several independent + * verifications. This function grades each claim exactly as `gradeFor` does and marks the later + * claim with `duplicateOf`, leaving the verdict and the count untouched: the flag is for a reader + * deciding how much evidence a run really produced. + */ +export function gradeClaims(claims: readonly GradeableClaim[]): GradedClaim[] { + const firstSeen = new Map() + return claims.map((claim) => { + const grade = gradeFor(claim.evidence, claim.execution) + const key = claimCheckKey({ ...claim.evidence, title: claim.title }) + if (!key) return { id: claim.id, ...grade } + const seenAt = `${key}\u0000${grade.verdict}` + const canonical = firstSeen.get(seenAt) + if (canonical === undefined) { + firstSeen.set(seenAt, claim.id) + return { id: claim.id, ...grade } + } + return { id: claim.id, ...grade, duplicateOf: canonical } + }) +} + +/** + * The identity two claims must share to be one verification counted twice: the same command, the + * same expectation, and the same sentence. A check shared across claims that say different things + * is one instrument used several times, which is not a duplicate. + * + * Returns `undefined` when the claim cannot be keyed at all — no check, or no title to compare — + * so an unkeyable claim is never flagged as a repeat of another unkeyable one. The key is JSON + * rather than a joined string, so no separator can collide with text inside a check. + */ +export function claimCheckKey(claim: { + check?: string + expect?: string + title?: string +}): string | undefined { + const check = claim.check?.trim() + if (!check) return undefined + const title = claim.title?.trim().replace(/\s+/g, ' ').toLowerCase().replace(/\.md$/, '') + if (!title) return undefined + return JSON.stringify([check, claim.expect?.trim() ?? '', title]) +} + /** The verdict alone, for graders that report a lattice member and not a reason. */ export function verdictFor( evidence: Pick, @@ -282,6 +417,38 @@ export async function verifyGradeableEvidence( } } +const WORD_CHARACTER = /[A-Za-z0-9_]/ + +/** + * Whether the check text carries the value it is graded against. + * + * A check that contains its own expectation prints a value the claim's subject did not have to + * produce, so the comparison that follows tests the author's typing rather than the artifact. + * The test is the whole trimmed expectation appearing verbatim in the command, which is narrow + * on purpose: it costs an author only a rewrite of the command, and it catches the shape + * `isConstantEmitter` cannot, where a real command's arguments already spell the answer. + * + * An occurrence glued to a word character on a side where the expectation is itself a word + * character does not count. Without that, a short expectation refuses the check that reads it: + * `expect: 6` appears inside `node report6.mjs`, and refusing a correct claim is the cost this + * file's rules are calibrated to avoid. + */ +function selfCertifyingNote(evidence: Pick): string | undefined { + const wanted = evidence.expect?.trim() + const check = evidence.check + if (!check || !wanted) return undefined + const startsInsideWord = WORD_CHARACTER.test(wanted[0] as string) + const endsInsideWord = WORD_CHARACTER.test(wanted[wanted.length - 1] as string) + for (let at = check.indexOf(wanted); at !== -1; at = check.indexOf(wanted, at + 1)) { + const before = check[at - 1] + const after = check[at + wanted.length] + if (startsInsideWord && before !== undefined && WORD_CHARACTER.test(before)) continue + if (endsInsideWord && after !== undefined && WORD_CHARACTER.test(after)) continue + return SELF_CERTIFYING_NOTE + } + return undefined +} + function checkRefusalNote(check: string | undefined): string | undefined { if (!check) return NO_CHECK_NOTE if (isConstantEmitter(check)) return CONSTANT_EMITTER_NOTE @@ -293,11 +460,6 @@ function checkRefusalNote(check: string | undefined): string | undefined { function expectationRefusalNote(expect: string | undefined): string | undefined { if (!expect?.trim()) return NO_EXPECTATION_NOTE if (expect.includes('\n') || expect.includes('\r')) return MULTILINE_EXPECTATION_NOTE - const keyValueTokens = expect - .trim() - .split(/\s+/) - .filter((token) => token.includes('=')) - if (keyValueTokens.length >= 3) return MULTI_VALUE_EXPECTATION_NOTE return undefined } @@ -320,10 +482,14 @@ async function runBash( }, (error, stdout, stderr) => { const diagnostic = error && typeof error.code !== 'number' ? `${stderr}\n${error}` : stderr + // A process killed at the deadline reports no exit status of its own, and its name + // separates the deadline from a caller's abort, which is a different verdict. + const timedOut = error?.killed === true && error.name !== 'AbortError' resolve({ exitCode: typeof error?.code === 'number' ? error.code : error ? 127 : 0, stdout: String(stdout ?? ''), stderr: String(diagnostic ?? ''), + ...(timedOut ? { timedOut: true } : {}), }) }, ) diff --git a/src/index.ts b/src/index.ts index a2db178..51ee2f4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,6 +37,7 @@ export * from './lint' export * from './material-facts-metric' export * from './memory/index' export type { + KnowledgeMutationHold, KnowledgeMutationLock, KnowledgeMutationOptions, KnowledgeReadOptions, @@ -45,7 +46,9 @@ export type { } from './mutation-lock' export { inspectPendingKnowledgeMutation, + isKnowledgeMutationHeld, recoverPendingKnowledgeMutation, + runInKnowledgeMutationScope, withKnowledgeMutation, withKnowledgeRead, } from './mutation-lock' diff --git a/src/mutation-lock.ts b/src/mutation-lock.ts index 5132a50..eb56a29 100644 --- a/src/mutation-lock.ts +++ b/src/mutation-lock.ts @@ -68,6 +68,17 @@ interface DurableFileLock { } export interface KnowledgeMutationOptions { + /** + * How long a lock may go without a heartbeat before another taker treats it as abandoned. + * Defaults to 15 minutes, and is floored at 5 seconds. The holder heartbeats at a third of this + * window, so the window is also the bound on how long a crashed holder wedges every other + * writer: a process that dies mid-write blocks the next one for up to `staleMs`. + * + * The default is sized for a knowledge-improvement run that holds the lock across agent turns. + * A consumer whose holds are short should say so and take the shorter wedge — measured holds of + * this lock are 9 ms for one page, 586 ms for a 50-page batch, and about 390 ms for the longest + * promotion in the largest store, against a 900 s default. + */ staleMs?: number resumeTransaction?: { purpose: string @@ -99,20 +110,42 @@ export interface KnowledgeReadOptions { waitMs?: number } +/** + * A store lock over a knowledge root that a caller took by its own path, and the one question + * this package asks of it. + * + * A consumer that batches its own writes around a read-modify-write takes the lock itself and + * then needs to call this package inside that window. It supplies the hold; this package supplies + * the scope, so a nested `withKnowledgeMutation` on the same root runs inline instead of blocking + * against a lock the caller already owns. + */ +export interface KnowledgeMutationHold { + /** + * Throw when the lock is no longer owned. Every mutation asks before it begins and again after + * it returns, so a lock lost to a stale-timeout takeover stops the write rather than letting it + * land under a lock somebody else now holds. + */ + assertOwned(): void +} + +/** + * Whether the current async context already holds the store lock for `root`. + * + * A caller that must decide between taking the lock and joining the one it is inside reads this + * rather than guessing; taking a held lock from inside its own scope would block against itself. + */ +export function isKnowledgeMutationHeld(root: string): boolean { + return activeRoots.getStore()?.get(resolve(root))?.active === true +} + export async function withKnowledgeMutation( root: string, mutate: (lock: KnowledgeMutationLock) => Promise | T, options: KnowledgeMutationOptions = {}, ): Promise { const resolvedRoot = resolve(root) - const active = activeRoots.getStore() - const existing = active?.get(resolvedRoot) - if (existing?.active) { - existing.lock.assertOwned() - const result = await mutate(existing.lock) - existing.lock.assertOwned() - return result - } + const existing = activeRoots.getStore()?.get(resolvedRoot) + if (existing?.active) return await runInlineMutation(existing.lock, mutate) return withSafeDirectory(resolvedRoot, '.agent-knowledge', true, async (cacheDir) => { const acquired = await acquireDurableFileLock(resolvedRoot, { @@ -122,83 +155,136 @@ export async function withKnowledgeMutation( options.retries ?? ({ retries: 100, factor: 1.1, minTimeout: 10, maxTimeout: 200, randomize: true } as const), }) - let recovery: KnowledgeMutationRecovery | undefined - const mutationLock: KnowledgeMutationLock = { - transactionRoot: join(cacheDir, 'file-transactions'), - get recovery() { - return recovery - }, - assertOwned: acquired.assertOwned, - } - const scope: KnowledgeMutationScope = { active: true, lock: mutationLock } try { - const pendingState = await inspectKnowledgeFileTransaction({ - root: resolvedRoot, - transactionRoot: mutationLock.transactionRoot, - }) - const pending = pendingState?.transaction ?? null - const resume = pending - ? assertKnowledgeTransactionResume(pending, options.resumeTransaction) - : undefined - const locks = new Map(active) - locks.set(resolvedRoot, scope) - return await activeRoots.run(locks, async () => { - const epoch = await beginMutationEpoch(cacheDir) - const finishEpoch = async (completed: boolean) => { - mutationLock.assertOwned() - const stillPending = await loadKnowledgeFileTransaction({ + return await runOwnedMutation(resolvedRoot, cacheDir, acquired, options, mutate) + } finally { + await acquired.release() + } + }) +} + +/** + * Run `body` inside this package's mutation scope for `root`, on a lock the caller already holds. + * + * The caller owns acquiring and releasing that lock; this package owns the scope, the file + * transaction it opens, and the epoch a concurrent reader watches. Inside `body`, every + * lock-taking function in this package sees the root as held and runs inline, which is what makes + * a consumer's own read-modify-write composable with this package instead of exclusive of it. + * + * The hold is asked `assertOwned()` at the same points a lock this package acquired is asked, so + * an externally held lock carries the same guarantee and not a weaker one. Entering a scope for a + * lock the caller does not actually hold is the one way to defeat the store's single-writer rule; + * the hold is required, rather than assumed, so that is a statement the caller has to make. + */ +export async function runInKnowledgeMutationScope( + root: string, + hold: KnowledgeMutationHold, + body: (lock: KnowledgeMutationLock) => Promise | T, + options: KnowledgeMutationOptions = {}, +): Promise { + const resolvedRoot = resolve(root) + const existing = activeRoots.getStore()?.get(resolvedRoot) + if (existing?.active) return await runInlineMutation(existing.lock, body) + + return withSafeDirectory(resolvedRoot, '.agent-knowledge', true, (cacheDir) => + runOwnedMutation(resolvedRoot, cacheDir, hold, options, body), + ) +} + +/** The reentrant path: the root is already held in this async context, so no lock is taken. */ +async function runInlineMutation( + lock: KnowledgeMutationLock, + mutate: (lock: KnowledgeMutationLock) => Promise | T, +): Promise { + lock.assertOwned() + const result = await mutate(lock) + lock.assertOwned() + return result +} + +/** The owning path, shared by a lock this package acquired and a lock the caller supplies. */ +async function runOwnedMutation( + resolvedRoot: string, + cacheDir: string, + hold: KnowledgeMutationHold, + options: KnowledgeMutationOptions, + mutate: (lock: KnowledgeMutationLock) => Promise | T, +): Promise { + let recovery: KnowledgeMutationRecovery | undefined + const mutationLock: KnowledgeMutationLock = { + transactionRoot: join(cacheDir, 'file-transactions'), + get recovery() { + return recovery + }, + assertOwned: () => hold.assertOwned(), + } + const scope: KnowledgeMutationScope = { active: true, lock: mutationLock } + try { + const pendingState = await inspectKnowledgeFileTransaction({ + root: resolvedRoot, + transactionRoot: mutationLock.transactionRoot, + }) + const pending = pendingState?.transaction ?? null + const resume = pending + ? assertKnowledgeTransactionResume(pending, options.resumeTransaction) + : undefined + const locks = new Map(activeRoots.getStore()) + locks.set(resolvedRoot, scope) + return await activeRoots.run(locks, async () => { + const epoch = await beginMutationEpoch(cacheDir) + const finishEpoch = async (completed: boolean) => { + mutationLock.assertOwned() + const stillPending = await loadKnowledgeFileTransaction({ + root: resolvedRoot, + transactionRoot: mutationLock.transactionRoot, + }) + if (completed && stillPending) { + throw new Error('knowledge mutation returned with an unfinished transaction') + } + if (!stillPending) await finishMutationEpoch(cacheDir, epoch) + } + try { + if (pending) { + if (!resume) + throw new Error( + `knowledge transaction '${pending.purpose}' requires its owner to resume`, + ) + await recoverKnowledgeFileTransaction({ root: resolvedRoot, transactionRoot: mutationLock.transactionRoot, + expectedPurpose: resume.purpose, + direction: resume.direction, + finish: resume.deferFinish !== true, + validate: resume.validate, + assertOwned: mutationLock.assertOwned, + }) + recovery = Object.freeze({ + transactionId: pending.transactionId, + purpose: pending.purpose, + direction: resume.direction ?? pendingState?.direction ?? 'apply', + transaction: pending, }) - if (completed && stillPending) { - throw new Error('knowledge mutation returned with an unfinished transaction') - } - if (!stillPending) await finishMutationEpoch(cacheDir, epoch) } + mutationLock.assertOwned() + const result = await mutate(mutationLock) + mutationLock.assertOwned() + await finishEpoch(true) + return result + } catch (error) { try { - if (pending) { - if (!resume) - throw new Error( - `knowledge transaction '${pending.purpose}' requires its owner to resume`, - ) - await recoverKnowledgeFileTransaction({ - root: resolvedRoot, - transactionRoot: mutationLock.transactionRoot, - expectedPurpose: resume.purpose, - direction: resume.direction, - finish: resume.deferFinish !== true, - validate: resume.validate, - assertOwned: acquired.assertOwned, - }) - recovery = Object.freeze({ - transactionId: pending.transactionId, - purpose: pending.purpose, - direction: resume.direction ?? pendingState?.direction ?? 'apply', - transaction: pending, - }) - } - mutationLock.assertOwned() - const result = await mutate(mutationLock) - mutationLock.assertOwned() - await finishEpoch(true) - return result - } catch (error) { - try { - await finishEpoch(false) - } catch (finishError) { - throw new AggregateError( - [error, finishError], - 'knowledge mutation failed and its durable state could not be inspected', - ) - } - throw error + await finishEpoch(false) + } catch (finishError) { + throw new AggregateError( + [error, finishError], + 'knowledge mutation failed and its durable state could not be inspected', + ) } - }) - } finally { - scope.active = false - await acquired.release() - } - }) + throw error + } + }) + } finally { + scope.active = false + } } export async function inspectPendingKnowledgeMutation( diff --git a/tests/mutation-lock.test.ts b/tests/mutation-lock.test.ts index 657192c..fd7285a 100644 --- a/tests/mutation-lock.test.ts +++ b/tests/mutation-lock.test.ts @@ -2,6 +2,7 @@ import { chmod, mkdir, mkdtemp, readdir, readFile, rm, writeFile } from 'node:fs import { tmpdir } from 'node:os' import { join } from 'node:path' import { setTimeout as delay } from 'node:timers/promises' +import lockfile from 'proper-lockfile' import { describe, expect, it } from 'vitest' import { applyKnowledgeFileTransaction, @@ -10,7 +11,13 @@ import { } from '../src/file-transaction' import { inspectPendingKnowledgeMutation, recoverPendingKnowledgeMutation } from '../src/index' import { buildKnowledgeIndex } from '../src/indexer' -import { withKnowledgeMutation } from '../src/mutation-lock' +import { + isKnowledgeMutationHeld, + type KnowledgeMutationHold, + runInKnowledgeMutationScope, + withKnowledgeMutation, + withKnowledgeRead, +} from '../src/mutation-lock' import { loadSourceRegistry } from '../src/sources' import { initKnowledgeBase } from '../src/store' @@ -297,3 +304,123 @@ async function chmodTree( } await chmod(root, directoryMode) } + +describe('an externally held store lock can be joined instead of blocked against', () => { + async function takeTheLockOutside(root: string): Promise<{ + hold: KnowledgeMutationHold + release: () => Promise + }> { + // What a consumer with its own lock wrapper does: the same lockfile this package uses, taken + // by a path this package did not hand out. + await mkdir(join(root, '.agent-knowledge'), { recursive: true }) + let compromised: Error | undefined + const release = await lockfile.lock(root, { + lockfilePath: join(root, '.agent-knowledge', 'mutation.lock.durable'), + realpath: false, + stale: 30_000, + update: 10_000, + onCompromised(error) { + compromised = error + }, + }) + return { + hold: { + assertOwned() { + if (compromised) throw compromised + }, + }, + release, + } + } + + it('blocks a nested mutation without the scope, and runs it inline inside it', async () => { + await withRoot(async (root) => { + const { hold, release } = await takeTheLockOutside(root) + try { + // The self-block: the caller holds the lock, so this package cannot take it and the + // consumer's only option was to forbid itself from calling this package while holding it. + await expect( + withKnowledgeMutation(root, () => 'never reached', { retries: 0 }), + ).rejects.toThrow() + + const order: string[] = [] + const result = await runInKnowledgeMutationScope(root, hold, async () => { + order.push('outer') + const inner = await withKnowledgeMutation(root, () => { + order.push('inner') + return 'inner ran' + }) + order.push('after') + return inner + }) + + expect(result).toBe('inner ran') + expect(order).toEqual(['outer', 'inner', 'after']) + } finally { + await release() + } + }) + }) + + it('leaves the epoch closed, so a reader is not left waiting on it', async () => { + await withRoot(async (root) => { + const { hold, release } = await takeTheLockOutside(root) + try { + await runInKnowledgeMutationScope(root, hold, async () => { + await withKnowledgeMutation(root, () => undefined) + }) + } finally { + await release() + } + await expect(withKnowledgeRead(root, () => 'read', { retries: 2 })).resolves.toBe('read') + }) + }) + + it('asks the hold whether the lock is still owned, and stops the write when it is not', async () => { + await withRoot(async (root) => { + const { release } = await takeTheLockOutside(root) + try { + let entered = false + const lost: KnowledgeMutationHold = { + assertOwned() { + if (entered) throw new Error('the external lock was lost') + }, + } + // A lost hold reaches the caller the same way a lock this package acquired and lost does: + // the mutation fails, and the aggregate carries the loss the hold reported. + const failure = await runInKnowledgeMutationScope(root, lost, () => { + entered = true + return 'wrote under a lock somebody else holds' + }).catch((error: unknown) => error) + expect(failure).toBeInstanceOf(AggregateError) + expect((failure as AggregateError).errors.map(String)).toContain( + 'Error: the external lock was lost', + ) + } finally { + await release() + } + }) + }) + + it('reports whether this async context holds the root', async () => { + await withRoot(async (root) => { + expect(isKnowledgeMutationHeld(root)).toBe(false) + + await withKnowledgeMutation(root, async () => { + expect(isKnowledgeMutationHeld(root)).toBe(true) + expect(isKnowledgeMutationHeld(join(root, 'other'))).toBe(false) + }) + expect(isKnowledgeMutationHeld(root)).toBe(false) + + const { hold, release } = await takeTheLockOutside(root) + try { + await runInKnowledgeMutationScope(root, hold, async () => { + expect(isKnowledgeMutationHeld(root)).toBe(true) + }) + } finally { + await release() + } + expect(isKnowledgeMutationHeld(root)).toBe(false) + }) + }) +})