Skip to content

feat: ship portable mutation journal helper (v0.7.0) - #21

Merged
TueJon merged 7 commits into
mainfrom
feature/portable-mutation-journal
Sep 19, 2026
Merged

TueJon merged 7 commits into
mainfrom
feature/portable-mutation-journal

Conversation

@TueJon

@TueJon TueJon commented Sep 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • ship dependency-free TypeScript/JavaScript mutation-journal helpers with portable flock / lockf -k locking
  • wire valid, invalid, and cleanup mutations through durable pre-dispatch and settlement hooks
  • cover runner exclusion, bounded-waiter cleanup, lock fallback/fail-closed behavior, persistence failure, crash recovery, migration, cleanup ordering, explicit effect proof, and TS/JS parity
  • prepare the coordinated v0.7.0 manifests, changelog, and release notes

Verification

  • npm run check (54 tests)
  • node --test tests/manifest-lock.test.mjs (11 tests)
  • git diff --check

Integration note

PR #20 is still open and reserves v0.6.1. This branch is intentionally based on current main; it will be rebased after #20 lands, as required by #17.

Closes #17

@TueJon TueJon added enhancement New feature or request webmcp WebMCP agent surface labels Sep 19, 2026
@TueJon TueJon self-assigned this Sep 19, 2026
@TueJon
TueJon marked this pull request as draft September 19, 2026 16:44
@TueJon
TueJon marked this pull request as ready for review September 19, 2026 16:45

@TueJon TueJon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Blocking — lock ownership can disappear while the parent still authorizes a mutation. skills/webmcpify/templates/mutation-journal.ts:351 checks #holder only once, before the first await; all reads, replacement/fsync work, and the return that authorizes the caller to dispatch happen afterward. I opened a journal, sent SIGKILL to its flock holder, and immediately called beforeDispatch; it returned an execution ID and durably appended started after the kernel lock was gone. A successor can therefore acquire the sidecar while the first runner continues into executeTool (webmcp.spec.ts:309). The same unfenced window exists in startup/migration (:329-340), settlement (:409-437), and release (:446-464; an exit between the initial check and listener registration can also leave close() waiting for an event already emitted).

    Closure checklist:

    • Make startup/migration, beforeDispatch, settle, and final release share an ownership mechanism whose loss fences every manifest write and dispatch authorization.
    • Ensure beforeDispatch cannot return authorization after ownership loss, including loss at each asynchronous read/write/fsync boundary used by the Playwright template.
    • Make close() handle holder exit at every point without a missed-event hang.
    • Add killed-holder/concurrent-successor regressions for startup, dispatch, settlement, and release, and keep the TS/JS twins in sync.
  2. Blocking — malformed journal data is silently erased or ignored, so uncertain mutations fail open. skills/webmcpify/templates/mutation-journal.ts:177-195 treats every non-array mutationExecutions value as a legacy absence and replaces it with []; arrays are scanned by filtering only exact state === 'started', so malformed/unknown entries are likewise treated as safe. Reproduction: a v4 tool containing mutationExecutions: { executionId: "uncertain", state: "started" } opened successfully, reported unresolved: [], and rewrote the value to an empty array. That violates the documented “never erase existing journal entries” boundary and permits a new mutation when the durable safety record is corrupt.

    Closure checklist:

    • Migrate only a genuinely absent legacy field; reject present non-array values without rewriting the manifest.
    • Validate every journal entry and state before unresolved scanning or any write; unknown/malformed states must fail closed rather than disappear from the scan.
    • Reject ambiguous identity/linkage such as duplicate execution IDs or invalid parent ownership before beforeDispatch/settle can select an entry.
    • Add preservation/fail-closed regressions for non-array journals, malformed entries, unknown states, duplicates, and bad parent links in both helper variants.
  3. Blocking — a repository-controlled manifest.lock symlink can overwrite an arbitrary writable file. skills/webmcpify/templates/mutation-journal.ts:322-338 creates/opens the sidecar by pathname, then writeOwnerMetadata reopens that pathname and truncates it at :199-204. I made .webmcpify/manifest.lock a symlink to a victim file; openMutationJournal() replaced the victim contents with owner metadata. Path re-resolution also means the file checked/created by the parent is not guaranteed to be the inode the lock utility owns.

    Closure checklist:

    • Reject symlink, non-regular, and multiply-linked sidecars without touching their targets.
    • Bind creation, advisory locking, metadata writes, and identity checks to the same verified inode without an unchecked pathname reopen/swap window.
    • Add regressions proving a symlink target remains byte-identical and a replaced sidecar cannot create two lock identities; mirror the fix in TS and JS.
  4. Blocking — this exact head conflicts with current main at the self-hosted-runner safety gate. .github/workflows/ci.yml:13 has the new self-hosted label but lacks current main's fork-PR guard (if: github.event_name == 'push' || ...head.repo.full_name == github.repository). GitHub reports this head CONFLICTING, and the PR also states that it must be rebased after #20 before the 0.7.0 release metadata is final.

    Closure checklist:

    • Rebase after the prerequisite release head lands and resolve the CI conflict while retaining main's fork guard.
    • Reconcile changelog/version/release ancestry so 0.7.0 follows the final 0.6.1 head.
    • Re-run the exact-head manifest/version and CI checks after that integration.

