feat: prepare v0.6.0 with input-aware re-verification and scoped harness access - #16
Conversation
TueJon
left a comment
There was a problem hiding this comment.
skills/webmcpify/references/reverify.md:57 — Blocking: interrupted mutations still have no durable pre-execution state to reconcile. The new policy says a resumed run must inspect an uncertain mutation before retrying, but the manifest schema only records status, failure, attempts, and post-success verifiedAgainst; the VERIFY loop at skills/webmcpify/SKILL.md:410-422 executes the valid example before its next manifest write. If the agent/session dies after the server accepted a mutation but before the pass/fail write, the manifest is indistinguishable from a tool that was never invoked (integrated). A fresh full/verify run therefore has no signal that triggers step 5 and can execute the mutation again. That leaves the release's central "reconcile before retry" guarantee unenforceable.
Closure checklist:
- Extend the Manifest v4 protocol with durable per-execution state (scoped at least by tool, role/fixture, and argument fingerprint) whose
started/uncertain record is atomically written before dispatch. - Make VERIFY and HEAL update/clear that record only after independent read-path reconciliation and required cleanup; timeouts, cancellation, and process death must leave a state that cannot be mistaken for never-started work.
- Make every resumable runner that can execute a real mutation—including smoke/model eval runs—scan and reconcile that state before any replay, blocking when the outcome cannot be established.
- Define migration semantics so old completed manifests without the new field require fresh evidence but are not falsely treated as an in-flight mutation.
README.md:175-177,189 — Non-blocking: the release removes modern-web-guidance as the live guidance source, but the status and related-project text still says webmcpify uses/pulls that package's guides at integration time. The changed SKILL now reads the Chrome guides and CG draft directly and mentions modern-web-guidance only as an optional, separately approved CLI. Update both README claims to match that behavior so users do not infer the removed package execution is still the normal path.
Verification: npm ci --ignore-scripts and npm run check passed at ab65b6e (43/43 tests). I also swept all changed distribution manifests, resume/phase transitions, VERIFY/HEAL execution paths, eval execution guidance, security boundaries, release notes, and current official Chrome/Puppeteer API claims.
|
Addressed both findings in cb42f0d.
Validation: npm run check passes (43 tests plus type/syntax checks); diff check passes. Targeted protocol walkthrough covers pre-dispatch failure, process death after server acceptance, uncertain cleanup, model-runner replay, and legacy migration. This is a skill workflow protocol; the vendored runtime does not automatically implement host journaling. Generic skill validator still rejects the pre-existing argument-hint/tags fields. Please limit recheck to these finding classes and their regressions. No merge or release has occurred. |
TueJon
left a comment
There was a problem hiding this comment.
skills/webmcpify/references/reverify.md:56-68 — Blocking regression: the lock identity is undefined across the required atomic manifest replacement. The new protocol says to take an exclusive "manifest lock" and then replace manifest.json by atomic rename. A straightforward advisory lock on the manifest file descriptor protects the old inode; after the rename, a second runner opens the new inode and can acquire a second lock while the first runner still owns the old one. Both runners can then append and dispatch, so one can overwrite the other's started record and the pre-dispatch journal no longer prevents duplicate mutations. This mechanism did not exist at the round-1 head (ab65b6e), so its inode-swap race could not have been established in that sweep; it was introduced by the review-fix commit cb42f0d and amends the durable-execution checklist.
Closure checklist:
- Define a stable sidecar lock identity that is never replaced with
manifest.json, acquired before the initial scan/read and held through durable reconciliation/settlement. - Define atomic ownership and stale-owner recovery so recovery cannot remove or bypass a live runner's lock.
- Add a concurrency acceptance test in which runner A holds the lock and atomically replaces the manifest while runner B remains blocked until A settles and releases ownership.
Verification: npm ci --ignore-scripts and npm run check passed at cb42f0d (43/43 tests). I rechecked all four round-1 closure items, the README correction, and every runner/migration/cleanup surface changed by the fix commit.
TueJon
left a comment
There was a problem hiding this comment.
No blocking findings. The stable sidecar lock now preserves one ownership identity across atomic manifest replacement, stale metadata cannot bypass a live kernel lock, and the two-runner regression proves runner B waits until runner A durably settles and releases.
Targeted verification at 2619f21: npm run check passed (44/44 tests). I rechecked all three round-2 lock checklist items and the original durable-execution surfaces touched by this fix.
TueJon
left a comment
There was a problem hiding this comment.
No blocking findings. The only change since the previously reviewed head is the v0.6.0 changelog date, and it introduces no regression in the closed durable-execution or locking finding classes.
Exact-head CI passed (44/44 tests).
Resuming a verified integration previously lacked input provenance, so changed app or browser inputs could retain stale success. This release adds input-aware re-verification and read-path reconciliation before retrying uncertain mutations.
It also removes automatic unpinned guidance execution, scopes browser access to approved test contexts, and strengthens effect assertions, lifecycle guidance and bounded optional evals. Distribution manifests and release notes are aligned at proposed v0.6.0; runtime APIs remain unchanged.
Validation:
npm run checkpasses type/syntax checks and 43 tests.npm run proof:verifypasses on Chrome 150.0.7871.186 using JSON-string input. This is prepared native-fixture evidence, not a full agent run, framework certification, or model-eval result. Fresh audit badges require directory re-indexing.Part of the harness overhaul. Merge/tag/release remain gated on the combined shipment decision after the site and distribution preparation.