feat(studio-server): files route extensions#2194
Conversation
1f8fc74 to
bbe1db5
Compare
858368d to
6ecac1c
Compare
bbe1db5 to
90663ec
Compare
6ecac1c to
13c0d74
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga
vanceingalls
left a comment
There was a problem hiding this comment.
🟢 LGTM — no new attack surface, no auth/traversal/tenant changes, batch semantics match sequential-of-N.
Focus items — verified.
Scope of the change. Despite the title "files route extensions", this PR does not add a route or a new file-serving path — it adds one variant (shift-positions-batch) to the existing discriminated-union type GsapMutationRequest and dispatches it in the two existing executor functions (executeGsapMutationAcorn line 1112-1119, and the recast/parser-loaded async path line 1482-1489). No changes to resolveProjectPath, no new HTTP route registration, no new file-read/write primitive.
Auth / tenant boundary. Handler at api.post("/projects/:id/gsap-mutations/*", ...) (line 2038) runs resolveProjectPath (line 2039-2041) unchanged from main — that helper is what enforces the project boundary and rejects ../absolute paths. Since the new op only substitutes a new branch inside the same executor, it inherits the exact same auth surface as shift-positions, scale-positions, etc. No cross-tenant read/write introduced.
Path-traversal. No new path parsing. The type: "shift-positions-batch" payload contains only shifts: Array<{ targetSelector: string; delta: number }> — CSS selectors, not file paths. The traversal risk vector isn't reachable through this variant.
Response shape. Same as sibling ops (line 2119-2128 in the shared response builder): { ok, changed, mutated, parsed, before, after, scriptText, path, backupPath }. No new fields, no internal metadata leaked (server absolute paths never enter the payload; res.filePath at line 2127 is the project-relative form, unchanged from siblings).
Batch semantics vs sequential (the test that actually pins the invariant). Test at lines 10-58 sets up two servers, runs two separate single shift-positions requests on one, and one batch request on the other with the same two {targetSelector, delta} pairs — asserts batch.after === seqAfter. That's a real atomicity + associativity check: folding shiftPositionsInScript over N selectors in one write must equal N sequential single writes. Passes only if shiftPositionsInScript is composition-safe (its second call sees the mutated script from its first call) — which the loop at lines 1112-1117 arranges by threading script = shiftPositionsInScript(script, ...) through iterations. Good.
HOLD_SYNC_MUTATION_TYPES inclusion (line 822-825). shift-positions-batch is added to the hold-sync set alongside shift-positions. The comment above the set (~L819) explains why: a shift can move a keyframed position tween's start across t=0, changing hold-before-first-keyframe need. Same reasoning applies to N shifts. Correct.
Validation. findUnsafeMutationValues (line 2048-2051) recursively walks the entire body and rejects any null / non-finite number — so shifts[i].delta = NaN or null gets caught at the outer gate before executor dispatch. The per-item !Number.isFinite(s.delta) || s.delta === 0 || !s.targetSelector guard at line 1114/1485 is a belt-and-suspenders skip for the zero-delta case (which the outer walker allows) — silent-skip rather than error is consistent with shift-positions at line 1109.
Two consistency nits (non-blocking, cheap follow-up):
- Empty-block fallback exclusion. The "no GSAP block found" bail-out at line 2074 checks
body.type === "shift-positions" || body.type === "scale-positions"and returns{ok: true, changed: false}.shift-positions-batchfalls through to{error: "no GSAP script found in file"}at line 2088 with 400. If the intent is "batch of shifts on a file with no timeline is a no-op success" — mirror the sibling — addshift-positions-batchto that condition. - Missing
shiftsfield crashes.for (const s of body.shifts)at line 1114/1485 throwsTypeErrorif a client sends{type: "shift-positions-batch"}with noshiftsarray. Handled by Hono's error middleware as 500, and the trust model here is "studio front-end sends its own JSON", so it's not a security issue — but it's the same pre-existing pattern (shift-positionshas the same shape assumption ontargetSelector). Consider a defensiveArray.isArray(body.shifts) || return 400at the switch entry point in a follow-up if you're already touching the runtime shape guards.
R1 by Via
13c0d74 to
ba49f01
Compare
90663ec to
b5bf0cf
Compare
ba49f01 to
6e7a79c
Compare
b5bf0cf to
9adc2d3
Compare
9adc2d3 to
66a029d
Compare
6e7a79c to
b970d96
Compare
What: the studio-server files route at its final NLE-stack form, with its test suite (25 tests). Why: standalone package seam — the server-side dependency of the studio asset workflow, reviewable in isolation. How: additive route behavior; existing route consumers unchanged. Test plan: bunx vitest run src/routes/files.test.ts in packages/studio-server; tsc --noEmit in packages/studio-server; fallow audit clean.
66a029d to
89066ef
Compare
b970d96 to
6890ed1
Compare

What
the studio-server files route at its final NLE-stack form, with its test suite (25 tests).
Why
standalone package seam — the server-side dependency of the studio asset workflow, reviewable in isolation.
How
additive route behavior; existing route consumers unchanged.
Test plan
bunx vitest run src/routes/files.test.ts in packages/studio-server; tsc --noEmit in packages/studio-server; fallow audit clean.
Stack position 3/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary
TEMP(studio-dnd)fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.