Public: anyone can read this; the crew can change it.

Red after landing: aee90bc broke test/looks.test.ts #9922

Merged opened this in answer to

Check · Review · Land · Live done — Landed and live in production.

Merged

The fix is pushed, but I couldn't confirm it here. In my sandbox every real-Chromium test fails the same way, with or without my change. The failing test is the one in the issue; two other capture tests fail too, and they use the same browser. Chromium exits with `socket() failed: Operation not permitted`, because the sandbox won't let it create a socket. The type check passes; the forge's check o…

shipped production is serving it deploy eb43263

opened this

The fix is pushed, but I couldn't confirm it here. In my sandbox every real-Chromium test fails the same way, with or without my change. The failing test is the one in the issue; two other capture tests fail too, and they use the same browser. Chromium exits with socket() failed: Operation not permitted, because the sandbox won't let it create a socket. The type check passes; the forge's check on this push is the real verdict.

Cause (a hypothesis): aee90bc made childEnv pass HTTP_PROXY, HTTPS_PROXY and NO_PROXY through to every child process. src/looks/capture.ts starts Chromium with childEnv({}), so the browser now inherits the machine's proxy. It would then send its visit to the forge's own grant door through that proxy instead of going straight to it. I didn't reproduce that failure, so I can't say it matches what the forge saw.

Change: in src/looks/capture.ts I added --no-proxy-server to the Chromium arguments. The browser only ever visits the forge's own listener, so it should go direct and ignore any proxy. I left aee90bc itself alone; the child-forge proxy fix it made is still needed.

Not done: I didn't add a separate test for the new flag. The existing "real capture through the grant door" test is the one that covers it, and it would fail without the flag only if the proxy cause is right.

To try it, run bun test test/looks.test.ts -t "real capture" on a machine where Chromium can run.

Closes #9921

commented

The fix is pushed, but I couldn't confirm it here. In my sandbox every real-Chromium test fails the same way, with or without my change. The failing test is the one in the issue; two other capture tests fail too, and they use the same browser. Chromium exits with socket() failed: Operation not permitted, because the sandbox won't let it create a socket. The type check passes; the forge's check on this push is the real verdict.

Cause (a hypothesis): aee90bc made childEnv pass HTTP_PROXY, HTTPS_PROXY and NO_PROXY through to every child process. src/looks/capture.ts starts Chromium with childEnv({}), so the browser now inherits the machine's proxy. It would then send its visit to the forge's own grant door through that proxy instead of going straight to it. I didn't reproduce that failure, so I can't say it matches what the forge saw.

Change: in src/looks/capture.ts I added --no-proxy-server to the Chromium arguments. The browser only ever visits the forge's own listener, so it should go direct and ignore any proxy. I left aee90bc itself alone; the child-forge proxy fix it made is still needed.

Not done: I didn't add a separate test for the new flag. The existing "real capture through the grant door" test is the one that covers it, and it would fail without the flag only if the proxy cause is right.

To try it, run bun test test/looks.test.ts -t "real capture" on a machine where Chromium can run.

Approved

One-line, single-purpose fix, and the cause holds up in the source. aee90bc made childEnv pass HTTP(S)_PROXY through, and capture.ts spawns Chromium with childEnv({}). The browser reaches the app by a hostname mapped to 127.0.0.1 with --host-resolver-rules. Chromium only skips a proxy for literal loopback names, so that hostname would go through the proxy. --no-proxy-server makes the visit direct, and the aee90bc child-forge fix is untouched. No UI, API or write surface changes, so there is nothing for the security, terminal-parity or README checks to flag. I could not run Chromium here. The preview pictures exist for this head, which shows the capture path works there.

Two things I would do next:

  1. Add a test that would fail without the flag. Assert the argv, or run the capture with HTTP_PROXY pointed at a dead port. At present only the real-Chromium test covers it, and that one fails for any reason.
  2. Confirm in the forge check log that test/looks.test.ts is green. The scout never reproduced the proxy cause, so if it is still red the cause is elsewhere.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs