Skip to content

web: deep-link openApp/appArgs/autoOpen — pre-select and pre-fill an app from the URL (#1577)#1671

Closed
cliffhall wants to merge 2 commits into
v2/1576-deeplink-connectfrom
v2/1577-deeplink-openapp
Closed

web: deep-link openApp/appArgs/autoOpen — pre-select and pre-fill an app from the URL (#1577)#1671
cliffhall wants to merge 2 commits into
v2/1576-deeplink-connectfrom
v2/1577-deeplink-openapp

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1577

Stacked on #1670 (#1576). Based on v2/1576-deeplink-connect so this diff shows only the #1577 changes. Both are superseded by the Wave 4 rollup; this PR is for focused review.

Summary

Extends the deep link to land on a rendered MCP App with one navigate + one click (or zero clicks with autoOpen):

  • &openApp=<toolName> — once connected and the tool appears in the app list, InspectorView switches to the Apps tab and pre-selects it.
  • &appArgs=<base64url(JSON)> — pre-fills the tool form; values are merged over collectSchemaDefaults() so required-with-default fields don't disable "Open App".
  • &autoOpen=<token> — token-gated auto-click of "Open App".

Adaptation notes (differs from PR #1510 reference)

  • Tab state is lifted to App.tsx (Persist per-screen selection and search/filter across tab navigation; clear only on user action or disconnect #1417), so the openApp effect lives in InspectorView and drives onActiveTabChange("Apps") + onAppsUiChange (seed selection + merged form values) + onSelectApp, guarded by deepLinkOpenAppRef to fire once.
  • AppsScreen owns the running/iframe state, so autoOpen is threaded to it as a prop. Its fire-once effect defers the open one microtask past the synchronous effect body — satisfying react-hooks/set-state-in-effect the same way App.tsx's connect effect does. resetAppChannels is memoized so the effect deps don't churn.

Tests

  • InspectorView: tab-switch + pre-select once connected; appArgs merged over schema defaults into the pre-filled form; a non-app openApp is ignored (no tab switch).
  • AppsScreen: autoOpen fires once with the seeded values and mounts the renderer; autoOpen with no selection stays idle.

Coverage gate green (npm run test:coverage, EXIT 0; AppsScreen.tsx 97.7/96.3/100/100).

Part of the #1579 decomposition (Wave 4). Per AGENTS.md the Closes line won't auto-fire on v2/main; the issue is closed and its board card moved on the Wave 4 rollup merge.

🤖 Generated with Claude Code

…p from the URL (#1577)

Closes #1577

Extends the deep link (#1576) to land on a rendered MCP App:

- `&openApp=<toolName>` — once connected and the tool appears in the app list,
  InspectorView switches to the Apps tab and pre-selects it.
- `&appArgs=<base64url(JSON)>` — pre-fills the tool form; values are merged
  **over** `collectSchemaDefaults()` so required-with-default fields don't
  disable "Open App".
- `&autoOpen=<token>` — token-gated auto-click of "Open App".

Adapted to the current architecture (differs from PR #1510's reference):
- Tab state is lifted to App.tsx (#1417), so the openApp effect lives in
  InspectorView and drives `onActiveTabChange("Apps")` + `onAppsUiChange` seed
  + `onSelectApp`, guarded by `deepLinkOpenAppRef` to fire once.
- AppsScreen owns the `running`/iframe state, so `autoOpen` is threaded to it
  as a prop; a fire-once effect defers the open one microtask past the
  synchronous effect body (satisfying `react-hooks/set-state-in-effect`,
  matching App.tsx's connect effect). `resetAppChannels` memoized so the
  effect's deps don't churn.

Tests: InspectorView (tab-switch + pre-select + appArgs-over-defaults merge +
non-app openApp ignored), AppsScreen (autoOpen fires once with seeded values;
no-selection stays idle). README documents the params. Coverage gate green.

Part of the #1579 decomposition (Wave 4).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 12, 2026
@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 2m 7s —— View job


Review: deep-link openApp / appArgs / autoOpen

  • Read the diff
  • Review InspectorView.tsx (openApp effect)
  • Review AppsScreen.tsx (autoOpen)
  • Review tests & docs
  • Post review

Solid, well-scoped PR. The two-part split — InspectorView drives tab-switch + pre-select + form seeding, AppsScreen owns the fire-once auto-open because it holds running/iframe state — is the right seam given the #1417 tab lift. The ref guards, the collectSchemaDefaults-then-overlay merge, and the microtask defer are all correct and clearly documented. Tests cover the meaningful paths (connect→select, appArgs-over-defaults, non-app ignored, autoOpen fires once, autoOpen idle with no selection). Verified the supporting wiring: AppsScreen receives tools={appTools} (already app-filtered), and in App.tsx onSelectApp is a no-op while onAppsUiChange={setAppsUi} does the real seeding — so the effect's triple call is consistent with real usage. Nothing blocking.

A few minor, non-blocking observations:

1. Auto-open isn't scoped to the deep-link's target app (theoretical race)AppsScreen.tsx:525-534
The fire-once effect triggers on whatever selectedTool is first present while autoOpen is true; it doesn't check that the selection is the deep-link target. In practice InspectorView drives the selection to the target immediately on connect, well before a human could click, so this is very low risk. But if a user manually selected a different app in the narrow window before the seeding effect lands, autoOpen would fire on their pick. Since AppsScreen has no knowledge of the target name, tightening this would mean threading the expected tool name down — probably not worth it, but worth a mental note.

2. Redundant guards (harmless)InspectorView.tsx:1193

autoOpen={Boolean(deepLink?.openApp) && (deepLink?.autoOpen ?? false)}

DeepLink.autoOpen is a non-optional boolean (see deepLink.ts:19), so ?? false is dead. And AppsScreen only fires when selectedTool is present — which only happens once openApp matched — so Boolean(deepLink?.openApp) is also belt-and-suspenders. Both are safe; just noting they can't be false in the way the expression implies.

3. appArgs merge is shallowInspectorView.tsx:889-892

const formValues = {
  ...collectSchemaDefaults(target.inputSchema),
  ...deepLink.appArgs,
};

A nested-object field present in both a schema default and appArgs is fully replaced by the appArgs value rather than deep-merged. This is almost certainly the intended semantics ("URL wins"), and matches how the schema form treats top-level fields — just flagging it in case a future nested-default case surprises someone.

4. Doc nitREADME.md
The appArgs row says malformed/non-object values "fall back to {}", which is accurate but lives in the decodeAppArgs/parseDeepLink layer (base PR #1576), not this diff. Fine to keep since this table is the user-facing contract, just calling out that the behavior is inherited, not added here.

None of these require changes. Nice work on the coverage and the automation-contract docs.
· v2/1577-deeplink-openapp

DeepLink.autoOpen is a non-optional boolean, so the `?? false` was dead once
openApp was truthy. Collapse to Boolean(deepLink?.openApp && deepLink.autoOpen)
— still false when there's no deep link, still gated on openApp so a malformed
autoOpen-without-openApp link can't fire.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall

Copy link
Copy Markdown
Member Author

Thanks — simplified the redundant autoOpen guard in b8cd4fd4 to Boolean(deepLink?.openApp && deepLink.autoOpen) (finding #2). #1 (auto-open not scoped to the target app name) is very low risk as you noted — InspectorView drives the selection to the target on connect before a human could pick another — so I left it rather than thread the tool name into AppsScreen. #3 (shallow appArgs merge) is the intended 'URL wins' semantics, and #4 is inherited from the #1576 base.

@claude review

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 46s —— View job


Re-review: openApp / appArgs / autoOpen (after b8cd4fd4)

  • Read the new commit
  • Verify the simplified autoOpen guard
  • Confirm the other findings' dispositions are reasonable
  • Post verdict

The follow-up commit is clean — LGTM. ✅

Finding #2 (fixed correctly). InspectorView.tsx:1193 now reads:

autoOpen={Boolean(deepLink?.openApp && deepLink.autoOpen)}

This preserves the three cases exactly:

  • no deep link → deepLink?.openApp short-circuits to undefinedBoolean(undefined) = false
  • openApp set but autoOpen false → false
  • both truthy → true

And it keeps autoOpen gated on openApp, so a malformed autoOpen-without-openApp link still can't fire — matching the commit message's stated intent. I confirmed DeepLink.autoOpen is a non-optional boolean (utils/deepLink.ts:19) and is only ever set to a real boolean via the token comparison (deepLink.ts:171), so the dropped ?? false was genuinely dead. No behavior change, just less noise. Good call also to keep the Boolean() wrapper — without it the expression would surface the raw openApp string / undefined to the boolean prop.

Dispositions on #1, #3, #4 — all reasonable.

Nothing outstanding. Ship it. 🚀
· v2/1577-deeplink-openapp

@cliffhall

Copy link
Copy Markdown
Member Author

Superseded by the Wave 4 rollup #1676, which contains this change (reviewed to LGTM here) merged base→tip. Closing in favor of the rollup.

@cliffhall cliffhall closed this Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

web: deep-link openApp/appArgs/autoOpen — pre-select and pre-fill an app from the URL

1 participant