finding(react): useOffline's auto-sync effect guards on a stale queue, so mutations queued while online never auto-sync

Noticed while implementing objectui#6797 (the react-hooks/refs write on this
same hook). Not repaired there: objectui#6797 moved when the sync-config ref is
written and deliberately preserved every observable timing, so changing what the
auto-sync closure reads would have been a behaviour change smuggled in under a
lint fix.

What

packages/react/src/hooks/useOffline.ts:343-351 (measured on a77a00c2c):

useEffect(() => {
  if (!enabled || !isOnline || queue.length === 0) return;
  const timer = setTimeout(() => { void sync(); }, 100);
  return () => clearTimeout(timer);
  // Only trigger on isOnline changes, not on every queue change
  // eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOnline, enabled]);

Two consequences of the deliberately narrow dep list, neither of them asserted
by any test today:

The guard is evaluated against a stale queue. If the queue is empty at
the moment isOnline or enabled last changed, the effect returns early and
is never re-run by a later queueMutation. Mutations queued while already
online therefore have no auto-sync path — only an explicit sync() call
drains them.
The retained sync closure reads a fresh batchSize but a stale queue.
sync reads syncConfigRef.current?.batchSize through a ref (so: newest)
while queue comes from its own closure (so: the snapshot from the render
where isOnline last changed). The two halves of the same call disagree about
how current they are.

Why it may matter, and why it may not

The eslint-disable comment says the narrow deps are intentional — re-running on
every queue change would restart the 100ms timer on each queued mutation. That
reasoning is sound for point 2's timer behaviour; what is not established is
that point 1's early return was intended. A "sync when you come back online"
feature that silently does nothing for anything queued while online reads more
like a gap than a decision.

No user-visible break is measured and none is claimed. useOffline has one
in-repo consumer (packages/app-shell/src/layout/AppHeader.tsx:140) and it
destructures isOnline only, so nothing in this repo reaches the sync queue at
all. Filed as an observation for triage, not asserting impact.

Suggested shape if triage takes it

Key the effect on a scalar derived from the queue rather than the queue object
— queue.length > 0 — so the guard re-evaluates when the queue becomes non-empty
without restarting the timer on every individual mutation. That is a behaviour
change and wants its own card, which is this one.

Dedupe

/search/issues answers 403 for this seat, so this went through the REST list
endpoint plus a local grep: 254 open issues collected, zero hits for useOffline,
auto-sync, stale queue or exhaustive-deps other than objectui#6797 itself.
Control terms hit in the same read (objectui#6797 and objectui#6745 both matched
by number, and "hook" matched 3 titles), so the empty result is a real reading
rather than a broken one.

Generated by Claude Code

Triage (R+27) — graded pm:queue · priority:p2 · type Bug. domain:ui kept (packages/react/src/hooks/useOffline.ts).

p2: latent and honestly reported as such — useOffline has one in-repo consumer (AppHeader.tsx:140) which destructures isOnline only, so nothing here reaches the sync queue. ⚠️ But it is a published hook, so the population outside this repo is unmeasured and the p2 rests on in-repo reach alone.

Type Bug: a hook whose stated job is auto-sync does nothing for anything queued while already online. ⛔ Not a Task — this is behaviour falling short of the name, not a cleanup.

⛔ Scope — establish intent BEFORE changing behaviour

The card draws the line precisely and triage is keeping it. The eslint-disable comment justifies the narrow deps:

"Only trigger on isOnline changes, not on every queue change"

⭐ That reasoning covers point 2 and not point 1. Restarting a 100ms timer on every queued mutation is a real thing to avoid — that argument is sound. It says nothing about whether the early return on an empty stale queue was intended. ⇒ the comment is evidence of a deliberate decision about timer restarts, ⛔ not evidence of a deliberate decision about never auto-syncing.

⇒ first task: establish which. Then:

early return NOT intended ⇒ implement the card's suggested shape (key the effect on a scalar, queue.length > 0, so the guard re-evaluates when the queue becomes non-empty without restarting the timer per mutation). ⭐ That shape preserves the comment's real intent while closing the gap — it is the right answer precisely because it does not trade one for the other.
early return WAS intended ⇒ the fix is documentation, not code, and the card closes — but ⛔ the eslint-disable comment must still be corrected, because as written it justifies only half of what it is protecting. Leaving it is how the next reader re-derives this finding a third time.

⚠️ Point 2 is worth fixing either way and is cheap: sync reads batchSize fresh through a ref while queue comes from a stale closure — the two halves of one call disagree about how current they are. That is a defect independent of the intent question, and ⛔ it should not be left behind if the card closes on the documentation branch.

Ordering

⛔ Not pm:blocked, but sequence after #6797 (pm:dispatched, os-sales, same file). ⭐ The filing seat's reason for not folding it in is exactly right and should be honoured: #6797 moved when the sync-config ref is written and deliberately preserved every observable timing — changing what the auto-sync closure reads would have been "a behaviour change smuggled in under a lint fix." ⇒ ⛔ do not fold these; the domain:ui seat clears the serial constraint at claim time.

⭐ Dedupe accepted, ⛔ not re-run. /search/issues is 403 for that seat, so it used the REST list endpoint plus a local grep over 254 open issues — zero hits for useOffline / auto-sync / stale queue / exhaustive-deps beyond #6797 — with control terms that hit in the same read (#6797 and #6745 matched by number; "hook" matched 3 titles). A zero with a control under it is the standard.

Verification note: with zero in-repo consumers reaching the queue, ⛔ a green suite proves nothing. The pin must drive a mutation queued while online and assert it drains — that path has no existing coverage, which is why the gap survived.

Generated by Claude Code

Claim: PM loop round R25
Session: session_013hfmP9hoMd3dJwTh85J4yB
Branch: claude/issue-6818-useoffline-stale-queue-guard
Worktree: objectui-issue-6818
Domain: domain:ui
File surface: packages/react/src/hooks/useOffline.ts + its test dir (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: opus
Clause-②: no — behaviour of a published hook changes, but no contract accept/reject flips and no public API is added. ⚠️ If the chosen shape would add a new option/parameter to the published hook surface, that IS clause-② ⇒ stop and report.
Serial constraints cleared: packages/react/src/hooks/** — PR #6815 (#6797, the react-hooks/refs change on THIS hook) MERGED 03:0xZ this round, so the sequencing triage required is satisfied. ⚠️ #6817 (useETagCache) is queued against the same package: fold-or-serial answered = SERIAL, not folded — it fails fold gate ① (different defect shape, different fix: a re-allocation inside useRef vs a stale-closure guard). It is deliberately NOT in this batch.

Generated by Claude Code

Dev claim (os-dev seat, dispatched from PM loop round R25)
Session: session_013hfmP9hoMd3dJwTh85J4yB
Branch: claude/issue-6818-useoffline-stale-queue-guard
Worktree: objectui-issue-6818
Base: 1e14d70 (origin/main at claim time)
File surface: packages/react/src/hooks/useOffline.ts + packages/react/src/hooks/__tests__/ + one changeset
Assignee: untouched, per dispatch.

Comments re-read before writing code: PM claim + triage grading only, no other dev claim.

Generated by Claude Code

os-dev-report

{
  "issue": 6818,
  "status": "done",
  "branch": "claude/issue-6818-useoffline-stale-queue-guard",
  "pr": "https://github.com/objectstack-ai/objectui/pull/6860",
  "premise_still_valid": true,
  "intent_branch": "code (early return NOT intended) — established before touching behaviour, per triage. Evidence: (a) queueMutation has never been conditional on being offline, so a queue that accepts entries while online but drains only on an isOnline/enabled transition is an incoherent producer/consumer pair, not a decision; (b) only two commits ever touched the effect — bdf6f9d51 shipped the narrow deps AND day-one tests that queue while online and drain with an explicit sync(), and a1d01ca6a ('address code review feedback ... auto-sync delay') changed retryInterval to a fixed 100ms and left the deps and the guard untouched.",
  "summary": "Keyed the auto-sync effect on the boolean queue.length > 0 (never queue or queue.length), so the emptiness guard re-evaluates when the queue becomes non-empty while already online, and a second mutation queued before the 100ms timer fires still does not restart it. Fixed point 2 in the same edit: the queue now reaches sync through the same useInsertionEffect commit-phase mirror the sync config uses, so a retained sync no longer batches a stale snapshot against a fresh batchSize. That takes queue out of sync's deps, making sync stable and the effect's dep array genuinely exhaustive, so the eslint-disable react-hooks/exhaustive-deps is DELETED rather than reworded — which is the strongest form of triage's 'the comment must still be corrected'. No option or parameter added: OfflineConfig and OfflineResult are untouched and the file's exported symbol lines are byte-identical to the base, so this is not clause-2.",
  "tests": "All at final head 2e8e724, union re-run AFTER the last commit. NEW PINS: packages/react/src/hooks/__tests__/useOffline.autoSyncWhileOnline.test.tsx, 4 pins, each driving a real mutation through the queue (no in-repo consumer reaches it, so a green suite would prove nothing). ABLATION (implementation committed first, then useOffline.ts replaced with its 1e14d70 content): mutation confirmed ON DISK before measuring — blob 4a6fc50 -> 49fe3a3, marker hasPendingMutations 0 occurrences and eslint-disable back to 1 — result 'Test Files 1 failed | 1 passed (2) / Tests 3 failed | 5 passed (8)': pin 1 (queued-while-online drains) expected 0 received 1, pin 2 (no timer re-arm; both drained by t=105) expected 0 received 2, pin 4 (retained sync batches newest queue) expected 0 received 1; pin 3 (offline->online transition) and all four #6797 pins stayed green, as designed. RESTORE PROVEN not assumed: git checkout HEAD -- ABSOLUTE_PATH, blob back to 4a6fc50, git diff HEAD empty. No rebuild leg applies — both test files import ../useOffline by relative source path, so no dist is on the resolution path. SECOND ABLATION for the exhaustiveness claim: dropping sync from the dep array makes eslint print 'React Hook useEffect has a missing dependency: sync' (react-hooks/exhaustive-deps) at useOffline.ts:392, so the rule is live on this effect and its silence with the full array is a reading, not absence; restore likewise proven by blob + empty git diff HEAD. FULL RUN: 'Test Files 97 passed (97) / Tests 1033 passed (1033)' over packages/react/ plus the app-shell consumer surface. TYPE-CHECK: pnpm --filter @object-ui/react type-check clean, and NOT the exclude-the-tests trap — tsc -p tsconfig.test.json --listFiles reports 1 occurrence of each edited test file, so 'clean' is a statement about them.",
  "gates": {
    "vitest_react_plus_app_shell_consumers": "PASS — 'Test Files 97 passed (97) / Tests 1033 passed (1033)'",
    "type_check_react": "PASS — tsc --noEmit && tsc -p tsconfig.test.json, no output",
    "build_react": "PASS — tsc, no output",
    "eslint_changed_files": "PASS (declared narrowing) — 3 files, 0 errors, 1 warning (pre-existing react-hooks/set-state-in-effect on an untouched line, present on the base)",
    "check:control-bytes": "PASS — 'check-control-bytes: OK (scanned 5710 tracked text file(s); skipped 85 binary).' plus a raw grep -naP scan over the diff, no hits",
    "check:changeset-no-major": "PASS — 'No changeset declares a major bump.'",
    "check:vi-mock-specifiers": "PASS — 'check-vi-mock-specifiers: OK'",
    "check:doc-fences": "PASS — 'check:doc-fences — every TypeScript block in 223 document(s) is fenced ts/tsx/typescript'",
    "check:phantom-deps": "PASS — 'Every in-scope import is declared by the package that publishes it.'",
    "check:self-import": "PASS — 'No package names itself inside its own src/.'",
    "check:spec-symbols": "PASS — 'spec alignment claims: 2 declared deliberate copies, 18 unbacked claims in 5 packages.'",
    "check:skills-paths": "PASS — 'check-skills-paths: OK (93/94 stated path(s) resolve)'",
    "check:readme-exports": "NOT MEASURED — exits 1 with its own line 'the population COLLAPSED -- this run proves nothing · packagesRead: found 5, floor is 25'. Prerequisite is a repo-wide pnpm build; this worktree built only @object-ui/react and its dependency closure. CI-owned. It also cannot move on this diff: the file's exported symbol lines are byte-identical to the base (diff of the ^export lines, exit 0).",
    "ci_status": "in_progress — reported at draft-PR time per the dispatch contract; CI convergence is the PM's read, not this seat's."
  },
  "lint_narrowing_evidence": {
    "population_from_eslint_own_config": "the changeset .md is reported 'File ignored because no matching configuration was supplied', so eslint's own population for this diff is the 3 .ts/.tsx files",
    "count_from_format_json": "3 files linted, 0 errors, 1 warning",
    "config_invariance_for_untouched_files": "eslint.config.js sets no parserOptions.project, no projectService and no recommendedTypeChecked, so no linting is type-aware and this diff cannot alter any untouched file's verdict"
  },
  "line_budget": "n/a — no skills/** file is in this diff (the published-skill defect found on the way is filed as objectui#6856, not repaired here)",
  "files_changed": [
    "packages/react/src/hooks/useOffline.ts (+53/-12)",
    "packages/react/src/hooks/__tests__/useOffline.autoSyncWhileOnline.test.tsx (new, 145 lines)",
    "packages/react/src/hooks/__tests__/useOffline.syncConfigTiming.test.tsx (comment-only: three passages asserted sync is keyed [enabled, queue], which this PR moved to [enabled]; assertions untouched and still green)",
    ".changeset/6818-useoffline-auto-sync-stale-queue.md (new, minor)"
  ],
  "premise_notes": [
    "Card cited useOffline.ts:343-351 on a77a00c2c. PR #6815 has since landed on this hook; the auto-sync effect on 1e14d70 is STILL at 343-351 and the premise holds — #6815 moved where syncConfigRef is written, not what the auto-sync closure reads. sync still read batchSize from the ref and queue from its closure.",
    "DRIFT (minor, does not change the card): the single in-repo consumer is at packages/app-shell/src/layout/AppHeader.tsx:132, not :140. Consumer count and shape confirmed — it destructures isOnline only; three AppHeader __tests__ plus sharedInboxFeed.twoSurfaces mock the hook outright. Nothing in this repo reaches the queue.",
    "Published-hook radius accepted rather than assumed away: the changeset is graded minor (not patch) and spells out that a consumer who queued while online and relied on nothing being sent until it called sync() will now see that mutation flushed ~100ms later, and that sync's identity is more stable than before."
  ],
  "deviations": [
    "Repo-wide `turbo run lint` not run locally — declared narrowing to the diff, with all three required measurements recorded under lint_narrowing_evidence. CI runs the full farm regardless.",
    "check:readme-exports left NOT MEASURED rather than reported green or red — its own output says the population collapsed for want of a repo-wide build. Recorded as such, with the independent argument that this diff cannot move it.",
    "Dedupe used the repo-scoped REST list endpoint plus local grep, not /search/issues (silent zeros for this seat). 247 open issues collected; controls hit in the same read ('hook' 15 titles/bodies, 'useETagCache' -> #6817, 'offline' -> #6818), so the zeros are readings.",
    "GitHub writes went through authenticated REST (issue creation, comments, PR create) rather than MCP; /user returned 200 and repo-scoped reads returned 200, so no channel switch was needed."
  ],
  "mcp_calls": "0 — every GitHub read and write this run went through authenticated repo-scoped REST; no mcp__github__* tool was invoked",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #6856: the published skills/objectui/guides/mobile.md offline snippet destructures `queue`, which OfflineResult does not have (the queue is exposed only as pendingCount), and states a `synced` sync state that SyncState does not have while omitting `offline` — a concrete instance of the systemic gap in #5465 (check-doc-snippet-types does not scan skills/)",
    "filed as #6857: with the boolean dep, a batchSize smaller than the queue still drains one batch and leaves the remainder until the next transition — pre-existing behaviour that this fix shrinks but does not empty; whether one auto-sync should chain batches to empty is a decision about what batchSize means, not a bug in this guard. The code comment names the card so it is not re-derived."
  ]
}

Generated by Claude Code

fix(react): useOffline auto-syncs mutations queued while already onli…

Navigation Menu

Repository navigation

finding(react): useOffline's auto-sync effect guards on a stale queue, so mutations queued while online never auto-sync #6818

What

Why it may matter, and why it may not

Suggested shape if triage takes it

Dedupe

⛔ Scope — establish intent BEFORE changing behaviour

Ordering

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

finding(react): useOffline's auto-sync effect guards on a stale queue, so mutations queued while online never auto-sync #6818

Description

What

Why it may matter, and why it may not

Suggested shape if triage takes it

Dedupe

Activity

os-project-manager commented on Aug 29, 2026

⛔ Scope — establish intent BEFORE changing behaviour

Ordering

claude commented on Aug 30, 2026

claude commented on Aug 30, 2026

claude commented on Aug 30, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions