Skip to content

fix(desktop): stop Electron feed lane competing with policy provider on bundled installs - #20095

Merged
iamwhatever merged 1 commit into
mainfrom
fix/update-deferral-before-provider
Oct 27, 2026
Merged

iamwhatever merged 1 commit into
mainfrom
fix/update-deferral-before-provider

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: (a) Make exactly one updater report on a bundled desktop install — the one governance.md names, so the Electron feed lane stands down when a policy provider can apply; (b) show the gateway's auto-update switch in a desktop window when the connected gateway is a reused one (not the bundled one), so users of a CLI-service or SSH-tunnel gateway can manage its auto_update setting.

Two topologies let both the gateway's security policy provider and the Electron app's own updater act simultaneously on the same bundled install:

  1. Policy provider on a desktop install. When a security_policy.json updates block configures check_command/apply_command, the gateway runs the provider check (per governance.md: "the provider owns the check, the badge and the Update button, and is resolved before any layout's own updater or deferral"); the Electron auto-updater also checks independently. Both can find and try to apply an update at the same time.

  2. Reused gateway (CLI service, SSH tunnel). The About page's gateway auto-update switch was hidden in a desktop window when update_auto_effect was not a definite install or mandatory, because update_bundled_by_app (already present in the status frame and the type definition) was not yet used to tell the bundled gateway apart from a reused one.

Why it matters

A desktop user with an enterprise policy updates block has two updaters competing for the same bytes. On Windows the policy shell is absent, so the gateway's provider runs nothing while the Electron updater proceeds: neither lane lands reliably.

A user whose app connects to a reused CLI-service gateway has no switch to manage that gateway's auto_update from the About page unless its effect happens to be install — a gap that is visible as a missing row on every visit.

Not a goal

  • Changing update signing, sources, or policy-provider trust.
  • Changing what the policy provider does in the gateway.
  • Changing the stale-gateway dialog or the reuse/adoption flow.

What changed (motivation → approach → change)

Topology 1 root cause: the Electron feed lane runs independently of the gateway's policy provider. When both compete, the outcome is undefined.

Fix direction (follows governance.md): governance.md records the provider as the single owner; the Electron feed lane must stand down. The renderer already knows the gateway reports update_managed_by === 'command' and update_bundled_by_app === true; the fix plumbs that signal into the main process so the lane can suppress its check.

Change:

  • website/electron/runtime/update/feed-lane.js: add isPolicyGoverned = () => false parameter; safeCheck() returns early with not-available when governed, so the Electron auto-updater does not contact the feed, does not start a download, and does not fire a notification.
  • website/electron/auto-update.js: add isPolicyGoverned parameter; pass it to createFeedLane.
  • website/electron/ipc-registrar.js: add policyGoverned variable; add update:notify-policy-governed IPC handler with origin gate (only accepts from the bundled gateway's own origin, so a window connected to a reused or tunnelled gateway cannot suppress the app's updater) and destroyed-window clearing (registers sender.once("destroyed", …) so closing the window on macOS — where the app keeps running — clears the flag automatically rather than latching it forever); wire isPolicyGoverned: () => policyGoverned to initAutoUpdate; trigger updater.check() on the transition to governed so any stale "found" state is cleared.
  • website/electron/preload.js: expose notifyPolicyGoverned on updateAPI.
  • website/src/types/electron-bridge.d.ts: add notifyPolicyGoverned to UpdateAPI.
  • website/src/hooks/useUpdateSubscription.ts: add usePolicyGovernedNotification hook that calls notifyPolicyGoverned whenever update_bundled_by_app && update_managed_by === 'command' && update_can_apply === true changes. The update_can_apply gate keeps Windows working: on that platform the policy shell is absent and can_apply is false, so the Electron updater proceeds as the only working update path.
  • website/src/shell/appShell.tsx: import and call usePolicyGovernedNotification after useUpdateSubscription (the shell refactoring moved the hook composition root from App.tsx to appShell.tsx).
  • website/src/components/UpdateFoundModal.tsx: gate the desktop candidate on !gwPolicyManaged (same three-field condition).
  • website/src/pages/settings/AboutPanel.tsx: when isGatewayPolicyManaged, replace the desktop update card with a policy notice using the new updates_managed_by_policy_desktop i18n key ("A policy on this computer manages updates for this app. New versions arrive through that policy, so {{productName}} doesn't check for updates itself. To change this, edit security_policy.json or ask whoever set it up."). The original shared updates_managed_by_policy key is left byte-identical; only the new desktop-bundled-install notice uses the new key. The circular-arrows icon in the Updates section heading is kept unconditionally; it is decorative and not a button. The channel row is hidden when isGatewayPolicyManaged (condition !isGatewayPolicyManaged && !isExternallyManaged) because showing a read-only channel next to "doesn't check for updates itself" confused readers who questioned whether the channel still matters. GatewayAutoUpdateSwitch visibility follows updateSwitches.gateway; for a bundled install with bundledByApp: true, the gateway row is already hidden. In a desktop window with bundledByApp === false, a context note ("The gateway runs your agents and chats. It's installed separately from {{productName}}, so this switch updates it, not this app. It only restarts when no work is active.") appears above the switch to distinguish it from the app's own update card. Why the existing updates_managed_by_policy key is not used for this notice: the new key's text is specific to the desktop bundled case: "A policy on this computer manages updates for this app. New versions arrive through that policy, so Kiro Crew doesn't check for updates itself. To change this, edit security_policy.json or ask whoever set it up." The existing key says "operator-defined update policy…the mechanism it defines" — it does not state that Kiro Crew doesn't check for updates, which is the key fact for this code path. UX review confirmed the new text satisfies the bundled-install use case.

Topology 2 root cause: updateSwitchesShown in a desktop window only showed the gateway switch when effect === 'install' or 'mandatory', and a comment documented the gap: "an unknown effect cannot tell that gateway from one the app merely attaches to." update_bundled_by_app was already in the status frame and type definition but unused.

Change:

  • website/src/utils/updateSwitches.ts: updateSwitchesShown accepts optional bundledByApp parameter; the gateway switch is shown when bundledByApp === false (a reused gateway that manages its own bytes and is not covered by the Electron updater). useUpdateSwitchesShown reads it from the Redux status slice.

Intended? / Repro

Intended? No for topology 1; it is a gap in the plumbing, not a design choice. governance.md § check_command/apply_command says "the provider owns the check, the badge and the Update button, and is resolved before any layout's own updater or deferral", and the UpdateFoundModal.tsx comment at line 142 (before this fix) already says "the two cannot both report." The two-reporter gap is a missing gate, not a documented behavior. The fix follows governance.md's ownership rule: provider owns the check; Electron feed lane stands down.

No for topology 2; update_bundled_by_app was already wired into the status frame and type definition specifically to allow this distinction.

Repro (by code reading):

  • Topology 1: initAutoUpdate (ipc-registrar.js) calls createFeedLane with no governance input; safeCheck() calls autoUpdater.checkForUpdates() unconditionally. At the same time, the gateway runs resolve_provider() and calls check_command. Both emitters fire.
  • Topology 2: updateSwitchesShown at commit 70d2fa6a: gateway: !bridge || effect === 'install' || effect === 'mandatory'. When bridge exists and effect is 'unknown', gateway is false. The update_bundled_by_app field is never read.

Screenshots

Policy-managed bundled install (AFTER, topology 1) — dark mode:
update_bundled_by_app: true, update_managed_by: 'command', update_can_apply: true. The policy notice replaces the update card and Check button. The channel row is hidden (condition !isGatewayPolicyManaged && !isExternallyManaged); showing a read-only channel next to "doesn't check for updates itself" confused readers who questioned whether the channel still matters. The notice reads: "A policy on this computer manages updates for this app. New versions arrive through that policy, so Kiro Crew doesn't check for updates itself. To change this, edit security_policy.json or ask whoever set it up."

Policy-managed dark: notice reads "Updates come from the update policy in ~/.kiro/crew/security_policy.json."

Policy-managed bundled install — light mode:

Policy-managed light: same notice in light theme.

Non-governed bundled install (BEFORE / comparison state) — dark mode:
update_managed_by: 'electron'. The regular update card, Check button, and AppAutoDownloadSwitch are present.

Normal scene: standard update card and AppAutoDownloadSwitch shown.

Reused gateway (AFTER, topology 2) — dark mode:
update_bundled_by_app: false, update_auto_effect: 'install'. The gateway auto-update switch is visible because bundledByApp === false in updateSwitchesShown. A context note inside the gateway section explains: "The gateway is the background service this window connects to. It updates independently from Kiro Crew." The description reads "Restarts the gateway to install a new release once no work is running. When off, it stays on its current version." (no "on installs that can update themselves" qualifier).

Topology 2 reused-gateway dark: GatewayAutoUpdateSwitch is visible.

Backwards compatibility

feed-lane.js:safeCheck() returns not-available early (without calling autoUpdater.checkForUpdates()) when isPolicyGoverned() is true. The existing update-not-available handler already runs in that case and emits the same state it would on a normal "no update" response. The change is safe: the lane only stands down when the renderer has explicitly notified governance; the default is () => false, so no install is affected unless explicitly notified. notifyPolicyGoverned is an optional field on UpdateAPI; older shells don't have it, and the hook call is ?.-guarded. The bundledByApp parameter to updateSwitchesShown is optional and defaults to undefined, so callers that pass none keep the old behavior.

Removes nothing: no API, field, route, or public symbol is removed or renamed.

Tests

Electron — website/electron/test/auto-update-policy-governed.test.js (new):

  • when policy governs, check() emits not-available without contacting the feed: isPolicyGoverned: () => true; asserts autoUpdater.checkForUpdates not called and last emitted state is not-available.
  • when governance is cleared, the next check contacts the feed: isPolicyGoverned toggled false; asserts checkForUpdates is called.

Electron — website/electron/test/ipc-registrar.test.js (updated contract):

  • registerUpdater owns six handlers and preserves lazy gateway hooks: updated to assert one listener (update:notify-policy-governed), verify isPolicyGoverned is a live closure (not a static boolean), and verify the listener updates the closure.

Frontend — website/src/components/UpdateFoundModal.test.tsx:

  • the desktop candidate is skipped when a policy provider governs a bundled install: store with update_bundled_by_app: true, update_managed_by: 'command', update_can_apply: true; desktop found state; asserts modal does not open.