npm run check passes all 54 tests on this head; the three focused reproductions above exercise missing failure boundaries rather than duplicating that suite.

@TueJon
TueJon marked this pull request as draft September 19, 2026 17:41
# Conflicts:
#	.claude-plugin/marketplace.json
#	.claude-plugin/plugin.json
#	.cursor-plugin/plugin.json
#	.github/workflows/ci.yml
#	CHANGELOG.md
#	gemini-extension.json
#	package-lock.json
#	package.json
#	skill.json
@TueJon

TueJon commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Finding 1 — fixed in e2aab82. The runner process now owns the advisory lock through its retained descriptor; there is no separately killable holder process, so ownership cannot disappear while the runner continues toward dispatch. Startup, beforeDispatch, settle, and release all use that same descriptor, while stable sidecar identity checks fence path replacement. Regressions cover exclusion across all four phases plus runner death during startup. Verified on final ancestry e5bc0bd with npm run check (69/69).

@TueJon

TueJon commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Finding 2 — fixed in e2aab82. Migration now applies only when mutationExecutions is genuinely absent. Present non-arrays, malformed entries, unknown states, duplicate execution IDs, missing parents, and cross-tool parent ownership all fail closed before rewrite or dispatch. The preservation regression exercises every listed malformed case and asserts the manifest remains byte-identical. Verified on final ancestry e5bc0bd with npm run check (69/69).

@TueJon

TueJon commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Finding 3 — fixed in e2aab82. Sidecars are opened with no-follow semantics, must be regular and singly linked, and are bound to the retained locked descriptor. Metadata is written through that descriptor; acquisition and live-use identity checks reject pathname swaps. Regressions prove symlink and hard-link victims remain byte-identical and that acquisition-time or live sidecar replacement cannot create a second operational journal. Verified on final ancestry e5bc0bd with npm run check (69/69).

@TueJon

TueJon commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Finding 4 — fixed in e5bc0bd. PR #20 is merged and current main is integrated without rewriting history. The CI job retains the self-hosted fork-PR guard, v0.7.0 now follows v0.6.1 in the changelog and release comparison, and the merged harness-pin tests/docs are present. The exact local merge result passes npm run check (69/69); hosted exact-head gates are now running.

@TueJon
TueJon marked this pull request as ready for review September 19, 2026 18:48

@TueJon TueJon left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review: all four prior closure checklists are satisfied at this head; no regressions were found in ownership/dispatch fencing, journal validation, sidecar identity, or the v0.6.1 ancestry and CI integration.

Exact-head CI (npm run check, 69 tests) passed. No blocking findings.

@TueJon
TueJon merged commit e6dba64 into main Sep 19, 2026
2 checks passed
@TueJon
TueJon deleted the feature/portable-mutation-journal branch September 22, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request webmcp WebMCP agent surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ship a portable mutation-journal helper for verification runners

1 participant