[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.

[github-actions]

Edit accuracy: accurate 267 (base branch 267), smooth 120 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): 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

[terencecho]

Updated in 26fdd177b. The review uploader now checks the destination against exact origins from trusted host configuration before opening the file. It refuses unexpected origins and still does not follow redirects. Regression tests verify that an unapproved destination receives no request and that a configured destination can receive the stream without following a redirect.

All 21 sharing tests passed. CodeQL completed successfully on the updated PR. The alert remains open because the authorized upload still sends file data.

[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)