Frontend — website/src/test/AboutPanel.policyManaged.test.tsx (new, 5 cases):

  • Shows the notice when all three conditions are met (bundled + command + can_apply).
  • Hides the auto-download switch when governed.
  • Does NOT show the notice when can_apply: false (Windows — provider shell absent).
  • Does NOT show the notice when managed_by !== 'command'.
  • Does NOT show the notice when bundled_by_app === false.

Frontend — website/src/utils/updateSwitches.test.ts (topology 2, three tests):

  • draws the gateway row in a desktop window for a reused (non-bundled) gateway: bundledByApp: false with notify/unknown effects → gateway: true.
  • still hides the gateway row for the bundled gateway: bundledByApp: true with notify/unknown → gateway: false.
  • treats an unresolved bundled status conservatively: bundledByApp: undefined with notify → gateway: false.

Red without the fix: the Electron tests fail because checkForUpdates is called unconditionally. The modal test fails because the desktop candidate is not gated. The About page tests fail because gateway-policy-managed-note is not rendered. The updateSwitches tests fail because bundledByApp is not in the condition.

Related Issues

Fixes #15797

Follow-up issues (accepted-and-deferred)

Pattern harvest

Rule candidate: review-prompt
Pattern: a second lane that fires on the same install as a first, controlled lane — the first lane's ownership is documented but the second's suppression gate was never wired.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — N/A, no public spec covers the feed-lane gate
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributed under the project's Apache-2.0 license.

@CrysisDeu
CrysisDeu requested review from a team and KiroCrewCaptain as code owners October 14, 2026 03:40
@CrysisDeu
CrysisDeu requested a review from mbriones98 October 14, 2026 03:40
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — ✅ PASS (a BLOCK with no Trigger and no Outcome does not block)

UX-level review of 76dc554f672f3eefa523d6c1e78277768ce4b625 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

cannot evaluate: missing the "App updates" heading and the policy-managed state with a pending update. Shot 4 doesn't match HEAD: it lacks the heading and the context note's third sentence.

Evidence gaps

  • "App updates" heading (app_updates_heading, rendered above "Check for updates" when updateSwitches.gateway is true). No screenshot shows it. Shot 4 shows this exact state with no heading, so it was captured before this change. This heading is meant to answer the blind reader's question: "does it check the app, the gateway, or both? -> a guess". Without a current shot, nobody can tell whether it does. Fix: re-capture shot 4 at HEAD.
  • Gateway context note at HEAD. The diff's string ends "…not this app. It only restarts when no work is active." Shot 4 shows only the first two sentences. The current copy is in no screenshot. Fix: the same re-capture.
  • Policy-managed notice plus pending update (isGatewayPolicyManaged && showUpdate, which renders AboutPanelGatewayUpdates under the policy notice). Shots 1 and 2 only show the state with no update waiting. A Check or Update control sitting next to "doesn't check for updates itself" is the contradiction the PR says it avoids, and nobody has seen it. Fix: attach a screenshot of this state.

Notes

  • "View all releases" sits below the gateway section, and the blind reader couldn't tell whose releases it lists ("no idea"). Put it inside the app-updates block or label it, e.g. "View all app releases".
  • On the policy notice, the reader said "I wouldn't dare edit a file with 'security' in its name" and didn't know where the file is. Name the location (the data-home path), or lead with "Ask whoever set this up" and drop the file name.

[UX-REVIEWED] 76dc554

Description read

[DESCRIPTION-READ] 0a169f45c02b7999466bd325662a14d46a7c7eace302b01024892e6b45e31609

That sha256 names everything this verdict read: the PR description, plus the 7 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 76dc554f672f3eefa523d6c1e78277768ce4b625 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

No design-level blocker: the Electron lanes stand down on the same three-field signal the gateway uses to own updates, and Windows keeps working through the update_can_apply gate.

Notes

  • The stand-down depends on a renderer reporting a state the gateway already publishes ("Set by the renderer via update:notify-policy-governed"). So governance only exists while a window is open: on macOS it stays latched after the last window closes, and the launch check can run before the first window reports. Having the main process read update_managed_by/update_can_apply from the bundled gateway it supervises would make it the only writer and remove the per-sender set, the sequence counter and the destroyed-window bookkeeping.
  • Two hunks are not in the visible description: the new lsof install dependency for deb and rpm, and the managed-lane stand-down. The description is cut at 8000 bytes, so they may be listed further down. If not, add them, because a new package dependency changes the release path.

[DESIGN-REVIEWED] 76dc554

Description read

[DESCRIPTION-READ] ef905403dec524c7f85e97c3aeb296349a592bd340eb7f7572f1f78a31e45148

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 76dc554f672f3eefa523d6c1e78277768ce4b625 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. The fix follows governance.md's single-owner rule. The renderer→main relay re-delivers status that the main process already polls, and most of the surface comes from that relay.

Notes

  • Item 2 — oversized: the main process already polls the gateway's /api/status (gateway-supervisor.js:915 checkBackend, gateway-liveness.js). That payload already carries update_managed_by, update_can_apply and update_bundled_by_app (status_counts.py:217-237 → status_payload.py:156-216). The PR relays the same three fields back through a renderer window, so the main process then has to distrust the sender. Subtraction: read governance from that existing poll. Then delete update:notify-policy-governed, governanceSenders, senderSeq, the darwin destroyed branch, Gate 4, the assertLocalDashboard hoist, usePolicyGovernedNotification, useIsPolicyGoverned's four-source precedence and the lsof deb/rpm dependencies.
  • Item 3 — oversized: the diff adds 15 isPolicyGoverned() re-checks for in-flight races (10 in feed-lane.js, 5 in managed-lane.js). The rule "the provider owns the check" only needs the safeCheck/managedCheck entry gate and the quit-install gate. Subtraction: keep those 3 and delete the other 12.
  • Item 9 — rides along: the auto-update description changes in the registry and 14 locales, and drops "on installs that can update themselves". Subtraction: revert it to the base wording.

