feat(cli): stable public URL for hyperframes publish (re-publish updates the same link)#2363
Conversation
… team id file) Resolve a stable project id (committed team id > machine store > mint), send it when authenticated so an owned re-publish updates the same URL in place, and persist the server id+url. Report updated-vs-created honestly, add --update to target a project explicitly, write a committable .hyperframes/project.json so a team shares one link, and show the prior URL on re-publish.
Publish -> edit -> re-publish asserts one URL with updated content against a live EF (HYPERFRAMES_E2E_API_URL + an authenticated runner); skipped otherwise.
3b37481 to
0fd03f1
Compare
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 0fd03f15.
Solid feature — the tri-source id resolution (--update > team-file > machine-local) is well-thought-out, the archive path stays token-free, and the anonymous flow is preserved. Two-file architecture (machine-local ~/.hyperframes/projects.json for the personal reuse case, committable .hyperframes/project.json for the team case) reads cleanly. Inline comment on one seam that affects the "Publish failed" UX; a few body-level notes below.
Concerns
- Cross-PR seam with EF#41962 — the client parser at
publishProject.ts:64-73now accepts either (claimed: true) OR (non-emptyclaim_token). This is a new implicit contract between HF and EF: authenticated publishes MUST come back withclaimed: true, and only anonymous ones needclaim_token. If the EF side ever omitsclaimedon the owned path OR keeps returning a claim_token for owned publishes, this client silently reports the owned publish as anonymous (falls through to the claim-token print path even for authenticated flows). Home has EF#41962 — flagging so the pairing gets sanity-checked in both directions. Would be worth a snapshot test on the EF side pinning the two response shapes.
Nits
parseUpdateTarget(publish.ts:26-36) silently accepts any string as a project id.hyperframes publish --update https://hyperframes.dev/settings/profileextracts"profile"and sends it; the server rejects and the flow falls through to "Created new project" per the PR body's design. That's fine when the user actually mistargeted a real project — but a typo'd URL (or an unrelated hyperframes.dev URL) produces a new project instead of an error. Optional tightening: validate the parsed segment against an expected id shape (hfp_prefix?) and emit a clear error before the round-trip. Non-blocking.writeProjectLinksswallows all persistence errors (projectLink.ts:41-46). Comment says "Project links must never prevent local CLI commands from running." — agreed on the never-throw invariant, but if~/.hyperframesis unwritable (readonly HOME, sandboxed exec), every subsequenthyperframes publishsilently mints a new id (viaensureProjectId→ fresh UUID becausereadProjectLinksreturns{}after each silent write failure). Consider a one-shotconsole.warnon the catch so the user knows their machine-local link isn't sticking. Related class of issue is the inline one below.- Race on concurrent publishes across different project dirs —
readProjectLinks→ in-memory merge →writeProjectLinkshas no file locking on~/.hyperframes/projects.json. Two concurrent publishes could each lose the other's entry. Low probability (unusual to run twohyperframes publishat once), non-blocking.
What I didn't verify
- Didn't run the env-gated E2E test (
HYPERFRAMES_E2E_API_URLunset locally, and the test asks for auth viahyperframes auth login). - Didn't trace
buildAuthHeadersinauth/client.js— took the auth-attach-on-metadata-only claim at face value; the PUT to the presigned S3 URL clearly doesn't pass auth headers.
| ); | ||
| } | ||
| if (readTeamProjectId(dir) === null) { | ||
| const file = writeTeamProjectId(dir, published.projectId); |
There was a problem hiding this comment.
🟠 writeTeamProjectId inside the publish try/catch conflates local-write failure with publish failure.
At this point (line 116) the publish has already succeeded — the server owns the project at published.url. If the project dir is read-only (Docker mount, CI checkout with locked-down perms, EROFS, EACCES on the dot-dir mkdir), writeTeamProjectId throws and execution jumps to the outer catch at line 158, which stops the spinner with c.error("Publish failed") and prints the EACCES message. But publish already succeeded; the URL is live and unreachable to the user.
Suggested fix — wrap just this call, keep publish-success feedback intact:
if (readTeamProjectId(dir) === null) {
try {
const file = writeTeamProjectId(dir, published.projectId);
console.log();
console.log(
` ${c.dim(`Wrote ${relative(dir, file) || file} — commit it so your team publishes to this link.`)}`,
);
} catch {
// Committable team file is a convenience; failing to write it should not shadow a successful publish.
}
}Or move the team-file write outside the outer try after printing the URL. Either way, decoupling local persistence from publish-succeeded UX matters — a user seeing “Publish failed” after a successful publish has no path back to their live URL.
— Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
R1 by Via — posted after Rames-D's parallel review; independent verification.
Peer-lens with Rames-D: convergent on the parseUpdateTarget URL-shape gap. Unique from this review: one P1 — --update is silently ignored on the unauthenticated path (server mints a fresh unrelated URL, only the generic "run auth login" tip prints, no acknowledgment the explicit target was dropped — the exact failure mode this PR eliminates, resurfaced under a different code path). Also flag: team file is written once and never refreshed if the current user doesn't own the id it points at (cross-account/cross-clone loops keep creating new projects, team file stays stale), and .hyperframes/ is in this repo's own root .gitignore so the "commit it" hint is defeated for dogfooding. Rames' writeProjectLinks silent-swallow and concurrent-publish race are additive to my findings.
PR #2363 — feat(cli): stable public URL for hyperframes publish (re-publish updates the same link)
Verdict: 🟡 LGTM-conditional
Head: 0fd03f15 on main
Status: mergeable=true, mergeable_state=blocked (needs approvals); CI all green (Lint, Typecheck, Build, Test, Fallow audit, Semantic PR title, CLI smoke, cross-OS shims — 27 success / 4 skipped / 0 fail); zero prior reviews.
Summary of change
Adds a three-tier stable-project-id resolver in the CLI: --update <url|id> (explicit) → <projectDir>/.hyperframes/project.json (committable team id) → ~/.hyperframes/projects.json (per-machine per-directory store, auto-minted UUID on first read). When a credential is resolvable, the id rides along as project_id in the JSON body of both the staged (/publish/complete) and legacy multipart (/publish) requests; anonymous publishes drop the id and behave exactly as before (fresh project + claim token). On successful authenticated publish the server's returned id+url are persisted back to the local store, and if no team file exists a fresh one is written and the user is prompted to commit it. Output distinguishes "Updated existing project" from "Created new project" by comparing the returned id against the sent id.
Findings
🟡 P1 — --update is silently ignored when the user is not authenticated. packages/cli/src/commands/publish.ts sets requestedProjectId = parseUpdateTarget(args.update), but publishProjectArchive in packages/cli/src/utils/publishProject.ts:381 does const projectId = credential ? opts.projectId : undefined;. With no credential the id is dropped, the server mints a fresh project, and the user gets a brand-new URL plus the generic "Tip: run 'hyperframes auth login' first" line. From the user's perspective they typed a specific target and got a different link with no acknowledgment that --update was ignored — the exact failure mode this PR is trying to eliminate, resurfaced under a different code path. Fail-fast (console.error("--update requires 'hyperframes auth login'"); process.exitCode = 1; return;) or at minimum a distinct warning above the tip. Refute attempt: "The tip already tells them to log in" — the tip is generic and always printed on any unauthed publish; it doesn't say --update was dropped, doesn't say the URL you just got is not the URL you asked to update, and doesn't help a scripted / CI caller who won't read stdout. Survives.
🟢 nit — Team file is written once and never refreshed if it points at a project the current user doesn't own. publish.ts:139-145: the team file is written only when readTeamProjectId(dir) === null. If Alice publishes → commits .hyperframes/project.json with id Y → Bob pulls and publishes as his own account, the server (per EF #41962's owner-scoped upsert semantics) doesn't recognize Y for Bob, creates a new project Z, returns claimed:true — CLI prints "Created new project" but does NOT update the team file, so every subsequent publish by Bob keeps sending the dead Y and keeps getting fresh projects created. Bob's per-machine local store does have Z, but team-file priority in the resolver (readTeamProjectId(dir) ?? ensureProjectId(dir)) overrides it. Fix: after publish, if team file exists AND readTeamProjectId(dir) !== published.projectId, either warn (recommended) or offer to rewrite. Refute attempt: "This is a legitimate cross-account/cross-org scenario the server enforces, CLI shouldn't second-guess" — fair, but the CLI silently loops without ever telling the user why the team-file happy path isn't taking. A one-line warning costs nothing. Survives at nit severity.
🟢 nit — Team-file "commit it" hint is defeated when .hyperframes/ is gitignored in the user's project. The hyperframes repo's own .gitignore at HEAD already has .hyperframes/ (unanchored, so it matches at any depth). Dogfooding the CLI from a project inside this repo would silently no-op the "commit it" hint. Downstream user projects usually won't have this rule, but a git check-ignore packages/cli/src/utils/projectLink.ts-style pre-flight would harden the hint (if git check-ignore reports ignored: print "Note: this file is gitignored in the current repo — commit with 'git add -f'"). Non-blocking. Refute attempt: "The repo's own .gitignore is not the CLI's problem" — true, but the failure mode leaks into any project template that copies the hyperframes gitignore. Kept at nit.
🟢 nit — parseUpdateTarget returns the last path segment, so unexpected URL shapes silently misparse. publish.ts:34-42: new URL(trimmed).pathname.split("/").filter(Boolean).pop(). For the documented /p/<id> shape this is correct. For https://hyperframes.dev/p/hfp_xyz/edit (or any URL with trailing segments), pop returns edit and that gets sent as the project id. And for https://hyperframes.dev/ (pathname /) the filter returns [], pop returns undefined, and the whole URL is returned as the "id" and forwarded to the server. Anchor on the /p/<id> shape or reject unrecognized paths with a clear error. Refute attempt: "The server will just reject a garbage id" — yes, but the error surface will be a generic 4xx, not a "your URL doesn't look like a hyperframes project link." Kept at nit.
🟢 nit — ensureProjectId writes a mint to the local store on every unauthed publish, then never sends it. projectLink.ts:76-79 mints + persists a UUID before we know whether the credential exists; publishProjectArchive drops the id on the unauthed path. Result: ~/.hyperframes/projects.json accumulates zero-URL entries for directories the user only ever anonymously published from. Non-blocking (state is idempotent, url:"" is treated as "no prior link" by the priorLink?.url check), but the resolver could be reordered to check auth first. Refute attempt: "Minting before auth-check is intentional so the id is stable across subsequent login" — plausible; if a user runs publish, then auth login, then publish, the second call uses the same UUID. That's a real property worth preserving. Kept at nit as a note-only.
Positive observations (not findings, worth noting for the record):
- Credential is attached to
/publish/uploadand/publish/complete(metadata) but not to the presigned S3 PUT — verified bypublishProject.test.ts:305-315(expectAuthorizedHeaders(fetchMock, 0)for upload metadata,not.toHaveProperty("authorization")for S3,expectAuthorizedHeaders(fetchMock, 2)for complete). Matches the PR body claim. - Anonymous path never sends
project_idand never writes to the local store — verified bypublishProject.test.ts:412-427("never sends a project id or persists a link when anonymous"). Also matches the PR body claim. - Wire contract:
project_idin JSON body for staged flow and in FormData for direct fallback. Both paths gated oncredential. Aligned with EF #41962's expected receiver shape (based on the "authenticated publish" title). - Corrupt-file resilience in
projectLink.ts(readProjectLinksswallows JSON.parse errors → empty map) with a test covering the case. writeProjectLinksusesmode: 0o700for dir and0o600for file — appropriate for a home-dir config that may eventually contain sensitive metadata.- Test coverage is real-dispatch, not aspirational:
projectLink.test.tsandpublishProject.test.tsbothimport from "./projectLink.js"/"./publishProject.js", no local helpers reproducing production logic, no referenced fix SHAs. The.e2e.test.tsisdescribe.skipunlessHYPERFRAMES_E2E_API_URLis set — appropriate for a live-server round-trip. - No
useEffect-style CLI refactor land-mines in the deleted code. The +401/-309 delta onpublishProject.test.tsis largely helper extraction (indexHtmlFiles,localizeSingleAsset,expectLocalizedHtml,stagedFetch,directFetch) followed by new authenticated-flow tests — not a coverage drop. Old localizer/upload tests are preserved via the extracted helpers.
Cross-checks
- Head SHA current: yes (
0fd03f1578af315a2dcf403e552a182861c5adfd, unchanged pre-post per secondgh apicall) - Prior reviewers on record: none (0 reviews, 0 requested reviewers)
- Mergeability:
mergeable=true,mergeable_state=blocked(needs approvals) - CI lanes state: all required green — Lint, Typecheck, Build, Test, Format, Fallow audit, Semantic PR title, CLI smoke (required), CLI/npx shim ×3 OS, Tests + Render on windows-latest, CodeQL, File size check
- PR envelope clean (no
Co-Authored-By: Claude/ 🤖 footer): yes (both commit messages and PR body checked, no matches)
R1 by Via
… write - --space + committed .hyperframes/project.json (projectId+spaceId) send X-Space-Id so a team converges on one link; personal space stays the default for solo users - --update/--space error when unauthenticated, and warn loudly when a resolved-but-invalid token silently downgrades to a new anonymous URL (no more generic-tip-only) - team-file write in its own try/catch so a read-only dir can't fake 'Publish failed' - parseUpdateTarget handles scheme-less URLs + query/hash; X-Space-Id on metadata only (not S3 PUT) - share readJsonRecord across the local + team descriptors
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R2 addendum by Rames D Jusso — verified at head b99d554a (CI green — Format, Lint, Typecheck, Build, Test, CLI smoke all pass). Cross-read of the wire seam with EF#41962 R2; layered on Via's R1 findings I didn't originally cover.
R1 items I flagged that are resolved
writeTeamProjectIdinside outer try/catch → readonly project dir prints "Publish failed" after successful publish. Resolved.publish.ts:170-183— the team-file write is now wrapped in its own innertry/catchwhose catch swallows silently. The outer catch owns publish failures only. Comment made the intent explicit: "Convenience file only; never shadow a successful publish with a local write failure."- Wire-seam parser branching on
claimed:trueOR non-emptyclaim_token. Resolved.publishProject.ts:65-73—claimed = data["claimed"] === true(claim-detection is strictly theclaimedfield); the!claimed && !claimTokenbranch only rejects the malformed anonymous case. Matches the EF-sideclaimed=True ⇒ claim_token=""contract now pinned in EF's route tests. --update/--spaceunauth continuity (Via P1). Resolved on both branches:- Pre-publish fail-fast when no credential (
publish.ts:109-122) — errors with "Run 'hyperframes auth login' first" and setsprocess.exitCode = 1. - Post-publish loud warning when a resolved credential downgrades to anonymous (
publish.ts:190-208): "Your login looks expired or invalid, so--updatewas ignored and a NEW url was created above."
- Pre-publish fail-fast when no credential (
- X-Space-Id header actually rides on the wire.
publishProject.ts:534—metadataHeaders = spaceId ? { ...authHeaders, "x-space-id": spaceId } : authHeaders. Test-locked bypublishProject.test.ts:691-702— the header rides on both metadata requests (0=upload-init, 2=complete) and does NOT ride on the S3 PUT (1) where extra headers would break the presigned signature. Anonymous path drops the header entirely. - Committable
.hyperframes/project.jsonwith{projectId, spaceId?}.projectLink.ts:82-113— read + write,spaceIdoptional (personal-space omits the key), path relative to the project dir (not~/). Round-tripped inprojectLink.test.ts:100-115including a "no token" security assertion on the on-disk body.
First-publish + backwards-compat traces I did
- First publish with
--space <team>:publish.ts:128—spaceId = spaceOverride ?? committedTeam?.spaceId.spaceOverrideis populated from--spacebefore any committed file exists, so the header rides on the first publish. Good. - First publish WITHOUT
--spaceon a fresh team:spaceIdis undefined → noX-Space-Idheader → server'sSpaceAuthorizationresolves personal space. Project is created in the caller's personal shard. Correct — matches the "personal space when the header is absent" behavior Miguel documented on the EF side. - N-1 CLI backwards-compat: old client that never sends
X-Space-Id→ server defaults to personal space → username-scoped project. Confirmed the server-side branch inrender_routes.py:_resolve_optional_owner. - Cross-team migration (project committed with
spaceId=X, user is now member of onlyY): headerX-Space-Id: Xis sent, server'sSpaceAuthorizationaborts 403 (non-member). Loud failure, not silent swap. Correct.
Layered on Via's R1 findings I didn't originally cover
parseUpdateTargethas no test. Confirmed.parseUpdateTargetis defined atpublish.ts:29-42, is NOT exported, and has no test that exercises the URL / bare-id / trailing-segment / non-URL cases. Via's shape-nit from R1 stands unaddressed — ahttps://hyperframes.dev/p/hfp_xyz/editstill parses asedit,https://hyperframes.dev/still returns the whole URL as the "id." The server-side warning-log (_finalize_publishat EFproject_logic.py:301) catches malformed ids as "not owned by space; creating a new project" — so the failure mode is "surprising new URL" rather than "silent misroute", but tightening the parser (or at minimum exporting it for a small test) is low-effort. Optional nit, matches Via's R1 severity.- Team-file stale on cross-account clone loop (Via R1 nit). Unaddressed.
publish.ts:170-172— team file is (re)written only whencommittedTeam === nullOR thespaceIdchanged. If Alice commits{projectId: Y, spaceId: T}and Bob (not a member of T's owning shard forY) publishes, server returns a freshZ; CLI prints "Created new project" but the team file keeps pointing atY. Next Bob publish sendsYagain → fresh project loop. Non-blocking (self-healing after Bob switches to--update <Z>once), but worth a follow-up: emit a one-line warning whenpublished.projectId !== committedTeam.projectId. I'll defer to Via's original framing since she raised it. .hyperframes/gitignored in the hyperframes repo (Via R1 nit). Confirmed at.gitignore:89— dogfooding the CLI from inside the hyperframes repo silently no-ops the "commit it" hint. Downstream user repos usually won't hit this. Non-blocking.
Nits (non-blocking, my own)
writeProjectLinksstill all-error-swallow (projectLink.ts:48-55) and no file locking on~/.hyperframes/projects.json. My original R1 body findings — unchanged in R2. Mitigated in the primary team-share flow because the committable.hyperframes/project.jsonprovides a stable id without needing the machine-local store; only affects unauthed / no-team-file callers. Non-blocking, worth a follow-up if we ever see reports of the machine-local store not persisting.
Verdict
LGTM from my side at b99d554a — leaving as a comment. HF-side R1 items are cleanly addressed; the CLI wire-seam parser matches the EF-side contract; the X-Space-Id header pattern is correct and test-locked. Remaining nits from Via's R1 (parseUpdateTarget shape / cross-account team-file loop) are non-blocking and can carry forward as follow-ups.
— R2 addendum by Rames D Jusso
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R3 by Rames D Jusso — verified at head 47469c79. Delta from R2 head b99d554a is a single commit (47469c79, test(cli): e2e team-space convergence + cross-space hijack guard); no CLI source / logic changes.
R3 delta — coverage-only
The only change since R2 is packages/cli/src/utils/publishProject.e2e.test.ts (+32 / -0), adding one env-gated e2e case: "two identities in a shared team space converge on one URL, and other spaces can't hijack it". This exercises the exact team-share round-trip Miguel and I confirmed on the wire in R2 — Alice publishes into a shared space, Bob publishes into the same space, both converge on one URL; and a caller with a different (or absent) X-Space-Id cannot overwrite the first party's project. The test is describeE2E-gated (skipped in normal CI, requires HYPERFRAMES_E2E_API_URL), which is the right posture for a live-server round-trip.
No regressions in the rest of the CLI diff; all other files unchanged from b99d554a.
Cross-repo note — EF companion (experiment-framework#41962)
Companion BE at 6896d6d6 has one blocker in R3 that will block the wire from landing: the content-addressed refactor moved the S3 write inside session_scope(), which violates the documented "no I/O inside sessions" rule (docs/db.md:235-256). I've posted a full write-up on that PR. HF CLI is not affected by that fix directly — no wire changes needed on this side once Miguel splits the scope on the BE.
R2 findings status — full-state
writeTeamProjectIdinside outer try/catch → readonly project dir shadowing publish success. Resolved at R2, unchanged at R3.- Wire-seam
claimed:trueparser. Resolved at R2, unchanged at R3. Matches the EF-sideclaimed=True ⇒ claim_token=""contract. --update/--spaceunauth continuity. Resolved at R2, unchanged at R3. Pre-publish fail-fast + post-publish loud warning still in place atpublish.ts:109-122andpublish.ts:190-208.X-Space-Idheader on wire. Resolved at R2, unchanged at R3.publishProject.ts:534still gatesx-space-idonspaceIdand drops it on the S3 PUT (indices 0/2 authorized, 1 unauthorized) — locked inpublishProject.test.ts:691-702.- Committable
.hyperframes/project.json. Resolved at R2, unchanged at R3.
Nits carried forward from R1/R2 (still unaddressed, still non-blocking)
parseUpdateTargetshape robustness (Via R1): unexported, no dedicated test;https://hyperframes.dev/p/hfp_xyz/editstill returnsedit. Low-cost tightening but not merge-blocking — server-side rejects malformed ids as "not owned; creating fresh."- Team-file stale on cross-account clone loop (Via R1):
publish.ts:170-172only writes the team file whencommittedTeam === nullorspaceIdchanged. If a user without ownership pulls a repo committing a team file with an id they don't own, they get "Created new project" but the team file keeps the old id → fresh-project loop. Self-healing after one--update <new-id>, worth a one-line warning follow-up. .hyperframes/gitignored in the hyperframes repo (Via R1): defeats the "commit it" hint when dogfooding from inside this repo. Downstream repos usually won't hit this.writeProjectLinksall-error-swallow + no file-locking on~/.hyperframes/projects.json(my R1): unchanged. Mitigated in the primary team-share flow because the committable file provides a stable id independent of the machine-local store.
Verdict
LGTM from my side at 47469c79 — leaving as a comment. HF CLI is in shipping shape; the new e2e test tightens exactly the team-share scenario I called out in R2. Merge unblocked HF-side once EF#41962's scope-split lands (the two need to ship together for the stable-URL feature to be usable, but that's a coordination question, not a code question).
— R3 by Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: hold for the EF-side fix. The HF-side of this PR is clean at 47469c79; my R4 blocker lives on the EF companion (#41962) — _try_update_owned_in_place in project_logic.py:249-260 holds a DB connection across a 2 GB S3 upload (violates docs/db.md §3.2, genesis-code-reviewer[bot] already flagged). Since the pair is planned to merge EF-first, HF unblocks once EF R5 lands with the three-scope split.
HF-side verification
- Content-addressed source contract on the CLI side (client sends only
X-Space-Id; server owns key generation) — clean. parseUpdateTargetuntested (R3 nit) — still not remediated at head, still non-blocking, still cheap.
R4 findings that touch HF
🟢 CLI signal for cross-space fallthrough
The new HF E2E asserts expect(c.projectId).not.toBe(a.projectId) for the "different space can't hijack" case, but the response body carries no explicit "we created a new project instead of updating yours" flag — only the projectId change. Server logs a warning (project_logic.py:319-323), but the CLI doesn't surface it to the user.
Consider a distinguishable response field (e.g. updateTargetHijacked: true or an explicit reason string on the fallthrough) or a client-side message so users don't quietly lose their intended target id. Follow-up sized; not R4-blocking.
Review by Via
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: LGTM — unblocked once EF-first merge lands.
R5 delta on this side is test-only: parseUpdateTarget exported + 5 URL-shape unit tests (publish.test.ts).
Covered:
- Full published URL
- Scheme-less URL (which
new URL()rejects) ?query+#hashstrip- Bare id + whitespace trim
- Non-
/p/last-segment fallback
Not covered (all harmless-fallthrough → server 404 → fresh project, non-blocking):
- Bare host
https://hyperframes.dev/(my R3 partial nit) /settings/profilelast-segment case (my R3 partial nit)/editlast-segment case (Rames R2 nit —https://hyperframes.dev/p/hfp_xyz/editreturnsedit)- Uppercase id (my R2 sibling)
The publish.ts diff is just function parseUpdateTarget → export function parseUpdateTarget — nothing else touched. HF side clean at this head.
Full-state R2/R3/R4/R5 status on my R1/R2/R3 findings:
- ✅
writeTeamProjectIdinside outer try/catch — resolved at R2. - ✅ Wire-seam
claimed:trueparser — resolved at R2. - ✅
--update/--spaceunauth continuity — resolved at R2. - ✅
X-Space-Idheader on wire (only on metadata, not S3 PUT) — resolved at R2. - ✅ Committable
.hyperframes/project.json— resolved at R2. ⚠️ parseUpdateTargetshape robustness — partially covered at R5 (5 tests; 4 residual edge cases remain, all harmless-fallthrough).
Review by Via
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R2 LGTM stands at 9fe92299a. Delta from prior HF head (47469c79) verified test-only via gh pr diff / git diff --name-status:
- Added
packages/cli/src/commands/publish.test.ts— 5 vitest cases covering the URL shapes I asked about in R2: full published URL, scheme-less URL, trailing query+hash, bare id with whitespace, and non-/p/last-segment fallback. - Modified
packages/cli/src/commands/publish.ts— single-linefunction parseUpdateTarget→export function parseUpdateTarget(test export; body unchanged).
Paired EF PR heygen-com/experiment-framework#41962 also cleared its R3 🔴 at my R4. LGTM — leaving as COMMENT on my side.
Review by Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
Verdict: LGTM — cleared to ship.
R6 delta is 6/-2 in publish.ts:158-168. The cross-space fall-through warning (my R4 🟢 nit) now fires when (updateTarget || committedTeam) && !updatedInPlace, with targetDesc differentiating "The requested project" (--update) vs "The committed team project" (committed team id). Message text: "not found, or your space can't access it; a new project was created above instead."
Exactly what the nit asked for — a teammate whose space doesn't own the committed project no longer silently loses the shared link.
Review by Via
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R5 re-review @ af990f401
Verified the R4 🟠 non-blocker (cross-space CLI signal) at HEAD. Layering on Via's R5 approval.
Verified
Cross-space CLI signal — packages/cli/src/commands/publish.ts:161-170:
- Condition widened from
updateTarget && !updatedInPlaceto(updateTarget || committedTeam) && !updatedInPlace. So the warning now fires on the committed-team miss — a teammate whose space doesn't own the committed project id sees an explicit "not updated" message instead of a silent fresh-URL create. committedTeamcomes fromreadTeamProject(dir)at:124, which returnsnullwhen no.hyperframes/project.jsonis committed → the warning correctly stays quiet on the pure machine-local / anonymous flow.targetDescdisambiguates the wording —"The requested project"on--update,"The committed team project"on committed-team → readers know which surface fell through.- Message body itself is tightened:
"not found, or your space can't access it; a new project was created above instead."— matches the actual server behavior (the ownership check in EF_try_update_owned_in_placereturns False on a foreign-space id, so_finalize_publishfalls through to the fresh-id create).
PR body refresh — no leftover "in-place overwrite" language; describes content-addressed + pointer mechanism accurately.
Nit (non-blocking)
🟡 No unit-test on the new console-log branch — the E2E at publishProject.e2e.test.ts:78 covers the server-side "outsider gets a fresh project id" behavior but doesn't assert on the CLI's warning text. Adding a console.log-capturing unit-test around the publishProjectArchive boundary would be nice for a UX-only signal like this. Small polish, not gating.
Peer state
- Via R5 APPROVE at tip — LGTM on the R4 nit fix.
mergeStateStatus: BLOCKED, mergeable: MERGEABLE— waiting on the required stamp; unblocks once the EF-first merge lands.
CI: required checks (Preflight, Lint, Format, Typecheck, Build, CLI smoke, Producer/SDK/Runtime tests, CodeQL, Skills, Perf) green at HEAD; a few Windows-Test / global-install shards are pending on the fresh push, no reds observed.
Verdict from my side: LGTM at af990f401, comment-only.
Review by Rames D Jusso
What
hyperframes publishreturns a stable public URL that re-publishing updates in place — including a team-shared link the whole team publishes to.Why
Re-publishing the same project used to mint a different link each time, so a shared tab never updated and a team had no canonical URL. Publish already promised a "stable public URL"; this makes it true.
How
~/.hyperframes/projects.json. When authenticated, it's sent so the server updates that project in place. Anonymous publish is unchanged.--space <id>+ a committable.hyperframes/project.json({projectId, spaceId}) sendX-Space-Idon the metadata requests only (never the presigned S3 PUT). Teammates in that space converge on one link; personal space is the solo default.--update <url|id>targets an existing project;parseUpdateTargethandles full URLs, scheme-less URLs, and?query/#hash(unit-tested).--update/--spaceerror when unauthenticated; a resolved-but-invalid token that downgrades to a new anonymous URL is warned loudly; a known target (--updateor a committed team id) that the server couldn't update surfaces "a new project was created instead."hyperframes auth loginstack; stable URLs require it (logged-out publish is unchanged).Test plan
parseUpdateTarget: 37 unit tests + an env-gated E2E (solo round-trip, two-identity team convergence, cross-space hijack guard).heygen-com/experiment-framework#41962; anonymous publish keeps working until it ships.