frontend-patterns/research/reviews/ZulipTargetLookupCallPath.md

95 lines
7.7 KiB
Markdown
Raw Normal View History

# Zulip target-lookup and anchor-continuation guard trace — 2026-09-27
LEARN-0017; Codex author-model review. [Protocol](../evolution/cycles/2026-09-27-zulip-target-lookup.md)
committed as `884803c` before this inspection. Stream B, comparison group
B-independent-navigation-source-v1 — second cycle in this group, following
LEARN-0014. **Static source evidence only**: no application build, browser run,
upstream test execution or inference call.
Pinned Zulip revision `5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c`, deliberately
reused from the earlier navigation and acceptance/projection studies, not
claimed latest. [Source manifest](ZulipTargetLookupSources.json) records four
files, raw SHA256 and reviewed ranges. This is the same Zulip family already
known to this research through other paths, not a new independent family.
## Question
ZN07 (ZulipNavigationSourceStudy.md) located the auxiliary target-message
lookup and the anchor-navigation fetch continuation and noted they have "no
visible captured-current-list guard like ZN02," but did not trace their actual
callers or check whether an existing test covers a stale-navigation race for
either path. This cycle traces both call paths to their concrete completion
point and checks test coverage.
## Located mechanism
| Claim | Exact pinned locators | Observed code | Interpretation / limit |
| --- | --- | --- | --- |
| ZT01 — target-message lookup | [channel.get to /json/messages/:id](https://github.com/zulip/zulip/blob/5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c/web/src/message_view.ts#L663-L692) | On success or error, the callback calls `show(raw_terms, {...opts, fetched_target_message: true})` unconditionally — there is no check that the user is still on the same narrow/hash that triggered the lookup. | A completed lookup re-enters the main `show` function regardless of intervening navigation. `show` itself resets UI state on entry, so a stale re-entry would still apply its own reset, but no code at this call site rejects a stale re-entry before doing so. |
| ZT02 — anchor-navigation continuation | [navigate_to_anchor_message / load_messages_around_anchor](https://github.com/zulip/zulip/blob/5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c/web/src/message_view.ts#L1001-L1073) | The continuation closure (`select_anchor_using_data`) captures `message_lists.current` and a freshly constructed `msg_list_data` at call time, then, once the fetch completes, unconditionally calls `message_lists.update_current_message_list(...)` and `select_msg_id(...)`. | If the user navigates away while `load_messages_around_anchor` is in flight, the completion still installs a message list built from the pre-navigation `message_lists.current` as the new current list. Nothing in this path re-checks that the captured list is still current before installing it. |
| ZT03 — where the continuation is actually invoked | [process_result](https://github.com/zulip/zulip/blob/5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c/web/src/message_fetch.ts#L157-L206) calls `opts.cont(data, opts)` unconditionally whenever `opts.cont !== undefined`. | `load_messages_around_anchor` never sets `opts.msg_list` (only `opts.msg_list_data`), so this call path does not go through the `opts.msg_list !== message_lists.current` guard that exists elsewhere (see ZT04). The continuation always fires once the request resolves. |
| ZT04 — the guard that *does* exist, and why it doesn't apply here | [load_messages error path](https://github.com/zulip/zulip/blob/5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c/web/src/message_fetch.ts#L455-L464) rejects further processing when `opts.msg_list !== undefined && opts.msg_list !== message_lists.current && !opts.msg_list.is_combined_feed_view`; [get_messages_success](https://github.com/zulip/zulip/blob/5b06fa3d34f28fa01f459fbf3ba053e3c0858e9c/web/src/message_fetch.ts#L249-L259) has an analogous success-path check. | Both guards are conditioned on `opts.msg_list`, which the anchor-fetch caller never supplies. The mechanism ZN02 relied on (list-identity checking before continuing) exists in this file, but is opt-in per caller, not automatic for every fetch that carries a `cont` callback. |
## Test coverage
- `web/tests/message_fetch.test.cjs` is 40 lines total (already reviewed in
full at ZN08); it exercises `get_parameters_for_message_fetch_api`'s
date-anchor parameter handling only. No test in this file exercises
`opts.cont`, `process_result`, or a stale-`message_lists.current` scenario.
- `web/tests/message_view.test.cjs` mocks `message_fetch.load_messages_around_anchor`
directly (bypassing `message_fetch.ts` entirely) in its
`fast_track_current_msg_list_to_anchor` tests (L1106–L1244); the mock invokes
the callback synchronously with no intervening navigation. No occurrence of
`target_id` or `fetched_target_message` was found anywhere in this file, so
the target-message-lookup path (ZT01) has no located test coverage at all.
## Interpretation and boundary
This corroborates and sharpens ZN07 rather than revising it: the two flagged
continuations genuinely lack the kind of captured-list guard that protects
`message_view.ts`'s main narrow-fetch continuation (ZN02) and that
`message_fetch.ts` itself implements for callers that pass `opts.msg_list`
(ZT04). The absence is now traced to a concrete cause — `load_messages_around_anchor`
and the target-lookup `channel.get` simply don't participate in that
opt-in mechanism — rather than only observed as "no visible guard." Neither
path has any located automated test coverage for the stale-navigation case.
This is **not** a reproduced race, a defect report, or a claim about runtime
behavior: `show()` itself performs its own state reset on entry (see ZT01),
which may or may not make a stale re-entry externally observable as wrong
behavior — that requires actual execution, which is out of scope here. No
production dependency, upstream message, or bug report follows from a source
trace. Do not treat this as established misbehavior without reproduction.
## Pattern disposition: corroboration, no promotion
CAND-0009 (URL-addressable State) and CAND-0010 (Progressive Enhancement)
remain draft/hypothesis; no candidate, relation or maturity change. The
existing "Zulip auxiliary target/anchor callbacks... remain unverified" gap
is sharpened, not resolved: it is now specifically "no captured-current-list
guard exists on these two paths, and no test covers the gap," rather than a
general unknown. Independent full-contract use counts stay unassessed.
## Scope closure and follow-up
Four files, all already implicated by ZN07; no further callees were fetched.
Files were inspected at the manifest's ranges except the two full test files,
which were short enough to review completely. No upstream code or frozen
evaluation artifact changed. Tests were read, not executed.
[Effort receipt](../evolution/cycles/2026-09-27-zulip-target-lookup-effort.json)
records active source work; token/cost unknown. This is the second cycle in
comparison group B-independent-navigation-source-v1 (with LEARN-0014); still
short of the three comparable cycles WP-0006 T03 needs before a returns review
of this specific group, and the workplan's own rule against manufacturing
cycles to reach that count still applies — a third cycle here should only
happen if a genuinely useful next question arises, not to close the count.
WP-0003 T01 follow-up, if pursued: before any defect or race claim, a
preregistered reproduction would need to actually run Zulip's frontend code
(not just read it) with a controlled clock/interception to hold the target-lookup
or anchor-fetch response while installing a competing navigation — outside the
static-source protocol used here. T02 retains independent human/AT and offline-case
obligations. WP-0006 T03 retains review-date/comparable-window work; the local
authored navigation fixture remains paused.