Fix external integration URL updates not reflecting without app restart

[apoorvdarshan]

What changed and why

Fixes #3309. After saving integration URL changes on the update-app screen, the app detail page kept using a stale local App object because both refresh paths only compared appHomeUrl, name, and description. Edits limited to auth-step URLs, setup-completed URLs, webhook URLs, or setup-instructions paths therefore did not trigger a refresh until a full app restart.

This PR:

• Detects user-visible/editable external-integration URL field changes and refreshes the detail page.
• Re-runs setup-completion and instructions loading after those changes.
• Extracts a shared _loadSetupInstructionsMarkdown helper used by initState and _onExternalIntegrationUpdated.
• Guards in-flight checkSetupCompleted / getAppMarkdown callbacks with generation counters plus requested URL/path checks so a stale response cannot overwrite newer config.

An accidental later push had truncated app_detail.dart; that file is restored. Diff is the intended 3-file set only.

Product invariants affected

none

How it was verified

• Local line-length check: app/test/unit/app_detail_config_changed_test.dart lines 69–70 are now well under 120 columns (the Formatting failure).
• git diff --stat vs BasedHardware/omi main is 3 files, +259 / −44 (app_detail.dart, app_detail_config.dart, the unit test).
• Bounded pre-push against upstream main passed. Flutter SDK is not in this VM, so Dart Analyze and dart format were not executed here; CI is the authority.
• local_segment_store_test.dart is out of scope; if Dart Analyze & Tests still fails on that capture/WAL flake, it is unrelated.

Manual verify path:

1. Open an external integration app you own.
2. Edit the app and change only the auth-step URL or setup-completed URL (leave name/home URL unchanged).
3. Save and return to the app detail page.
4. Confirm the setup button/link and setup-completed check use the new URLs without force-quitting the app.

Tests

app/test/unit/app_detail_config_changed_test.dart covers auth-step URL-only changes, setup-completed URL-only changes, webhook URL changes, hidden-field no-ops, and name changes.

Failure class (fixes)

Failure-Class: none

[cubic-dev-ai]

1 issue found across 3 files

Confidence score: 3/5

• In app_detail.dart, a late setup-check response can update the UI with completion status for a previous URL after a reload, showing stale configuration state to users; guard responses by the requested URL or configuration generation before updating state.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/lib/pages/apps/app_detail/app_detail.dart">

<violation number="1" location="app/lib/pages/apps/app_detail/app_detail.dart:269">
P2: When an earlier setup check completes after this reload check, it can display the completion status for the old URL. Guard setup-check responses with the requested URL or a configuration generation before updating `setupCompleted` and triggering auto-install.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[Git-on-my-level]

Thanks for the clean, well-scoped fix and the honest verification notes — the regression tests are exactly the right shape for #3309.

What I verified:

• All 9 tests in app/test/unit/app_detail_config_changed_test.dart ran and passed in CI (confirmed from the test-run diagnostics), including the auth-step-only and setup-completed-only change cases that reproduce the bug.
• The Dart Analyze & Tests failure is not from this PR: the single failing test is local_segment_store_test.dart → "CaptureProvider listener persists when a live session is active" (expects a persisted-segment content hash, gets the empty-input hash). That is the capture/WAL subsystem, untouched by this diff, and main-branch runs of the same suite were green today — no action needed from you on that one.

Blocking (small):

• The Formatting check fails on app/test/unit/app_detail_config_changed_test.dart: lines 69–70 are 121 characters, just over the 120-column limit, so dart format --line-length 120 --set-exit-if-changed rejects the file. Running dart format --line-length 120 app/test/unit/app_detail_config_changed_test.dart and pushing should turn it green.

Non-blocking, worth considering:

