[terencecho]

Add @hyperframes/share, a host-neutral runtime for creating video review links, managing access, uploading versions, and resuming interrupted operations. CLI and local Studio can share the same project identity and persisted retry state.

Project locks serialize mutations across processes, including on filesystems that do not support hard links. Existing snapshots retain their symlink behavior, while CLI render fingerprints detect changes to linked media. Unreadable sharing records surface recovery instructions and preserve the existing identity.

Validation: kernel regression tests, declaration build, packed-consumer checks, and Docker workspace checks. The CLI integration is in #7135.

The share package budget is 60,820 bytes packed and 240,537 bytes unpacked. The increase covers persisted-state validation, safe upload retries, and review feedback attribution. I verified the tarball contents and measured the new size locally and in CI.

[github-actions]

Edit accuracy: accurate 267 (base branch 267), smooth 108 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

• seqplainkeys-none-center-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-center-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): fail / pass / pass, passes
• seqplainkeys-none-center-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-pct-r0-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-pct-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r0-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): pass, passes
• seqplainkeys-none-px-r30-nested-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run
• seqplainkeys-none-px-r30-root-z100 (fixed by studio: undo/redo on a keyframed plain element sometimes leaves the box 230-400px off (edit-accuracy seqplainkeys-none-*) #6989): not run

Unstable (1)

• seqfastkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.07, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

[jrusso1020]

Reviewed at 26fdd177b202c962427ad0ba45075485c585874b.

The package reuses the Desktop sharing state machine, retains operation IDs across failed registrations, and limits uploads to configured exact origins before opening the file. The 21 sharing tests and declaration build pass. Two persistence issues remain before publishing with the stated shared-directory concurrency and recovery guarantees.

Important — held-version status can erase another process's pending upload in the same project directory. packages/share/src/reviewExport.mjs:574–586 (recheckHeld) checks only its process's inFlight map and writes through finish without the export file lock. I independently reran a two-process probe using the actual runtime:

1. Process A has a held version and starts a delayed status read.
2. Process B starts uploading a newer render, holds review-export.lock, and persists its new operation before its create request.
3. A receives ready for the old version and writes pending: null plus the old last, overwriting B's pending operation while B still owns the lock.
4. B loses its create reply. Its saved operation is gone, so a retry cannot recover the possibly-created version under that ID.

The probe's before/after record shows B's export-… operation replaced by pending: null and the old render. Code inspection identifies markFiles at :654–657 as another export-record read/modify/write guarded only by process-local state and needs the same lock discipline. refreshReviewExport already demonstrates the common export-lane boundary.

Fix: put every export-record mutation behind the export lane, including held completion and markFiles, and re-read/revalidate pending operation or version identity inside that lock. Prefer reading status outside the lock, then acquire it, re-read and verify the same pending operation before committing the answer; never commit a captured record over another uploader. Add the delayed-held-read/new-upload two-process regression and an attach/new-upload interleaving, asserting the newer operation ID survives, then is reused on retry after a lost response. For attachment, hold its state read while the uploader persists new pending/last fields and assert those fields survive the files-flag write. The reproduced race concerns multiple processes sharing one project directory. This is new exposure in the multi-process kernel; the copied Desktop algorithm's process-local checks are insufficient for the new contract.

Important — malformed but valid JSON silently discards sharing identity. packages/share/src/reviewShare.mjs:48–55 returns null when a present record lacks a string studioProjectId. recordToShare then creates a new identity and overwrites the file. The new recovery behavior in storage.mjs:72–85 catches parsing/I/O errors only. My actual shareForReview probe preserves invalid JSON and makes zero registrations, but each existing {}, [] and null record is overwritten and makes one new registration. This bypasses recovery and can consume another project slot; an identity whose fields were damaged may be lost. A second probe of reviewExport.mjs:109–117 uses a present malformed pending object: pendingReviewExport returns null, and addVersion overwrites it and creates a new operation. reviewSwitch.mjs:53–56 also collapses malformed shapes into missing journals; that sibling is a code-audit observation, not a separately reproduced switch flow.

Fix: distinguish ENOENT from present-but-invalid data at each record decoder. Validate the fields needed to preserve identity/operation ownership, retain the original bytes and throw the recovery error for malformed records; keep explicitly supported legacy shapes. Add valid-JSON malformed fixtures alongside the existing invalid-JSON tests, asserting zero registration/create calls and byte preservation.

Nit: packages/share/README.md:9 places reviewUploadOrigins: […] inside an import list, which is invalid JavaScript. Move that property into the bindShareHost({...}) example.

Validation: node --test packages/share/tests/*.test.mjs — 21 passed; bun run --cwd packages/share build — passed; node --test scripts/verify-packed-manifests.test.mjs — 26 passed; node --test packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs — 7 passed. Local probe artifacts: review-artifacts/oss7134/adversary-held-race.mjs (independently rerun) and malformed-probe.mjs / export-schema-probe.mjs; these invoke this head's runtime with local fake clients. The locked control (node review-artifacts/oss7134/adversary-held-race.mjs control) preserves the newer operation after the lost response; it also passes its assertions. These are not live backend/app acceptance tests. No Docker image or full repository build run.

Audited: new runtime modules, identity/operation persistence, project/export locks, snapshot/link handling, auth/HTTP/upload boundaries, export and package declarations, release/build/Docker workspace additions. Trusting: existing backend semantics and host policy; CLI adoption in #7135 is outside this verdict. Compared the extracted export state machine with HFI and checked subsequent source history; no claim of byte-identical extraction.

Need: yes · Size: ok for the cohesive runtime extraction · Reuse: Desktop sharing state machine and one common export-lock boundary · Simpler: reuse the existing locked recovery path for every export-state writer · Structure: fine
Risk: medium — cross-process persistence controls publication identity and recovery after ambiguous server responses.

Verdict: REQUEST CHANGES
Reasoning: An unlocked completion can delete another process's pending operation, and malformed present records can be treated as fresh identity. Enforce the shared mutation boundary and preserve invalid records before publishing this runtime.

— Rames (hf-review)

[terencecho]

Addressed the review findings in 3c7c94d78.

• Held status responses now reacquire the export lock and recheck the pending operation before writing. Snapshot attachment also rereads the record under that lock and updates only the matching version.
• Malformed sharing records and workspace switch journals retain their original bytes and surface recovery instructions. They cannot trigger a fresh registration or export operation.
• Fixed the bindShareHost example.

All 27 sharing tests passed, including six new regressions that fail on the reviewed commit. The declaration build, 26 manifest tests, and seven Docker workspace tests also passed. Hosted CI is pending.

[jrusso1020]

Reviewed at 3c7c94d

Need: yes · Size: ok for the runtime extraction and focused repair · Reuse: common export lane and shared record/recovery helpers · Simpler: existing lock boundary now covers every export writer · Structure: fine
Risk: medium — shared-directory persistence controls publication identity and recovery after lost responses.

Both Important findings from my review at 26fdd177b2 are fixed; the README nit is fixed too. This is a delta re-review of those repairs, retaining the earlier whole-package audit.

• Previously reported export-state races fixed: reviewExport.mjs:645–674 keeps the held status request outside the lock, then reacquires the export lane, rereads disk and checks video, operation and project identity before finishing. A busy uploader is left alone; an uploader that has already released its lock is protected by the identity check. markFiles at :737–750 also rereads inside that lane, checks the target version/project and preserves the current pending record. reviewFiles.mjs:61 supplies the project ID. The other export-record writes remain inside the uploader or locked recovery runner.
• Malformed record recovery fixed: identity, export and workspace-switch decoders now distinguish missing files from malformed JSON shapes, retain the bytes, and report SharingStateError. Export validation runs before preparing a new upload. The new tests assert no registration/create/upload for the malformed fixtures; they include {}, arrays, null and incomplete identity/operation records. Optional legacy identity fields and export fingerprint/project fields remain supported by the decoders.
• README.md:21 now puts reviewUploadOrigins inside bindShareHost, rather than the import list.

Validation at this head:

• node --test packages/share/tests/*.test.mjs: 27 passed, 0 failed.
• Ran this head's six new persistence tests against the previous reviewed runtime: 0 passed, 6 failed, with no cancelled tests. The same tests pass on this head. They cover delayed held answers both while the other process owns the lock and after it releases it, attachment against a second-process uploader, and all three malformed-record families. Lost-response retries assert the same operation ID.
• Independently reran the original two-process race probe from round 1 with the expected preservation assertion: the newer operation ID and fingerprint survive the old held answer and the lost create response.
• Independently reran an additional decoder probe: 11 legacy-compatible record forms accepted, including omitted optional identity, fingerprint/project, last-version and switch fields.
• Declaration build passed; packed-manifest unit tests 26 passed; Docker workspace tests 7 passed.

Local probes use the actual runtime with filesystem/process interleavings and fake service clients. No live backend/app test, full repository build, Docker image build or full packed-consumer gate was run. CLI adoption #7135 remains outside this review.

Verdict: APPROVE
Reasoning: The shared mutation boundary now protects the newer operation on both stale-answer interleavings, and malformed records no longer turn into fresh identity or upload state. No remaining blocker found in this repair.

— Rames (hf-review)

[jrusso1020]

Reviewed at 87e2b4f

Need: yes · Size: ok for the sharing runtime and focused additions · Reuse: existing export lane, authenticated request loop and host feedback hook · Simpler: keep cleanup ownership on the persisted operation · Structure: fine
Risk: medium — export retries now also consume persisted feedback activity.

Delta re-review since my approval at 3c7c94d78, retaining the earlier whole-package audit. The prior export-record locking and malformed-record recovery fixes remain present; the 33 sharing tests pass. Title updates carry the expected-title precondition, and legacy record handling has focused controls.

Important — an accepted export whose reply is lost can consume later feedback activity on retry. packages/share/src/reviewExport.mjs:383–390 computes the mark in the request-body factory but only attaches reviewSent to the pending record after createExport returns. An ambiguous failure therefore leaves no persisted cutoff. The retry reuses the operation ID but takes a new timestamp and mark; finish at :551 then calls published through that newer timestamp.

I independently reran a probe against this head's actual runtime and authenticated client:

1. At time 100, operation X sends from_review: "no"; the fake service accepts it and the response is lost.
2. A new feedback action is recorded after that request.
3. At time 200, the retry sends operation X with from_review: "yes"; the service returns the original export.
4. Observed: published(dir, 200) and onAdded({…, fromReview: "yes"}). Thus activity newer than the original export is consumed, and the callback attributes the original export using the retry's mark.

This replay is consistent with the backend implementation: review_export_registration.py::_request_digest excludes from_review, and register_reserved_export returns the existing video for the operation. I inspected that source at EF f2eee9fb83666a3b94b10c69ba01c0e411d01117; I did not run a live server.

Fix: persist the operation's cleanup cutoff before its first possible dispatch and preserve that cutoff across unanswered/ambiguous retries. Keep the current per-auth-attempt read for the outbound analytics field, so consent changes still apply, but do not advance cleanup ownership merely because a replay answered. When the accepted attempt cannot be determined, omit the callback's claimed accepted attribution (or obtain it from the server). Advance the cutoff after a definite refusal such as 401 only when no earlier dispatch remains ambiguous; a later 401 does not resolve a previous lost reply. Add the lost-accepted-response/new-feedback/retry regression beside the existing 401 test, asserting that the later action remains for the next version. This is introduced by the new attribution path, not one of the earlier repaired persistence issues.

Validation at this head:

• node --test packages/share/tests/*.test.mjs: 33 passed, 0 failed.
• Declaration build passed.
• Packed-manifest and Docker workspace unit tests: 33 passed, 0 failed combined.
• Local probe review-artifacts/oss7134-r3/lost-attribution.mjs reproduces the issue with the same operation ID on both requests, a lost response, and cleanup at 200.

Local fake-client/filesystem checks only. No live backend/app, full repository build, Docker image build or complete packed-consumer gate. CLI adoption #7135 remains outside this verdict.

Verdict: REQUEST CHANGES
Reasoning: Retrying an ambiguous publication must preserve feedback activity recorded after the original request. The new attribution cleanup currently consumes that activity.

— Rames (hf-review)

[terencecho]

Addressed the export retry finding in 3a57f7d0d. The cleanup cutoff is saved before dispatch and survives an unanswered request, including an intervening 401. Each authenticated attempt still reads the current feedback mark for the request; a replay omits callback attribution when the accepted attempt is unknown.

The regression tests exercise a lost accepted reply with and without an intervening 401, and verify that later feedback is attributed to the next version. All 35 share tests, the declaration build, and the packed size check passed locally. Ready for another look.

[jrusso1020]

Reviewed at 3a57f7d

Need: yes · Size: ok, focused retry repair · Reuse: existing export lock, pending operation and authenticated request factory · Simpler: cleanup ownership stays with the persisted operation · Structure: fine
Risk: medium — ambiguous publication retries control feedback cleanup and export identity.

The Important finding from 87e2b4f08 is fixed. This is a delta re-review of reviewExport.mjs and its attribution tests. Review of unchanged modules is carried forward from earlier rounds; the 35-test rerun here includes the earlier export-lock and malformed-record regressions.

• reviewExport.mjs:386–420 persists the cleanup cutoff before dispatch. An ambiguous retry keeps that cutoff and operation ID, rereads the current outbound mark after auth, and omits callback attribution when the accepted attempt is unknown. A later refusal does not erase an earlier ambiguous send. A definite 401 with no prior ambiguity can advance to the renewed attempt's cutoff.
• The new tests cover lost accepted replies with and without an intervening 401, preserve the feedback added later, and send that feedback on the next version. Both fail against the previously reviewed runtime (0 passed, 2 failed) and pass here (2 passed).
• I reran the original actual-runtime/authenticated-client probe with preservation assertions: the same operation sends no then fresh yes, cleanup remains at 100, callback attribution is omitted, and feedback at 150 survives. Eight additional independently authored scenarios also pass on my rerun: 408/500/malformed accepted replies followed by snapshot refusal, snapshot-fallback lost replies, renewed-auth lost replies, active-creator/recovery exclusion and fresh definitive refusal. These check both preservation and successful progress.

Validation at this head: 35 sharing tests passed; declaration build passed; 33 packed-manifest/Docker workspace unit tests passed. Packed share artifact measures 60,723 bytes packed / 241,442 unpacked; the repository's listPackageSizeIssues returns [] under its existing 1% tolerance. The initial manifest test attempt lacked root TypeScript; after exposing the installed package compiler to that runner, the rerun passes.

Local filesystem/process and fake-service tests only; no live backend/app, full repository build, Docker image or complete packed-consumer gate. CLI adoption #7135 remains outside this verdict.

Verdict: APPROVE
Reasoning: The original publication owns cleanup across ambiguous retries, while each outbound attempt still uses the current mark. The reported loss is closed and no new blocker was found in this repair.

— Rames (hf-review)

[jrusso1020]

Reviewed at 121f14e

Need: yes · Size: ok, one documentation sentence · Reuse: existing trusted upload-origin configuration · Simpler: already minimal · Structure: fine
Risk: low — the delta changes documentation only; runtime and tests are unchanged from the previously reviewed head.

Delta re-review since my approval at 3a57f7d0d49d17a87120986148d328f48d445bfc. The sole new commit removes “including any customer-managed storage this host can use” from packages/share/README.md. It retains the instruction to configure the signer's exact bucket-specific HTTPS origins. I read the full README and checked host.mjs and reviewUpload.mjs: the host/environment allowlist and exact-origin check are unchanged. This neither widens upload destinations nor changes the earlier retry/persistence repairs.

Validation: node --test packages/share/tests/*.test.mjs — 35 passed, 0 failed at this head. Git diff confirms no other changed files since the last review. The prior runtime review and regression evidence remain applicable to the identical runtime; declaration, package-size and workspace checks were not rerun for this documentation-only delta. No live backend/app test.

Verdict: APPROVE
Reasoning: The wording no longer implies an additional storage integration. No new blocker found.

— Rames (hf-review)