Repository navigation
feat: make new promptfiddle user-friendly - #3864
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHeader values now flow through Rust and WebAssembly payloads, proxy requests can be rewritten through a Boundary proxy using ChangesBoundary proxy and playground wiring
Sequence Diagram(s)sequenceDiagram
participant SplitPreview.tsx
participant proxy-config.ts
participant gateway.ts
participant ApiKeysDialog.tsx
participant SessionStore
SplitPreview.tsx->>proxy-config.ts: configureProxyEnvVar({ visible: true, url: BOUNDARY_PROXY_URL })
SplitPreview.tsx->>gateway.ts: initPlaygroundEnv()
gateway.ts->>SessionStore: seed defaults and BOUNDARY_PROXY_URL once
ApiKeysDialog.tsx->>gateway.ts: onToggleProxy(!proxyEnabled)
gateway.ts->>SessionStore: set or remove BOUNDARY_PROXY_URL
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.69.3)Trivy execution failed: 2026-06-27T05:36:53Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: cloudformation scan error: fs filter error: fs filter error: walk error range error: stat typescript2/app-fiddleproxy/doctor.config.json: no such file or directory: range error: stat typescript2/app-fiddleproxy/doctor.config.json: no such file or directory 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 |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
![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: 38ad18ead2
ℹ️ 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".
Binary size checks passed✅ 7 passed
Generated by |
sxlijin
force-pushed
the
push-mmuzomoksnnx
branch
from
June 27, 2026 00:05
38ad18e to
0a98b8b
Compare
![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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
typescript2/pkg-playground/src/components/ApiKeysDialog.tsx (1)
104-122: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDon’t hide a live
BOUNDARY_PROXY_URLwhen the proxy UI is disabled.These filters remove
BOUNDARY_PROXY_URLfrom every list even whenshowProxyEnvVarisfalse. In those hosts, the generic add/import flows can still set the key, and the WASM runtime will still honor it for request rewriting, so users can end up with a hidden proxy target carrying their provider-authenticated traffic. Either reject this key when the dedicated toggle is hidden, or show it as a normal editable row instead of filtering it out.🤖 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 `@typescript2/pkg-playground/src/components/ApiKeysDialog.tsx` around lines 104 - 122, The key filtering in ApiKeysDialog is hiding BOUNDARY_PROXY_URL even when the proxy toggle is disabled, which leaves a live proxy target uneditable and invisible. Update the primaryKeys and shellOnlyKeys logic so BOUNDARY_PROXY_URL is only excluded when showProxyEnvVar is enabled and the dedicated proxy UI is handling it; otherwise surface it as a normal editable row or reject it in the add/import path. Use the ApiKeysDialog memoized key lists and the BOUNDARY_PROXY_URL_KEY handling as the main places to fix.
🤖 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 `@baml_language/crates/baml_lsp_server/src/playground_runs.rs`:
- Around line 215-218: The redaction test for HeaderObservation in
playground_runs.rs only checks the fixture shape, so it can miss regressions
where redacted headers still serialize a value. Update the assertions around the
HeaderObservation snapshot/patch path to explicitly verify that value is null
whenever value_redacted is true, using the existing HeaderObservation and
requestHeaders serialization logic as the target for the check.
In `@baml_language/crates/bex_events/src/run.rs`:
- Around line 441-458: HeaderObservation::observe is currently trusting the
header name alone, which lets response headers with
DISPLAY_SAFE_HEADER_ORIGINAL_URL bypass redaction. Update the header handling
around HeaderObservation so the display-safe exemption is only applied on the
request path used by the proxy rewrite in build_request/mod.rs, and keep the
response-header collection path in playground_http.rs fully redacted. Use
separate request vs response observation logic, and keep the trusted
original-url value only for the proxy-injected request header.
In `@typescript/packages/fiddle-proxy/server.js`:
- Around line 103-127: The proxy routing in router() currently accepts any
caller-supplied baml-original-url and only checks apiKeyInjectionAllowed later
in proxyReq, which leaves this as an open proxy/SSRF path. Update router() so it
validates the parsed destination origin against the allowlist before returning
it, and reject any non-allowlisted origin immediately; keep the existing
proxyReq header injection logic in sync with the same allowlist source so both
routing and header injection use the same origin check.
- Around line 77-89: The auth gate in the Express middleware in server.js can be
bypassed when PROXY_PROMPTFIDDLE_COM_TOKEN is unset because only the
empty-string case is rejected and the header check is skipped for falsy values.
Update the middleware around the app.use token check to fail closed by treating
both missing and empty expectedToken as a configuration error, logging via
logger.error and returning 403 before any proxying occurs, while keeping the
existing PROXY_TOKEN_HEADER comparison and deletion logic for the valid-token
path.
In `@typescript/packages/fiddle-proxy/server.test.js`:
- Around line 8-16: The ALL_KEYS fixture is missing AI_GATEWAY_API_KEY, so the
buildApiKeyInjectionAllowed() setup fails before the API-key assertions run.
Update the ALL_KEYS object in server.test.js to include every provider key
expected by buildApiKeyInjectionAllowed() in server.js, including
AI_GATEWAY_API_KEY, so the test allowlist matches the real production list.
In `@typescript/packages/fiddle-proxy/set-origin-token-header.ts`:
- Around line 127-158: The entrypoint fetch handling in
set-origin-token-header.ts incorrectly treats any failed Cloudflare response as
an empty ruleset, which can overwrite unrelated rules on the subsequent PUT.
Update the logic around existingResp/existingJson so only a confirmed “no
entrypoint exists” case falls back to an empty array, and any other unsuccessful
response is surfaced or preserved instead of using []. Keep the existing
rule-merging flow around existingRules, newRule, and updatedRules, but guard it
so late-transform rules are not deleted when the fetch fails.
In `@typescript2/pkg-playground/src/ExecutionPanel.tsx`:
- Around line 327-343: The RequestUrlLabel reconstruction logic is incorrectly
appending parsed.pathname from the proxy URL, which breaks when the proxy itself
is configured with a path prefix. Update the de-proxying branch in
RequestUrlLabel to strip any configured proxy pathname before combining the
original upstream URL with the request path, or validate that the proxy URL is
origin-only before using it. Keep the fix localized to the URL rebuild logic
that uses findHeaderValue, ORIGINAL_URL_HEADER, and new URL(url).
In `@typescript2/pkg-playground/src/gateway.ts`:
- Around line 61-100: installGateway() currently stores gateway state in
module-global installedStore and gatewayUrl, so a second call overwrites the
first and the first cleanup can null out reconciliation for the remaining store.
Update the gateway management in reconcile() and installGateway() to either keep
per-store state keyed by the SessionStore instance or reject a second active
install with a clear error; make sure cleanup only affects the matching store
and does not disable another mounted installation.
---
Outside diff comments:
In `@typescript2/pkg-playground/src/components/ApiKeysDialog.tsx`:
- Around line 104-122: The key filtering in ApiKeysDialog is hiding
BOUNDARY_PROXY_URL even when the proxy toggle is disabled, which leaves a live
proxy target uneditable and invisible. Update the primaryKeys and shellOnlyKeys
logic so BOUNDARY_PROXY_URL is only excluded when showProxyEnvVar is enabled and
the dedicated proxy UI is handling it; otherwise surface it as a normal editable
row or reject it in the add/import path. Use the ApiKeysDialog memoized key
lists and the BOUNDARY_PROXY_URL_KEY handling as the main places to fix.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50089646-2bf2-4f63-a956-5a0a14758656
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
baml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_runs.rsbaml_language/crates/bex_events/src/run.rsbaml_language/crates/bex_events/src/run_wire.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/sys_llm/src/build_request/anthropic.rsbaml_language/crates/sys_llm/src/build_request/mod.rstypescript/packages/fiddle-proxy/.dockerignoretypescript/packages/fiddle-proxy/Dockerfiletypescript/packages/fiddle-proxy/package.jsontypescript/packages/fiddle-proxy/server.jstypescript/packages/fiddle-proxy/server.test.jstypescript/packages/fiddle-proxy/set-origin-token-header.tstypescript2/app-promptfiddle/src/playground/SplitPreview.tsxtypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/components/ApiKeysDialog.tsxtypescript2/pkg-playground/src/gateway.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/proxy-config.tstypescript2/pkg-playground/src/run-store-projections.tstypescript2/pkg-playground/src/worker-protocol.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@typescript/packages/fiddle-proxy/Dockerfile`:
- Around line 6-8: The Dockerfile is installing dependencies with npm even
though fiddle-proxy declares pnpm, which can make builds drift. Update the image
build to use pnpm in the Dockerfile for fiddle-proxy, and make sure the matching
lockfile is copied into the build context before installation. Use the package’s
declared manager with a frozen lockfile install so dependency resolution stays
deterministic.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 95e28f3c-2c66-4efb-84f1-41827c823222
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
baml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_runs.rsbaml_language/crates/bex_events/src/run.rsbaml_language/crates/bex_events/src/run_wire.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/sys_llm/src/build_request/anthropic.rsbaml_language/crates/sys_llm/src/build_request/mod.rstypescript/packages/fiddle-proxy/.dockerignoretypescript/packages/fiddle-proxy/Dockerfiletypescript/packages/fiddle-proxy/package.jsontypescript/packages/fiddle-proxy/server.jstypescript/packages/fiddle-proxy/server.test.jstypescript2/app-promptfiddle/src/playground/SplitPreview.tsxtypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/components/ApiKeysDialog.tsxtypescript2/pkg-playground/src/gateway.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/proxy-config.tstypescript2/pkg-playground/src/run-store-projections.tstypescript2/pkg-playground/src/worker-protocol.ts
✅ Files skipped from review due to trivial changes (4)
- typescript/packages/fiddle-proxy/.dockerignore
- baml_language/crates/baml_lsp_server/src/playground_runs.rs
- baml_language/crates/sys_llm/src/build_request/anthropic.rs
- typescript2/pkg-playground/src/components/ApiKeysDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- typescript2/pkg-playground/src/index.ts
- typescript2/pkg-playground/src/worker-protocol.ts
- baml_language/crates/bridge_wasm/src/wasm_http.rs
- typescript2/app-promptfiddle/src/playground/SplitPreview.tsx
- typescript2/pkg-playground/src/run-store-projections.ts
- typescript2/pkg-playground/src/proxy-config.ts
- typescript/packages/fiddle-proxy/package.json
- baml_language/crates/bex_events/src/run.rs
- baml_language/crates/bex_events/src/run_wire.rs
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- baml_language/crates/sys_llm/src/build_request/mod.rs
- typescript2/pkg-playground/src/gateway.ts
- typescript2/pkg-playground/src/ExecutionPanel.tsx
- typescript/packages/fiddle-proxy/server.js
Contributor
|
wait for #3844 |
sxlijin
force-pushed
the
push-mmuzomoksnnx
branch
from
June 27, 2026 00:49
0a98b8b to
2f29c66
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@typescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsx`:
- Around line 41-45: The Sonnet client configuration is using the wrong
environment variable for Anthropic credentials. Update the `client<llm> Sonnet`
setup in `PlaygroundProvider` so the Anthropic provider reads the
Anthropic-specific env var instead of `env.OPENAI_API_KEY`; keep the fix
localized to the `options` block for `provider anthropic` so the default
template routes correctly when the gateway toggle is off.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77178cdd-9713-4c7d-8f93-22916f1fd5ee
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltypescript2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
baml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_runs.rsbaml_language/crates/bex_events/src/run.rsbaml_language/crates/bex_events/src/run_wire.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/sys_llm/src/build_request/anthropic.rsbaml_language/crates/sys_llm/src/build_request/mod.rstypescript/README.mdtypescript/packages/fiddle-proxy/Dockerfiletypescript/packages/fiddle-proxy/package.jsontypescript/packages/fiddle-proxy/server.jstypescript2/app-fiddleproxy/.dockerignoretypescript2/app-fiddleproxy/.gitignoretypescript2/app-fiddleproxy/Dockerfiletypescript2/app-fiddleproxy/fly.tomltypescript2/app-fiddleproxy/package.jsontypescript2/app-fiddleproxy/server.jstypescript2/app-fiddleproxy/server.test.jstypescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsxtypescript2/app-promptfiddle/src/playground/SplitPreview.tsxtypescript2/app-promptfiddle/src/playground/baml-lsp-worker.tstypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/components/ApiKeysDialog.tsxtypescript2/pkg-playground/src/gateway.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/proxy-config.tstypescript2/pkg-playground/src/run-store-projections.tstypescript2/pkg-playground/src/worker-protocol.ts
💤 Files with no reviewable changes (4)
- typescript/packages/fiddle-proxy/Dockerfile
- typescript/README.md
- typescript/packages/fiddle-proxy/server.js
- typescript/packages/fiddle-proxy/package.json
✅ Files skipped from review due to trivial changes (4)
- typescript2/app-fiddleproxy/.dockerignore
- typescript2/app-fiddleproxy/Dockerfile
- typescript2/app-fiddleproxy/package.json
- baml_language/crates/baml_lsp_server/src/playground_runs.rs
🚧 Files skipped from review as they are similar to previous changes (11)
- typescript2/pkg-playground/src/index.ts
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- typescript2/pkg-playground/src/worker-protocol.ts
- baml_language/crates/bex_events/src/run.rs
- baml_language/crates/sys_llm/src/build_request/anthropic.rs
- typescript2/pkg-playground/src/run-store-projections.ts
- baml_language/crates/bridge_wasm/src/wasm_http.rs
- typescript2/pkg-playground/src/proxy-config.ts
- baml_language/crates/bex_events/src/run_wire.rs
- baml_language/crates/sys_llm/src/build_request/mod.rs
- typescript2/pkg-playground/src/ExecutionPanel.tsx
sxlijin
force-pushed
the
push-mmuzomoksnnx
branch
from
June 27, 2026 03:50
2f29c66 to
251dd2b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@typescript2/app-fiddleproxy/Dockerfile`:
- Around line 1-14: The Dockerfile for the proxy still runs the container as
root, so update the build steps to switch to an unprivileged user before the
final CMD. Keep the existing install and copy flow in place, then add a non-root
USER after the app files are copied so `server.js` starts without root
privileges. Use the Dockerfile’s existing WORKDIR and startup CMD as the anchor
points when inserting the user switch.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 93aa31df-f2d0-43ee-b76c-6296e4b4eb38
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltypescript2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
baml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_runs.rsbaml_language/crates/bex_events/src/run.rsbaml_language/crates/bex_events/src/run_wire.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/sys_llm/src/build_request/anthropic.rsbaml_language/crates/sys_llm/src/build_request/mod.rstypescript/README.mdtypescript/packages/fiddle-proxy/Dockerfiletypescript/packages/fiddle-proxy/package.jsontypescript/packages/fiddle-proxy/server.jstypescript2/app-fiddleproxy/.dockerignoretypescript2/app-fiddleproxy/.gitignoretypescript2/app-fiddleproxy/Dockerfiletypescript2/app-fiddleproxy/fly.tomltypescript2/app-fiddleproxy/package.jsontypescript2/app-fiddleproxy/server.jstypescript2/app-fiddleproxy/server.test.jstypescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsxtypescript2/app-promptfiddle/src/playground/SplitPreview.tsxtypescript2/app-promptfiddle/src/playground/baml-lsp-worker.tstypescript2/pkg-grammar/package.jsontypescript2/pkg-playground/package.jsontypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/components/ApiKeysDialog.tsxtypescript2/pkg-playground/src/gateway.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/proxy-config.tstypescript2/pkg-playground/src/run-store-projections.tstypescript2/pkg-playground/src/worker-protocol.tstypescript2/pkg-proto/package.json
💤 Files with no reviewable changes (4)
- typescript/README.md
- typescript/packages/fiddle-proxy/package.json
- typescript/packages/fiddle-proxy/Dockerfile
- typescript/packages/fiddle-proxy/server.js
✅ Files skipped from review due to trivial changes (3)
- typescript2/app-fiddleproxy/package.json
- typescript2/app-fiddleproxy/.dockerignore
- baml_language/crates/baml_lsp_server/src/playground_runs.rs
🚧 Files skipped from review as they are similar to previous changes (17)
- typescript2/app-promptfiddle/src/playground/SplitPreview.tsx
- baml_language/crates/bex_events/src/run_wire.rs
- baml_language/crates/sys_llm/src/build_request/anthropic.rs
- typescript2/pkg-playground/src/proxy-config.ts
- baml_language/crates/bridge_wasm/src/wasm_http.rs
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- typescript2/pkg-playground/src/worker-protocol.ts
- typescript2/pkg-playground/src/gateway.ts
- baml_language/crates/bex_events/src/run.rs
- baml_language/crates/sys_llm/src/build_request/mod.rs
- typescript2/pkg-playground/src/components/ApiKeysDialog.tsx
- typescript2/pkg-playground/src/run-store-projections.ts
- typescript2/pkg-playground/src/index.ts
- typescript2/app-promptfiddle/src/playground/baml-lsp-worker.ts
- typescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsx
- typescript2/app-fiddleproxy/server.js
- typescript2/pkg-playground/src/ExecutionPanel.tsx
sxlijin
force-pushed
the
push-mmuzomoksnnx
branch
from
June 27, 2026 05:36
251dd2b to
28a6cf5
Compare
sxlijin
enabled auto-merge
June 27, 2026 05:38
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
typescript2/app-fiddleproxy/server.test.js (2)
128-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all configured provider mappings here.
This only spot-checks two entries even though
ALL_KEYSnow exercises multiple providers, including the new gateway path. A broken mapping for any of the other origins would still pass this test, so please either assert every expected origin/header pair or tighten the test name to match the narrower coverage.🤖 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 `@typescript2/app-fiddleproxy/server.test.js` around lines 128 - 132, The test in buildApiKeyInjectionAllowed only verifies two provider origins, so it misses regressions in the other configured mappings. Update the server.test.js coverage around buildApiKeyInjectionAllowed and ALL_KEYS to assert every expected origin-to-header pair, including the new gateway path, or rename the test to clearly indicate it only spot-checks a subset.
114-123: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAlso assert that
baml-original-urlis stripped before forwarding.That header is now part of the proxy control plane, just like the token header. If it reaches the upstream provider, we leak internal routing metadata without this suite noticing. Add an assertion that the fake upstream never receives
baml-original-url.🤖 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 `@typescript2/app-fiddleproxy/server.test.js` around lines 114 - 123, The proxy forwarding test in server.test.js only verifies PROXY_TOKEN_HEADER is stripped, but it also needs to ensure baml-original-url is removed before the request reaches the upstream. Update the existing request-forwarding test around the upstream.received assertion to explicitly check the fake upstream never receives baml-original-url, using the same upstream.received[0].headers assertions alongside the current token check.
🤖 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 `@typescript2/pkg-playground/src/gateway.ts`:
- Around line 65-68: The enable path in setGatewayEnabled currently always
writes getProxyEnvVarConfig().url into BOUNDARY_PROXY_URL_KEY even when no proxy
URL exists. Update setGatewayEnabled so that when enabled is true it first
checks the proxy config URL from getProxyEnvVarConfig(), and only sets the env
var if a valid URL is present; otherwise leave the env var unset and keep the
gateway disabled. Keep the existing disable branch behavior unchanged.
---
Nitpick comments:
In `@typescript2/app-fiddleproxy/server.test.js`:
- Around line 128-132: The test in buildApiKeyInjectionAllowed only verifies two
provider origins, so it misses regressions in the other configured mappings.
Update the server.test.js coverage around buildApiKeyInjectionAllowed and
ALL_KEYS to assert every expected origin-to-header pair, including the new
gateway path, or rename the test to clearly indicate it only spot-checks a
subset.
- Around line 114-123: The proxy forwarding test in server.test.js only verifies
PROXY_TOKEN_HEADER is stripped, but it also needs to ensure baml-original-url is
removed before the request reaches the upstream. Update the existing
request-forwarding test around the upstream.received assertion to explicitly
check the fake upstream never receives baml-original-url, using the same
upstream.received[0].headers assertions alongside the current token check.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 17436495-314d-4ba2-b1fb-e951ad04e94f
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltypescript2/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
baml_language/crates/baml_lsp_server/src/playground_http.rsbaml_language/crates/baml_lsp_server/src/playground_runs.rsbaml_language/crates/bex_events/src/run.rsbaml_language/crates/bex_events/src/run_wire.rsbaml_language/crates/bridge_wasm/src/wasm_http.rsbaml_language/crates/sys_llm/src/build_request/anthropic.rsbaml_language/crates/sys_llm/src/build_request/mod.rstypescript/README.mdtypescript/packages/fiddle-proxy/Dockerfiletypescript/packages/fiddle-proxy/package.jsontypescript/packages/fiddle-proxy/server.jstypescript2/app-fiddleproxy/.dockerignoretypescript2/app-fiddleproxy/.gitignoretypescript2/app-fiddleproxy/Dockerfiletypescript2/app-fiddleproxy/fly.tomltypescript2/app-fiddleproxy/package.jsontypescript2/app-fiddleproxy/server.jstypescript2/app-fiddleproxy/server.test.jstypescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsxtypescript2/app-promptfiddle/src/playground/SplitPreview.tsxtypescript2/app-promptfiddle/src/playground/baml-lsp-worker.tstypescript2/pkg-grammar/package.jsontypescript2/pkg-playground/package.jsontypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/components/ApiKeysDialog.tsxtypescript2/pkg-playground/src/gateway.tstypescript2/pkg-playground/src/index.tstypescript2/pkg-playground/src/proxy-config.tstypescript2/pkg-playground/src/run-store-projections.tstypescript2/pkg-playground/src/worker-protocol.tstypescript2/pkg-proto/package.json
💤 Files with no reviewable changes (4)
- typescript/README.md
- typescript/packages/fiddle-proxy/server.js
- typescript/packages/fiddle-proxy/package.json
- typescript/packages/fiddle-proxy/Dockerfile
✅ Files skipped from review due to trivial changes (3)
- typescript2/app-fiddleproxy/package.json
- typescript2/app-fiddleproxy/.dockerignore
- baml_language/crates/baml_lsp_server/src/playground_runs.rs
🚧 Files skipped from review as they are similar to previous changes (20)
- typescript2/pkg-grammar/package.json
- typescript2/app-fiddleproxy/Dockerfile
- typescript2/pkg-proto/package.json
- typescript2/pkg-playground/src/index.ts
- typescript2/app-promptfiddle/src/playground/SplitPreview.tsx
- baml_language/crates/bex_events/src/run_wire.rs
- baml_language/crates/baml_lsp_server/src/playground_http.rs
- typescript2/pkg-playground/src/worker-protocol.ts
- typescript2/app-promptfiddle/src/playground/baml-lsp-worker.ts
- baml_language/crates/bridge_wasm/src/wasm_http.rs
- baml_language/crates/bex_events/src/run.rs
- typescript2/pkg-playground/src/components/ApiKeysDialog.tsx
- baml_language/crates/sys_llm/src/build_request/mod.rs
- typescript2/pkg-playground/package.json
- typescript2/app-promptfiddle/src/playground/PlaygroundProvider.tsx
- baml_language/crates/sys_llm/src/build_request/anthropic.rs
- typescript2/pkg-playground/src/ExecutionPanel.tsx
- typescript2/pkg-playground/src/proxy-config.ts
- typescript2/pkg-playground/src/run-store-projections.ts
- typescript2/app-fiddleproxy/server.js
aaronvg
added a commit
that referenced
this pull request
Jun 30, 2026
The bridge_wasm runtime + pkg-playground protocol renamed runId -> boundaryId (#3844/#3864), but the website worker still read msg.runId and forwarded notification.runId — both undefined at runtime, so run snapshots/subscriptions never matched and the run UI never updated. - rename runId -> boundaryId across run handlers (cancelRun, respondToInput, respondToEnv, snapshot, subscribe) and the runSnapshot/runCursorExpired notification forwarders - add the missing listHistory/openHistory/readValue handlers and the historyList/valueBody notification forwarders (matches app-promptfiddle) Co-Authored-By: Claude Opus 4.8 <[email protected]>
7 tasks done
This branch was previously deployed
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
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 by CodeRabbit