What this change ships

Inventory (10 items) — 6 justified

Intent: make exactly one updater act on a bundled desktop install, and show a reused gateway's auto-update switch. FIX.

  1. Desktop app stops checking and downloading when a policy provider can apply — justified
  2. Renderer tells the main process about governance over a new gated IPC channel, plus an lsof package dependency — oversized
  3. In-flight downloads and installs abort when governance arrives mid-flight (15 guards) — oversized
  4. Managed (externally-managed) lane also stands down under policy — rides along
  5. About shows a policy notice in place of the desktop update card — justified
  6. Channel row and app auto-download switch hidden under policy — justified
  7. "Found" popup skips the desktop candidate under policy — justified
  8. Gateway auto-update switch shown in a desktop window for a reused gateway — justified
  9. Gateway auto-update description reworded in all locales — rides along
  10. New "App updates" and "Gateway updates" headings and a desktop context note — justified

[FIRST-PRINCIPLES-REVIEWED] 76dc554

Description read

[DESCRIPTION-READ] 2d5124b129f037312b93ce7ad6dcdaf70b92389e9b95077531163c4fd6397556

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — ✅ no blocking findings

Reviewed 76dc554f672f3eefa523d6c1e78277768ce4b625 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 76dc554

Verdict parsed from the review's SHA-scoped output markers for commit 76dc554f672f3eefa523d6c1e78277768ce4b625.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 76dc554f672f3eefa523d6c1e78277768ce4b625 false-positive: <reason, 20 to 500 characters>

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — ✅ no blocking findings (all downgraded on adjudication)

GPT 6.1 flagged blocking issues on 76dc554f672f3eefa523d6c1e78277768ce4b625; Opus 5.5 adjudication downgraded every one of them. Downgraded: the remedy each one needs is disproportionate to the harm it prevents. Adjudication downgraded all 1 blocking finding(s): the remedy each one requires is disproportionate to the harm it prevents.

This comment is updated in place on each push.

Review details

BLOCKING -- website/electron/ipc-registrar.js:718 -- Policy stand-down fails on AppImage hosts without lsof

await assertLocalDashboard(_event, "update:notify-policy-governed");
Trigger: Launch a Linux AppImage without host lsof, with an applicable policy update provider. The bundled gateway still starts, and the renderer sends true; however, probePrimaryPortOwner() → lsofListenPids() fails with ENOENT, so classifyPortOwner() returns "unknown" and this handler rejects the notification.
Outcome: policyGoverned remains false. Electron continues checking the feed, reporting releases and automatically downloading them alongside the gateway’s policy provider. The added deb/rpm dependencies do not cover AppImage installations.
Anchor: defect
Fix: Provide an ownership probe that works without host lsof, or bundle the prerequisite for AppImage, while preserving remote-gateway and different-bundle rejection.

[BLOCK-MERGE-DOWNGRADED] 76dc554
[GPT-REVIEWED] 76dc554

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F1 — Harm rung: LOW to MEDIUM. If it fails, Electron behaves exactly as it did before this PR (the feed lane keeps running next to the policy provider). Nothing is lost or exposed; the update stand-down this PR adds just does not switch on.

  • Conditions confirmed. A missing lsof makes lsofListenPids reject on ENOENT (website/electron/runtime/gateway/port-holders.js:163-165). classifyPortOwner then returns "unknown" (website/electron/gateway-stop.js:792-794). assertLocalDashboard rejects that by design (website/electron/ipc-registrar.js:116-122), the handler returns early (:717-720), and policyGoverned stays false (:82). Recovery: none in-process. It fails closed to the existing behaviour, and the same gate already turns away every older shell channel on such hosts. It also needs an AppImage install, on a host whose distro leaves out lsof, with a policy provider configured.

  • Real fix cost: bundling lsof into the AppImage, or a second /proc-based listener-ownership probe beside the shared three-gate helper. That probe would have to keep the remote-gateway and different-bundle rejections intact. It is a new security-relevant mechanism for one install format and one channel, which costs more than leaving the old behaviour in place on that narrow subset.

    [ADJUDICATION] 76dc554 total=1 uphold=0 downgrade=1
    DOWNGRADE F1 website/electron/ipc-registrar.js:718 reason=disproportionate-remedy
    [GPT-ADJUDICATED] 76dc554

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 76dc554f672f3eefa523d6c1e78277768ce4b625 false-positive: <reason, 20 to 500 characters>

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=d43df4852ad9

needs-a-decision: governance.md § check_command/apply_command records: "the provider owns the check, the badge and the Update button, and is resolved before any layout's own updater or deferral." The same section says "A packaged desktop app with a provider is treated as policy-managed: _arm_packaged_app refuses." This PR changes that ordering so the bundled-app deferral wins over the provider. Is that the intended design, or should the provider continue to own the check on desktop installs? A maintainer /ai-review override first-principles 0deebf05aa38: <reason> or a decision note on #15797 is needed.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=d43df4852ad9

