diff --git a/.spiral/cycles/CYC-20260919-TTZ7C-36.md b/.spiral/cycles/CYC-20260919-TTZ7C-36.md new file mode 100644 index 0000000..053d08d --- /dev/null +++ b/.spiral/cycles/CYC-20260919-TTZ7C-36.md @@ -0,0 +1,56 @@ +# Cycle: Reliable Runtime Contracts And Readable Workflows + +Status: Accepted. Human accepted on 2026-09-19 after cycle evaluation. +Branch: spiral/CYC-20260919-TTZ7C-36-runtime-contracts. +Base: a60545cc68058302d82ba5dcd1847fe722396e51 (master). + +Goal: correct the three demonstrated refactor regressions and make the affected +runtime, inspection and recovery workflows easier to reason about. + +Plan continuity: deliberately precede the remaining randomized/soak roadmap +work with this maintainer-requested correction. SRC-001, REQ-001, DES-001 and +SRC-20260919-TTZ7C-18 / REQ-20260919-TTZ7C-20 remain governing durability +direction. The accepted power-loss implementation and runtime refactor are in +place; this cycle repairs their lifecycle boundaries before broader testing. + +Scope: concurrent shutdown completion; acceptance failure checks; duplicate-ID +compatibility; same-level inspection and recovery coordinating methods; +regression evidence; the already requested local acceptance/integration rule. + +Non-goals: format changes, migrations, automatic rerun, new recovery policy, +dependency upgrades, worker-pool redesign, VM2 replacement, full rebuild or +broader randomized/filesystem/soak testing. Keep the existing classes and public +functions. A newly discovered regression caused by this work remains in scope. + +Evaluation: reproduce the failures before repair; prove runtime contracts with +controlled interleavings and real process shutdown evidence; rerun the existing +112-test regression suite and focused lint; review abstraction consistency and +validate causal history. Stop at human cycle acceptance. Integration is separate. + +Pause/replan if fixes require a changed durability/recovery contract or new +public/disk format. The prior review plus explicit cycle instruction satisfy +the intent/reality checkpoint; no second confirmation is required. + +## Evaluation + +Implementation and human acceptance are complete. All 118 regression +tests and focused lint pass. The six new tests first failed against unchanged +product code, then passed after repair. EVD-20260919-TTZ7C-41 records the baseline, +acceptance matrix and final implementation verification. Review confirms the +same-level workflow changes preserve sequencing, audit and cleanup semantics. +No scope expansion, new class, dependency or format was needed. + +The branch identity has been checked before each semantic commit. The existing +AGENTS.md acceptance/integration clarification was preserved and committed here. +Do not merge into master without separate integration authorization. + +## Human acceptance + +After reviewing the cycle outcome and its validation, the maintainer replied: + +> accepted + +This accepts the evaluated branch tip +`061571e48c453e5f3d7086a33af0d94926b2f7ea`. The cycle is closed and ready to +propose for integration. Integration into master is not authorized by this +acceptance; the branch and its immutable causal history remain intact. diff --git a/.spiral/cycles/CYC-20260919-TTZ7C-36.ttl b/.spiral/cycles/CYC-20260919-TTZ7C-36.ttl new file mode 100644 index 0000000..2c60eda --- /dev/null +++ b/.spiral/cycles/CYC-20260919-TTZ7C-36.ttl @@ -0,0 +1,19 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . + +project:CYC-20260919-TTZ7C-36 + a sd:Cycle ; + dcterms:identifier "CYC-20260919-TTZ7C-36" ; + sd:repositoryPath ".spiral/cycles/CYC-20260919-TTZ7C-36.md" ; + sd:status sd:Accepted ; + sd:derivedFrom [ + a sd:ArtifactReference ; + sd:artifact project:REQ-20260919-TTZ7C-39 ; + sd:gitCommit "13f9b33f3fe7d7dc97ad9e3b53870764685b95bd" + ] ; + sd:transforms [ + a sd:ArtifactReference ; + sd:artifact project:CYC-20260919-TTZ7C-36 ; + sd:gitCommit "061571e48c453e5f3d7086a33af0d94926b2f7ea" + ] . diff --git a/.spiral/designs/DES-20260919-TTZ7C-40.md b/.spiral/designs/DES-20260919-TTZ7C-40.md new file mode 100644 index 0000000..ce6091b --- /dev/null +++ b/.spiral/designs/DES-20260919-TTZ7C-40.md @@ -0,0 +1,62 @@ +# Design: Complete Lifecycle Contracts + +Keep DES-20260919-TTZ7C-32 as the broader runtime architecture. This design adds +explicit contracts where its refactor lost ordering and completion semantics. + +## Corrections + +Close owns one completion promise. The first call synchronously stops admission +and starts resource closure; all subsequent calls await the same outcome, +including rejection. A separate closeResources() holds the existing ordered +acceptance/runner/pool/ownership cleanup. Preserve retention after storage +failure. Do not add another server-only shutdown guard: it would mask a broken +runtime contract for other callers. The server may safely await close from both +signal handlers. + +After persisting acceptance and passing its existing fault point, check storage +health again before queue insertion and success return. A concurrent command's +failure may invalidate admission while an append is awaited. Retain any records +already written; never roll back evidence or automatically rerun. Reject later +admission through the existing failed state. + +Command preparation parses and validates identity, then recognizes an existing +ID before validating a new command's name or serializing it. Move the existing +duplicate-result branch into that position rather than adding a second lookup +path or weakening validation of new commands. + +## Readability + +StoreInspector.inspect() coordinates path validation, inventory, command +history, integrity, reconstruction, artifact checks and stable-snapshot/report +creation. loadCommandHistory() groups reading log/status records, constructing +commands and validating committed order. Prefix validity is initialized where +reconstruction begins, after integrity/history diagnostics are available. + +RecoveryApplication.runApprovedCommand() coordinates freshness, selection, +activation, execution, result assessment and commit. markCommandActive() owns +attempt numbering, audit-before-status ordering and status serialization. Keep +recordAttempt() as the existing audit operation. No new classes or generic +workflow engine; no changes to ordering, reports, locks or integrity checks. + +## Verification and risk + +Reproduce regression tests before changing product code. Use controlled promise +barriers for command execution and ownership release, test concurrent close +success/failure, and prove rejection of admission while closing. Inject failure +at the acceptance persistence boundaries and assert no enqueue or success, +retained records and ownership. Verify known ID-only retries before and after +execution while rejecting unknown incomplete commands. + +Use a subprocess with a command blocked on an explicit release file to deliver +both shutdown signals before allowing completion. Observe signal handling, then +release the command, require done status and released locks, and restart/query +the committed state. Bound test waits and release barriers during cleanup. + +All existing runtime, offset, startup, ACID, crash, handler, power-loss and +administrator/storage suites remain required. Read the final coordinator code +as well as running lint; method length alone is not proof of useful abstraction. + +The observed failure is composition risk (adopted WPF-HUMAN-001-W4); verify the +server/runtime interaction, rather than treating isolated final-state tests as +sufficient evidence. The existing acceptance/integration clarification controls +authority drift locally; no process-core or culture changes are required. diff --git a/.spiral/designs/DES-20260919-TTZ7C-40.ttl b/.spiral/designs/DES-20260919-TTZ7C-40.ttl new file mode 100644 index 0000000..47cb0b9 --- /dev/null +++ b/.spiral/designs/DES-20260919-TTZ7C-40.ttl @@ -0,0 +1,24 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . +@prefix culture: . + +project:DES-20260919-TTZ7C-40 + a sd:Design ; + dcterms:identifier "DES-20260919-TTZ7C-40" ; + sd:repositoryPath ".spiral/designs/DES-20260919-TTZ7C-40.md" ; + sd:status sd:Active ; + sd:satisfies [ + a sd:ArtifactReference ; + sd:artifact project:REQ-20260919-TTZ7C-39 ; + sd:gitCommit "13f9b33f3fe7d7dc97ad9e3b53870764685b95bd" + ] ; + sd:shapedBy [ + a sd:ArtifactReference ; + sd:artifact culture:CUL-MUZE-001 ; + sd:gitCommit "9958f0e296e84169c21c02aa478ac4aae5a06a41" + ], [ + a sd:ArtifactReference ; + sd:artifact culture:CUL-MUZE-LIB-001 ; + sd:gitCommit "9958f0e296e84169c21c02aa478ac4aae5a06a41" + ] . diff --git a/.spiral/evidence/EVD-20260919-TTZ7C-41.md b/.spiral/evidence/EVD-20260919-TTZ7C-41.md new file mode 100644 index 0000000..d375968 --- /dev/null +++ b/.spiral/evidence/EVD-20260919-TTZ7C-41.md @@ -0,0 +1,59 @@ +# Evidence: Runtime Contract Corrections + +## Baseline before product changes + +Test additions against the unchanged runtime at 665a97d reproduce all three +review findings. Running node --test --test-reporter=tap test/store-runtime.mjs +produces six passes (existing tests) and six failures (new tests): + +- overlapping close resolves before the active command finishes; +- overlapping close reports success despite ownership-release failure; +- storage failure at each of two acceptance boundaries still returns success; +- a known ID-only retry returns 422 rather than its existing status; +- SIGTERM followed by SIGINT exits with command status active, rather than done. + +The process test holds execution on an explicit release file, observes both +signals, then checks status and ownership and restarts the store. Direct tests +use controlled promises and real temporary log files. No product file changed +before this baseline run. The full pre-cycle review suite passed 112 tests; +these failures demonstrate its missing interleaving/compatibility coverage. + +## Final implementation verification + +Implementation at 5d9e81218d5a51d8d3171f7132e73c2e4bfc298a passes all 118 tests: the existing 112 plus +six new runtime contract cases. No failures, skips or TODOs. Command: + +```sh +node --test --test-reporter=tap test/store-runtime.mjs \ + test/durability-startup.mjs test/conversion-offsets.mjs \ + test/command-offsets.mjs test/acid-baseline.mjs \ + test/durability-crash.mjs test/after-change-handlers.mjs \ + test/power-loss-baseline.mjs test/storage.mjs test/admin-recovery.mjs +``` + +Local-server suites require socket access outside this environment's restricted +sandbox; the run was authorized and used disposable stores. Focused ESLint passes +for the three modified production modules and test/store-runtime.mjs. Staged +provenance and whitespace validation passed before every semantic commit. + +| Criterion | Result | +| --- | --- | +| R1 Closure | Controlled active execution blocks both close callers. Both observe cleanup rejection; repeated close retains that failure. The real process receives SIGTERM and SIGINT before its command is released, then records done, releases ownership and restarts with the committed value. | +| R2 Acceptance | Injected storage failure after log persistence or after accepted-status persistence rejects success and leaves the queue empty. Logged command evidence remains; subsequent admission is refused and ownership retained. | +| R3 Compatibility | A known ID-only retry returns accepted or done status without a second append/execution. An unknown ID without a name still returns 422. | +| R4 Readability | inspect() now lists inspection phases; prefix bookkeeping lives in reconstruction. runApprovedCommand() delegates activation numbering, audit and status serialization to markCommandActive(). No workflow order or report shape changed. | +| R5 Preservation | Full startup, offset, ACID, crash, handler, power-loss and administrative suites pass. | +| R6 Review boundary | The prior AGENTS.md edit is committed on the new cycle branch. Cycle acceptance and integration remain separate; master is unchanged. | + +Production diff: runtime +8, inspection +4, recovery +6 net lines. Three +supporting methods clarify existing concerns; no new classes or dependencies. +The new tests add 209 net lines, including controlled interleavings and the +subprocess/restart proof. The failing baseline remains in Git at 959e2cd92181be07f3fe38d38328f10ba41923b3. + +## Evaluation limits + +This is evidence for the specified lifecycle contracts and preservation of the +covered behaviors, not a new filesystem/hardware durability claim. Randomized +and destructive soak work remains deferred under the governing roadmap. +Human cycle acceptance is pending. Integration is not authorized by acceptance +alone. diff --git a/.spiral/evidence/EVD-20260919-TTZ7C-41.ttl b/.spiral/evidence/EVD-20260919-TTZ7C-41.ttl new file mode 100644 index 0000000..0d99d6a --- /dev/null +++ b/.spiral/evidence/EVD-20260919-TTZ7C-41.ttl @@ -0,0 +1,19 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . + +project:EVD-20260919-TTZ7C-41 + a sd:VerificationEvidence ; + dcterms:identifier "EVD-20260919-TTZ7C-41" ; + sd:repositoryPath ".spiral/evidence/EVD-20260919-TTZ7C-41.md" ; + sd:status sd:Active ; + sd:verifies [ + a sd:ArtifactReference ; + sd:artifact project:IMP-20260919-TTZ7C-33 ; + sd:gitCommit "5d9e81218d5a51d8d3171f7132e73c2e4bfc298a" + ] ; + sd:transforms [ + a sd:ArtifactReference ; + sd:artifact project:EVD-20260919-TTZ7C-41 ; + sd:gitCommit "959e2cd92181be07f3fe38d38328f10ba41923b3" + ] . diff --git a/.spiral/implementations/IMP-20260919-TTZ7C-33.md b/.spiral/implementations/IMP-20260919-TTZ7C-33.md index 298422a..704e8e2 100644 --- a/.spiral/implementations/IMP-20260919-TTZ7C-33.md +++ b/.spiral/implementations/IMP-20260919-TTZ7C-33.md @@ -78,3 +78,26 @@ restore candidates retain their locks and audit evidence; source ownership is released under the same conditions. Existing duplicate reads and later-dataset queries in inspection are retained to avoid mixing race or performance semantics into the readability refactor. + +## Cycle 36: complete lifecycle contracts + +DES-20260919-TTZ7C-40 supplements the existing architecture. All close callers +now await one completion promise, including cleanup rejection. The server can +await it from both signal handlers without prematurely exiting. Acceptance +rechecks storage health after persistence and before queueing/acknowledgment. +Command preparation recognizes an existing valid ID before applying new-command +name requirements, restoring ID-only retry compatibility. + +StoreInspector groups log/status interpretation in loadCommandHistory() and +initializes prefix validity inside reconstruction. RecoveryApplication groups +attempt numbering, audit persistence and active-status serialization in +markCommandActive(). The coordinating methods stay at workflow level; the order +of checks, writes, parsing, audit and cleanup remains unchanged. No new classes +or dependencies are introduced. + +The preceding implementation version added six failing regression cases before +product changes. Those tests now pass: overlapping closure waits for execution +and shares cleanup failure; both persistence-boundary failures reject acceptance; +known ID-only retries return status while incomplete new commands fail; and a +real process receiving both shutdown signals commits, unlocks and restarts. +EVD-20260919-TTZ7C-41 preserves the baseline and records final suite results. diff --git a/.spiral/implementations/IMP-20260919-TTZ7C-33.ttl b/.spiral/implementations/IMP-20260919-TTZ7C-33.ttl index 566725c..bee07a8 100644 --- a/.spiral/implementations/IMP-20260919-TTZ7C-33.ttl +++ b/.spiral/implementations/IMP-20260919-TTZ7C-33.ttl @@ -19,14 +19,23 @@ project:IMP-20260919-TTZ7C-33 sd:artifact project:DES-20260919-TTZ7C-32 ; sd:gitCommit "f7ced34e7821e3aa02606c2d1abe6f1bb1c08c5b" ] ; - sd:transforms [ + sd:implements [ + a sd:ArtifactReference ; + sd:artifact project:DES-20260919-TTZ7C-40 ; + sd:gitCommit "665a97d1c366b4cce2b9004d6e649a389d18a2c4" + ] ; + sd:changeCausedBy [ + a sd:ArtifactReference ; + sd:artifact project:EVD-20260919-TTZ7C-41 ; + sd:gitCommit "959e2cd92181be07f3fe38d38328f10ba41923b3" + ], [ a sd:ArtifactReference ; - sd:artifact project:IMP-20260919-TTZ7C-26 ; - sd:gitCommit "e31e07c9bc8d4fc6458da1d213609dbf1aa4a7a2" + sd:artifact project:DES-20260919-TTZ7C-40 ; + sd:gitCommit "665a97d1c366b4cce2b9004d6e649a389d18a2c4" ] ; sd:transforms [ a sd:ArtifactReference ; sd:artifact project:IMP-20260919-TTZ7C-33 ; - sd:gitCommit "f0c1616c828db6c63c84cd63dcf4c4f4324188c1" + sd:gitCommit "959e2cd92181be07f3fe38d38328f10ba41923b3" ] ; sd:implementationChangeKind sd:MixedChange . diff --git a/.spiral/project-context.md b/.spiral/project-context.md index 39bedad..ed279f7 100644 --- a/.spiral/project-context.md +++ b/.spiral/project-context.md @@ -18,7 +18,21 @@ Project causal-graph namespace: `https://github.com/simplyedit/simplystore/spira Spiral core source: `.spiral-core/`, git submodule for `https://github.com/muze-labs/spiral-developer.git`, currently checked out at `8d4b2c738a413abd4cccca740ce958f486e5f7af`. -Latest accepted Spiral cycle: `.spiral/cycles/CYC-20260919-TTZ7C-31.md` (`Readable +Latest accepted cycle: CYC-20260919-TTZ7C-36, Reliable Runtime Contracts And Readable +Workflows, on spiral/CYC-20260919-TTZ7C-36-runtime-contracts. The maintainer +confirmed this cycle after independent review found three regressions in the +accepted runtime refactor. It deliberately precedes remaining randomized/soak +work to repair shutdown completion, storage-failure acceptance and duplicate-ID +compatibility, and clarify inspection/recovery coordinators. Source SRC-37, +understanding UND-38, request REQ-39 and design DES-40 use the full +20260919-TTZ7C namespace. The maintainer accepted the evaluated branch tip +061571e48c453e5f3d7086a33af0d94926b2f7ea on 2026-09-19. Integration is pending +a separate decision; no cycle is active. Implementation and acceptance are complete: +118 regression tests and focused lint pass; EVD-20260919-TTZ7C-41 records six +new regression cases and the restored contracts. The production change adds +18 net lines across runtime, inspection and administrative recovery. + +Previously accepted Spiral cycle: `.spiral/cycles/CYC-20260919-TTZ7C-31.md` (`Readable Store Runtime And Dependency Maintenance`), opened on `spiral/CYC-20260919-TTZ7C-31-runtime-boundaries` from `master` at `e31e07c9bc8d4fc6458da1d213609dbf1aa4a7a2`. It deliberately precedes the @@ -27,7 +41,7 @@ exposed by the accepted durability cycle. Its source, understanding, request and design are `SRC-20260919-TTZ7C-28`, `UND-20260919-TTZ7C-29`, `REQ-20260919-TTZ7C-30` and `DES-20260919-TTZ7C-32`. The maintainer accepted the cycle on 2026-09-19. The accepted branch was integrated locally into `master` at -`0d23ed0354fb9f1285d9bc3b15c0a1089583d65b`; no new cycle is active. +`0d23ed0354fb9f1285d9bc3b15c0a1089583d65b`; the correction cycle above is accepted and awaits integration. Accepted-cycle outcome: `IMP-20260919-TTZ7C-33` moves opened-store state and lifecycle into one cohesive `StoreRuntime`, leaving diff --git a/.spiral/requests/REQ-20260919-TTZ7C-39.md b/.spiral/requests/REQ-20260919-TTZ7C-39.md new file mode 100644 index 0000000..e1a9926 --- /dev/null +++ b/.spiral/requests/REQ-20260919-TTZ7C-39.md @@ -0,0 +1,16 @@ +# Request: Correct And Clarify Runtime Boundaries + +Retain the accepted architecture while correcting the reviewed regressions. + +| Criterion | Required evidence | +| --- | --- | +| R1 Closure | Every overlapping/repeated close waits for the same cleanup, including any failure. SIGTERM followed by SIGINT must wait for active commands, release healthy ownership and exit successfully only after completion. | +| R2 Acceptance | If storage fails during persistence, no new success acknowledgment or queue insertion follows. Retain written evidence, reject subsequent admission and keep ownership. | +| R3 Compatibility | Parsed commands with a valid existing ID return the current status before new-command name validation. Incomplete new commands still return 422 and write nothing. | +| R4 Readability | Inspection and approved-command recovery coordinators express phases at a consistent abstraction level; parsing, persistence and prefix bookkeeping belong in cohesive supporting methods. No new class hierarchy or general infrastructure. | +| R5 Preservation | Log order, durable barriers, query publication, recovery predicates, audit reports and lock retention remain covered by existing regression suites. No disk-format or public endpoint changes. | +| R6 Review boundary | Record the already requested AGENTS.md acceptance/integration clarification. Finish at evaluation; human acceptance and integration remain separate. | + +Use deterministic runtime interleavings and a real subprocess signal test in +addition to the existing suites. Test awaited outcomes, not promise identity or +method names. Readability needs human/code review as well as behavioral evidence. diff --git a/.spiral/requests/REQ-20260919-TTZ7C-39.ttl b/.spiral/requests/REQ-20260919-TTZ7C-39.ttl new file mode 100644 index 0000000..f7c95a4 --- /dev/null +++ b/.spiral/requests/REQ-20260919-TTZ7C-39.ttl @@ -0,0 +1,19 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . + +project:REQ-20260919-TTZ7C-39 + a sd:Request ; + dcterms:identifier "REQ-20260919-TTZ7C-39" ; + sd:repositoryPath ".spiral/requests/REQ-20260919-TTZ7C-39.md" ; + sd:status sd:Active ; + sd:derivedFrom [ + a sd:ArtifactReference ; + sd:artifact project:UND-20260919-TTZ7C-38 ; + sd:gitCommit "65b79da346f98c6314d543794e34c8c13e2e5bd9" + ] ; + sd:constrainedBy [ + a sd:ArtifactReference ; + sd:artifact project:REQ-20260919-TTZ7C-20 ; + sd:gitCommit "a60545cc68058302d82ba5dcd1847fe722396e51" + ] . diff --git a/.spiral/sources/SRC-20260919-TTZ7C-37.md b/.spiral/sources/SRC-20260919-TTZ7C-37.md new file mode 100644 index 0000000..e1eb59b --- /dev/null +++ b/.spiral/sources/SRC-20260919-TTZ7C-37.md @@ -0,0 +1,26 @@ +# Source: Runtime Review Corrections + +Maintainer conversation on 2026-09-19; no external conversation locator. + +The maintainer requested an independent review of the accepted runtime refactor +against the Programming for Wizards chapters and Muze engineering philosophy. +The review compared master at a60545c with the preceding cycle baseline and +reported three reproduced regressions: overlapping shutdowns may exit before +completion, acceptance may acknowledge after concurrent storage failure, and +duplicate IDs are now checked after command-name validation. All 112 existing +tests and focused lint passed. These findings are assistant observations, not +human-authored requirements or proof of a broader durability envelope. + +The review supported retaining the runtime and internal recovery classes, while +improving the mixed abstraction levels in StoreInspector.inspect() and +RecoveryApplication.runApprovedCommand(). It recommended explicit lifecycle +contracts and interleaving tests rather than further broad decomposition. + +The maintainer then instructed: + +> lets start a new cycle to fix the regressions and improve the refactor. + +This confirms the reviewed outcome and evidenced gaps and authorizes execution +of a new cycle. The preceding request to add the acceptance/integration rule to +AGENTS.md also remains authorized; its existing uncommitted edit will be retained +with this cycle. Human acceptance does not authorize integration by itself. diff --git a/.spiral/sources/SRC-20260919-TTZ7C-37.ttl b/.spiral/sources/SRC-20260919-TTZ7C-37.ttl new file mode 100644 index 0000000..e67c5f8 --- /dev/null +++ b/.spiral/sources/SRC-20260919-TTZ7C-37.ttl @@ -0,0 +1,9 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . + +project:SRC-20260919-TTZ7C-37 + a sd:Source ; + dcterms:identifier "SRC-20260919-TTZ7C-37" ; + sd:repositoryPath ".spiral/sources/SRC-20260919-TTZ7C-37.md" ; + sd:status sd:Active . diff --git a/.spiral/understandings/UND-20260919-TTZ7C-38.md b/.spiral/understandings/UND-20260919-TTZ7C-38.md new file mode 100644 index 0000000..4733c10 --- /dev/null +++ b/.spiral/understandings/UND-20260919-TTZ7C-38.md @@ -0,0 +1,22 @@ +# Understanding: Reliable Runtime Contracts + +The maintainer wants to retain the useful runtime/recovery boundaries, repair +the three reproduced compatibility/lifecycle regressions, and complete the +identified readability improvements. The preceding review established the +effective behavior and gaps; the instruction to start this cycle confirms them. + +The shared cause is incomplete contracts at moved boundaries: awaiting close +must mean closure completed for every caller; acceptance must recheck storage +health after asynchronous persistence; duplicate lookup must precede validation +that applies only to new commands. Passing final-state tests did not establish +these ordering properties. Verify the interleavings and the process composition. + +Keep inspection and approved-recovery coordinators at one abstraction level. +Use existing cohesive classes and a few meaningful supporting methods. Avoid +new frameworks, public APIs or broad renaming. Preserve disk records, replay +eligibility, audit evidence and lock-retention policies. + +Assumptions: restoring ID-only duplicate lookup is compatibility, not permission +to execute incomplete new commands. Overlapping close calls must share failure +as well as success. Storage failure remains terminal and retains ownership. +Inspection/recovery cleanup must remain unchanged by readability-only changes. diff --git a/.spiral/understandings/UND-20260919-TTZ7C-38.ttl b/.spiral/understandings/UND-20260919-TTZ7C-38.ttl new file mode 100644 index 0000000..0648583 --- /dev/null +++ b/.spiral/understandings/UND-20260919-TTZ7C-38.ttl @@ -0,0 +1,14 @@ +@prefix sd: . +@prefix dcterms: . +@prefix project: . + +project:UND-20260919-TTZ7C-38 + a sd:Understanding ; + dcterms:identifier "UND-20260919-TTZ7C-38" ; + sd:repositoryPath ".spiral/understandings/UND-20260919-TTZ7C-38.md" ; + sd:status sd:Active ; + sd:derivedFrom [ + a sd:ArtifactReference ; + sd:artifact project:SRC-20260919-TTZ7C-37 ; + sd:gitCommit "f5ea142ec16911b410137af015ad683446bd07f2" + ] . diff --git a/AGENTS.md b/AGENTS.md index c52123e..b8be8c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,6 +69,7 @@ Do not create Spiral artifacts merely to populate folders. Add `.spiral/` artifa - Before allocating a new artifact ID, check `node .spiral-core/bin/spiral.mjs status`. This checkout has worktree-local allocator state under `.git/spiral`; other clones/worktrees will have their own namespace. - Preserve Git history as evidence. Do not amend, rebase, squash, reset, or force-push causal cycle history. - An Active cycle branch must not merge into `master` or another Active cycle branch. Record human acceptance by changing the cycle to `sd:Accepted` before proposing integration. +- Human acceptance closes the cycle and authorizes proposing it for integration; it does not authorize merging it. Treat integration as a separate review and decision boundary. After recording acceptance and validating the accepted commit, stop and present the pull request or equivalent integration proposal unless the human has explicitly authorized the merge. When authorized, integrate with a normal merge commit. - Before proposing or merging accepted cycle work, run `node .spiral-core/bin/spiral.mjs validate integration --base --head --base-branch master --head-branch `, or rely on a hosting check that validates the exact prospective merged result with branch metadata. - When accepted `master` work is merged into an Active cycle branch, prefer `git merge --no-commit master` if governed artifacts may have changed on both sides. If both parent histories materially changed the same governed artifact, reconcile the merge version and record both immediate predecessor versions with `sd:transforms`. - Before every semantic causal commit, run `python3 scripts/validate-spiral-provenance.py --staged` (or the Node entry point with `PYTHON` set) and `git diff --cached --check`. The optional hook in `scripts/hooks/pre-commit` runs staged validation; see `docs/spiral-validation.md` before installing it. diff --git a/src/admin-recovery.mjs b/src/admin-recovery.mjs index aa2d0b2..c889a41 100644 --- a/src/admin-recovery.mjs +++ b/src/admin-recovery.mjs @@ -395,15 +395,21 @@ class RecoveryApplication { async runApprovedCommand(id) { await assertFresh(this.plan) const command = this.freshPlan.commands.find(item => item.id === id) + const active = await this.markCommandActive(command) + const result = await this.executeCommand(id, command) + await this.assertSuccessfulResult(id, result) + await this.commitCommandResult(id, command, active, result) + } + + async markCommandActive(command) { + const { id } = command const active = nextActiveCommandStatus(id, command.history.at(-1)) await this.recordAttempt(id, active) await appendRecord( this.config.commandStatus, JSONTag.stringify(active) ) - const result = await this.executeCommand(id, command) - await this.assertSuccessfulResult(id, result) - await this.commitCommandResult(id, command, active, result) + return active } async recordAttempt(id, active) { diff --git a/src/store-inspection.mjs b/src/store-inspection.mjs index 95e975e..64673d2 100644 --- a/src/store-inspection.mjs +++ b/src/store-inspection.mjs @@ -157,6 +157,15 @@ class StoreInspector { async inspect() { await this.validateConfiguredPaths() this.files = await inventory(this.config) + await this.loadCommandHistory() + await this.loadIntegrityManifest() + await this.reconstructCommittedState() + this.validateStoreArtifacts() + await this.verifyStableSnapshot() + return this.createReport() + } + + async loadCommandHistory() { const log = await this.readRecords( this.config.commandLog, 'command log' @@ -168,12 +177,6 @@ class StoreInspector { this.readCommands(log) const doneOrder = this.applyStatusHistory(statuses) this.validateCommittedOrder(doneOrder) - await this.loadIntegrityManifest() - this.prefixValid = this.errors.length === 0 - await this.reconstructCommittedState() - this.validateStoreArtifacts() - await this.verifyStableSnapshot() - return this.createReport() } async validateConfiguredPaths() { @@ -356,6 +359,7 @@ class StoreInspector { } async reconstructCommittedState() { + this.prefixValid = this.errors.length === 0 await this.readBaseDataset() for (const command of this.commands) { await this.inspectCommandDataset(command) diff --git a/src/store-runtime.mjs b/src/store-runtime.mjs index f1f862d..71ba36c 100644 --- a/src/store-runtime.mjs +++ b/src/store-runtime.mjs @@ -174,6 +174,7 @@ export class StoreRuntime { this.commandRunnerActive = false this.storageFailed = false this.closing = false + this.closure = null this.ownership = null this.queryWorkerPool = null this.slowQueryWorkerPool = null @@ -334,15 +335,6 @@ export class StoreRuntime { return candidate.result } const { command, line } = candidate - if (this.status.has(command.id)) { - return { - code: 200, - value: { - command: command.id, - ...this.status.get(command.id) - } - } - } const accepted = { command: command.id, @@ -361,6 +353,9 @@ export class StoreRuntime { await this.mechanisms.faultPoint( 'after-command-accepted-status-before-response' ) + if (this.storageFailed) { + throw new Error('Store failed during acceptance') + } this.commandQueue.push({ id: command.id, command: line }) return { @@ -400,6 +395,17 @@ export class StoreRuntime { } } } + if (this.status.has(command.id)) { + return { + result: { + code: 200, + value: { + command: command.id, + ...this.status.get(command.id) + } + } + } + } if (!command.name) { return { result: { @@ -576,14 +582,16 @@ export class StoreRuntime { } } - async close() { - if (this.closing) { - return + close() { + if (!this.closure) { + this.closing = true + this.closure = this.closeResources() } - this.closing = true - await this.serializeAcceptance(async () => { + return this.closure + } - }) + async closeResources() { + await this.serializeAcceptance() await this.runner this.queryWorkerPool.close() this.slowQueryWorkerPool.close() diff --git a/test/store-runtime.mjs b/test/store-runtime.mjs index 3dfa782..fa83d57 100644 --- a/test/store-runtime.mjs +++ b/test/store-runtime.mjs @@ -1,10 +1,21 @@ import assert from 'node:assert/strict' import fs from 'node:fs/promises' +import path from 'node:path' import test from 'node:test' +import { setImmediate as nextTurn } from 'node:timers/promises' import JSONTag from '@muze-nl/jsontag' import StoreRuntime from '../src/store-runtime.mjs' import { appendFile } from '../src/util.mjs' -import { makeServerFixture } from './durability-helpers.mjs' +import { + getCommandStatus, + getOpenPort, + makeServerFixture, + postCommand, + queryPersons, + startServer, + waitForExit, + waitForServer +} from './durability-helpers.mjs' function command(id, name = 'addPerson') { return JSONTag.stringify({ @@ -259,3 +270,201 @@ test('runtime does not require a storage failure observer', async t => { assert.equal(opened.runtime.storageFailed, true) await opened.runtime.close() }) + +test('overlapping closes wait for the active command and cleanup', async t => { + const started = Promise.withResolvers() + const result = Promise.withResolvers() + const opened = await openRuntime(t, { + executeWorker() { + started.resolve() + return result.promise + } + }) + await opened.runtime.acceptCommand(command('A')) + opened.runtime.runQueuedCommands() + await started.promise + + let completed = 0 + const closures = [opened.runtime.close(), opened.runtime.close()] + .map(async closing => { + await closing + completed++ + }) + try { + await nextTurn() + assert.equal(completed, 0) + assert.equal(opened.ownershipReleased(), false) + const refused = await opened.runtime.acceptCommand(command('B')) + assert.equal(refused.code, 503) + } + finally { + result.resolve({}) + await Promise.all(closures) + } + assert.equal(completed, 2) + assert.equal(opened.runtime.getCommandStatus('A').value.status, 'done') + assert.equal(opened.ownershipReleased(), true) + assert.equal(opened.events.filter(event => { + return event === 'ownership released' + }).length, 1) + await opened.runtime.close() +}) + +test('overlapping and repeated closes share cleanup failure', async t => { + const releasing = Promise.withResolvers() + const released = Promise.withResolvers() + const failure = new Error('ownership release failed') + let releases = 0 + const opened = await openRuntime(t, { + async acquireOwnership() { + return { + release() { + releases++ + releasing.resolve() + return released.promise + } + } + } + }) + const results = Promise.allSettled([ + opened.runtime.close(), + opened.runtime.close() + ]) + await releasing.promise + released.reject(failure) + for (const result of await results) { + assert.equal(result.status, 'rejected') + assert.equal(result.reason, failure) + } + await assert.rejects(opened.runtime.close(), failure) + assert.equal(releases, 1) +}) + +for (const boundary of [ + 'after-command-log-before-accepted-status', + 'after-command-accepted-status-before-response' +]) { + test(`acceptance refuses success after storage failure at ${boundary}`, + async t => { + const failure = new Error('concurrent command persistence failed') + const opened = await openRuntime(t, { + async faultPoint(name) { + if (name === boundary) { + opened.runtime.failStorage(failure) + } + } + }) + await assert.rejects( + opened.runtime.acceptCommand(command('A')), + /Store failed during acceptance/ + ) + assert.equal(opened.runtime.commandQueue.length, 0) + const refused = await opened.runtime.acceptCommand(command('B')) + assert.equal(refused.code, 503) + const records = readRecords( + await fs.readFile(opened.fixture.commandLog, 'utf8') + ) + assert.deepEqual(records.map(record => record.id), ['A']) + await assert.rejects(opened.runtime.close()) + assert.equal(opened.ownershipReleased(), false) + }) +} + +test('known ID-only retries return status but new commands need a name', + async t => { + let executions = 0 + const opened = await openRuntime(t, { + async executeWorker() { + executions++ + return {} + } + }) + await opened.runtime.acceptCommand(command('A')) + const retry = await opened.runtime.acceptCommand('{"id":"A"}') + assert.equal(retry.code, 200) + assert.equal(retry.value.status, 'accepted') + await opened.runtime.runQueuedCommands() + const completed = await opened.runtime.acceptCommand('{"id":"A"}') + assert.equal(completed.code, 200) + assert.equal(completed.value.status, 'done') + const invalid = await opened.runtime.acceptCommand('{"id":"B"}') + assert.equal(invalid.code, 422) + await opened.runtime.runQueuedCommands() + assert.equal(executions, 1) + const records = readRecords( + await fs.readFile(opened.fixture.commandLog, 'utf8') + ) + assert.deepEqual(records.map(record => record.id), ['A']) + await opened.runtime.close() + }) + +test('both shutdown signals wait for a command and leave a restartable store', + { timeout: 15000 }, async t => { + const fixture = await makeServerFixture(t, { + commandsSource: `import fs from 'node:fs' +export default { + addPerson(data, command) { + const pause = new Int32Array(new SharedArrayBuffer(4)) + while (!fs.existsSync(command.releaseFile)) { + Atomics.wait(pause, 0, 0, 10) + } + data.persons.push(command.value) + } +} +` + }) + const releaseFile = path.join(fixture.dir, 'release-command') + await fs.appendFile(fixture.runner, ` +process.on('SIGTERM', () => console.log('observed SIGTERM')) +process.on('SIGINT', () => console.log('observed SIGINT')) +`) + const port = await getOpenPort() + const running = startServer(t, fixture, { port }) + await waitForServer(running.child, running.getOutput, port) + const response = await postCommand(port, { + id: 'A', + name: 'addPerson', + value: { name: 'A' }, + releaseFile + }) + assert.equal(response.status, 202) + await waitUntil(async () => { + const status = await getCommandStatus(port, 'A') + return status.status === 'active' + }) + + try { + for (const signal of ['SIGTERM', 'SIGINT']) { + running.child.kill(signal) + await waitUntil(() => { + return running.getOutput().includes(`observed ${signal}`) + }) + } + assert.equal(running.child.exitCode, null) + } + finally { + await fs.writeFile(releaseFile, '') + } + const exit = await waitForExit(running.child) + assert.equal(exit.code, 0) + const records = readRecords( + await fs.readFile(fixture.commandStatus, 'utf8') + ) + assert.equal(records.at(-1).status, 'done') + await assert.rejects( + fs.stat(path.join(fixture.dir, '.simplystore-lock')), + { code: 'ENOENT' } + ) + const restarted = startServer(t, fixture, { port }) + await waitForServer(restarted.child, restarted.getOutput, port) + const persons = await queryPersons(port) + assert.deepEqual(persons.map(person => person.name), ['A']) + }) + +async function waitUntil(predicate) { + const deadline = Date.now() + 5000 + while (!await predicate()) { + assert.ok(Date.now() < deadline, 'Timed out waiting for test event') + await new Promise(resolve => setTimeout(resolve, 10)) + } +}