Feat/automation build #48
Conversation
- Add BuildConfig.ts with AUTOMATED_MODE flag - Update AIChatPanel.ts to support automated mode - Bypass OAuth authentication in automated mode - Auto-enable evaluation mode for Docker deployments
- Simple Dockerfile that uses pre-built local files - Uses nginx:alpine for lightweight serving - Designed for local development workflows
WalkthroughAdds a build-time AUTOMATED_MODE to Docker builds, introduces a BuildConfig for AI Chat, includes it in GN build, defaults evaluation to enabled in automated mode, and suppresses OAuth UI when automated. Updates Docker README to document automated vs manual modes and corresponding build/run instructions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Docker as Docker Build
participant Build as BuildConfig.ts
participant UI as AIChatPanel
participant Eval as EvaluationConfigStore
Note over Docker,Build: Automated build path
Docker->>Build: sed AUTOMATED_MODE: true (when ARG AUTOMATED_MODE=true)
Note over UI,Eval: App startup
UI->>Eval: loadFromLocalStorage()
alt Automated mode AND no prior setting
Eval->>Eval: set ai_chat_evaluation_enabled = 'true'
Eval-->>UI: return settings (evaluation enabled)
else Otherwise
Eval-->>UI: return existing settings
end
UI->>UI: determine showOAuthLogin()
alt Automated mode
UI-->>User: OAuth login hidden
else Manual mode
UI-->>User: OAuth login shown if no credentials
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
docker/Dockerfile.local (1)
5-10: Confirm nginx port and add healthcheck (optional).Ensure docker/nginx.conf listens on 8000 to match EXPOSE. Consider adding a simple HEALTHCHECK for faster failure diagnosis.
Example:
+HEALTHCHECK --interval=30s --timeout=3s --retries=3 CMD wget -qO- http://localhost:8000/ || exit 1front_end/panels/ai_chat/core/BuildConfig.ts (1)
5-20: Fix lint: trailing whitespace and missing EOF newline.Resolve ESLint errors and keep diffs minimal.
- * + * @@ -} as const; \ No newline at end of file +} as const;docker/Dockerfile (1)
55-58: Make the override resilient to formatting drift (optional).The current sed is brittle. Consider generating the file instead.
-RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ - sed -Ei 's/(AUTOMATED_MODE:\s*)false/\1true/' front_end/panels/ai_chat/core/BuildConfig.ts; \ - fi +RUN node -e "const fs=require('fs');const p='front_end/panels/ai_chat/core/BuildConfig.ts';let s=fs.readFileSync(p,'utf8');s=s.replace(/AUTOMATED_MODE:\s*false/,'AUTOMATED_MODE: true');fs.writeFileSync(p,s)"docker/README.md (2)
25-30: Document that automated mode still requires provider creds for actual LLM calls.Bypassing OAuth UI ≠ skipping API keys/endpoints. Add a note to prevent confusion.
-# Build with automated mode (default - bypasses OAuth, auto-enables evaluation) +# Build with automated mode (default - bypasses OAuth panel, auto-enables evaluation; provider credentials/endpoints still required for LLM calls)
76-98: Tighten wording on “no authentication required.”Clarify it’s “no OAuth UI” while still needing keys/endpoints; optionally link to Settings.
-### Automated Mode (Default) +### Automated Mode (Default) @@ -- **Authentication**: Bypasses OAuth panel - no manual setup required +- **Authentication**: Bypasses the OAuth login panel; you still need to provide provider API keys/endpoints in Settingsfront_end/panels/ai_chat/common/EvaluationConfig.ts (2)
7-7: Fix import order to satisfy lint.-import { createLogger } from '../core/Logger.js'; -import { WebSocketRPCClient } from './WebSocketRPCClient.js'; -import { BUILD_CONFIG } from '../core/BuildConfig.js'; +import { BUILD_CONFIG } from '../core/BuildConfig.js'; +import { createLogger } from '../core/Logger.js'; +import { WebSocketRPCClient } from './WebSocketRPCClient.js';
53-59: Auto-enable in automated mode is correct; trim trailing space.Behavior is gated on null-check, so user preference isn’t overridden. Remove trailing space to fix lint.
- if (BUILD_CONFIG.AUTOMATED_MODE && + if (BUILD_CONFIG.AUTOMATED_MODE && localStorage.getItem('ai_chat_evaluation_enabled') === null) {front_end/panels/ai_chat/ui/AIChatPanel.ts (2)
23-23: Fix import order (BuildConfig before LLMClient) to appease lint.-import { BUILD_CONFIG } from '../core/BuildConfig.js'; +import { BUILD_CONFIG } from '../core/BuildConfig.js';And move it above the LLM imports:
-import { LLMClient } from '../LLM/LLMClient.js'; +import { BUILD_CONFIG } from '../core/BuildConfig.js'; +import { LLMClient } from '../LLM/LLMClient.js';
1398-1401: Remove trailing whitespace.- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docker/Dockerfile(1 hunks)docker/Dockerfile.local(1 hunks)docker/README.md(3 hunks)front_end/panels/ai_chat/BUILD.gn(2 hunks)front_end/panels/ai_chat/common/EvaluationConfig.ts(2 hunks)front_end/panels/ai_chat/core/BuildConfig.ts(1 hunks)front_end/panels/ai_chat/ui/AIChatPanel.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T23:08:07.342Z
Learnt from: CR
PR: BrowserOperator/browser-operator-core#0
File: eval-server/nodejs/CLAUDE.md:0-0
Timestamp: 2025-08-29T23:08:07.342Z
Learning: Applies to eval-server/nodejs/src/logger.js : Log all RPC calls, evaluations, and connection events
Applied to files:
front_end/panels/ai_chat/common/EvaluationConfig.ts
🧬 Code graph analysis (2)
front_end/panels/ai_chat/common/EvaluationConfig.ts (1)
front_end/panels/ai_chat/core/BuildConfig.ts (1)
BUILD_CONFIG(11-20)
front_end/panels/ai_chat/ui/AIChatPanel.ts (1)
front_end/panels/ai_chat/core/BuildConfig.ts (1)
BUILD_CONFIG(11-20)
🪛 ESLint
front_end/panels/ai_chat/common/EvaluationConfig.ts
[error] 7-7: ../core/BuildConfig.js import should occur before import of ../core/Logger.js
(import/order)
[error] 54-54: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
front_end/panels/ai_chat/core/BuildConfig.ts
[error] 7-7: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
[error] 20-20: Newline required at end of file but not found.
(@stylistic/eol-last)
front_end/panels/ai_chat/ui/AIChatPanel.ts
[error] 23-23: ../core/BuildConfig.js import should occur before import of ../LLM/LLMClient.js
(import/order)
[error] 1399-1399: Trailing spaces not allowed.
(@stylistic/no-trailing-spaces)
🔇 Additional comments (4)
front_end/panels/ai_chat/core/BuildConfig.ts (1)
11-20: Config shape looks good; build-time flip is acceptable.Literal-typed flag via “as const” is OK; consumers use it as boolean. No changes needed.
front_end/panels/ai_chat/BUILD.gn (2)
51-52: LGTM: BuildConfig included in module.Included in sources ensures the flag is available to TS and metadata.
180-181: LGTM: BuildConfig added to metadata list.Keeps GRD checks aligned.
front_end/panels/ai_chat/ui/AIChatPanel.ts (1)
1994-1999: LGTM: OAuth UI suppressed in automated mode.Short-circuiting showOAuthLogin when BUILD_CONFIG.AUTOMATED_MODE is true matches the intended behavior.
| # Allow configurable automated mode | ||
| ARG AUTOMATED_MODE=true | ||
|
|
||
| # Set build-time flags based on Docker arg | ||
| RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ | ||
| sed -i 's/AUTOMATED_MODE: false/AUTOMATED_MODE: true/' \ | ||
| front_end/panels/ai_chat/core/BuildConfig.ts; \ | ||
| fi |
There was a problem hiding this comment.
Automated-mode sed edit may target a file that isn’t in the image (build will fail).
You never copy the repo’s front_end into the builder. The sed path will not exist in a vanilla devtools-frontend checkout.
Apply this to overlay our sources before the sed:
@@
-# Allow configurable automated mode
-ARG AUTOMATED_MODE=true
+# Allow configurable automated mode
+ARG AUTOMATED_MODE=true
@@
-# Set build-time flags based on Docker arg
-RUN if [ "$AUTOMATED_MODE" = "true" ]; then \
- sed -i 's/AUTOMATED_MODE: false/AUTOMATED_MODE: true/' \
- front_end/panels/ai_chat/core/BuildConfig.ts; \
- fi
+# Overlay local sources (includes ai_chat and BuildConfig.ts)
+COPY front_end /workspace/devtools/devtools-frontend/front_end
+
+# Set build-time flags based on Docker arg (robust match)
+RUN if [ "$AUTOMATED_MODE" = "true" ]; then \
+ sed -Ei 's/(AUTOMATED_MODE:\s*)false/\1true/' front_end/panels/ai_chat/core/BuildConfig.ts; \
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Allow configurable automated mode | |
| ARG AUTOMATED_MODE=true | |
| # Set build-time flags based on Docker arg | |
| RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ | |
| sed -i 's/AUTOMATED_MODE: false/AUTOMATED_MODE: true/' \ | |
| front_end/panels/ai_chat/core/BuildConfig.ts; \ | |
| fi | |
| # Allow configurable automated mode | |
| ARG AUTOMATED_MODE=true | |
| # Overlay local sources (includes ai_chat and BuildConfig.ts) | |
| COPY front_end /workspace/devtools/devtools-frontend/front_end | |
| # Set build-time flags based on Docker arg (robust match) | |
| RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ | |
| sed -Ei 's/(AUTOMATED_MODE:\s*)false/\1true/' front_end/panels/ai_chat/core/BuildConfig.ts; \ | |
| fi |
🤖 Prompt for AI Agents
In docker/Dockerfile around lines 51 to 58, the RUN sed targets
front_end/panels/ai_chat/core/BuildConfig.ts but the front_end sources are never
copied into the image, causing the sed to fail; add a COPY step before this RUN
to overlay the repo sources into the builder image (e.g., COPY front_end/
front_end/ or the specific path) so the file exists, or alternatively ensure the
directory is created and the specific file is copied into the image prior to
running sed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docker/Dockerfile (1)
53-57: sed targets a path that may not exist; overlay sources and fail if substitution is a no‑op.As-is, this RUN can fail when the file isn’t present in the image and will silently succeed if nothing is replaced. Overlay the repo’s front_end before sed, and assert the toggle occurred.
-# Set build-time flags based on Docker arg -RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ - sed -i 's/AUTOMATED_MODE: false/AUTOMATED_MODE: true/' \ - front_end/panels/ai_chat/core/BuildConfig.ts; \ - fi +# Overlay local sources (ensures BuildConfig.ts exists) +COPY front_end /workspace/devtools/devtools-frontend/front_end + +# Set build-time flags based on Docker arg (robust + fail-fast) +RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ + test -f front_end/panels/ai_chat/core/BuildConfig.ts && \ + sed -Ei 's/(AUTOMATED_MODE:\s*)false/\1true/' front_end/panels/ai_chat/core/BuildConfig.ts && \ + grep -Eq 'AUTOMATED_MODE:\s*true' front_end/panels/ai_chat/core/BuildConfig.ts; \ + fi
| # Build Browser Operator version with current changes | ||
| RUN npm run build |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
“with current changes” is misleading; build ignores PR workspace unless you COPY it.
This stage checks out upstream/main and never copies the PR’s tree, so local changes won’t be in the build. If the intent is to build this PR, COPY the needed paths (or the entire repo) before running npm run build.
Minimal overlay to include PR edits in the output:
-# Build Browser Operator version with current changes
-RUN npm run build
+# Build Browser Operator version with current changes
+# (already added above) COPY front_end /workspace/devtools/devtools-frontend/front_end
+RUN npm run buildIf you intentionally build a remote ref, consider making it explicit:
- RUN git fetch upstream
- RUN git checkout upstream/main
+ ARG SOURCE_REF=main
+ RUN git fetch upstream ${SOURCE_REF} && git checkout FETCH_HEAD🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing repo root:"
ls -la
echo
echo "Find Dockerfile(s) (using fd if available, fallback to find):"
if command -v fd >/dev/null 2>&1; then
fd -HI -t f 'Dockerfile' || true
else
find . -maxdepth 6 -type f -name 'Dockerfile' || true
fi
echo
echo "Search for 'RUN npm run build' across repo:"
rg -n --hidden -S "RUN npm run build" -g '!*node_modules/**' || true
echo
# Prefer docker/Dockerfile if present
if [ -f docker/Dockerfile ]; then
echo "---- docker/Dockerfile (with line numbers) ----"
nl -ba docker/Dockerfile | sed -n '1,240p'
echo "---- Relevant matches in docker/Dockerfile ----"
rg -n -C3 "git fetch|checkout upstream|checkout|upstream/main|COPY|WORKDIR|ARG SOURCE_REF|RUN npm run build" docker/Dockerfile || true
else
echo "docker/Dockerfile not found — printing every Dockerfile that contains 'npm run build'."
files=$(rg --no-line-number --hidden -S "RUN npm run build" --glob '!*node_modules/**' --files || true)
if [ -z "$files" ]; then
echo "No Dockerfile with 'npm run build' found."
else
for f in $files; do
echo
echo "---- $f (with line numbers) ----"
nl -ba "$f" | sed -n '1,240p'
echo "---- Relevant matches in $f ----"
rg -n -C3 "git fetch|checkout upstream|checkout|upstream/main|COPY|WORKDIR|ARG SOURCE_REF|RUN npm run build" "$f" || true
done
fi
fi
echo
echo "Also search repository for 'git fetch upstream' or 'checkout upstream' to confirm behavior:"
rg -n --hidden -S "git fetch upstream|checkout upstream|checkout FETCH_HEAD|checkout upstream/main" || trueLength of output: 3814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "---- docker/Dockerfile (full) ----"
sed -n '1,400p' docker/Dockerfile || true
echo
echo "---- Repo-wide searches ----"
echo "RUN npm run build occurrences:"
grep -nR --line-number --binary-files=without-match "RUN npm run build" || true
echo
echo "git fetch / checkout upstream occurrences:"
grep -nR --line-number --binary-files=without-match -E "git fetch|checkout upstream|checkout FETCH_HEAD|upstream/main" || true
echo
echo "COPY of front_end / repo files occurrences:"
grep -nR --line-number --binary-files=without-match -E "COPY .*front_end|COPY .*front-end|COPY \.|COPY .* /workspace" || true
echo
echo "---- Context around 'RUN npm run build' in docker/Dockerfile ----"
grep -n -C3 --line-number "RUN npm run build" docker/Dockerfile || trueLength of output: 3481
Build ignores local PR edits — Dockerfile checks out upstream/main before the final build.
docker/Dockerfile adds an upstream remote and runs git fetch && git checkout upstream/main before the second RUN npm run build and never COPYs the repo into the image, so “current changes” won’t be included. Location: docker/Dockerfile lines 46–60.
Minimal overlay to include PR edits in the output:
-# Build Browser Operator version with current changes
-RUN npm run build
+# Build Browser Operator version with current changes
+# (already added above) COPY front_end /workspace/devtools/devtools-frontend/front_end
+RUN npm run buildIf you intentionally build a remote ref, make that explicit:
- RUN git fetch upstream
- RUN git checkout upstream/main
+ ARG SOURCE_REF=main
+ RUN git fetch upstream ${SOURCE_REF} && git checkout FETCH_HEADCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docker/Dockerfile around lines 46–60 the Docker build adds an upstream remote
and checks out upstream/main before the final RUN npm run build, and the
repository contents are not copied into the image, so local PR edits are never
included; fix by removing the git fetch/checkout of upstream/main (or make the
remote ref explicit via a build ARG) and ensure the workspace is COPYed into the
image before running npm run build (i.e., add COPY . /app (or appropriate path)
and set WORKDIR before the build step) so the build uses current changes, or
alternatively make the Dockerfile accept a build-time ref/ARG and document that
it intentionally builds a remote ref.
|
@codex review |
![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 suggestions.
Reply with @codex fix comments to fix any unresolved comments.
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, or 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 fix this CI failure" or "@codex address that feedback".
| # Allow configurable automated mode | ||
| ARG AUTOMATED_MODE=false | ||
|
|
||
| # Set build-time flags based on Docker arg | ||
| RUN if [ "$AUTOMATED_MODE" = "true" ]; then \ | ||
| sed -i 's/AUTOMATED_MODE: false/AUTOMATED_MODE: true/' \ | ||
| front_end/panels/ai_chat/core/BuildConfig.ts; \ |
There was a problem hiding this comment.
[P1] Automated build never toggles automated mode
The README claims automated mode is the default build (docker build -f docker/Dockerfile -t browser-operator-automated .), but the Dockerfile declares ARG AUTOMATED_MODE=false and only flips the flag when the arg is explicitly true. With the documented command the sed step is skipped, leaving BUILD_CONFIG.AUTOMATED_MODE false so OAuth is still shown and evaluation stays manual. Builders will never get the advertised automated behavior unless they pass --build-arg AUTOMATED_MODE=true. Consider changing the default to true or update the instructions to pass the arg when automated mode is desired.
Useful? React with 👍 / 👎.
## Summary * **New Features** * Added Automated and Manual modes for Docker builds and runs. * Automated mode bypasses OAuth login UI and auto-enables evaluation by default (when no prior setting exists). * Manual mode preserves standard authentication and manual evaluation controls. * **Documentation** * Updated Docker README with mode-specific build/run instructions, image names, and a sample automated workflow. * Added guidance comparing Automated vs Manual modes, including authentication and evaluation behavior.
Summary by CodeRabbit
New Features
Documentation