feat(api): tutorial-video config nested under /config/builder#2329
Closed
WcaleNieWolny wants to merge 7 commits into
Closed
feat(api): tutorial-video config nested under /config/builder#2329WcaleNieWolny wants to merge 7 commits into
WcaleNieWolny wants to merge 7 commits into
Conversation
Adds a small private endpoint the CLI calls from
`build init --platform ios` to fetch the PiP tutorial video used in
the no-match-recovery flow. All three pieces of config are rotatable
as Cloudflare Worker secrets so the actual video URL, integrity hash,
and YouTube fallback can be swapped without redeploying the CLI:
BUILDER_TUTORIAL_VIDEO_R2_BUCKET — R2 bucket name
BUILDER_TUTORIAL_VIDEO_R2_PATH — object path inside the bucket
BUILDER_TUTORIAL_VIDEO_R2_ACCOUNT_ID — Cloudflare R2 account id
BUILDER_TUTORIAL_VIDEO_R2_ACCESS_KEY — R2 access key id
BUILDER_TUTORIAL_VIDEO_R2_SECRET_KEY — R2 secret access key
BUILDER_TUTORIAL_VIDEO_SHA1 — lowercase hex SHA1 of the file
BUILDER_TUTORIAL_VIDEO_YOUTUBE_URL — browser fallback when PiP fails
Behaviour:
GET /private/config/builder/tutorial-video
enabled=true response when all seven secrets are populated:
{
"enabled": true,
"presignedUrl": "https://<account>.r2.cloudflarestorage.com/...",
"sha1": "<40-hex>",
"youtubeFallback": "https://...",
"expiresInSeconds": 86400
}
Otherwise:
{ "enabled": false }
The presigned URL is generated fresh on every call (24h TTL) using the
existing `presignUrl` helper in supabase/functions/_backend/utils/aws4.ts
— that helper was already in the codebase but not previously called
from any endpoint. Returns a GET URL because the CLI only needs to
stream the bytes.
The enabled=false fallback lets us ship the endpoint NOW, before the
actual tutorial video is produced. The CLI's PiP path (PR Cap-go/capgo
#2308) will see `enabled: false` and quietly fall back to opening the
YouTube URL directly. Once the video exists and secrets are set, the
CLI starts using PiP without any client redeploy.
The "missing secret" log line names which secrets are missing so an
operator can tell at a glance what's left to provision.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a TutorialVideoConfig builder that validates R2/video env vars and presigns a 24-hour R2 GET URL; the config is computed per request and included in the config builder JSON response. Tests are updated to cover enabled/disabled states and validation cases. ChangesTutorial video config in config builder endpoint
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConfigHandler as "config_builder GET /"
participant Builder as "buildTutorialVideoConfig"
participant Presign as "presignUrl"
participant R2
Client->>ConfigHandler: GET /
ConfigHandler->>Builder: buildTutorialVideoConfig()
Builder->>Builder: read ENV (BUILDER_TUTORIAL_VIDEO_*)
Builder->>Builder: validate (sha1 format, parse youtubeFallback URL)
Builder->>Presign: presignUrl(bucket, resourcePath, 24h)
Presign->>R2: request presigned GET URL
Presign-->>Builder: presignedUrl or throw
Builder-->>ConfigHandler: tutorialVideo config (enabled or disabled)
ConfigHandler-->>Client: JSON response including tutorialVideo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Merging this PR will improve performance by 84.91%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cloudflare_workers/api/index.ts`:
- Line 6: The import for "builderTutorialVideo" is out of the required sorted
order; reorder the import statement for "import { app as builderTutorialVideo }
from '../../supabase/functions/_backend/private/builder_tutorial_video.ts'" so
that it follows the project's import-sorting convention (alphabetical/grouping)
and satisfies ESLint — locate the import of builderTutorialVideo and move it to
the correct position among the other imports.
In `@supabase/functions/_backend/private/builder_tutorial_video.ts`:
- Line 60: The code currently treats any non-empty BUILDER_TUTORIAL_VIDEO_SHA1
as present; update the logic around the sha1 variable (const sha1 = getEnv(c,
'BUILDER_TUTORIAL_VIDEO_SHA1').trim().toLowerCase()) to validate it matches a
40-character hex SHA1 (e.g. /^[a-f0-9]{40}$/) and only consider it present if it
passes that test; if it fails, treat it as missing and ensure the endpoint's
enabled flag (the same code path that uses sha1 to build integrity metadata) is
set to false. Apply the same strict validation to the other SHA checks
referenced around the same block (the checks at the later usage on lines ~77-78)
so malformed hashes are rejected consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c906569-c135-4eb4-85e4-c8a9911afc28
📒 Files selected for processing (2)
cloudflare_workers/api/index.tssupabase/functions/_backend/private/builder_tutorial_video.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83f07dfa2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| appPrivate.route('/website_stats', publicStats) | ||
| appPrivate.route('/config', config) | ||
| appPrivate.route('/config/builder', configBuilder) | ||
| appPrivate.route('/config/builder/tutorial-video', builderTutorialVideo) |
There was a problem hiding this comment.
This commit wires /private/config/builder/tutorial-video only in cloudflare_workers/api/index.ts, but the parallel Supabase private entrypoint (supabase/functions/private/index.ts) is not updated, so environments that hit the Supabase private function (including local bun backend/Supabase-only deployments) will return 404 for the new endpoint while Cloudflare works. Add the same import and appGlobal.route('/config/builder/tutorial-video', ...) mapping in the Supabase private router to avoid platform-dependent behavior.
Useful? React with 👍 / 👎.
Replaces the standalone /private/config/builder/tutorial-video endpoint introduced in 83f07df with a nested `tutorialVideo` field on the existing /private/config/builder response. The CLI fetches both the Android (Google OAuth) and iOS (tutorial video) builder config in one round-trip instead of two. Response shape: { enabled: true, clientId, clientSecret, scopes, tutorialVideo } { enabled: false, tutorialVideo } The two halves are independent — `enabled` (OAuth) and `tutorialVideo.enabled` (video) gate on different secret groups, and either can be on while the other is off. iOS users who don't need Google OAuth still get the video block when its secrets are set. `tutorialVideo` shape (unchanged from the deleted standalone endpoint): { enabled: true, presignedUrl, sha1, youtubeFallback, expiresInSeconds } { enabled: false } Reads the same seven worker secrets the previous endpoint did: BUILDER_TUTORIAL_VIDEO_R2_BUCKET BUILDER_TUTORIAL_VIDEO_R2_PATH BUILDER_TUTORIAL_VIDEO_R2_ACCOUNT_ID BUILDER_TUTORIAL_VIDEO_R2_ACCESS_KEY BUILDER_TUTORIAL_VIDEO_R2_SECRET_KEY BUILDER_TUTORIAL_VIDEO_SHA1 BUILDER_TUTORIAL_VIDEO_YOUTUBE_URL Backwards-compatible with the existing CLI parser (cli/src/build/onboarding/android/oauth-config.ts) — it does `if (!parsed.enabled) return null` on the OAuth-config path and ignores extra fields, so adding `tutorialVideo` doesn't change Android onboarding behaviour. Files: - Removes: supabase/functions/_backend/private/builder_tutorial_video.ts - Removes: the extra route registration in cloudflare_workers/api/index.ts - Extends: supabase/functions/_backend/private/config_builder.ts with the buildTutorialVideoConfig helper (reads + presigns) and inlines the field into both OAuth-enabled/disabled response branches.
WcaleNieWolny
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/private/config_builder.ts`:
- Around line 78-86: buildTutorialVideoConfig currently calls presignUrl
directly so any exception from presignUrl will bubble up and cause the whole
/private/config/builder endpoint to 500; wrap the presignUrl call in a try/catch
inside buildTutorialVideoConfig (or around the block that sets presignedUrl) and
on error log the exception and return tutorialVideo: { enabled: false } (or set
presignedUrl to undefined and enabled:false) instead of throwing; reference the
presignUrl invocation and the presignedUrl variable so the fix localizes to that
creation step.
- Around line 49-70: The code currently treats any non-empty sha1 and
youtubeFallback as valid; update the validation in config_builder.ts to reject
malformed values by testing the sha1 variable against a SHA-1 hex regex (e.g.
/^[a-f0-9]{40}$/) and validate youtubeFallback is a well-formed YouTube URL or
ID (e.g. check URL parsing and host/path patterns or a safe regex), and if
either check fails push the corresponding env name into the missing array (same
way as other env vars) so the existing cloudlog({ context:
'config_builder.tutorial_video', ... }) branch returns { enabled: false } for
invalid inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62e39995-5af5-4210-81b9-f4698ee75bbf
📒 Files selected for processing (1)
supabase/functions/_backend/private/config_builder.ts
…rl try/catch
Two review findings, both still valid against current code in
config_builder.ts:
1) SHA1 + YouTube URL format validation (lines 49-70)
Treated any non-empty trimmed string as valid, so a misconfigured
secret (uppercase SHA1, wrong-length hash, "wwww.youtube.com" typo)
would happily round-trip through the API and only fail at the CLI's
SHA1-compare step — every request.
Now the format is verified before the missing-array check:
- sha1 must match /^[a-f0-9]{40}$/ (we already .toLowerCase()'d,
so this guards against wrong-length / non-hex / wrong-algo
paste accidents).
- youtubeFallback must parse as a URL via `new URL()`. Not
restricted to youtube.com host — operators may want Loom,
Vimeo, docs URLs, etc. The parse step catches the typical
missing-protocol / mistyped-host class of errors.
Format failures push a descriptive entry onto the missing array
(e.g. "BUILDER_TUTORIAL_VIDEO_SHA1 (invalid format — expected 40
lowercase hex chars)") so the existing cloudlog branch logs the
exact reason and returns { enabled: false }. The response shape is
unchanged; CLI continues to use YouTube fallback for misconfigured
secrets.
2) presignUrl exception → endpoint 500 (lines 78-86)
buildTutorialVideoConfig called presignUrl directly so any thrown
error would bubble up through the app.get('/') handler and 500 the
whole /private/config/builder endpoint — which ALSO serves the
Google OAuth config to Android onboarding. A bug in the iOS
tutorial-video path could have broken Android.
Wrapped the presignUrl invocation + presignedUrl variable in a
try/catch. On error: log via cloudlog with reason=presignUrl
failed + the message, return { enabled: false }. The OAuth half
of the response is unaffected because it runs in a different code
path (and was always isolated from this one — but the shared
endpoint route meant a thrown error there would still 500 the
whole response).
Validated locally: file still parses (the worker is Deno-style with
.ts extension imports, no separate typecheck step in this dir; the
Cloudflare worker build catches any syntax error at deploy time).
Behavior is back-compat — when all secrets are valid the response is
identical to before; new behavior only activates on misconfiguration.
Lint failure on PR #2329 (e18e/prefer-url-canparse): the try/catch around `new URL(youtubeFallback)` was rejected by the rule because `URL.canParse()` is the modern, throw-free alternative. Same validation semantics — returns true if the string parses as a URL, false otherwise — but no thrown exception and no eslint-disable escape hatch. Verified locally: `bun lint:backend` (eslint on supabase/**) is clean.
Existing 9 OAuth tests used exact-equality assertions that didn't
account for the new tutorialVideo field on both response branches —
they would have failed as soon as lint went green.
Two changes to keep the existing tests passing:
- beforeEach now resets all 7 BUILDER_TUTORIAL_VIDEO_* env vars
alongside the OAuth ones, so the baseline shape is consistently
`{ enabled: false, tutorialVideo: { enabled: false } }`.
- 5 `toEqual({...})` assertions updated to include
`tutorialVideo: TUTORIAL_VIDEO_DISABLED`.
Plus 5 new tests for the previously-uncovered tutorialVideo paths:
- Happy path: 7 valid secrets → enabled:true with a presigned URL
matching the expected R2 hostname/bucket/path pattern + the
declared SHA1 + youtube fallback + 24h expiry.
- OAuth-independence: video enabled while OAuth disabled — the two
halves of the response don't gate each other.
- SHA1 normalization: uppercase is .toLowerCase()'d before validation,
so it remains a valid input (locks in this behaviour so a future
"reject uppercase" change is explicit, not silent).
- SHA1 rejection: wrong length / non-hex → enabled:false (the cases
that would produce a real mismatch against Node's createHash output
at the CLI side).
- YouTube URL rejection: unparseable string → enabled:false.
- Bucket-secret missing: spot-checks the missing-secret short-circuit.
The malformed-SHA1 and unparseable-URL cases use a manual
vi.unstubAllEnvs() + re-apply-baseline cycle inside the test because
calling vi.stubEnv twice for the same key in a single test doesn't
reliably override the previously-stubbed value under this
Bun + vitest + Hono test harness. A getSkipping() helper lets the
bucket-missing test use the beforeEach blank instead of re-stubbing.
Both patterns avoid the override-doesn't-stick footgun.
Verified: bunx vitest run tests/config-builder.unit.test.ts → 14
passed. bun lint:backend clean.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/config-builder.unit.test.ts (1)
56-264: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse
it.concurrent()instead ofit()for parallel test execution.The tests use
it(), but the coding guidelines requireit.concurrent()for parallel execution within the same file to optimize CI/CD performance. These tests appear concurrent-safe since each has isolated env stubbing viabeforeEach/afterEach.♻️ Example conversion
- it('returns enabled:true with clientId, clientSecret, and default scopes when both required env vars are set', async () => { + it.concurrent('returns enabled:true with clientId, clientSecret, and default scopes when both required env vars are set', async () => {Apply the same change to all
it(calls in this file.As per coding guidelines: "Design all tests for parallel execution across files; use it.concurrent() instead of it() to run tests in parallel within the same file for faster CI/CD"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/config-builder.unit.test.ts` around lines 56 - 264, Replace all synchronous it(...) test declarations with it.concurrent(...) in this test file so tests run in parallel; update every occurrence of it( to it.concurrent( including the blocks that reference helpers and symbols shown (get, getSkipping, VALID_VIDEO_ENV, TUTORIAL_VIDEO_DISABLED) and ensure beforeEach/afterEach env stubbing remains unchanged so each concurrent test keeps isolated environment setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/config-builder.unit.test.ts`:
- Around line 201-204: The comment incorrectly claims the test "Uses
`getSkipping`" while the code actually calls get({ ...VALID_VIDEO_ENV,
BUILDER_TUTORIAL_VIDEO_SHA1: sha1 }) and relies on a manual unstub/restub cycle
around each iteration; update or remove the misleading sentence so the comment
accurately describes that this test uses get(...) with a manual unstub/re-stub
per iteration to restub SHA1 (and optionally mention the related
"bucket-missing" test for context), ensuring references to get and
BUILDER_TUTORIAL_VIDEO_SHA1 remain so readers can locate the behavior being
described.
- Around line 230-246: Remove the redundant environment reset and re-stubbing in
the test "tutorialVideo: rejects unparseable youtubeFallback — enabled:false":
delete the call to vi.unstubAllEnvs() and the repeated vi.stubEnv(...) lines
(the manual re-stubbing of GOOGLE_OAUTH_*, BUILDER_TUTORIAL_VIDEO_* vars) and
simply call get({ ...VALID_VIDEO_ENV, BUILDER_TUTORIAL_VIDEO_YOUTUBE_URL: 'not a
url' }) as the beforeEach already provides the clean baseline; keep the existing
assertions unchanged.
- Around line 197-228: The test "tutorialVideo: rejects malformed SHA1 (wrong
length / non-hex) — enabled:false" manually calls vi.unstubAllEnvs() and
re-stubs the beforeEach envs inside a for-loop; replace that loop with
it.concurrent.each(malformed)(...) so each case runs in its own fresh
beforeEach/afterEach lifecycle, remove the vi.unstubAllEnvs() and the subsequent
re-stub block, and keep the existing call to get({ ...VALID_VIDEO_ENV,
BUILDER_TUTORIAL_VIDEO_SHA1: sha1 }) and assertions unchanged so each iteration
receives a clean environment automatically.
---
Outside diff comments:
In `@tests/config-builder.unit.test.ts`:
- Around line 56-264: Replace all synchronous it(...) test declarations with
it.concurrent(...) in this test file so tests run in parallel; update every
occurrence of it( to it.concurrent( including the blocks that reference helpers
and symbols shown (get, getSkipping, VALID_VIDEO_ENV, TUTORIAL_VIDEO_DISABLED)
and ensure beforeEach/afterEach env stubbing remains unchanged so each
concurrent test keeps isolated environment setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75d444f7-86a8-46be-a80b-56dcef9080b2
📒 Files selected for processing (1)
tests/config-builder.unit.test.ts
CI typos checker (crate-ci/typos@v1, dictionary v1.46.2) flagged
"unparseable" as a misspelling. Renamed two occurrences in
tests/config-builder.unit.test.ts:
- Test title: "rejects unparseable youtubeFallback" → "rejects
unparsable youtubeFallback"
- Inline doc comment in the bucket-missing test that referenced the
URL-validation test by its prior name.
No behavioural change. All 14 tests still pass locally.
Three review nits on tests/config-builder.unit.test.ts: 1) The SHA1 malformed test's inline comment claimed "Uses `getSkipping`" — incorrect, that test calls `get(...)` and used to wrap each loop iteration in a manual vi.unstubAllEnvs() + re-stub cycle. The comment was stale (copy-paste from the bucket-missing test below). Replaced the manual for-loop with `it.each(malformed)` so each case runs in its own test() lifecycle with a fresh beforeEach — that's the structural fix the comment was trying to work around. No more manual unstub/restub. NOT `it.concurrent.each`: vi.stubEnv mutates process.env, which is process-global; concurrent iterations would race on the shared video env keys. The comment now states this explicitly so a future reader doesn't try the concurrent variant. 2) The youtube-fallback test had the same manual unstub + 10-line re-stub block. Going from beforeEach's '' to a single test stub of 'not a url' is the same pattern as the VALID_VIDEO_ENV test (which works without any manual unstubbing), so the block was pure noise. Dropped — now just one call to get(...). 3) (Outside-diff suggestion to convert all `it(...)` → `it.concurrent(...)` deliberately NOT applied: the happy-path test reads back specific stubbed values (SHA1, youtube URL) and would race against any concurrent test that stubs different values for the same keys. The video keys are all aliased on the same process.env, so concurrent tests aren't safe here.) Verified: bunx vitest run tests/config-builder.unit.test.ts → 16 passed (3 new SHA1 cases from it.each + 13 prior). `bun lint:backend` clean.
|
WcaleNieWolny
added a commit
that referenced
this pull request
May 22, 2026
The PiP tutorial (precompile Swift helper + predownload video + SHA1-verified playback + 5s budget + YouTube fallback) is gone — too much infrastructure for a feature that the actual tutorial video doesn't yet exist for, and the related Capgo PR #2329 would have to land + secrets provisioned before any of it could even work. Replaced with a much simpler "make the manual path less attractive" nudge: when the user picks "🌐 Open Apple Developer Portal" from the recovery menu, we no longer fire-and-forget into the portal. We route to a new `import-portal-explanation` step that: - Lists the SIX manual steps the user would need to do in the portal (sign in to the right team, create app_store profile, pick the right App ID, tick the right cert in the allowed list, download the .mobileprovision, get it back into the CLI). - When the cert is available on Apple's side AND distribution is app_store: leads with a green "💡 Recommended: let Capgo do this for you" nudge — explains that "✨ Create a new App Store profile for this cert via Apple" does all six steps automatically via the Apple API. - When that automatic path isn't available (ad_hoc, currently unsupported by createProfile): explains the constraint and points at the file-from-disk path instead. - Three actions: ✨ Use "Create a new App Store profile" instead (recommended) 🌐 Open the portal anyway (advanced) 📁 I already have a .mobileprovision on disk — let me pick it ↩️ Back to recovery menu The "Open anyway" path still opens the developer.apple.com URL in the browser and routes back to the recovery menu so the user can pick "🔄 Rescan Apple API" when ready — same final UX as before, just behind a deliberate confirmation that reads the room. Removed: - cli/src/build/onboarding/pip-tutorial.ts (entire file) - PipTamperError / precompilePipHelper / predownloadVideo / verifyAndPlayPip imports - pipHelperPromiseRef / pipVideoPromiseRef + their useRef declarations - PIP_VIDEO_URL / PIP_YOUTUBE_FALLBACK constants - ensurePipTasksStarted helper + its two call sites (setup-method- select onChange import branch, mount-time hydration) - The 70-line PiP/YouTube race block inside the Open Portal handler Net: −336 LOC (pip-tutorial.ts) − ~70 LOC (PiP wiring) + ~140 LOC (explainer step) = −266 LOC. Cap-go/capgo PR #2329 (the /private/config/builder tutorialVideo endpoint) is now orphaned — the CLI no longer consumes it. Leaving that PR open for the user to close manually since they may want the plumbing for a future revival.
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.
Summary
Extends the existing
/private/config/builderendpoint with a nestedtutorialVideofield so the CLI fetches both the Android (Google OAuth) and iOS (tutorial video) builder config in a single round-trip. Used by PR #2308 (CLI side: PiP precompile + SHA1-verified play in the iOS no-match-recovery flow).This PR went through two iterations:
83f07dfa2added a standalone/private/config/builder/tutorial-videoendpoint.eb632dcdb(current head) refactors that into a nested field on the existing endpoint per review feedback — one fetch, one parser, one route.Response shape
The two halves are independent —
enabled(OAuth) andtutorialVideo.enabled(video) gate on different secret groups, and either can be on while the other is off.tutorialVideoshape:Presigned URL is generated fresh on every call (24h TTL) using the existing
presignUrlhelper insupabase/functions/_backend/utils/aws4.ts(it was already in the codebase but not previously called from any endpoint).Rotatable secrets
BUILDER_TUTORIAL_VIDEO_R2_BUCKETBUILDER_TUTORIAL_VIDEO_R2_PATHBUILDER_TUTORIAL_VIDEO_R2_ACCOUNT_IDBUILDER_TUTORIAL_VIDEO_R2_ACCESS_KEYBUILDER_TUTORIAL_VIDEO_R2_SECRET_KEYBUILDER_TUTORIAL_VIDEO_SHA1BUILDER_TUTORIAL_VIDEO_YOUTUBE_URLIf any are missing,
tutorialVideo.enabledisfalseand a log line names which secrets are unset. The CLI quietly falls back to opening the YouTube URL directly in that case.Backwards compatibility
The existing CLI parser (
cli/src/build/onboarding/android/oauth-config.ts) does:…on the OAuth-config path, then reads
clientId / clientSecret / scopes. It ignores fields it doesn't know about. AddingtutorialVideois invisible to that path — Android onboarding behaviour is unchanged.Ship-now, populate-later
The endpoint can ship before the actual tutorial video is produced —
tutorialVideo.enabled: falseis the default until secrets are populated. Once the video exists and the secrets are set in wrangler, the CLI starts using PiP without any client redeploy.Files
supabase/functions/_backend/private/config_builder.ts— extended withbuildTutorialVideoConfig()helper + inlines the field into both OAuth-enabled/disabled response branches.cloudflare_workers/api/index.ts— unchanged (the OAuth route was already there).Test plan
GET /private/config/builderwith OAuth + video secrets unset → bothenabled: falseandtutorialVideo.enabled: falseenabled: false, tutorialVideo: { enabled: true, presignedUrl, sha1, youtubeFallback, expiresInSeconds: 86400 }Related
Summary by CodeRabbit
New Features
Bug Fixes
Tests