Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Owner
Author
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThis PR implements production-ready feature gating for automations and macOS VM, introduces webhook gateway readiness detection with Tailscale Funnel support, adds GitHub webhook ingress handling, and extends CLI/IPC surfaces for gateway management while simplifying the automation rule editor UI. ChangesAutomations & Linear Webhook Gateway Production Gating
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Copilot stopped work on behalf of
arul28 due to an error
June 3, 2026 01:26
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
350
to
+382
| const ensureRelayEndpoint = async (force = false): Promise<RelayEnsureWebhookResponse | null> => { | ||
| const config = buildRelayConfig(); | ||
| if (!config.configured) { | ||
| updateStatus({ | ||
| relay: { | ||
| configured: false, | ||
| healthy: false, | ||
| status: "disabled", | ||
| webhookUrl: null, | ||
| endpointId: null, | ||
| }, | ||
| }); | ||
| return null; | ||
| const direct = buildDirectWebhookConfig(); | ||
| if (!direct.configured) { | ||
| updateStatus({ | ||
| relay: { | ||
| configured: false, | ||
| healthy: false, | ||
| status: "disabled", | ||
| webhookUrl: direct.webhookUrl, | ||
| endpointId: null, | ||
| lastError: direct.webhookUrl | ||
| ? `Missing ${DIRECT_WEBHOOK_SECRET_REF}.` | ||
| : "Missing webhook gateway public URL.", | ||
| }, | ||
| }); | ||
| return null; | ||
| } | ||
| try { | ||
| localSigningSecret = direct.signingSecret!; | ||
| const existing = await args.linearClient.listWebhooks(); | ||
| if (!existing.some((entry) => entry.url === direct.webhookUrl)) { | ||
| await args.linearClient.createWebhook({ | ||
| url: direct.webhookUrl!, | ||
| secret: direct.signingSecret!, | ||
| label: `ADE ${args.projectId} workflow ingress`, | ||
| resourceTypes: ["Issue", "IssueLabel"], | ||
| allPublicTeams: true, | ||
| }); | ||
| } | ||
| const payload: RelayEnsureWebhookResponse = { | ||
| endpointId: "direct", |
There was a problem hiding this comment.
ensureRelayEndpoint creates a new Linear webhook when direct.webhookUrl doesn't already exist in the list of registered webhooks. But when webhookGatewayPublicUrl is changed (via setWebhookGatewayPublicUrl), the new URL produces a different direct.webhookUrl. The check !existing.some((entry) => entry.url === direct.webhookUrl) now misses the old entry and registers a second webhook. Both old and new URLs remain active on Linear's side, causing every event to be delivered twice (duplicate handleIngressEvent calls) until the stale registration is manually removed.
The same issue applies in reverse: if the gateway URL is cleared, the previously registered webhook is abandoned on Linear but continues to receive deliveries that will now 404 or go unprocessed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/services/cto/linearIngressService.ts
Line: 350-382
Comment:
**Orphaned Linear webhooks on gateway URL change**
`ensureRelayEndpoint` creates a new Linear webhook when `direct.webhookUrl` doesn't already exist in the list of registered webhooks. But when `webhookGatewayPublicUrl` is changed (via `setWebhookGatewayPublicUrl`), the new URL produces a different `direct.webhookUrl`. The check `!existing.some((entry) => entry.url === direct.webhookUrl)` now misses the old entry and registers a second webhook. Both old and new URLs remain active on Linear's side, causing every event to be delivered twice (duplicate `handleIngressEvent` calls) until the stale registration is manually removed.
The same issue applies in reverse: if the gateway URL is cleared, the previously registered webhook is abandoned on Linear but continues to receive deliveries that will now 404 or go unprocessed.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
271
to
290
| }; | ||
| } | ||
|
|
||
| function isSourceCheckoutRuntimeModule(modulePath: string): boolean { | ||
| return /[/\\]apps[/\\]ade-cli[/\\](?:src|dist)[/\\]bootstrap\.(?:ts|js|cjs)$/i.test(modulePath); | ||
| } | ||
|
|
||
| function automationsEnabledForHeadlessRuntime(): boolean { | ||
| const override = readAutomationsEnvOverride(process.env); | ||
| if (override !== null) return override; | ||
| return isSourceCheckoutRuntimeModule(__filename); | ||
| } | ||
|
|
||
| function macosVmEnabledForHeadlessRuntime(): boolean { | ||
| const override = readMacosVmEnvOverride(process.env); | ||
| if (override !== null) return override; | ||
| return isSourceCheckoutRuntimeModule(__filename); | ||
| } | ||
|
|
||
| function resolveCurrentAdeCliEntry(): string | null { |
There was a problem hiding this comment.
isSourceCheckoutRuntimeModule regex matches dist/bootstrap.cjs in the source tree
The pattern matches both src/bootstrap.ts (dev) and dist/bootstrap.cjs (local build). A developer who builds the CLI and runs it outside the source tree would still be gated correctly, but this asymmetry is worth keeping in mind if the dist/ artifacts are ever distributed separately from the packaged build.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ade-cli/src/bootstrap.ts
Line: 271-290
Comment:
**`isSourceCheckoutRuntimeModule` regex matches `dist/bootstrap.cjs` in the source tree**
The pattern matches both `src/bootstrap.ts` (dev) and `dist/bootstrap.cjs` (local build). A developer who builds the CLI and runs it outside the source tree would still be gated correctly, but this asymmetry is worth keeping in mind if the `dist/` artifacts are ever distributed separately from the packaged build.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/renderer/browserMock.ts (1)
3936-4003:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the webhook gateway mock stateful.
setWebhookGatewayPublicUrl()ignores its input and always returns the hard-coded URL, andrefreshWebhookGatewayStatus()never reflects any prior change. That makes the new gateway-management UI look broken in browser preview and hides renderer/IPC contract regressions in the exact flow this PR adds.Keep a mutable mock ingress-status object and update
webhookGateway.publicUrl/lastCheckedAtbefore returning it from these methods.🤖 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 `@apps/desktop/src/renderer/browserMock.ts` around lines 3936 - 4003, The current mocks (getIngressStatus, refreshWebhookGatewayStatus, setWebhookGatewayPublicUrl) always return hard-coded objects and ignore prior changes; make the mock stateful by creating a shared mutable ingressStatus object used by getIngressStatus, refreshWebhookGatewayStatus, and setWebhookGatewayPublicUrl, have setWebhookGatewayPublicUrl update ingressStatus.webhookGateway.publicUrl and ingressStatus.webhookGateway.lastCheckedAt from the input, and have refreshWebhookGatewayStatus return the current ingressStatus (updating lastCheckedAt when simulating a refresh) so subsequent calls and the UI/IPC flow reflect prior changes.apps/desktop/src/renderer/components/automations/ActionRow.tsx (1)
69-87:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't strand existing per-step runtime/model settings.
ActionRowValuestill carriestargetLaneId,condition,continueOnFailure,timeoutMs,retry,modelConfig, andpermissionConfig, and the save path still preserves them, but this UI no longer lets users inspect, edit, or clear any of those fields. Existing rules can now show acustom modelbadge while giving the user no way to change the underlying config. Either keep the editors or migrate/strip unsupported fields explicitly on load/save.Also applies to: 152-251
🤖 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 `@apps/desktop/src/renderer/components/automations/ActionRow.tsx` around lines 69 - 87, ActionRow currently accepts and preserves fields on ActionRowValue (targetLaneId, condition, continueOnFailure, timeoutMs, retry, modelConfig, permissionConfig) but the UI no longer exposes editors for them; either re-introduce editors for those fields in ActionRow (or linked subcomponents) so users can inspect/edit/clear them, or explicitly migrate/strip unsupported fields when loading/saving: update the component logic that consumes/returns ActionRowValue to transform incoming value (removing or mapping modelConfig/permissionConfig and other runtime fields) and ensure onChange/onRemove handlers emit the cleaned shape, and add a clear migration function (e.g., normalizeActionRowValue) used in ActionRow and any save path to avoid stranded configs.apps/ade-cli/src/bootstrap.ts (1)
927-947:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPackaged runtimes still wire
linearIngressService.
createHeadlessLinearServices()is unconditional here, so packaged/headless runtimes still construct and expose Linear ingress even though this PR is supposed to block production runtime wiring for it. Hiding the CLI commands alone won't cover the socket-backed/runtime path if callers can still reach this service directly. Based on learnings: For ADE CLI changes, verify both headless mode and the desktop socket-backed ADE RPC path.🤖 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 `@apps/ade-cli/src/bootstrap.ts` around lines 927 - 947, The code unconditionally calls createHeadlessLinearServices (producing headlessLinearServices and wiring linearIngressService) which causes packaged/headless runtimes to expose Linear ingress; guard this creation with the same runtime/feature flag used to block production wiring (e.g., check isPackagedRuntime or enableLinearIngress) so createHeadlessLinearServices is only invoked when Linear should be available; update the caller site where headlessLinearServices is used and any socket-backed ADE RPC path to respect the same condition so neither headless nor desktop socket-backed paths construct/expose the Linear ingress service.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/adeActions/registry.test.ts (1)
98-147: ⚡ Quick winAdd override-path coverage for packaged builds.
These tests only cover the default packaged/dev states. The load-bearing behavior in this PR is that packaged builds can be re-enabled via internal env overrides, so regressions in
ADE_ENABLE_AUTOMATIONS/ADE_ENABLE_MACOS_VMwould still pass here.🤖 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 `@apps/desktop/src/main/services/adeActions/registry.test.ts` around lines 98 - 147, Update the test suite for getAdeActionDomainServices to add cases that verify packaged builds respect the ADE_ENABLE_AUTOMATIONS / ADE_ENABLE_MACOS_VM override envs (and their corresponding ADE_DISABLE_* counterparts) by using withEnv to set override variables and asserting that automations/automation_planner and macos_vm domains are present when enabled and absent when explicitly disabled; reference the existing tests around getAdeActionDomainServices and withEnv to add these packaged-override scenarios so regressions in ADE_ENABLE_AUTOMATIONS and ADE_ENABLE_MACOS_VM behavior are covered.
🤖 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 `@apps/ade-cli/src/cli.ts`:
- Around line 248-264: The path-based gate is using the raw CLI_ENTRY_PATH (from
process.argv[1]) which can be a symlink and cause false negatives; update
isSourceCheckoutCliEntryPath to resolve the path before testing (e.g., call a
safe realpath resolver like realpathSync in try/catch or a realpathSyncSafe
utility inside isSourceCheckoutCliEntryPath or at the call sites
automationsCliEnabled/macosVmCliEnabled) and then run the existing regex checks
against the resolved target (so the /apps/ade-cli/dist/cli.cjs and source paths
match actual file targets rather than symlinked argv values).
In `@apps/ade-cli/src/headlessLinearServices.ts`:
- Around line 1897-1908: The current try/catch around the whole loop allows the
first failing dispatch to abort remaining dispatches; update
buildLinearAutomationDispatches handling so each dispatched item is handled
independently: either map the dispatches to promises and use Promise.allSettled
to run them concurrently and log any failures, or wrap each await
automationService.dispatchIngressTrigger(dispatched) in its own try/catch so a
single failure (inside the loop over buildLinearAutomationDispatches(event))
only logs via args.logger.warn (include issueId and the specific dispatched info
and error) and continues processing the rest; reference
buildLinearAutomationDispatches, automationService.dispatchIngressTrigger, and
args.logger.warn (and issueId) when making the change.
In `@apps/desktop/src/main/services/adeActions/registry.test.ts`:
- Around line 12-34: The helper withEnv currently accepts a synchronous callback
and restores process.env immediately because it doesn't await async callbacks;
change withEnv to be asynchronous by updating its signature to async function
withEnv<T>(updates: Record<string, string | undefined>, run: () => Promise<T> |
T): Promise<T>, call and await the result of run() inside the try block (e.g.,
const result = await run()), then return that result; keep the existing env
save/restore logic in the finally block so environment variables remain set for
the entire duration of the awaited callback — reference the withEnv function to
make these edits.
In `@apps/desktop/src/main/services/automations/automationIngressService.ts`:
- Around line 164-180: The PR comment webhook path in
automationIngressService.ts fabricates a PR context without
headBranch/baseBranch or branch/targetBranch, so branch-filtered automations in
automationService.ts won't match; update the issue_comment handling (the block
that builds issue, computes issueIsPr and returns triggerType/pr) to either (A)
hydrate full PR details when issueIsPr is true by fetching the PR via the GitHub
API and populating headBranch/baseBranch and branch/targetBranch on the returned
object before returning, or (B) explicitly mark PR comments as unsupported by
returning null for PR triggers and/or returning a distinct unsupported flag so
callers skip branch-matching; target the code paths using readNested,
buildIssueContext, the issueIsPr boolean, and the returned pr object so the
dispatched trigger includes the branch information automationService.ts expects.
In `@apps/desktop/src/main/services/automations/automationService.ts`:
- Around line 1196-1261: refreshWebhookGatewayStatus currently updates
ingressStatusRef but does not re-schedule gateway-backed automations when the
webhook gateway flips ready or its publicUrl changes; detect changes to
readiness/publicUrl by comparing the newly-computed next (from
refreshWebhookGatewayStatus) with the previous ingressStatusRef.webhookGateway
(or store the previous ready/publicUrl at start of the function) and if ready or
publicUrl changed from false/null to true/non-null, call syncFromConfig() to
re-sync gateway-backed rules. Keep the existing updateIngressStatus({
webhookGateway: next }) behavior but invoke syncFromConfig() after
updateIngressStatus when the readiness/publicUrl transition is detected;
reference refreshWebhookGatewayStatus, ingressStatusRef.webhookGateway,
updateIngressStatus, and syncFromConfig in the implementation.
- Around line 65-75: normalizePublicWebhookUrl currently only checks the scheme;
update it to reject non-public hosts so internal addresses (localhost,
127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, IPv6 ::1, fc00::/7,
link-local fe80::/10, and literal "localhost" or ".local" names) return null. In
normalizePublicWebhookUrl(), after creating the URL and verifying protocol ===
"https:", extract url.hostname and if it's an IP use a standard IP check
(net.isIP or equivalent) to detect private/loopback ranges and reject them, or
if it's a hostname reject explicit "localhost" and local mDNS patterns (e.g.
ending with ".local"); only allow otherwise and then trim trailing slash as
before so defaultWebhookGatewayStatus()/getIngressStatus() won't treat
non-public endpoints as ready. Ensure error handling still returns null on parse
failures.
In `@apps/desktop/src/main/services/config/projectConfigService.ts`:
- Around line 1919-1923: Validate webhookGatewayPublicUrl as a proper HTTPS
origin before assigning it to out.webhookGatewayPublicUrl: after computing
webhookGatewayPublicUrl = asString(... )?.trim(), attempt to parse it with the
URL constructor and ensure url.protocol === 'https:' and that the URL represents
a base/origin (e.g. url.pathname === '/' or url.pathname === ''), and only then
set out.webhookGatewayPublicUrl = webhookGatewayPublicUrl; if the value fails
validation, skip assigning it (and optionally surface/record a validation error
elsewhere). This change targets the webhookGatewayPublicUrl handling in
projectConfigService (the webhookGatewayPublicUrl local variable and the out.ui
assignment).
In `@apps/desktop/src/main/services/cto/linearIngressService.ts`:
- Around line 369-399: The code only checks webhook.url when ensuring a webhook,
so if direct.signingSecret rotates the existing Linear webhook remains with the
old secret; modify the ensure logic around localSigningSecret /
args.linearClient.listWebhooks() to also compare the stored signing secret
(direct.signingSecret) against the existing webhook.secret and, if different,
call the appropriate update path (use args.linearClient.updateWebhook(...) if
available or delete + args.linearClient.createWebhook(...)) to replace the
secret for the matching direct.webhookUrl; update the
RelayEnsureWebhookResponse/relayWebhook and call updateStatus(...) after the
update so state reflects the new signing secret and health.
In `@apps/desktop/src/renderer/components/automations/RulesTab.tsx`:
- Around line 785-815: When switching selection to manualRunRule you must guard
against accidentally overwriting an in-memory draft and preserve the
created-lane draft; modify the onClick handlers that call
setSelectedRuleId(manualRunRule.id) (the "Create lanes for runs" and "Edit lane
mode" handlers) to first call confirmDiscardIfDirty() and, if it
returns/permits, set a one-shot override ref (e.g., draftOverrideRef) containing
the precomputed draftWithCreateLaneMode(toDraftFromRule(manualRunRule)) (for the
Create path) or the current draft (for Edit), then call
setSelectedRuleId(manualRunRule.id) and setDraft from that overrideRef instead
of relying on the selection effect to re-run toDraftFromRule(selected); also
clear the override ref after applying and keep savedSnapshotRef/current issue
state updates as before.
In `@apps/desktop/src/shared/automationAvailability.ts`:
- Around line 17-20: In readAutomationsEnvOverride, currently
envFlagEnabled(AUTOMATIONS_ENABLE_ENV) is checked before the disable flag so a
set disable flag can be overridden; change the logic so the disable flag takes
precedence (check envFlagEnabled(env[AUTOMATIONS_DISABLE_ENV]) first and return
false if set) or explicitly detect both flags set and return false (or throw if
you prefer strict behavior); apply the same change to the analogous function
around lines 32-35 (the other env-reading function) — reference symbols:
readAutomationsEnvOverride, AUTOMATIONS_ENABLE_ENV, AUTOMATIONS_DISABLE_ENV.
In `@apps/desktop/src/shared/ipc.ts`:
- Around line 446-447: The new IPC channels/APIs (constants
automationsRefreshWebhookGatewayStatus and automationsSetWebhookGatewayPublicUrl
in ipc.ts, the ipcMain.handle registrations in registerIpc.ts, and the preload
methods window.ade.automations.refreshWebhookGatewayStatus /
setWebhookGatewayPublicUrl plus typings) are not used anywhere in production
renderer code—only referenced in browserMock.ts; either wire them into the real
UI or remove them. To fix: if the UI needs them, add calls from the appropriate
renderer component(s) (search for automation UI pages/components) to invoke
window.ade.automations.refreshWebhookGatewayStatus() and
setWebhookGatewayPublicUrl() and update any component props/state to surface
results; otherwise remove the unused constants in ipc.ts, remove the
ipcMain.handle registrations in registerIpc.ts, strip the preload methods and
global.d.ts typings, and delete the mock usage in browserMock.ts so there are no
unused IPC entry points left.
---
Outside diff comments:
In `@apps/ade-cli/src/bootstrap.ts`:
- Around line 927-947: The code unconditionally calls
createHeadlessLinearServices (producing headlessLinearServices and wiring
linearIngressService) which causes packaged/headless runtimes to expose Linear
ingress; guard this creation with the same runtime/feature flag used to block
production wiring (e.g., check isPackagedRuntime or enableLinearIngress) so
createHeadlessLinearServices is only invoked when Linear should be available;
update the caller site where headlessLinearServices is used and any
socket-backed ADE RPC path to respect the same condition so neither headless nor
desktop socket-backed paths construct/expose the Linear ingress service.
In `@apps/desktop/src/renderer/browserMock.ts`:
- Around line 3936-4003: The current mocks (getIngressStatus,
refreshWebhookGatewayStatus, setWebhookGatewayPublicUrl) always return
hard-coded objects and ignore prior changes; make the mock stateful by creating
a shared mutable ingressStatus object used by getIngressStatus,
refreshWebhookGatewayStatus, and setWebhookGatewayPublicUrl, have
setWebhookGatewayPublicUrl update ingressStatus.webhookGateway.publicUrl and
ingressStatus.webhookGateway.lastCheckedAt from the input, and have
refreshWebhookGatewayStatus return the current ingressStatus (updating
lastCheckedAt when simulating a refresh) so subsequent calls and the UI/IPC flow
reflect prior changes.
In `@apps/desktop/src/renderer/components/automations/ActionRow.tsx`:
- Around line 69-87: ActionRow currently accepts and preserves fields on
ActionRowValue (targetLaneId, condition, continueOnFailure, timeoutMs, retry,
modelConfig, permissionConfig) but the UI no longer exposes editors for them;
either re-introduce editors for those fields in ActionRow (or linked
subcomponents) so users can inspect/edit/clear them, or explicitly migrate/strip
unsupported fields when loading/saving: update the component logic that
consumes/returns ActionRowValue to transform incoming value (removing or mapping
modelConfig/permissionConfig and other runtime fields) and ensure
onChange/onRemove handlers emit the cleaned shape, and add a clear migration
function (e.g., normalizeActionRowValue) used in ActionRow and any save path to
avoid stranded configs.
---
Nitpick comments:
In `@apps/desktop/src/main/services/adeActions/registry.test.ts`:
- Around line 98-147: Update the test suite for getAdeActionDomainServices to
add cases that verify packaged builds respect the ADE_ENABLE_AUTOMATIONS /
ADE_ENABLE_MACOS_VM override envs (and their corresponding ADE_DISABLE_*
counterparts) by using withEnv to set override variables and asserting that
automations/automation_planner and macos_vm domains are present when enabled and
absent when explicitly disabled; reference the existing tests around
getAdeActionDomainServices and withEnv to add these packaged-override scenarios
so regressions in ADE_ENABLE_AUTOMATIONS and ADE_ENABLE_MACOS_VM behavior are
covered.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1696eb2-cb60-49bb-a823-c00285afb7fb
📒 Files selected for processing (38)
apps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/headlessLinearServices.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/automations/automationIngressService.test.tsapps/desktop/src/main/services/automations/automationIngressService.tsapps/desktop/src/main/services/automations/automationPlannerService.test.tsapps/desktop/src/main/services/automations/automationPlannerService.tsapps/desktop/src/main/services/automations/automationService.test.tsapps/desktop/src/main/services/automations/automationService.tsapps/desktop/src/main/services/config/projectConfigService.test.tsapps/desktop/src/main/services/config/projectConfigService.tsapps/desktop/src/main/services/cto/linearIngressService.tsapps/desktop/src/main/services/cto/linearIntake.test.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/TabNav.tsxapps/desktop/src/renderer/components/automations/ActionList.tsxapps/desktop/src/renderer/components/automations/ActionRow.tsxapps/desktop/src/renderer/components/automations/AdeActionEditor.tsxapps/desktop/src/renderer/components/automations/AutomationsComingSoon.tsxapps/desktop/src/renderer/components/automations/AutomationsPage.tsxapps/desktop/src/renderer/components/automations/AutomationsTemplatesPage.tsxapps/desktop/src/renderer/components/automations/RulesTab.tsxapps/desktop/src/renderer/components/automations/components/RuleEditorPanel.tsxapps/desktop/src/renderer/components/automations/designTokens.tsapps/desktop/src/renderer/components/automations/shared.tsapps/desktop/src/renderer/components/graph/WorkspaceGraphPage.tsxapps/desktop/src/shared/adeCliGuidance.tsapps/desktop/src/shared/automationAvailability.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/automations.tsapps/desktop/src/shared/types/config.ts
Comment on lines
+248
to
+264
| function isSourceCheckoutCliEntryPath(modulePath: string): boolean { | ||
| return ( | ||
| isSourceCliEntryPath(modulePath) || | ||
| /[/\\]apps[/\\]ade-cli[/\\]dist[/\\]cli\.cjs$/i.test(modulePath) | ||
| ); | ||
| } | ||
|
|
||
| function automationsCliEnabled(): boolean { | ||
| const override = readAutomationsEnvOverride(process.env); | ||
| if (override !== null) return override; | ||
| return isSourceCheckoutCliEntryPath(CLI_ENTRY_PATH); | ||
| } | ||
|
|
||
| function macosVmCliEnabled(): boolean { | ||
| const override = readMacosVmEnvOverride(process.env); | ||
| if (override !== null) return override; | ||
| return isSourceCheckoutCliEntryPath(CLI_ENTRY_PATH); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate cli.ts and print relevant regions around the reported lines
FILE="apps/ade-cli/src/cli.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"; exit 1
fi
echo "== Context around lines 220-310 =="
nl -ba "$FILE" | sed -n '220,310p'
# Find where CLI_ENTRY_PATH and any dist/module path constants are defined in this file
echo "== Definitions/uses of CLI_ENTRY_PATH and dist constants in cli.ts =="
rg -n "CLI_ENTRY_PATH|CLI_DIST_PATH|process\.argv\[1\]|argv\[1\]|realpath|realpathSync|realpathSyncSafe|sourceCliEntryPath|isSourceCliEntryPath" "$FILE"
# Print the implementations of helpers used here (isSourceCliEntryPath, read*EnvOverride) if in this file
echo "== Helper implementations (isSourceCliEntryPath/readAutomationsEnvOverride/readMacosVmEnvOverride) =="
rg -n "function (isSourceCliEntryPath|readAutomationsEnvOverride|readMacosVmEnvOverride|read.*EnvOverride)|const (CLI_ENTRY_PATH|CLI_DIST_PATH)" "$FILE"
# If functions are found, show their bodies
for sym in isSourceCliEntryPath readAutomationsEnvOverride readMacosVmEnvOverride; do
line=$(rg -n "function $sym" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-20)); end=$((line+80))
echo "== $sym around $line =="
nl -ba "$FILE" | sed -n "${start},${end}p"
fi
done
# Search for realpathSyncSafe in repo (to see if it's available/used elsewhere)
echo "== realpathSyncSafe usage in repo =="
rg -n "realpathSyncSafe" -S .
# Search for CLI_ENTRY_PATH derivation elsewhere (if imported)
echo "== Any other references to CLI_ENTRY_PATH (name-based) =="
rg -n "CLI_ENTRY_PATH" -S apps/ade-cli/src
# Search for gating command logic around automations/macOS-vm
echo "== References to automationsCliEnabled/macosVmCliEnabled in cli.ts =="
rg -n "automationsCliEnabled\(|macosVmCliEnabled\(|automations(?!CliEnabled)|macos-vm" "$FILE" || trueRepository: arul28/ADE
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/ade-cli/src/cli.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"; exit 1
fi
echo "== Context around lines 220-310 =="
cat -n "$FILE" | sed -n '220,310p'
echo "== Definitions/uses of CLI_ENTRY_PATH and dist constants in cli.ts =="
rg -n "CLI_ENTRY_PATH|CLI_DIST_PATH|process\.argv\[1\]|argv\[1\]|realpath|realpathSync|realpathSyncSafe|sourceCliEntryPath|isSourceCliEntryPath" "$FILE" || true
echo "== Helper implementations (isSourceCliEntryPath/readAutomationsEnvOverride/readMacosVmEnvOverride) =="
rg -n "function (isSourceCliEntryPath|readAutomationsEnvOverride|readMacosVmEnvOverride)|const (CLI_ENTRY_PATH|CLI_DIST_PATH)" "$FILE" || true
for sym in isSourceCliEntryPath readAutomationsEnvOverride readMacosVmEnvOverride; do
line=$(rg -n "function $sym" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-20)); end=$((line+100))
echo "== $sym around $line =="
cat -n "$FILE" | sed -n "${start},${end}p"
fi
done
echo "== realpathSyncSafe usage in repo =="
rg -n "realpathSyncSafe" -S . || true
echo "== Any other references to CLI_ENTRY_PATH (name-based) =="
rg -n "CLI_ENTRY_PATH" -S apps/ade-cli/src || true
echo "== References to automationsCliEnabled/macosVmCliEnabled in cli.ts =="
rg -n "automationsCliEnabled\(|macosVmCliEnabled\(|automations(?!CliEnabled)|macos-vm" "$FILE" || trueRepository: arul28/ADE
Length of output: 11246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/ade-cli/src/cli.ts"
echo "== CLI_ENTRY_PATH definition context =="
cat -n "$FILE" | sed -n '180,235p'
echo "== realpathSyncSafe implementation =="
cat -n "$FILE" | sed -n '12020,12090p'
echo "== realpathIfExists usage helper (if present) =="
# Show the import location around line ~1-60
cat -n "$FILE" | sed -n '1,80p'
echo "== bootstrap.ts where ADE_CLI_ENTRY_PATH is set =="
FILE2="apps/ade-cli/src/bootstrap.ts"
if [ -f "$FILE2" ]; then
cat -n "$FILE2" | sed -n '360,430p'
fi
echo "== Where is CLI_ENTRY_PATH overridden / sourced from env/global =="
rg -n "ADE_CLI_ENTRY_PATH|CLI_ENTRY_PATH\s*=" "$FILE" apps/ade-cli/src/bootstrap.tsRepository: arul28/ADE
Length of output: 11719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/ade-cli/src/cli.ts"
echo "== Find automations/macOS-vm gating usage in cli.ts (help + command dispatch) =="
rg -n "automationsCliEnabled\(|macosVmCliEnabled\(|assertAutomationsCliEnabled\(|assertMacosVmCliEnabled\(|AUTOMATIONS_COMING_SOON_MESSAGE|MACOS_VM_COMING_SOON_MESSAGE|linear ingress|macos-vm" "$FILE"
echo "== Snippet around where automationsCliEnabled is called =="
# Since rg might not show context, show nearby lines for any occurrences
for pat in "automationsCliEnabled(" "macosVmCliEnabled(" "assertAutomationsCliEnabled" "assertMacosVmCliEnabled"; do
echo "--- occurrences for $pat ---"
rg -n "$pat" "$FILE" | head -n 20
done
echo "== Show a broader window around the first few matches for command gating/help =="
matches=$(rg -n "automationsCliEnabled\(|macosVmCliEnabled\(|assertAutomationsCliEnabled\(|assertMacosVmCliEnabled\(|linear ingress|macos-vm" "$FILE" | cut -d: -f1 | head -n 20)
i=0
for line in $matches; do
i=$((i+1))
start=$((line-15)); end=$((line+60))
echo "=== window around line $line (set $i) ==="
sed -n "${start},${end}p" "$FILE" | nl -ba -w2 -s' ' | head -n 110
doneRepository: arul28/ADE
Length of output: 5969
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/ade-cli/src/cli.ts"
echo "== automations/macOS-vm gating: windows around lines 480-560 =="
cat -n "$FILE" | sed -n '430,570p'
echo "== help command map entries: windows around lines 1260-1360 =="
cat -n "$FILE" | sed -n '1260,1360p'
echo "== where HELP_BY_COMMAND['macos-vm'] / automations filtering happens (around 7400-7600) =="
cat -n "$FILE" | sed -n '7400,7560p'
echo "== command dispatch area: around 10180-10520 =="
cat -n "$FILE" | sed -n '10080,10560p'
echo "== search: ADE_CLI_ENTRY_PATH usage in cli.ts =="
rg -n "ADE_CLI_ENTRY_PATH" "$FILE" || true
echo "== search: process.argv[1] usage elsewhere in cli.ts for gating expectations =="
rg -n "process\.argv\[1\]" "$FILE" | head -n 50Repository: arul28/ADE
Length of output: 40334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for ade-cli/bin / executable entrypoints =="
rg -n "ade-cli|dist/cli\.cjs|src/cli\.ts|bin/ade|#!/usr/bin/env node" -S apps/ade-cli . || true
echo "== List package.json entrypoints for ade-cli (bin/module) =="
fd -e package.json apps/ade-cli | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}" | head -n 80' || true
echo "== Search for bin field or cli file references in package.json files =="
rg -n "\"bin\"|\"main\"|\"exports\"" apps/ade-cli -S || trueRepository: arul28/ADE
Length of output: 50739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/ade-cli/README.md"
if [ ! -f "$FILE" ]; then echo "Missing $FILE"; exit 1; fi
cat -n "$FILE" | sed -n '60,100p'Repository: arul28/ADE
Length of output: 2049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n apps/ade-cli/package.json | sed -n '1,60p'Repository: arul28/ADE
Length of output: 2182
Use a realpath-based check for source-checkout gating (avoid argv[1] path-string matches).
CLI_ENTRY_PATH is derived from process.argv[1] (only path.resolved) and isSourceCheckoutCliEntryPath() gates automations/macos-vm help + command dispatch purely via regexes on that path. When ade is invoked through npm link/global shims (symlinked executable path like .../bin/ade rather than .../apps/ade-cli/dist/cli.cjs), the gate can flip off and hide/block automations and macos-vm even from a source/dev build.
Fix by resolving the executable target path (e.g., realpathSyncSafe(modulePath)) before applying the /src/cli.ts or /apps/ade-cli/dist/cli.cjs checks, or by deriving the check from the resolved package/module root rather than the raw argv[1] string. [Major: apps/ade-cli/src/cli.ts isSourceCheckoutCliEntryPath + automationsCliEnabled/macosVmCliEnabled call sites]
🤖 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 `@apps/ade-cli/src/cli.ts` around lines 248 - 264, The path-based gate is using
the raw CLI_ENTRY_PATH (from process.argv[1]) which can be a symlink and cause
false negatives; update isSourceCheckoutCliEntryPath to resolve the path before
testing (e.g., call a safe realpath resolver like realpathSync in try/catch or a
realpathSyncSafe utility inside isSourceCheckoutCliEntryPath or at the call
sites automationsCliEnabled/macosVmCliEnabled) and then run the existing regex
checks against the resolved target (so the /apps/ade-cli/dist/cli.cjs and source
paths match actual file targets rather than symlinked argv values).
Comment on lines
+1897
to
+1908
| const automationService = args.getAutomationService?.() ?? null; | ||
| if (!automationService) return; | ||
| try { | ||
| for (const dispatched of buildLinearAutomationDispatches(event)) { | ||
| await automationService.dispatchIngressTrigger(dispatched); | ||
| } | ||
| } catch (error) { | ||
| args.logger.warn("linear.headless_automation_dispatch_failed", { | ||
| issueId, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Don't let one automation dispatch abort the rest.
The try/catch wraps the whole loop, so the first dispatchIngressTrigger rejection stops every later dispatch generated from the same Linear event. One bad rule can suppress unrelated automations here. Catch per dispatch or use Promise.allSettled.
🤖 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 `@apps/ade-cli/src/headlessLinearServices.ts` around lines 1897 - 1908, The
current try/catch around the whole loop allows the first failing dispatch to
abort remaining dispatches; update buildLinearAutomationDispatches handling so
each dispatched item is handled independently: either map the dispatches to
promises and use Promise.allSettled to run them concurrently and log any
failures, or wrap each await
automationService.dispatchIngressTrigger(dispatched) in its own try/catch so a
single failure (inside the loop over buildLinearAutomationDispatches(event))
only logs via args.logger.warn (include issueId and the specific dispatched info
and error) and continues processing the rest; reference
buildLinearAutomationDispatches, automationService.dispatchIngressTrigger, and
args.logger.warn (and issueId) when making the change.
Comment on lines
+12
to
+34
| function withEnv<T>(updates: Record<string, string | undefined>, run: () => T): T { | ||
| const previous = new Map<string, string | undefined>(); | ||
| for (const key of Object.keys(updates)) { | ||
| previous.set(key, process.env[key]); | ||
| const value = updates[key]; | ||
| if (value === undefined) { | ||
| delete process.env[key]; | ||
| } else { | ||
| process.env[key] = value; | ||
| } | ||
| } | ||
| try { | ||
| return run(); | ||
| } finally { | ||
| for (const [key, value] of previous) { | ||
| if (value === undefined) { | ||
| delete process.env[key]; | ||
| } else { | ||
| process.env[key] = value; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Make withEnv await async callbacks.
finally runs as soon as run() returns, so await withEnv(..., async () => ...) restores process.env before the async body finishes. The current signature makes that misuse look valid.
Suggested fix
-function withEnv<T>(updates: Record<string, string | undefined>, run: () => T): T {
+async function withEnv<T>(
+ updates: Record<string, string | undefined>,
+ run: () => T | Promise<T>,
+): Promise<T> {
const previous = new Map<string, string | undefined>();
for (const key of Object.keys(updates)) {
previous.set(key, process.env[key]);
const value = updates[key];
if (value === undefined) {
@@
}
try {
- return run();
+ return await run();
} finally {
for (const [key, value] of previous) {
if (value === undefined) {
delete process.env[key];
} else {🤖 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 `@apps/desktop/src/main/services/adeActions/registry.test.ts` around lines 12 -
34, The helper withEnv currently accepts a synchronous callback and restores
process.env immediately because it doesn't await async callbacks; change withEnv
to be asynchronous by updating its signature to async function
withEnv<T>(updates: Record<string, string | undefined>, run: () => Promise<T> |
T): Promise<T>, call and await the result of run() inside the try block (e.g.,
const result = await run()), then return that result; keep the existing env
save/restore logic in the finally block so environment variables remain set for
the entire duration of the awaited callback — reference the withEnv function to
make these edits.
Comment on lines
+164
to
+180
| if (githubEvent === "issue_comment") { | ||
| if (action && action !== "created") return null; | ||
| const rawIssue = readNested(payload, "issue"); | ||
| const comment = readNested(payload, "comment"); | ||
| const issueIsPr = Boolean(readNested(rawIssue, "pull_request")); | ||
| const issue = buildIssueContext(rawIssue, repo); | ||
| if (!issue) return null; | ||
| return { | ||
| triggerType: issueIsPr ? "github.pr_commented" : "github.issue_commented", | ||
| summary: `GitHub ${issueIsPr ? "PR" : "issue"} #${issue.number} commented: ${issue.title}`, | ||
| author: readUserLogin(comment) ?? readLogin(readNested(payload, "sender")), | ||
| labels: issue.labels, | ||
| issue: issueIsPr ? null : issue, | ||
| pr: issueIsPr | ||
| ? { ...issue, baseBranch: undefined, headBranch: undefined, draft: false, merged: false } | ||
| : null, | ||
| }; |
There was a problem hiding this comment.
PR comment webhooks lose the branch data needed for matching.
apps/desktop/src/main/services/automations/automationService.ts matches github.pr_commented branch filters against trigger.branch, but this issue_comment path fabricates a PR context from the issue payload without headBranch/baseBranch and never sets branch/targetBranch. Branch-filtered PR comment automations that work via apps/desktop/src/main/services/automations/githubPollingService.ts:240-310 will be silently missed here. Either hydrate the PR details before dispatch or treat PR comments as unsupported on the direct webhook path.
🤖 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 `@apps/desktop/src/main/services/automations/automationIngressService.ts`
around lines 164 - 180, The PR comment webhook path in
automationIngressService.ts fabricates a PR context without
headBranch/baseBranch or branch/targetBranch, so branch-filtered automations in
automationService.ts won't match; update the issue_comment handling (the block
that builds issue, computes issueIsPr and returns triggerType/pr) to either (A)
hydrate full PR details when issueIsPr is true by fetching the PR via the GitHub
API and populating headBranch/baseBranch and branch/targetBranch on the returned
object before returning, or (B) explicitly mark PR comments as unsupported by
returning null for PR triggers and/or returning a distinct unsupported flag so
callers skip branch-matching; target the code paths using readNested,
buildIssueContext, the issueIsPr boolean, and the returned pr object so the
dispatched trigger includes the branch information automationService.ts expects.
Comment on lines
+65
to
+75
| function normalizePublicWebhookUrl(value: string | null | undefined): string | null { | ||
| const trimmed = value?.trim(); | ||
| if (!trimmed) return null; | ||
| try { | ||
| const url = new URL(trimmed); | ||
| if (url.protocol !== "https:") return null; | ||
| return url.toString().replace(/\/$/, ""); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject non-public HTTPS hosts here.
normalizePublicWebhookUrl() currently accepts any https: URL, so values like https://127.0.0.1/..., https://localhost/..., or RFC1918 hosts will flow through as “ready” via defaultWebhookGatewayStatus()/getIngressStatus() and let external-trigger automations be enabled even though GitHub/Linear can never reach them. This needs host-level validation, not just a scheme check.
🤖 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 `@apps/desktop/src/main/services/automations/automationService.ts` around lines
65 - 75, normalizePublicWebhookUrl currently only checks the scheme; update it
to reject non-public hosts so internal addresses (localhost, 127.0.0.0/8,
10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, IPv6 ::1, fc00::/7, link-local
fe80::/10, and literal "localhost" or ".local" names) return null. In
normalizePublicWebhookUrl(), after creating the URL and verifying protocol ===
"https:", extract url.hostname and if it's an IP use a standard IP check
(net.isIP or equivalent) to detect private/loopback ranges and reject them, or
if it's a hostname reject explicit "localhost" and local mDNS patterns (e.g.
ending with ".local"); only allow otherwise and then trim trailing slash as
before so defaultWebhookGatewayStatus()/getIngressStatus() won't treat
non-public endpoints as ready. Ensure error handling still returns null on parse
failures.
Comment on lines
+1919
to
+1923
| const webhookGatewayPublicUrl = asString(value.webhookGatewayPublicUrl)?.trim(); | ||
| const out: NonNullable<ProjectConfigFile["ui"]> = {}; | ||
| if (linearBatchLaunchDefaultPrompt) out.linearBatchLaunchDefaultPrompt = linearBatchLaunchDefaultPrompt; | ||
| if (webhookGatewayPublicUrl) out.webhookGatewayPublicUrl = webhookGatewayPublicUrl; | ||
| return Object.keys(out).length ? out : undefined; |
There was a problem hiding this comment.
Enforce HTTPS URL validation for webhookGatewayPublicUrl before persisting config.
Line 1919 currently accepts any trimmed string, which conflicts with the HTTPS base-URL contract and allows invalid/insecure values into effective config.
Suggested fix
function coerceProjectUiConfig(value: unknown): ProjectConfigFile["ui"] {
if (!isRecord(value)) return undefined;
const linearBatchLaunchDefaultPrompt = asString(value.linearBatchLaunchDefaultPrompt)?.trim();
- const webhookGatewayPublicUrl = asString(value.webhookGatewayPublicUrl)?.trim();
+ const webhookGatewayPublicUrlRaw = asString(value.webhookGatewayPublicUrl)?.trim();
+ let webhookGatewayPublicUrl: string | undefined;
+ if (webhookGatewayPublicUrlRaw) {
+ try {
+ const parsed = new URL(webhookGatewayPublicUrlRaw);
+ if (parsed.protocol === "https:") {
+ webhookGatewayPublicUrl = parsed.toString().replace(/\/$/, "");
+ }
+ } catch {
+ // ignore invalid URL
+ }
+ }
const out: NonNullable<ProjectConfigFile["ui"]> = {};
if (linearBatchLaunchDefaultPrompt) out.linearBatchLaunchDefaultPrompt = linearBatchLaunchDefaultPrompt;
if (webhookGatewayPublicUrl) out.webhookGatewayPublicUrl = webhookGatewayPublicUrl;
return Object.keys(out).length ? out : undefined;
}📝 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.
Suggested change
| const webhookGatewayPublicUrl = asString(value.webhookGatewayPublicUrl)?.trim(); | |
| const out: NonNullable<ProjectConfigFile["ui"]> = {}; | |
| if (linearBatchLaunchDefaultPrompt) out.linearBatchLaunchDefaultPrompt = linearBatchLaunchDefaultPrompt; | |
| if (webhookGatewayPublicUrl) out.webhookGatewayPublicUrl = webhookGatewayPublicUrl; | |
| return Object.keys(out).length ? out : undefined; | |
| function coerceProjectUiConfig(value: unknown): ProjectConfigFile["ui"] { | |
| if (!isRecord(value)) return undefined; | |
| const linearBatchLaunchDefaultPrompt = asString(value.linearBatchLaunchDefaultPrompt)?.trim(); | |
| const webhookGatewayPublicUrlRaw = asString(value.webhookGatewayPublicUrl)?.trim(); | |
| let webhookGatewayPublicUrl: string | undefined; | |
| if (webhookGatewayPublicUrlRaw) { | |
| try { | |
| const parsed = new URL(webhookGatewayPublicUrlRaw); | |
| if (parsed.protocol === "https:") { | |
| webhookGatewayPublicUrl = parsed.toString().replace(/\/$/, ""); | |
| } | |
| } catch { | |
| // ignore invalid URL | |
| } | |
| } | |
| const out: NonNullable<ProjectConfigFile["ui"]> = {}; | |
| if (linearBatchLaunchDefaultPrompt) out.linearBatchLaunchDefaultPrompt = linearBatchLaunchDefaultPrompt; | |
| if (webhookGatewayPublicUrl) out.webhookGatewayPublicUrl = webhookGatewayPublicUrl; | |
| return Object.keys(out).length ? out : undefined; | |
| } |
🤖 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 `@apps/desktop/src/main/services/config/projectConfigService.ts` around lines
1919 - 1923, Validate webhookGatewayPublicUrl as a proper HTTPS origin before
assigning it to out.webhookGatewayPublicUrl: after computing
webhookGatewayPublicUrl = asString(... )?.trim(), attempt to parse it with the
URL constructor and ensure url.protocol === 'https:' and that the URL represents
a base/origin (e.g. url.pathname === '/' or url.pathname === ''), and only then
set out.webhookGatewayPublicUrl = webhookGatewayPublicUrl; if the value fails
validation, skip assigning it (and optionally surface/record a validation error
elsewhere). This change targets the webhookGatewayPublicUrl handling in
projectConfigService (the webhookGatewayPublicUrl local variable and the out.ui
assignment).
Comment on lines
+369
to
+399
| try { | ||
| localSigningSecret = direct.signingSecret!; | ||
| const existing = await args.linearClient.listWebhooks(); | ||
| if (!existing.some((entry) => entry.url === direct.webhookUrl)) { | ||
| await args.linearClient.createWebhook({ | ||
| url: direct.webhookUrl!, | ||
| secret: direct.signingSecret!, | ||
| label: `ADE ${args.projectId} workflow ingress`, | ||
| resourceTypes: ["Issue", "IssueLabel"], | ||
| allPublicTeams: true, | ||
| }); | ||
| } | ||
| const payload: RelayEnsureWebhookResponse = { | ||
| endpointId: "direct", | ||
| webhookUrl: direct.webhookUrl!, | ||
| signingSecret: direct.signingSecret!, | ||
| lastDeliveredAt: null, | ||
| }; | ||
| relayWebhook = payload; | ||
| updateStatus({ | ||
| relay: { | ||
| configured: true, | ||
| healthy: true, | ||
| status: "ready", | ||
| webhookUrl: payload.webhookUrl, | ||
| endpointId: payload.endpointId, | ||
| lastDeliveryAt: null, | ||
| lastError: null, | ||
| }, | ||
| }); | ||
| return payload; |
There was a problem hiding this comment.
Recreate or update the Linear webhook when the signing secret changes.
Both branches treat url as the only identity check. If the public URL stays stable but the secret rotates, ADE updates localSigningSecret locally and skips createWebhook, so the existing Linear webhook keeps signing with the old secret and every delivery starts failing HMAC validation until someone deletes the webhook manually.
Also applies to: 436-446
🤖 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 `@apps/desktop/src/main/services/cto/linearIngressService.ts` around lines 369
- 399, The code only checks webhook.url when ensuring a webhook, so if
direct.signingSecret rotates the existing Linear webhook remains with the old
secret; modify the ensure logic around localSigningSecret /
args.linearClient.listWebhooks() to also compare the stored signing secret
(direct.signingSecret) against the existing webhook.secret and, if different,
call the appropriate update path (use args.linearClient.updateWebhook(...) if
available or delete + args.linearClient.createWebhook(...)) to replace the
secret for the matching direct.webhookUrl; update the
RelayEnsureWebhookResponse/relayWebhook and call updateStatus(...) after the
update so state reflects the new signing secret and health.
Comment on lines
+785
to
+815
| <Button | ||
| size="sm" | ||
| variant="primary" | ||
| onClick={() => { | ||
| const nextDraft = draftWithCreateLaneMode(toDraftFromRule(manualRunRule)); | ||
| setSelectedRuleId(manualRunRule.id); | ||
| setDraft(nextDraft); | ||
| savedSnapshotRef.current = JSON.stringify(toDraftFromRule(manualRunRule)); | ||
| setIssues([]); | ||
| setSimulationNotes([]); | ||
| setRequiredConfirmations([]); | ||
| setAcceptedConfirmations(new Set()); | ||
| setDetailView("editor"); | ||
| setManualRunRule(null); | ||
| setManualRunLaneId(""); | ||
| }} | ||
| > | ||
| Create lanes for runs | ||
| </Button> | ||
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={() => { | ||
| setSelectedRuleId(manualRunRule.id); | ||
| setDetailView("editor"); | ||
| setManualRunRule(null); | ||
| setManualRunLaneId(""); | ||
| }} | ||
| > | ||
| Edit lane mode | ||
| </Button> |
There was a problem hiding this comment.
Preserve the intended draft before switching rules.
If this modal is opened from a different rule than the one currently selected, both actions call setSelectedRuleId(manualRunRule.id), which triggers the effect on Lines 444-455 and reloads toDraftFromRule(selected). That wipes the draftWithCreateLaneMode(...) changes in the “Create lanes for runs” path, and it can also silently discard unsaved edits in the currently open draft.
Guard the rule switch with confirmDiscardIfDirty() and carry the intended draft through a one-shot override/ref so the selection effect does not immediately replace it.
🤖 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 `@apps/desktop/src/renderer/components/automations/RulesTab.tsx` around lines
785 - 815, When switching selection to manualRunRule you must guard against
accidentally overwriting an in-memory draft and preserve the created-lane draft;
modify the onClick handlers that call setSelectedRuleId(manualRunRule.id) (the
"Create lanes for runs" and "Edit lane mode" handlers) to first call
confirmDiscardIfDirty() and, if it returns/permits, set a one-shot override ref
(e.g., draftOverrideRef) containing the precomputed
draftWithCreateLaneMode(toDraftFromRule(manualRunRule)) (for the Create path) or
the current draft (for Edit), then call setSelectedRuleId(manualRunRule.id) and
setDraft from that overrideRef instead of relying on the selection effect to
re-run toDraftFromRule(selected); also clear the override ref after applying and
keep savedSnapshotRef/current issue state updates as before.
Comment on lines
+17
to
+20
| export function readAutomationsEnvOverride(env: EnvLike = process.env): boolean | null { | ||
| if (envFlagEnabled(env[AUTOMATIONS_ENABLE_ENV])) return true; | ||
| if (envFlagEnabled(env[AUTOMATIONS_DISABLE_ENV])) return false; | ||
| return null; |
There was a problem hiding this comment.
Make the disable override win on conflicting env flags.
Both readers check ADE_ENABLE_* before ADE_DISABLE_*, so setting both truthy still enables the feature. That weakens the production kill switch: an operator cannot reliably shut the feature off if a stale enable var is still present. Fail closed here by giving *_DISABLE_* precedence, or by throwing on conflicts.
Suggested fix
export function readAutomationsEnvOverride(env: EnvLike = process.env): boolean | null {
- if (envFlagEnabled(env[AUTOMATIONS_ENABLE_ENV])) return true;
if (envFlagEnabled(env[AUTOMATIONS_DISABLE_ENV])) return false;
+ if (envFlagEnabled(env[AUTOMATIONS_ENABLE_ENV])) return true;
return null;
}
@@
export function readMacosVmEnvOverride(env: EnvLike = process.env): boolean | null {
- if (envFlagEnabled(env[MACOS_VM_ENABLE_ENV])) return true;
if (envFlagEnabled(env[MACOS_VM_DISABLE_ENV])) return false;
+ if (envFlagEnabled(env[MACOS_VM_ENABLE_ENV])) return true;
return null;
}Also applies to: 32-35
🤖 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 `@apps/desktop/src/shared/automationAvailability.ts` around lines 17 - 20, In
readAutomationsEnvOverride, currently envFlagEnabled(AUTOMATIONS_ENABLE_ENV) is
checked before the disable flag so a set disable flag can be overridden; change
the logic so the disable flag takes precedence (check
envFlagEnabled(env[AUTOMATIONS_DISABLE_ENV]) first and return false if set) or
explicitly detect both flags set and return false (or throw if you prefer strict
behavior); apply the same change to the analogous function around lines 32-35
(the other env-reading function) — reference symbols:
readAutomationsEnvOverride, AUTOMATIONS_ENABLE_ENV, AUTOMATIONS_DISABLE_ENV.
Comment on lines
+446
to
+447
| automationsRefreshWebhookGatewayStatus: "ade.automations.refreshWebhookGatewayStatus", | ||
| automationsSetWebhookGatewayPublicUrl: "ade.automations.setWebhookGatewayPublicUrl", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify both new channels are registered (main), bridged + typed (preload), and consumed (renderer).
for ch in automationsRefreshWebhookGatewayStatus automationsSetWebhookGatewayPublicUrl; do
echo "===== $ch ====="
echo "--- main IPC registration ---"
rg -nP -C2 "\b$ch\b" -g 'apps/desktop/src/main/**'
echo "--- preload bridge + types ---"
rg -nP -C2 "\b$ch\b" -g 'apps/desktop/src/preload/**'
echo "--- renderer usage ---"
rg -nP -C2 "\b$ch\b" -g 'apps/desktop/src/renderer/**'
doneRepository: arul28/ADE
Length of output: 2328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search renderer for both IPC channel references and the preload-exposed method names.
rg -n --hidden --no-ignore \
-S "IPC\.automationsRefreshWebhookGatewayStatus|IPC\.automationsSetWebhookGatewayPublicUrl" \
apps/desktop/src/renderer || true
rg -n --hidden --no-ignore \
-S "refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl" \
apps/desktop/src/renderer || true
# Broaden slightly in case renderer calls are elsewhere (still within apps/desktop/src).
rg -n --hidden --no-ignore \
-S "refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl" \
apps/desktop/src | head -n 50 || trueRepository: arul28/ADE
Length of output: 4333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Renderer call sites (invocations) for preload-exposed methods
rg -n --hidden --no-ignore \
"(^|[^A-Za-z0-9_])(refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl)\s*\(" \
apps/desktop/src/renderer || true
# Also search for dotted access patterns commonly used off preload/global objects
rg -n --hidden --no-ignore \
"automations\.(refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl)" \
apps/desktop/src/renderer || true
# Search for usage via global/runtime namespaces (best-effort)
rg -n --hidden --no-ignore \
"(refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl)" \
apps/desktop/src/renderer/browserMock.ts || trueRepository: arul28/ADE
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for raw string action names in real renderer code
rg -n --hidden --no-ignore \
'"refreshWebhookGatewayStatus"|"setWebhookGatewayPublicUrl"' \
apps/desktop/src/renderer || true
# Search for likely action-layer usage in renderer
rg -n --hidden --no-ignore \
"adeActions|runtimeAction|callProjectRuntimeActionOr|projectRuntimeAction|automations\.|automations'" \
apps/desktop/src/renderer || true
# As a last broaden: search for the module prefix used in preload/global for actions
rg -n --hidden --no-ignore \
"automations|refreshWebhookGatewayStatus|setWebhookGatewayPublicUrl" \
apps/desktop/src/renderer --glob '!**/browserMock.ts' || trueRepository: arul28/ADE
Length of output: 15540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- preload.ts context (around refresh/set) ----"
sed -n '3900,3990p' apps/desktop/src/preload/preload.ts
echo "---- global.d.ts context (around automations typing) ----"
sed -n '960,1035p' apps/desktop/src/preload/global.d.tsRepository: arul28/ADE
Length of output: 7085
IPC channels are wired through main + preload; only renderer mock usage found.
apps/desktop/src/shared/ipc.ts: constants added forade.automations.refreshWebhookGatewayStatusandade.automations.setWebhookGatewayPublicUrlapps/desktop/src/main/services/ipc/registerIpc.ts: bothipcMain.handle(...)handlers are registeredapps/desktop/src/preload/preload.ts:window.ade.automations.refreshWebhookGatewayStatus()/setWebhookGatewayPublicUrl()invoke the matchingIPC.*channelsapps/desktop/src/preload/global.d.ts: correspondingwindow.ade.automationstypings exist- Missing concern: no production renderer call sites found for
window.ade.automations.refreshWebhookGatewayStatus/setWebhookGatewayPublicUrl—onlyapps/desktop/src/renderer/browserMock.tsreferences
If these are intended to be used by the UI, wire them into the relevant renderer component(s); otherwise remove/avoid keeping unused IPC channels.
🤖 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 `@apps/desktop/src/shared/ipc.ts` around lines 446 - 447, The new IPC
channels/APIs (constants automationsRefreshWebhookGatewayStatus and
automationsSetWebhookGatewayPublicUrl in ipc.ts, the ipcMain.handle
registrations in registerIpc.ts, and the preload methods
window.ade.automations.refreshWebhookGatewayStatus / setWebhookGatewayPublicUrl
plus typings) are not used anywhere in production renderer code—only referenced
in browserMock.ts; either wire them into the real UI or remove them. To fix: if
the UI needs them, add calls from the appropriate renderer component(s) (search
for automation UI pages/components) to invoke
window.ade.automations.refreshWebhookGatewayStatus() and
setWebhookGatewayPublicUrl() and update any component props/state to surface
results; otherwise remove the unused constants in ipc.ts, remove the
ipcMain.handle registrations in registerIpc.ts, strip the preload methods and
global.d.ts typings, and delete the mock usage in browserMock.ts so there are no
unused IPC entry points left.
arul28
deleted the
ade-75-automations-graph-ux-polish-false-warning-simulate-dead-end-lane-required-run-now-reparent-undo-double-click-rollback-order
branch
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\n- hide Automations behind a production coming-soon gate in packaged desktop builds while keeping source/dev builds available\n- block production CLI/runtime wiring for Automations, Linear ingress, and macOS VM unless internal env overrides are set\n- remove production-facing webhook settings affordances and update agent CLI guidance so agents do not treat these surfaces as usable in production\n\n## Validation\n- npm --prefix apps/ade-cli run typecheck -- --pretty false\n- npm --prefix apps/ade-cli run test\n- npm --prefix apps/desktop run typecheck -- --pretty false\n- npm --prefix apps/desktop exec -- vitest run src/main/services/adeActions/registry.test.ts src/main/services/automations/automationService.test.ts src/main/services/automations/automationIngressService.test.ts\n- npm --prefix apps/desktop run build\n- git diff --check\n- packaged CLI gate smoke: copied built cli.cjs outside source checkout and confirmed automations, linear ingress, and macos-vm are hidden/blocked without internal env overrides\n\n## Notes\nThis intentionally keeps Automations and macOS VM available for source/dev builds while packaged production builds present them as coming soon or unavailable.
Summary by CodeRabbit
Release Notes
New Features
UI Updates
Greptile Summary
This PR gates Automations, Linear webhook ingress, and macOS VM behind a production-build wall: packaged desktop builds show a "coming soon" screen and have all automation services nulled out, while source/dev builds retain full access. It also adds a webhook gateway abstraction (Tailscale Funnel or custom HTTPS URL) for directing external GitHub/Linear events to the local ADE runtime, plus a
/github-webhooksendpoint with HMAC verification.automationAvailability.tscentralises env-override logic;main.ts,bootstrap.ts, andregistry.tsall use it to conditionally create/expose services.AutomationsProductionGatein the renderer mirrors this for the UI.automationService.tsgainssetWebhookGatewayPublicUrl,refreshWebhookGatewayStatus(Tailscale probe viaexecFile), andassertWebhookGatewayReadyForRulegating;automationIngressService.tsadds a/github-webhooksroute with HMAC verification;linearIngressService.tsgrows a direct-webhook registration path bypassing the relay when a gateway URL is configured.WorkspaceGraphPage.tsx: in-placecompleted.reverse()mutations replaced with[...completed].reverse(), and anundoToastPendingguard prevents double-click undo races.Confidence Score: 3/5
The core feature-flag plumbing is solid, but two issues in the new webhook gateway path could cause incorrect data at runtime for users who enable the feature.
The gating logic in automationAvailability.ts, main.ts, bootstrap.ts, and registry.ts is straightforward and well-tested. The bug fixes in WorkspaceGraphPage.tsx are correct. The concern is concentrated in the new webhook integration code: linearIngressService.ts can accumulate duplicate Linear webhook registrations when the gateway public URL is changed, leading to doubled event delivery. Separately, automationService.ts getIngressStatus() silently overwrites ingressStatusRef on every call, potentially discarding freshly-refreshed Tailscale probe data.
apps/desktop/src/main/services/cto/linearIngressService.ts (direct-webhook deduplication on URL change) and apps/desktop/src/main/services/automations/automationService.ts (getIngressStatus side-effectful mutation of ingressStatusRef).
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[App Start / CLI Invoked] --> B{isPackaged?} B -- Yes / packaged --> C[automationsEnabled = false\nmacosVmEnabled = false] B -- No / dev --> D[automationsEnabled = true\nmacosVmEnabled = true] B -- ENV override set --> E[readAutomationsEnvOverride\nreadMacosVmEnvOverride] E --> C E --> D C --> F[automationService = null\nlinearIngressService = null\nmacosVmService = null] C --> G[UI: ProductionAutomationsComingSoon] C --> H[ADE action domains: automations/linear_ingress/macos_vm → null] D --> I[automationService created] I --> J[refreshWebhookGatewayStatus\n— Tailscale probe via execFile] J --> K{publicUrl configured?} K -- Custom HTTPS URL --> L[Gateway ready] K -- Tailscale .ts.net URL --> M{Funnel active?} M -- Yes --> L M -- No --> N[pending-approval] K -- None --> O[needs-public-url] I --> P[automationIngressService\n/automation-webhooks/:id\n/github-webhooks — HMAC verified] I --> Q[linearIngressService\ndirect webhook or relay path]Comments Outside Diff (3)
apps/desktop/src/main/services/automations/automationService.ts, line 2779-2807 (link)getIngressStatus()silently mutatesingressStatusRefgetIngressStatus()directly assigns toingressStatusRefwithout going throughupdateIngressStatus(), which means it bypasses theemit({ type: "ingress-updated" })side-channel. Callers reading status through subscribed events will not be notified of the re-computed values, while callers using the return value will see the updated data — two different pictures of the same state until the next explicit update. More importantly, every call to this method silently overwrites the currentingressStatusRef, including any freshly-refreshed Tailscale data written byrefreshWebhookGatewayStatus(), because the block unconditionally overwriteswebhookGateway.statuswith a fallback derivation that may lag the async refresh result.Prompt To Fix With AI
apps/desktop/src/main/services/automations/automationService.ts, line 2520-2531 (link)normalizePublicWebhookUrlwith diverging behaviorlinearIngressService.tsdefines its ownnormalizePublicWebhookUrlwith two differences: it also clearshashandsearchfrom the URL, and it strips all trailing slashes (/\/+$/) instead of just one (/\/$/). A URL likehttps://host.example.com/ade-webhooks//normalizes tohttps://host.example.com/ade-webhooks/inautomationService.tsbut tohttps://host.example.com/ade-webhooksinlinearIngressService.ts. The webhook URL appended byappendWebhookPathinlinearIngressService.tswould then not match thepublicUrlstored byautomationService, breaking theisReadychecks that compare the two values.Prompt To Fix With AI
apps/desktop/src/renderer/components/automations/AutomationsPage.tsx, line 144-176 (link)When
window.ade.app.getInfo()rejects (e.g., transient IPC error on startup), theuseAppPackagingStatehook resolves to"dev", rendering the full Automations UI to packaged users. The backend gate (automationsEnabledinmain.tsandgetAdeActionDomainServices) still prevents actual execution, so data won't be exposed, but users will briefly see the Automations interface. Defaulting to"packaged"on error would be more conservative for a production gate.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Gate automations and VM in production bu..." | Re-trigger Greptile