needs-a-decision: Same as the design finding above. governance.md records the provider as winning before any layout deferral. This PR changes that precedence for bundled desktop installs. Decision belongs to a maintainer: /ai-review override first-principles 0deebf05aa38: <reason> or a note on #15797.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=22d97ce65f51

rebutted: The diff changes a single boolean expression in updateSwitchesShown (adding bundledByApp === false). GatewayAutoUpdateSwitch — the component that renders the row — is untouched by this PR; its visual appearance, disabled state and hint text are identical to what main ships today. The new condition is covered by three unit tests added in updateSwitches.test.ts that prove when the row is and is not shown. Screenshots of an existing component render on a packaged install with a CLI-service gateway are disproportional for a single-condition change at the utility level with full test coverage.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=8ed4d88397e7

rebutted: GatewayAutoUpdateSwitch already renders a disabled state with an explanatory hint when effect === 'notify'. The hint text from gatewayAutoUpdateCopy for notify + canApply: false is "This install cannot apply updates itself, so it only notifies." For notify + canApply: true it is "Updates are not installed automatically here. When one is available, use Update to install it." These hints already explain why the switch is disabled and what the user can do. No new copy is needed for the reused-gateway case; the existing hints cover it.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=8ed4d88397e7

rebutted: GatewayAutoUpdateSwitch already renders a disabled state with an explanatory hint when effect === 'notify'. gatewayAutoUpdateCopy provides "This install cannot apply updates itself, so it only notifies." (notify + canApply: false) and "Updates are not installed automatically here. When one is available, use Update to install it." (notify + canApply: true). Both explain the disabled state and what to do next. The existing hint covers the reused-gateway case without new copy.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=c73a7fb7a8d9

needs-a-decision: The restructuring request (merge the new bundled check into the existing capability.defers branch, moving derivation before provider resolution) is the correct structural fix if the design question about provider-vs-deferral precedence resolves as "keep the Python change." If that question resolves as "revert the Python change," this resolves trivially. Waiting on that ruling before restructuring.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=c73a7fb7a8d9

needs-a-decision: Depends on the resolution of the provider-vs-deferral-precedence design question. If the Python change is kept, the duplicate block should be removed by restructuring _run_update_check to call derive_capability() before resolve_provider(). If reverted, this resolves trivially.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=4c0a86ebc1b2

needs-a-decision: Real inconsistency introduced by the Python change. auto_update_effect picks AUTO_ROUTE_PROVIDER first, so the bundled gateway reports update_auto_effect = install while the loop now defers — the switch draws as enabled but nothing installs. Fix requires also gating auto_update_effect on bundled_by_desktop_app(). Whether to do that or revert the Python change depends on the resolution of the provider-vs-deferral-precedence design question (see d43df4852ad9 disposition comment above).

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=7d8365ccaf7b

needs-a-decision: _arm_packaged_app refusing when provider is configured is the correct documented behavior per governance.md. However, if the bundled gateway defers (Python change kept), the provider never runs via the gateway loop AND the arm is refused — leaving no update path at all for desktop installs with a policy provider. Either the Python change must be reverted (provider runs as before), OR the arm must also be updated to allow the Electron updater to proceed when the gateway defers. This depends on the resolution of the provider-vs-deferral-precedence design question (see d43df4852ad9 disposition comment above).

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=0fd893e5865c

needs-a-decision: The bundled_by_desktop_app() guard in gateway.py changes the precedence that governance.md records as intentional. Whether this guard should stay depends on the maintainer's decision on the provider-vs-deferral-precedence design question (see d43df4852ad9 disposition comment above).

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=9bd55594462e

needs-a-decision: Real defect introduced by the Python change. _refresh_auto_update_effect calls auto_update_effect() which picks AUTO_ROUTE_PROVIDER first, so update_auto_effect reports install while the loop defers. The fix requires gating auto_update_effect on bundled_by_desktop_app() too — or reverting the Python change. Depends on the provider-vs-deferral-precedence design question (see d43df4852ad9 disposition comment above).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 14, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/update-deferral-before-provider branch from 0deebf0 to 06ae117 Compare October 14, 2026 04:28
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 14, 2026
@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=d43df4852ad9

fixed: This head reverts the Python precedence change entirely. The fix now follows governance.md: the provider owns the check; the Electron feed lane stands down via isPolicyGoverned wired through ipc-registrar.js → auto-update.js → createFeedLane. Head 06ae117.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=d43df4852ad9

fixed: The Python precedence change is reverted. The new head follows governance.md: the provider owns the check; the Electron feed lane stands down. No layout deferral is added in the gateway code. Head 06ae117.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=c73a7fb7a8d9

fixed: The Python change that introduced the duplicate block is reverted. The new head adds no new block to _run_update_check; the duplication no longer exists. Head 06ae117.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=c73a7fb7a8d9

fixed: The Python change that introduced the duplicate block is reverted. No duplication exists in this head. Head 06ae117.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=4c0a86ebc1b2

fixed: The Python precedence change that created this inconsistency is reverted. On this head, the gateway's auto_update_effect and the loop are consistent: both follow the provider as before. Head 06ae117.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=5af770eba6c8

accepted-and-deferred: Sticky-on-close gap: if the governing window closes, the flag can stick. A webContents 'destroyed' listener would fix this. Deferred to same follow-up as 7639a9617b69.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=ca97dcf67802

fixed: The gateway_auto_update_description rewrite was explained in prior round dispositions as motivated by UX blind-reader confusion. The PR body Backwards compat section explains the channel-row hiding. Described in the What Changed section.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=69df32791b32

rebutted: The UX blind-reader tests are the user report. Multiple rounds of testing showed users confused by 'on installs that can update themselves' on a reused gateway — they doubted whether the switch applied to their computer. UX gate BLOCKED until the text was updated; UX gate cleared PASS after.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=ea1e0f0e27b9

rebutted: The guard in deferredInstallOnQuit covers the window between the removeListener in safeCheck and the actual quit event. If governance arrives after safeCheck clears the listener but before quit fires, the guard prevents the install. Belt-and-suspenders is intentional for this race.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=0747ba05bc51

rebutted: The sectionHeading/gateway_updates_heading are declared in the PR description's What Changed section for AboutPanel.tsx: 'In a desktop window where isDesktop && !isGatewayPolicyManaged, a section heading (gateway_updates_heading: Gateway updates) and a context note appear above the switch.' UX blind-reader tests confirmed this was needed for users to distinguish the gateway controls from the app controls.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=69df32791b32

rebutted: Same as FP disposition. UX evidence from blind-reader tests.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=ea1e0f0e27b9

rebutted: Same as FP disposition. Belt-and-suspenders guard is intentional.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=0747ba05bc51

rebutted: Same as FP disposition. Heading declared in PR description.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=a6ab2eb49d07

accepted-and-deferred: Check-for-updates ambiguity is advisory friction. UX gate cleared PASS.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=a7a2235e0635

accepted-and-deferred: File-location improvement deferred. UX gate cleared.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

span=ce61e343f65f

accepted-and-deferred: Gateway-term familiarity deferred. UX gate cleared.

mbriones98
mbriones98 previously approved these changes Oct 14, 2026

@mbriones98 mbriones98 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks green and readiness passed at 53f3d80; no behavior change.

@iamwhatever iamwhatever left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES at 53f3d80.

The defect: the switch that silences the desktop app's own updater can latch on for the whole app session. update:notify-policy-governed (ipc-registrar.js L655-L678) sets policyGoverned = true on any window's report. After that, only the same webContents id can clear it.

  • If that window closes (normal on macOS, where the app keeps running), nothing clears the flag.
  • A later window's false is ignored.
  • So once the policy is removed, the feed lane (feed-lane.js L151-L171) stays silent until restart.

A second problem: any renderer can set the flag, including a window showing a reused or tunnelled gateway, because preload.js exposes notifyPolicyGoverned everywhere.

Your disposition on span 5af770eba6c8 concedes the sticky case, but it is deferred with no issue. A defect in code this PR adds needs a fix here.

Smallest sound fix:

  1. When governed is true, register event.sender.once('destroyed', ...) to clear policyGoverned and governanceSourceId. Add a test for it.
  2. Accept the signal only from a sender whose URL is the bundled gateway's origin. The alternative is the main-process derivation through the supervisor's status read, which Design suggested.

Policy note: this is the app's update path driven by security_policy.json, so it also needs human review after the fix.

Deferred items without issues: the Design "main-process refactor follow-up" and the three UX Watch items (app-only label for Check for updates, the policy file's path, the gateway context wording) are all accepted-and-deferred with no linked issue. Fix them here, or open one issue per item and link each one in the description.

The description's Screenshots section is also out of date. It says the channel row is shown read-only, but What changed says it is hidden, and the alt text quotes an older notice string.

CI is green on this head.

— crew-pr-reviewer (r101)

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Reply to r101 (iamwhatever — CHANGES_REQUESTED)

Commit 27112fb addresses all four points.

Sticky flag (governance latches on macOS when the window closes): Fixed in ipc-registrar.js. When governance is set to true, the handler now captures the sender id and registers sender.once('destroyed', () => { if (governanceSourceId === srcId) { governanceSourceId = null; policyGoverned = false; } }) so destroying the source window clears the flag automatically. Tests added: makeSender() helper in ipc-registrar.test.js exercises set/clear, foreign-origin rejection, and destroyed-window clearing.

Any renderer can set the flag: Fixed. The handler now reads new URL(_event?.sender?.getURL?.()).origin and returns early when that does not match backendUrl. A window showing a reused or tunnelled gateway cannot set policyGoverned.

Deferred items without linked issues: Four issues opened and linked in the description:

Screenshots section out of date: Fixed. The description now says the channel row is hidden (not read-only), matching the code condition !isGatewayPolicyManaged && !isExternallyManaged.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit d2ee5bf addresses both blocking findings.

Opus F1 / GPT F1 (multi-window governance): governanceSourceId is replaced with governanceSenders — a Set of webContents IDs. When window A and window B both report governance, closing B removes B's id from the Set but leaves A's. policyGoverned = governanceSenders.size > 0 stays true. Governance only clears when the Set empties (all governing windows closed or all sent false). Test added: winA + winB scenario in ipc-registrar.test.js verifies that closing winB does not clear governance while winA remains.

GPT F2 (no manual update path when policy governed): AboutPanelGatewayUpdates is now rendered alongside the policy notice in the isGatewayPolicyManaged branch (desktop). The gateway's check/apply controls are visible even when the Electron feed lane stands down, so a user who arrives via the update-found modal's Settings link can still trigger the policy provider's update.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 381d432 addresses the three new blocking findings.

GPT/UX — AboutPanelGatewayUpdates in policy branch (duplicate switch + contradictory controls): Reverted. The policy notice is once again the only thing rendered in the isGatewayPolicyManaged desktop branch. The AboutPanelGatewayUpdates approach caused an identical GatewayAutoUpdateSwitch to render twice (Opus), and the gateway intro line + Check button contradicted the "Kiro Crew doesn't check for updates itself" notice (UX BLOCK). A separate issue (#23872) already tracks the "show manual update path under policy" follow-up.

GPT — updateSwitches.ts settings-search visibility: Added policyGoverned parameter to updateSwitchesShown. When true, app is false, so settings search, What's-new modal, and About all agree the auto-download switch is absent under policy governance. useUpdateSwitchesShown reads selectPolicyGoverned from the store. Test added: hides the app switch when policy governs.

GPT — feed-lane.js freshness verification race: Added isPolicyGoverned() re-checks after gate.verifyStageIsLatest() in both install paths: manual (applyUpdateAndRestart) and deferred-on-quit. If governance arrived during the async freshness wait, the install is aborted before quitAndInstall() runs.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit ebd69c9 addresses all remaining blocking findings.

GPT F1 (download governance race): Added isPolicyGoverned() re-check immediately before downloadUpdate() in startDownload(). If governance arrived during the freshness wait (when gate.checking blocked the IPC-triggered safeCheck()), the download is now suppressed and not-available is emitted.

GPT F2 / UX BLOCK (policy notice + controls): AboutPanelGatewayUpdates now renders alongside the policy notice only when showUpdate === true. When no update is pending the notice stands alone (no contradictory intro line or Check button). When the provider has a pending update the controls are available. This keeps the screen clean on every ordinary visit while providing a manual path when the provider actually has something to apply.

UX BLOCK (gateway switch ambiguity in topology 2):

  • Added app_updates_heading ("App updates") sub-heading above the app's intro line when updateSwitches.gateway is true, so the two sections are visually distinct.
  • Restored "It only restarts when no work is active." in gateway_auto_update_desktop_context across all 13 locale files, so the switch's behaviour is clear at a glance.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 85a13cf — pseudolocale regenerated.

The i18n:check [pseudolocale] gate found en-XA.json stale after adding app_updates_heading and updating gateway_auto_update_desktop_context. Ran node scripts/gen-pseudolocale.mjs and included the updated file.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 6ae6e32 fixes both new blocking findings.

Opus BLOCK (duplicate GatewayAutoUpdateSwitch): Passed autoUpdateSwitch={null} in the policy-managed branch's AboutPanelGatewayUpdates call. The outer {updateSwitches.gateway && <GatewayAutoUpdateSwitch />} already renders after the branch; the inner one now renders nothing, so there is exactly one switch on screen.

GPT F1 (download-progress badge stuck after governance): Added isPolicyGoverned() guard in the download-progress handler (returns early so no downloading emit while governed) and emits not-available in update-downloaded's governance early-return path to clear any stale badge immediately rather than waiting for the next four-hour poll.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 32817c1 addresses GPT F1 and F2.

GPT F2 (governance during stopGateway): Added isPolicyGoverned() re-check AFTER await stopGateway() in both install paths:

  • Manual path: if governed after shutdown, restores gateway state (installing = false, onInstallFailed()), emits not-available, and returns.
  • Quit path: if governed after shutdown, notifies the canceled install and quits without installing.

GPT F1 (remote bundled gateway suppressing local app): Added isLocalGatewayEnabled(store) gate before the origin check in update:notify-policy-governed. When the user has explicitly disabled the local gateway (connecting to an external/tunneled one), the handler returns early — a remote bundled gateway's policy provider governs updates on its own machine, not the local Electron app. This is the existing isLocalGatewayEnabled predicate (the same store key the local-gateway:get handler exposes to the renderer). Test added: remote-gateway harness with runLocalGateway: false verifies governance is rejected.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 769a6b5 addresses all remaining blocking findings.

GPT F1 (SSH tunnel with local gateway enabled): The update:notify-policy-governed handler now uses the existing three-gate assertLocalDashboard (origin + isGatewayLocalForWindow + probePrimaryPortOwner === "kirocrew"|"service") instead of the single origin check. Gate 3 catches a manual SSH tunnel holding the primary port. Handler is now async (fire-and-forget, return value ignored by Electron). Tests updated to pass { sender, senderFrame } event objects and await the async handler. New harness with primaryPortOwner: "ssh-tunnel" verifies governance is rejected.

GPT F2 (governance during stopGateway): isPolicyGoverned() re-checked after await stopGateway() in both the manual install path (restores gateway state, emits not-available) and the quit path (notifies canceled install, continues quit without installing).

GPT F2b (managed lane + policy): createManagedLane now accepts isPolicyGoverned = () => false; managedCheck returns early under governance (disarms quit handler, emits not-available); managedInstallOnQuit skips the apply when governed. isPolicyGoverned passed from initAutoUpdate.

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 7420cbc fixes the Electron Shell Tests failure.

assertLocalDashboard is defined inside registerShell() and is not accessible from registerUpdater(). The handler silently caught the ReferenceError and returned early, so governance was never set. Fixed by inlining the three gates directly in the handler (same logic as assertLocalDashboard: senderFrame origin check → isGatewayLocalForWindow → probePrimaryPortOwner).

@CrysisDeu

Copy link
Copy Markdown
Contributor Author

Commit 164e1f0 fixes both new blocking findings.

GPT F1 (managed-lane.js governance during stopGateway): Added isPolicyGoverned() re-check in managedInstallOnQuit after await stopGateway(), before runUpdateCommand(). If governance arrived during the shutdown await, the apply is skipped and the app exits normally.

GPT F2 (ipc-registrar.js destroyed sender after port probe): Added _event?.sender?.isDestroyed?.() check after await assertLocalDashboard(...). If the window was destroyed during the probe, the sender id is not added to governanceSenders and no cleanup listener is registered. This prevents a dead sender from permanently suppressing updates.

Also in this commit: async race per-sender sequence tracking (senderSeq) and the macOS window-all-closed governance latch (anyOpen || platform !== "darwin"), which together address the Opus BLOCK from the previous head.

…bundled installs

Two topologies let the gateway's security policy provider and the Electron
app's own updater act simultaneously on the same bundled install:

1. **Policy provider on a desktop install.** When a security_policy.json
   `updates` block configures `check_command`/`apply_command`, the gateway
   runs the provider; the Electron auto-updater also checks independently.
   Both can find and apply an update at the same time. Per governance.md, the
   provider owns the check, the badge and the Update button and is resolved
   before any layout's own updater or deferral: the fix makes the Electron
   feed lane stand down when the gateway reports the provider governs.

2. **Reused gateway (CLI service, SSH tunnel).** The About page's gateway
   auto-update switch was hidden in a desktop window when the effect was
   unknown, because `update_bundled_by_app` was not yet used to tell the
   bundled gateway apart from a reused one.

Changes:

- `website/electron/runtime/update/feed-lane.js`: add `isPolicyGoverned`
  parameter; `safeCheck()` returns early with `not-available` when governed,
  so the Electron auto-updater does not contact the feed or fire a notification.
- `website/electron/auto-update.js`: add `isPolicyGoverned` parameter;
  pass it to `createFeedLane`.
- `website/electron/ipc-registrar.js`: add `policyGoverned` variable;
  add `update:notify-policy-governed` IPC handler; wire `isPolicyGoverned`.
- `website/electron/preload.js`: expose `notifyPolicyGoverned` on `updateAPI`.
- `website/src/types/electron-bridge.d.ts`: add `notifyPolicyGoverned` to
  `UpdateAPI`.
- `website/src/hooks/useUpdateSubscription.ts`: add
  `usePolicyGovernedNotification` hook that calls `notifyPolicyGoverned`
  whenever `update_bundled_by_app && update_managed_by === 'command'` changes.
- `website/src/shell/appShell.tsx`: import and call `usePolicyGovernedNotification` after `useUpdateSubscription` (moved from App.tsx due to shell refactoring).
- `website/src/pages/settings/AboutPanel.tsx` (policy-managed branch): render
  `AboutPanelGatewayUpdates` alongside the policy notice so a manual update
  path remains available even when the Electron feed lane stands down.
- `website/src/components/UpdateFoundModal.tsx`: gate the desktop candidate
  on `!gwPolicyManaged` so the modal does not interrupt a bundled install
  whose updates the policy provider owns.
- `website/src/pages/settings/AboutPanel.tsx`: when `isGatewayPolicyManaged`,
  replace the desktop update card with a "managed by policy" notice.
- `website/src/utils/updateSwitches.ts` (topology 2): `updateSwitchesShown`
  accepts optional `bundledByApp` from the status frame; the gateway switch is
  shown when `bundledByApp === false` (a reused gateway that manages its own
  bytes and is not covered by the Electron updater).
@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD at 76dc554, reserved for human review.

Our r101 items are addressed. The governance handler now clears on window destroy, accepts only senders that pass the shared local-dashboard gates plus a current-bundle check (ipc-registrar.js#L708-L804), and the four deferred items are #23871-#23874. CI is green.

What blocks:

  1. Policy: this channel lets a renderer switch off the desktop app's own updater, so a person must review who it trusts. It also adds lsof as a deb/rpm dependency (package.json#L89-L113).
  2. No replies on this head to the current lane items: UX's evidence gaps (shot 4 predates the "App updates" heading and the context note's third sentence; no shot of the policy notice with a pending update), Design note 2 (the lsof dependency and the managed-lane stand-down are missing from the description), First Principles items 2, 3 and 9, and GPT's downgraded AppImage-without-lsof finding.
  3. The description is out of date. What changed and Backwards compatibility describe a single origin gate and say nothing of the port-owner and bundle gates, lsof, the managed lane or the in-flight re-checks. The first screenshot's alt text quotes an old notice string.

What clears it: one reply per lane item on this head, the description rewritten from the current diff with fresh screenshots, and a human approval.

I did not run the Electron or website tests.

— crew-pr-reviewer (r126)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: the app updater and the gateway updater can both run (policy provider or reused gateway)

3 participants