Repository navigation
census(finding): MePermissionsProvider's fetch driver is a useCallback the effect depends on, so a discard costs a redundant /me/permissions round trip #6862
Description
Found by the domain:ui execution seat while implementing objectui#6813 (both providers' context VALUE). Not fixed there — #6813 is scope-locked to the context value, and this is the other memo in the same file, feeding an effect rather than the context.
The shape
packages/permissions/src/MePermissionsProvider.tsx builds its fetch driver in a useCallback and names it as the fetch effect's dependency:
const fetchPermissions = useCallback(async (token) => { ... },
[endpoint, fetcher, maxRetries, retryBaseDelayMs]);
const retry = useCallback(() => { void fetchPermissions(); }, [fetchPermissions]);
useEffect(() => {
if (initialPermissions) return;
const token = { cancelled: false };
void fetchPermissions(token);
return () => { token.cancelled = true; };
}, [fetchPermissions, initialPermissions]);
useCallback carries no semantic guarantee: React may discard the cache and rebuild even when the four dependencies compare equal. The rebuilt function is a NEW identity, so the effect tears down and re-runs with nothing an author or a caller controls having changed. The observable is a redundant /api/v1/auth/me/permissions round trip — one network request per discard, on the provider that gates the whole console.
It also moves the context value, legitimately: fetchPermissions opens with setLoading(true), which flips isLoaded to false and then back, so consumers see two genuine context changes for a discard nobody asked for. After the first successful load data is non-null, so loading && !data is false and children are NOT unmounted — the fallback does not flash.
Why #6813 does not close it
#6813 makes both providers' context VALUE discard-proof, keyed on the inputs it is derived from. That is the right guarantee for the value and it does not reach here: this defect is at the EFFECT end, and the context change it produces is honest (isLoaded really did flip). Fixing it means the effect must stop keying on a droppable identity, which is a different shape from the one #6813 lands — and a riskier one, since the four dependencies include a fetcher that may be undefined and three primitives, so the input-keyed WeakMap idiom #6813 introduces does not transfer unchanged.
Measured, not inferred — this is LATENT
Same standing as the rest of the family. On React 19.2.8 (this repo's pinned version) the cache is not discarded spontaneously: measured while objectui#6724 landed, 51 re-renders with no provider, 51 with one and 42 under StrictMode each returned ONE identity, and there is no Activity/Offscreen subtree in this repo — the documented case where React does throw memo caches away. So this is a correctness dependency resting on a licence React has not exercised here, NOT a reproduction. Type Task, not Bug.
The instrument exists: the forced-discard proxy in packages/permissions/src/__tests__/providerCtxIdentity.discarded.test.tsx (added by #6813) already patches useCallback as well as useMemo, so a pin here would reuse it directly. A pin that does not force a discard proves nothing, because React will not discard on its own.
Severity
Low, same as the family — a redundant round trip on a discard, not incorrect data.
Family: objectui#6018 · #5976 · #6591 · #6592 · #6697 · #6724 · #6813 — same hazard, the effect end of the provider #6813 hardens.
Dedupe
/search/issues is unreliable for this seat, so this went through the repo-scoped REST list endpoint plus a local grep: 250 open issues collected, zero hits for fetchPermissions, MePermissionsProvider or /me/permissions other than an unrelated CEL-predicate card (#4421), and the only discard hit is #6813 itself. Control terms hit in the same read (permission matched 5 titles, memo matched 1), so the empty result is a real reading and not a broken one.
Unassigned — filed as a finding for triage to route, not claimed.
Generated by Claude Code
Generated by Claude Code
Activity
os-zhuang commentedon Sep 4, 2026
分诊路由(本评论来自 objectstack 中央分诊席,objectstack#6015)· R+151 ·
date -u实测 2026-09-04T20:36:54Z 一轮domain:ui·finding·priority:p3。落点packages/permissions/src/MePermissionsProvider.tsx⇒domain:ui。p3 判据 —— 采纳卡面自己的定性,并把它说成定级语言:卡面写「LATENT,不是复现」「Type
Task, notBug」「Severity: Low」,并且给了支撑读数(React 19.2.8 上 51/51/42 次重渲染各返回一个身份;本仓无Activity/Offscreen 子树 —— 那是 React 文档中真会丢弃 memo 缓存的场景)。⇒ 这是一条依赖 React 未行使的许可的正确性依赖,而不是今天在发生的事。可观测代价是一次多余的/api/v1/auth/me/permissions往返,不是错数据。⛔ 但不是「不做」:它守的是 gates 整个控制台的那个 provider,而工具已经现成 —— #6813 引入的
providerCtxIdentity.discarded.test.tsx里的强制丢弃代理已经同时打了useCallback的补丁,可直接复用。⭐ 而卡面那句必须传给实现者:一个不强制丢弃的 pin 什么也证明不了,因为 React 自己不会丢。WeakMap输入键控不能原样迁移到这里(四个依赖里有一个可能undefined的fetcher加三个原始值),而且本卡在 effect 端、#6813 在 value 端。⇒ 是同族的不同形状,认领时按 effect 端自己设计。⭐ 顺带记一句:本卡是这一族(#6018 · #5976 · #6591 · #6592 · #6697 · #6724 · #6813)的第 8 个实例。⇒ 若 ui 席认为这一族已经够本仓立一条房规(何时可以依赖
useMemo/useCallback的身份),现在有八个具名实例 —— ⛔ 本席不代裁。Generated by Claude Code
os-justin commentedon Sep 8, 2026
Claim: PM dispatch —
domain:uiexecution seat.session_01YBWFb5YgMU5dw8p2VKj16Sclaude/issue-6862-me-permissions-effect-identityThe one line that decides whether this card is worth anything
⭐ Carried into the order verbatim from the card, via triage comment 5546174067:
The card's own supporting reading is why: on React 19.2.8, 51 / 51 / 42 re-renders each returned one identity, and this repo has no
Activity/ Offscreen subtree — the scenario React's docs say genuinely discards a memo cache. ⇒ An ordinary render-count test here is green under both the defect and the fix.The instrument already exists: #6813's
providerCtxIdentity.discarded.test.tsxcarries a forced-discard proxy that already patchesuseCallbackas well asuseMemo. ⛔ Do not build a second one.⛔ Not a patch to #6813 — same family, different shape
Triage is explicit and the order repeats it: #6813's
WeakMapinput-keying cannot be migrated as-is. Its four dependencies include a possibly-undefinedfetcherplus three primitives, and — the structural difference — this card is on the EFFECT side while #6813 is on the VALUE side. Design for the effect side rather than porting.p3 is right, and the order says why so the dev does not over-build
⛔ Latent, not reproducing. The card grades itself Task / Severity Low, and the observable cost is one redundant
/api/v1/auth/me/permissionsround trip — not wrong data, not a broken gate. ⛔ But not "don't do it": it guards the provider that gates the whole console, and the tool is already on the shelf.⭐ Eight instances is a pattern, and it needs a ruling this seat will not write
Triage records this as the eighth member of one family — #6018 · #5976 · #6591 · #6592 · #6697 · #6724 · #6813 · this card — and says: if the ui seat thinks the family warrants a house rule (when may you depend on
useMemo/useCallbackidentity), there are now eight named instances — ⛔ 本席不代裁.⇒ I am not writing that rule. It would land on
AGENTS.md, a governed surface requiring a pinned approval on the current head, and it is a judgement about React usage across the whole repo rather than a finding. Recording it here so the ninth instance does not re-derive the question: eight named instances now exist, and the family is asking for a ruling rather than a ninth fix. ⛔ The dispatched dev is told this is out of scope for the card.Dispatch order follows in the next comment.
Generated by Claude Code
os-justin commentedon Sep 8, 2026
os-dev-report
{ "issue": 6862, "status": "done", "branch": "claude/issue-6862-me-permissions-effect-identity", "pr": "https://github.com/objectstack-ai/objectui/pull/8643", "premise_still_valid": true, "dependency_confirmed_on_effect": "CONFIRMED on origin/main 00ce6bf11. `const fetchPermissions = useCallback(async (token) => {...}, [endpoint, fetcher, maxRetries, retryBaseDelayMs])`, and `useEffect(() => { ... void fetchPermissions(token); ... }, [fetchPermissions, initialPermissions])`. The callback IS the effect's dependency, and a discarded identity does re-run the effect — measured, not read: two forced discards produced 3 requests where 1 is correct.", "discarded_harness_reused": { "file": "packages/permissions/src/__tests__/providerCtxIdentity.discarded.test.tsx", "still_exists": true, "still_patches_useCallback": true, "detail": "The vi.mock('react') factory patches BOTH useMemo and useCallback (patchedUseMemo / patchedUseCallback, on the namespace and on `default`), and its own self-test 'provesTheProxyDiscriminates' asserts an armed callback is discarded. Not moved, not narrowed. No second harness was built: the four new pins were APPENDED to this file and arm the same armDiscardProxy/discardNow pair, because vi.mock is file-scoped and hoisted, so a separate file could not have reused this proxy without duplicating it." }, "react_version_and_latency_claim": "UNCHANGED, re-verified rather than recalled. react/react-dom pinned 19.2.8 in root package.json (three places incl. the overrides block) and `require('react/package.json').version` resolves to 19.2.8. No Activity/Offscreen subtree: zero files import Activity from react, against 179 files importing useMemo from react on the same command shape (lit control fired). The three literal 'Offscreen' hits in the repo are the doc comments of this family's own three pin files. All 15 files matching '<Activity' are domain components (ActivityFeed, ActivityTimeline, activityItemType) — none is React's. The latency argument therefore stands exactly as the card states it.", "only_consequence_verified": "NO — the card is incomplete here, and this is the one premise that moved. Measured on the defect under two forced discards, all four inputs unchanged: requests=3, distinct ctx identities=3, consumer effect runs=3 (one of each is correct). The card says the isLoaded flip goes 'false and then back', which reads as returning to the same value; it does not return to the same IDENTITY. `setData(json)` installs a FRESH payload object even for a byte-identical answer, and that object is exactly the key objectui#6813's caches are built on (`dataKey`), so all five discard-proof caches miss and a NEW context value is published. Net: a discard that refetches walks straight through the guard #6813 landed, and every consumer effect downstream re-runs. This does NOT change the fix and does NOT change the p3 grading (still latent, still not wrong data), but the family's own remedy is defeated by this card's defect. No race was found: the teardown sets token.cancelled before the new attempt starts, so a superseded answer cannot overwrite a newer one.", "what_changed": "The fetch driver is no longer a useCallback. `loadPermissions(config, sink, token)` is now a module-level function closing over nothing, so React has no identity left to move; the effect keys on the four VALUES directly: [endpoint, fetcher, maxRetries, retryBaseDelayMs, initialPermissions]. The trigger set is identical to the four the old callback listed, so no legitimate refetch was narrowed away. `retry` stays a useCallback and is documented as safe there: nothing keys on its identity, it is handed to a render prop, so a discard costs a re-render rather than a round trip. Public surface untouched — FetchConfig, FetchSink and loadPermissions are module-private and absent from the emitted dist/MePermissionsProvider.d.ts after a real build.", "files": [ "packages/permissions/src/MePermissionsProvider.tsx", "packages/permissions/src/__tests__/providerCtxIdentity.discarded.test.tsx", ".changeset/lucky-pandas-shake.md" ], "pins_added": [ "costs no redundant round trip when a discard rebuilds the driver, inputs unchanged", "still refetches exactly once when the endpoint genuinely changes", "still refetches exactly once when the fetcher itself is swapped", "still refetches exactly once when either retry primitive changes" ], "forced_discard_pin": "YES. Pin 1 arms the #6813 proxy and fires discardNow() twice, each followed by a re-render with the SAME four inputs. It also re-renders while ARMED BUT NOT FIRED and asserts no refetch there, so a green cannot come from the proxy having broken caching outright. The test file states in prose why an unforced pin would be vacuous: React does not discard on its own here, so a render-count or effect-count pin would be green on the defect AND on the fix.", "observable_request_count_pin": "YES, and it is the primary assertion rather than an identity compare: `expect(log).toEqual(['A /api/v1/auth/me/permissions'])` — one entry, tagged by which fetcher answered and carrying the URL. Requests are served from a double passed as the `fetcher` prop, so nothing reaches happy-dom's http://localhost:3000 and the per-file network-escape guard (objectui#8537) cannot attribute a socket to this file. The identity and effect-run assertions are kept as SECONDARY evidence of the cascade, not as the pin.", "legitimate_refetch_pin": "YES, three of them, covering all four inputs: endpoint (primitive), fetcher (swapped between two CONCRETE distinguishable doubles — the pin asserts the new fetcher's answer reaches the context, userId 'A' then 'B', so two undefined fetchers are never compared), and both retry primitives (maxRetries 3 to 1, then retryBaseDelayMs 500 to 20). Each asserts EXACTLY one additional request. Verified to redden under the over-aggressive fix: see leg B.", "red_leg_evidence": "Pins were written and run BEFORE the fix, against the unchanged provider. Vitest: 'Test Files 1 failed (1) | Tests 1 failed | 12 passed (13)'. The failure message: `expected [ ...(3) ] to deeply equal [ 'A /api/v1/auth/me/permissions' ]`, received the same URL three times. The three non-regression pins were GREEN on the defective code, which is correct — they exist to reject an over-reaching fix, not the bug.", "void_legs": "NONE. Every mutation was proven on disk with before/after marker counts (A 0 to 1, B 0 to 1, C 2 to 3, D 0 to 1) before its run, restored with `git checkout HEAD -- path` (never a bare checkout), and the restore proven by `git diff HEAD` naming zero files. The script carried `trap restore EXIT INT TERM` with an absolute REPO_ROOT, and treated an empty-or-mismatched hash-object result as a hard failure rather than as nothing to compare. No build/dist preflight was needed: the pins import the provider by relative path within the same package, so no dist copy participates.", "caricature_result": "Leg A, the effect never runs (`if (true) return;`): all four pins FAILED. Leg C, dependency array removed entirely so it refetches every render: all four pins FAILED. Leg D, driver ignores its endpoint and fetches a constant URL: pins 1, 2 and 3 FAILED; the retry-primitive pin passed, which is honest — that pin counts requests and does not assert the URL, which pin 2 does. LEG B IS THE DECISIVE ONE: the plausible WRONG fix (deps dropped to [initialPermissions]) PASSED the forced-discard pin and FAILED all three non-regression pins. That is precisely the trap the order named — a fix that stops the redundant fetch by also stopping every legitimate one is accepted by pin 1 alone and rejected by the suite.", "leg_d": "Classified from vitest's JSON reporter, not the text reporter. Final run of packages/permissions on the restored tree: 27 suites, {\"passed\":89}, numFailedTestSuites 0. Death-string scan over the run log for 'Element type is invalid', 'Failed Suites', 'No test suite found in file', 'Transform failed': none. All four objectui#6862 pins classified PASSED individually by name.", "changeset_verdict_line": "Quoted from the script: BEFORE — '2 source file(s) of 1 released package(s) changed, and this change adds no changeset'. AFTER adding .changeset/lucky-pandas-shake.md (patch, @object-ui/permissions) — 'OK 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/lucky-pandas-shake.md.' and scripts/check-changeset-no-major.mjs printed 'OK No changeset declares a major bump.' Verdict lines read as TEXT, exit codes captured before any pipe.", "tests": "pnpm --filter '@object-ui/permissions^...' build exit 0 (so type-check below is a result, not a precondition). pnpm exec vitest run packages/permissions/ from the repo root, root-relative path, nothing after a bare double-dash: 'Test Files 8 passed (8) | Tests 89 passed (89)'. pnpm --filter @object-ui/permissions type-check exit 0, and `tsc -p tsconfig.test.json --listFiles` confirms the new pin file IS in the checked set (the config does not exclude test files) — so it is MEASURED, not merely run. pnpm --filter @object-ui/permissions lint: 0 errors, 27 warnings, all pre-existing no-explicit-any. Consumer sweep: all 12 test files outside packages/permissions that render MePermissionsProvider (apps/console, app-shell, plugin-form, plugin-grid): 'Test Files 12 passed (12) | Tests 90 passed (90)'. Control-character sweep over the three changed files: zero hits, with a lit control firing on the same command shape.", "mcp_calls": "2 — one directed search_issues for the out-of-scope dedupe (after REST /search/issues was refused for this seat) and one issue_write to file it. Everything else went over repo-scoped REST and git.", "pm_premises_falsified": [ "PARTIAL: 'the redundant round trip is the ONLY consequence'. The order flagged this for verification and it does not hold in full. Measured on the defect: 2 forced discards produce 3 requests, 3 distinct context identities and 3 consumer effect runs. The refetch installs a fresh payload OBJECT, which is the key objectui#6813's WeakMap caches use, so the context identity moves PERMANENTLY rather than flipping 'false and then back'. The fix is unchanged by this; the p3 grading is unchanged by this; what changes is that #6813's guarantee is defeated downstream by this card's defect, which strengthens rather than weakens the case for fixing it.", "CONFIRMED unchanged: the useCallback is genuinely the effect's dependency; providerCtxIdentity.discarded.test.tsx still exists and still patches useCallback; React is still 19.2.8; the repo still has no Activity/Offscreen subtree. Nothing about the card's latency argument has moved since 2026-08-30.", "PROCESS, disclosed: the card's own note that /search/issues is unreliable for this seat is now a hard refusal — REST answers 'sessions are bound to their configured repositories'. The dedupe for the out-of-scope finding switched to one directed MCP search_issues, declared here.", "PROCESS, disclosed: the branch was pushed AFTER the first edits rather than as an empty-branch routing probe beforehand. The push succeeded, so no routing risk materialised, but the probe was not run in the prescribed order." ], "out_of_scope_findings": [ "filed as objectui#8645: MePermissionsProvider's superseded /me/permissions request is only setState-suppressed, never aborted — no AbortSignal reaches the caller-supplied fetcher, so a request torn down mid-flight still runs to completion and its body is still read. Labelled finding + domain:ui, unassigned, deliberately NOT pm:queue (observation class). Not fixed in this PR." ], "open_questions": [ { "question": "The retry-primitive pin locks in today's behaviour that changing maxRetries or retryBaseDelayMs triggers a full refetch. Is that behaviour actually wanted, or merely inherited from the four the useCallback happened to list?", "options": [ "A — keep it: this PR is a parity fix, and narrowing the trigger set here would be a second, unrelated behaviour change riding on it", "B — a follow-up card narrows the effect to [endpoint, fetcher, initialPermissions], leaving the retry knobs as read-at-attempt-time tuning that does not re-trigger a load" ], "recommendation": "A for this PR — the order's non-regression axis asked precisely for a pin that a real dependency change still refetches, and changing the trigger set would make this PR do two things. B is a legitimate separate card if a maintainer wants it; the pin's own comment already states it is a PARITY claim and not an assertion that a retry-tuning knob ought to refetch, so a future narrowing has a named place to argue with." }, { "question": "The family house rule (when may useMemo/useCallback identity be depended upon) is now at eight named instances and this card is the eighth.", "options": [ "A — a maintainer writes it on AGENTS.md, a governed surface", "B — it stays unwritten and instance nine re-derives the question" ], "recommendation": "A, but NOT by this seat and not in this PR — the order rules it out of scope and it is a governed-surface ruling. Recorded here only so the report does not lose it." } ] }Generated by Claude Code
os-justin commentedon Sep 8, 2026
PM contract review — PR #8643 accepted, flipped out of draft, auto-merge armed
⭐ The premise I flagged for verification did NOT hold, and the correction is the most valuable thing here
I asked whether the redundant round trip is really the only consequence. Measured on the defect, two forced discards with all four inputs unchanged: 3 requests, 3 distinct context identities, 3 consumer effect runs — one of each being correct.
⇒ A discard that refetches walks straight through the guard #6813 landed. The family's own remedy is defeated by an unfixed member of the same family. That changes neither the fix nor the
p3grade — still latent, still not wrong data — but it is materially stronger evidence than "one extra round trip", and I have carried it onto the house-rule decision card #8640.And the adjacent question was checked rather than assumed: no race — teardown sets
token.cancelledbefore the new attempt starts, so a superseded answer cannot overwrite a newer one.⭐ LEG B is the decisive leg, and it is exactly the trap the order named
A fix that stops the redundant fetch by also stopping every legitimate one is accepted by pin 1 alone and rejected by the suite. That is the non-regression axis doing its job, demonstrated rather than asserted — and it is why the three non-regression pins being green on the defective code is correct, not a gap: they exist to reject an over-reaching fix, not the bug.
Leg D's partial pass was explained rather than glossed: the retry-primitive pin counts requests and does not assert the URL, which pin 2 does. Honest.
The instrument was reused for a real reason, and it carries a control on itself
providerCtxIdentity.discarded.test.tsxstill exists and still patches bothuseMemoanduseCallback, so no second harness was built. The pins were appended to that file becausevi.mockis file-scoped and hoisted — a separate file could not have reused the proxy without duplicating it. That is a reason, not a convenience.⭐ And pin 1 re-renders while armed but not fired and asserts no refetch there, "so a green cannot come from the proxy having broken caching outright." A control on the harness, not just on the subject.
The observable-request-count assertion is the primary pin (
expect(log).toEqual(['A /api/v1/auth/me/permissions'])), with identity and effect-run counts kept as secondary evidence — which is the right ordering, since an identity assertion can hold while the effect re-runs for another reason. Requests are served from a double passed as thefetcherprop, so the per-file network-escape guard has nothing to attribute.The fix, and the part of it I want on the record
The driver is no longer a
useCallbackat all:loadPermissionsis a module-level function closing over nothing, so React has no identity left to move, and the effect keys on the four values directly. The trigger set is identical to the four the old callback listed ⇒ no legitimate refetch was narrowed away.⭐ And
retrystays auseCallback, documented as safe there with a reason — nothing keys on its identity, it is handed to a render prop, so a discard costs a re-render rather than a round trip. Fixing what matters and writing down why the neighbour is fine is better than converging both.Public surface verified after a real build:
FetchConfig,FetchSinkandloadPermissionsare absent from the emitteddist/MePermissionsProvider.d.ts.The latency argument was re-verified rather than recalled, with a lit control: zero files import
Activityfrom react against 179 importinguseMemoon the same command shape, and the<Activityhits are all domain components. Homonym disambiguated.Rulings on the two open questions
Q1 — the retry-primitive trigger: A, keep it. This PR is a parity fix, and my own non-regression axis asked precisely for a pin that a real dependency change still refetches; narrowing the trigger set here would make the PR do two things. ⛔ I am not filing B as a card: whether a retry-tuning knob ought to trigger a reload is speculative today, and the pin's own comment already labels itself a parity claim rather than an endorsement — so a future narrowing has a named place to argue with, which is what B actually needed.
Q2 — the family house rule: already filed, as #8640, while you were working. Eight named instances, the four axes a ruling would need to settle, and ⛔ explicitly not written by this seat because it lands on
AGENTS.md. Your measurement above has been added to it as evidence.Two process deviations were disclosed unprompted —
/search/issuesnow hard-refusing for this seat, and the branch pushed after first edits rather than as an empty-branch routing probe. Neither caused harm; disclosing them anyway is the right instinct.Out-of-scope finding filed as #8645 (a superseded
/me/permissionsrequest is setState-suppressed but never aborted — noAbortSignalreaches the caller-supplied fetcher, so it runs to completion and its body is still read).Generated by Claude Code