• _onExternalIntegrationUpdated() in app/lib/pages/apps/app_detail/app_detail.dart duplicates the instructions-fetch block from initState (the getAppMarkdown → ](assets/ rewrite → setState(instructionsMarkdown = ...) sequence). Extracting one helper called from both places would remove the copy and make it natural to guard the async callbacks with the path/config they were issued for, so a late-arriving older fetch cannot overwrite newer instructions or setupCompleted state.
• The doc comment on hasAppDetailConfigChanged in app/lib/pages/apps/app_detail/app_detail_config.dart says it compares "fields shown on the app detail page", but ExternalIntegration.toJson() also includes fields the page does not render (actions, triggers_on, chat_tools_manifest_url, mcp_server_url). The wider comparison is harmless today (worst case an extra refresh); trimming the comment or the compared field set would make the contract read true.

Once formatting is green, this still needs a maintainer to manually walk the #3309 repro (edit only an auth-step or setup-completed URL, save, return to the detail page) before merge — the save/return flow has no automated coverage and the unit tests stop at the comparison helpers.

This review was produced by automated tooling on behalf of the Omi maintainers.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[apoorvdarshan]

Addressed the requested changes on cursor/fix-integration-url-refresh-26a3 (rebased onto latest BasedHardware/omi main, no fork-main sync):

• Restored the intended 3-file diff after the accidental app_detail.dart truncation.
• Formatting: app_detail_config_changed_test.dart is wrapped under 120 columns.
• Shared _loadSetupInstructionsMarkdown for initState and _onExternalIntegrationUpdated.
• Stale in-flight checkSetupCompleted / markdown fetches are ignored via generation counters + requested URL/path checks.
• hasExternalIntegrationChanged compares only user-visible/editable URL fields for URL updates not reflecting in external integration app without restart #3309.

Please re-review. Dart Analyze still cannot be run in this VM (no Flutter SDK); CI is the authority. The local_segment_store_test.dart flake is unrelated if it reappears.

[Git-on-my-level]

Thanks @apoorvdarshan — all three items from the earlier review are addressed on d7e4ef9, verified against the diff and CI:

• Formatting: app/test/unit/app_detail_config_changed_test.dart now has no lines over 120 columns (checked directly at head), and the Formatting check is green.
• The duplicated instructions-fetch block in app/lib/pages/apps/app_detail/app_detail.dart is now a single _loadSetupInstructionsMarkdown() used by both initState and _onExternalIntegrationUpdated() — and it goes a step further than suggested: a _markdownLoadGeneration counter plus a re-check of setupInstructionsFilePath means a late-arriving fetch for an old path can no longer overwrite newer instructions. checkSetupCompleted() gets the same generation + requested-URL guard, and both refresh call sites (didChangeDependencies and the builder's _applyProviderAppUpdate) now re-run setup-completion and instructions loading after a config change — which is the actual URL updates not reflecting in external integration app without restart #3309 fix.
• hasExternalIntegrationChanged() in app/lib/pages/apps/app_detail/app_detail_config.dart now compares exactly the user-visible/editable URL fields (appHomeUrl, setupCompletedUrl, webhookUrl, setupInstructionsFilePath, auth-step name/url) and documents the ignored hidden fields, with the new "returns false when only hidden integration fields change" test pinning that contract.

Also confirmed: authSteps is non-nullable on ExternalIntegration (app/lib/backend/schema/app.dart defaults it and fromGenerated maps ?? const []), so the indexed comparison in the helper is safe. Dart Analyze & Tests is green this run with all 11 tests in the new unit file — the earlier local_segment_store_test.dart flake did not reappear — and the head diff is the intended clean 3-file set after the accidental truncation was restored. The earlier changes-requested review has been dismissed.

Remaining before merge: the unit tests stop at the comparison helpers, so the #3309 repro itself (edit only an auth-step or setup-completed URL → save → return to the detail page) still needs one manual walkthrough on a device/emulator. That's the only open item I'm aware of.

This review was produced by automated tooling on behalf of the Omi maintainers.

---

by AI on behalf of David — the manual #3309 walkthrough above is the one item needing a maintainer's hands before merge; no other follow-up is pending.

Dismissed: all items from this review are resolved at d7e4ef9 — formatting is green (no lines over 120 columns in app_detail_config_changed_test.dart), the instructions-fetch block is deduplicated into _loadSetupInstructionsMarkdown with stale-response guards, and hasExternalIntegrationChanged now matches its documented user-visible URL-field contract with a covering test.