PORTAL-SHOWMODEL — the public 3D viewer shipped, and nothing could mint a token that reached it - #438
Open
ibuilder wants to merge 1 commit into
Open
PORTAL-SHOWMODEL — the public 3D viewer shipped, and nothing could mint a token that reached it#438ibuilder wants to merge 1 commit into
ibuilder wants to merge 1 commit into
Conversation
…nt a token that reached it
`show_model` is the per-token opt-in that lets a share link fetch
`GET /shared/{token}/model.frag`. The backend has supported it end to end since
R22-PUBLIC-VIEWER: the route reads `body.get("show_model")`, `model_fragment`
gates on it, `_public_row` returns it, and `test_shared_model.py` proves the
200-vs-404 pair and that `show_payments` does not imply it.
`createShareToken` sent only `label` and `show_payments`. Every token this
product minted had the flag false, so that route 404'd for every link it has
ever produced.
TWO DEFECTS, and the second is the worse kind. The missing PARAMETER made the
capability unreachable. The missing ROW FIELD — `show_model` absent from the
`Tok` type — made it unauditable: the value was on the wire the whole time and
the type dropped it, so a geometry link would have rendered identically to a
digest-only one. R22-PUBLIC-VIEWER's record claims "the owner's token list shows
which links carry geometry"; that was true of the JSON and false of the product.
Corrected in place in roadmap-completed.md rather than rewritten.
WHY NO GATE SAW IT. Both sides were correct in isolation. `test_shared_model.py`
mints its own tokens with `json={"show_model": True}` — a body the product never
produces — so it passed over a client that could not ask for the thing it tests.
A test that builds its own request proves the server honours that request, never
that anything sends it. And the failure is invisible from outside: unknown token,
revoked token, no opt-in and no published fragment all return an identical 404 by
design, so a dark viewer is indistinguishable from a project with no model.
Closed in `clientPortal.ts` (fourth argument + the row field) and
`masterBuilder.ts` (a 3D-model opt-in beside the payments one, both grants marked
on every live link, both reset after each mint). The two flags stay separate
arguments — never one "share more" level — because the backend's rule is that
granting one must never imply the other.
Two new suites, each mutation-checked against the confusion it names.
`shareTokenGrants.test.ts` asserts the encoded BODY, not a mock's arguments;
`masterBuilder.test.ts` drives the real DOM, because a checkbox that renders and
is never read would satisfy the first test completely and leave the capability as
dark as it was. Measured: restoring the original defect fails 3 of 4 wire tests;
never reading the box fails 1; an unconditional marker fails the audit test;
`payCk || mdlCk` fails the independence test. The row-type half is NOT guarded by
vitest — deleting the field keeps it 4/4 green and turns `tsc` red in three
places — and the test says so rather than implying a coverage it does not have.
Scope held: the token still serves the converted fragment, never the source IFC.
Nothing here widens what a token grants; it makes the grant the backend already
defined askable and visible.
Verified: web typecheck clean, eslint clean on all four files, full vitest
206 files / 2069 tests green, and test_claude_md_gates / test_file_sizes /
test_shared_model pass. No backend file is touched by this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. So far, Strix has reviewed 28 pull requests, surfaced 3 security issues (1 critical/high) and blocked 2 risky merges across this workspace. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe web client now supports independent payment and 3D-model grants when creating share tokens. The master builder exposes both options, resets them after minting, and displays geometry metadata on token links. API and UI tests cover request encoding, grant combinations, and auditing. ChangesGeometry share-token flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Share links can now independently grant access to payment schedules and 3D models, with link metadata showing those grants. The change is ready to merge with no current merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant MasterBuilder
participant ClientPortal
participant ShareTokenAPI
MasterBuilder->>ClientPortal: createShareToken with payment and model opt-ins
ClientPortal->>ShareTokenAPI: POST encoded grant flags
ShareTokenAPI-->>ClientPortal: token row with grant metadata
ClientPortal-->>MasterBuilder: created token
MasterBuilder->>MasterBuilder: reset opt-ins and render link metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
show_modelis the per-token opt-in that lets a share link fetchGET /shared/{token}/model.frag. The backend has supported it end to end since R22-PUBLIC-VIEWER: the route readsbody.get("show_model"),client_portal.model_fragmentgates on it,_public_rowreturns it, andservices/api/test_shared_model.pyproves the 200-vs-404 pair and thatshow_paymentsdoes not imply it.createShareTokensent onlylabelandshow_payments. Every token this product minted had the flag false, so that route 404'd for every link it has ever produced.Two defects, and the second is the worse kind
The missing parameter made the capability unreachable. The missing row field —
show_modelabsent from theToktypeshareTokensreturns — made it unauditable: the value was on the wire the whole time and the type dropped it, so a geometry link would have rendered identically to a digest-only one.R22-PUBLIC-VIEWER's shipped record claims "the owner's token list shows which links carry geometry, because an opt-in nobody can audit after minting cannot be reviewed or regretted." That was true of the JSON and false of the product. Corrected in place in
docs/roadmap-completed.mdrather than rewritten, since what it got wrong is the useful part.Why no gate saw it
Both sides were correct in isolation.
test_shared_model.pymints its own tokens withjson={"show_model": True}— a body the product never produces — so it passed over a client that could not ask for the thing it tests. A test that builds its own request proves the server honours that request, never that anything sends it.And the failure is invisible from outside: unknown token, revoked token, no opt-in, and no published fragment all return an identical 404 by design, so a dark viewer is indistinguishable from a project with no model.
The change
apps/web/src/api/clientPortal.ts— fourth argument, plusshow_modelon the row type.apps/web/src/portal/panels/masterBuilder.ts— a🧊 3D modelopt-in beside the payments one; both grants marked on every live link; both reset after each mint.The two flags stay separate arguments, never one "share more" level, because the backend's rule is that granting one must never imply the other — a single toggle could not express that.
Verification
Two new suites, each mutation-checked against the confusion it names.
shareTokenGrants.test.tsasserts the encoded body, not a mock's arguments;masterBuilder.test.tsdrives the real DOM, because a checkbox that renders and is never read would satisfy the first test completely and leave the capability as dark as it was.mdlCk || payCkStated grade: the row-type half is not guarded by vitest — deleting the field keeps it 4/4 green and turns
tscred in three places. The test says so rather than implying coverage it does not have.Web typecheck clean, eslint clean on all four files, full vitest 206 files / 2069 tests green, and
test_claude_md_gates/test_file_sizes/test_shared_modelpass. No backend file is touched.Scope held
The token still serves the converted fragment, never the source IFC. Nothing here widens what a token grants; it makes the grant the backend already defined askable and visible.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation