luci-base: mark background poll XHRs with X-Ubus-No-Touch header

[micpf]

Mark LuCI's background XHR polls with an X-Ubus-No-Touch: 1 HTTP request header so that the server side can distinguish them from real user activity and skip refreshing the rpcd session idle timer for them. This is what finally makes option sessiontime in /etc/config/rpcd behave as documented for LuCI.

How it works

Capture-phase document listeners for common user-interaction event types (click, submit, change, input, keydown, mousedown, touchstart) record Date.now() in Request.lastUserInteraction on every gesture.

Request.request() computes opt.background synchronously — before the Promise.resolve(target) deferral — by comparing that timestamp against BACKGROUND_THRESHOLD (5 s). A request is foreground when it starts within the threshold of the last gesture. This survives async continuations: RPCs issued from .then() chains after an intervening network round-trip or modal confirmation (e.g. ui.changes.apply, uci.save) are treated as foreground as long as the user's last gesture is recent.

For same-origin requests, X-Ubus-No-Touch: 1 is set when opt.background is true. The header is omitted for cross-origin XHRs to avoid triggering CORS preflight.

For the batched ubus path, flushRequestQueue() sets the header on the merged XHR only when every queued entry carries opt.background, so a foreground RPC sharing a batch with a poll still touches the session.

Companion patches

• rpcd: session: make rpc_session_get() a pure lookup and add "notouch" to session/access rpcd#39
• uhttpd: ubus: honour "X-Ubus-No-Touch: 1" HTTP header on /ubus/ uhttpd#39

Verified

On an OpenWrt target with option sessiontime '30' in /etc/config/rpcd: a LuCI tab left idle reaches expires:0 within the timeout, and the next poll brings up the "Session expired" modal. Actively clicking the UI extends the session as expected.

[openwrt-ai]

Commit checks

• 93908ce "luci-base: mark background poll XHRs with _luci_bg=1" — the message (and the PR body) states that "Poll.tick() increments a Poll.tickDepth counter around each invocation of a registered callback". The diff does not touch Poll.tick(); it changes Poll.step(). Poll.tick is a numeric tick counter, not a function — see Poll.tick = (Poll.tick + 1) % Math.pow(2, 32) in the very same hunk, plus this.tick = 0 in start() and delete this.tick in stop(). Please retitle the reference to Poll.step() so the message matches the code.

---

Generated by Claude Code

[openwrt-ai]

Poll.tickDepth is ambient global state, not per-request context, so this tags any request that happens to be issued while some poll callback's promise chain is still outstanding — including genuinely user-initiated ones. Since the counter is only decremented when the whole chain settles, the window is at least one full RPC round-trip every poll interval, and it is unbounded if a poll callback returns a promise that never settles (hung XHR, callback awaiting a user action). A user clicking "Save & Apply" inside that window gets _luci_bg=1 on their request, the idle timer is not refreshed, and they can be logged out mid-edit — the opposite of what sessiontime should do.

RPC batching makes this concrete rather than theoretical. flushRequestQueue() at luci.js:562-591 coalesces all queued ubus calls — poll-originated and user-originated alike — into a single Request.request(rpcBaseURL, reqopt) at luci.js:575. That one XHR is marked or not purely by the value of Poll.tickDepth at requestAnimationFrame time, so a user's ubus call riding along in a poll batch is unconditionally reported as background.

Would an explicit opt-in be safer than inferring from global state — e.g. a background: true request option that Poll.step() threads into the calls it makes, with flushRequestQueue() only setting the flag when every batched entry carries it? That fails closed (session stays alive) instead of open.

---

Generated by Claude Code

[openwrt-ai]

The increment is unconditional but the decrement only happens if .finally() gets attached. If e.fn() throws synchronously, the exception propagates out of Promise.resolve(...) before .finally() is reached, so Poll.tickDepth is leaked permanently. From then on Poll.tickDepth > 0 is always true and every XHR for the remaining lifetime of the page — including every user click — is tagged _luci_bg=1, silently disabling idle-timer refresh entirely.

A synchronously throwing poll callback is arbitrary view code, so this is reachable. Today such a throw only wedges that one queue entry (e.r stays false); this change escalates it to a page-global failure. Guarding it also restores e.r on that path:

Suggested change

	Poll.tickDepth = (Poll.tickDepth || 0) + 1;
	Promise.resolve(e.fn()).finally(() => { e.r = true; Poll.tickDepth--; });
	Poll.tickDepth = (Poll.tickDepth ?? 0) + 1;
	try {
	Promise.resolve(e.fn()).finally(() => { e.r = true; Poll.tickDepth-- });
	}
	catch (err) {
	e.r = true;
	Poll.tickDepth--;
	throw err;
	}

---

Generated by Claude Code

[openwrt-ai]

nit: naming collision. Poll.tick (this line) is the existing wrap-around tick counter, and Poll.tickDepth is now an unrelated in-flight-callback depth — the shared prefix reads as if one derives from the other. Something like Poll.pending or Poll.inCallback would avoid the confusion.

Also, unlike tick (initialised in start()), tickDepth is never declared on the singleton — hence the || 0 fallback on line 1209. Declaring it as a /* private */ member alongside the other Poll state would let both sites just use Poll.tickDepth directly and would document it for anyone reading the class.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the force-pushed revision (93908ce → fd34f33). The only functional delta is the switch from a _luci_bg=1 query parameter to an X-Ubus-No-Touch: 1 request header; the Poll.step() side is unchanged.

The two still-open threads from the previous revision remain applicable and are not repeated here: the unguarded Poll.tickDepth increment leaking permanently when a poll callback throws synchronously, and the Poll.tick / Poll.tickDepth naming collision plus the undeclared singleton member.

CI on fd34f33 is green.

---

Generated by Claude Code

[openwrt-ai]

Switching from a query parameter to a header made the RPC-batch case worse, because a header survives the batch merge where the URL did not.

The assignment happens above the queueing check at if (isQueueableRequest(opt)) — luci.js:742, so a poll-originated entry is pushed onto requestQueue with opt.headers['X-Ubus-No-Touch'] = '1' already set. flushRequestQueue() then builds the batch options from the first queued entry — Object.assign({}, requestQueue[0][0], { content: [], nobatch: true }) — and state in the outer request() only overrides xhr/url/start, never headers. So entry 0's header is carried onto the single batched XHR at luci.js:575 and applied verbatim at luci.js:785.

Consequence: any user-initiated ubus call that lands in the same animation-frame batch behind a poll-initiated one is reported as background, and the idle timer is not refreshed for it. Unlike the previous _luci_bg=1 revision, this now happens even when Poll.tickDepth is back to 0 by the time requestAnimationFrame fires — e.g. a fire-and-forget poll callback that issues an RPC without returning the promise settles in a microtask, long before the flush.

Would it be safer to decide this per batch rather than per first-entry — record the intent on opt (e.g. opt.background = true) here, and have flushRequestQueue() set the header on reqopt only when every queued entry carries it? That fails closed (session stays alive) instead of open. This is a concrete instance of the ownership concern in the now-outdated thread on the previous revision, which I don't think the header switch addresses.

---

Generated by Claude Code

[openwrt-ai]

nit: the PR body still describes this as "Poll.tickDepth counter is bumped around each Poll callback (e.fn() on the Poll.tick list)". There is no Poll.tick list — the callback list is Poll.queue (iterated on line 1200 above), and Poll.tick is the wrap-around tick counter four lines below. The commit message has the same residue ("outside any Poll.tick"). Worth correcting both to Poll.queue / Poll.step() so the prose matches the identifiers.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the force-pushed revision (fd34f33 → 4ce3079). The synchronous-throw counter leak, the tickDepth naming collision and the undeclared singleton member from the previous revision are all addressed, and the header is no longer smuggled through opt.headers into the batch merge. CI on 4ce3079 is green.

Commit checks

• 4ce3079 "luci-base: mark background poll XHRs with X-Ubus-No-Touch header" — two statements in the message do not match the diff:
  • "Track when a Poll.step() callback is synchronously on the JS stack via a Poll.inCallback counter" — the counter is decremented in the callback promise's .finally(), so it tracks pending callbacks and stays set for the whole RPC round-trip. (That is necessary for the mechanism to work; see the inline note on line 1226.) The follow-on claim that "requests from real user actions […] are outside any Poll.step() callback" therefore only holds for actions that do not overlap a pending poll callback.
  • "the header is only set on the merged XHR when every queued entry carries the flag" — Request.request() re-derives opt.background from Poll.inCallback after flushRequestQueue() has set it, so the merged XHR is tagged even when a queued entry is user-initiated. See the inline comment on line 707.

---

Generated by Claude Code

[openwrt-ai]

This unconditionally overrides the flag flushRequestQueue() just computed, so the new all-entries guard never takes effect in the mixed-batch case it was added for.

flushRequestQueue() sets reqopt.background = allBackground (luci.js:576) and then calls Request.request(rpcBaseURL, reqopt). opt here is Object.assign({}, reqopt, state), so opt.background already carries false when the batch mixes a user-initiated call with a poll-initiated one — and this line flips it back to true whenever any poll callback is still pending. That is precisely the situation in such a batch: the poll callback's promise settles only once the batched reply arrives, so Poll.inCallback > 0 still holds when requestAnimationFrame(flushRequestQueue) (luci.js:749) fires. The merged XHR is then tagged X-Ubus-No-Touch and the user's ubus call does not refresh the idle timer — the outcome the commit message says is prevented.

Only infer the flag when the caller has not set it explicitly:

Suggested change

	if (Poll.inCallback > 0)
	opt.background = true;
	if (opt.background == null && Poll.inCallback > 0)
	opt.background = true;

This also gives user-intent call sites a way to opt out with an explicit background: false.

---

Generated by Claude Code

[openwrt-ai]

Poll.inCallback is decremented only when the callback's promise settles, so it stays > 0 for the whole RPC round-trip rather than for the synchronous invocation. Since the check in Request.request() (luci.js:707) reads ambient state with no per-request context, every request created inside that window is tagged — including genuinely user-initiated ones.

Worth recording explicitly: the deferred decrement is load-bearing and cannot simply be moved into a synchronous finally block. Request.request() runs its check inside the Promise.resolve(target).then(url => …) microtask (luci.js:697), i.e. after Poll.step() has already returned, so a synchronous decrement would leave every poll request untagged and make the feature a no-op.

Given that, the residual risk is one-directional: a user action landing inside the pending window is reported as background and does not refresh the idle timer, which is the mid-edit-logout case. With the opt.background == null guard suggested on line 707, would it be worth having the few user-intent entry points (form save, ui.changes.apply()) pass background: false explicitly, so those paths fail closed?

---

Generated by Claude Code

[openwrt-ai]

nit: the counter is described as callbacks "currently synchronously on the JS stack", but the decrement happens in the promise's .finally() on line 1226, so it actually tracks poll callbacks whose promise has not settled yet. The distinction is not cosmetic — the deferred decrement is required for the mechanism to work at all, so the comment as written invites a future reader to "fix" it into a synchronous decrement and silently disable the header.

Suggested change

	/* private: number of Poll.step() callbacks currently synchronously
	* on the JS stack. Consulted by Request.request() to mark outgoing
	* XHRs as background so the rpcd session idle timer is not
	* refreshed by autonomous polling.
	*/
	/* private: number of Poll.step() callbacks whose returned promise
	* has not settled yet. This deliberately outlives the synchronous
	* callback invocation, because Request.request() performs its
	* check from a `.then()` microtask, i.e. after Poll.step() has
	* already returned. Consulted by Request.request() to mark
	* outgoing XHRs as background so the rpcd session idle timer is
	* not refreshed by autonomous polling.
	*/

---

Generated by Claude Code

[micpf]

Revised approach in the latest force-push.

The previous revision used a Poll.inCallback counter incremented around each Poll.step() callback and decremented in the callback's .finally(). This worked, but relied on the async gap between e.fn() returning and its promise settling to keep inCallback > 0 while the RPC was in-flight — a subtle invariant that was easy to misread as a synchronous call-stack check.

The new approach inverts the default: Request.background is true by default, and user-initiated events are what opt out. Capture-phase document listeners for click, submit, change, input, keydown, mousedown and touchstart clear Request.background for the synchronous duration of the event handler; a microtask resets it. Because JS is single-threaded, no user event can fire while another handler is on the stack, so the flag is always in the right state when Request.request() captures it into opt.background. Poll.step() requires no changes at all.

This removes the counter, removes the try/catch guard, and makes the mechanism explicit: background is the default, user gestures are what touch the session.

[openwrt-ai]

Re-reviewed the force-pushed revision (4ce3079 → 854a69b). The Poll.inCallback counter is gone and the batch merge now decides per batch rather than per first entry, which resolves the previous revision's threads. The new event-listener approach, however, does not appear to work as described — details inline. CI on 854a69b reports no failures.

Commit checks

• 854a69b "luci-base: mark background poll XHRs with X-Ubus-No-Touch header" — the message states that "Request.request() defaults opt.background from that flag when the caller does not set it explicitly, so user-initiated RPCs leave background false (session is touched)". The diff does not achieve this: Request.request() reads Request.background from inside the Promise.resolve(target).then(…) microtask, which is queued after the capture listener's reset microtask, so the value read is always true. See the inline comment on line 717 for the ordering demonstration, and the one on line 521 for why the synchronous window would also be too narrow even with the ordering fixed.

---

Generated by Claude Code

[openwrt-ai]

Request.background is sampled one microtask too late, so opt.background ends up true even for an RPC issued directly from a user event handler.

request() defers its entire body into a microtask — return Promise.resolve(target).then(url => …) at luci.js:705-706. The capture-phase listener queues its reset (line 523) before the page's own handler calls request(), so the reset job sits ahead of this one in the microtask queue and runs first:

let bg = true;
bg = false; Promise.resolve().then(() => { bg = true });    // capture listener
Promise.resolve('u').then(() => console.log('sees', bg));   // Request.request()
// → sees true

The PR body's premise — "RPC calls queued by a user event handler are made synchronously on its call stack, [so] the flag is captured correctly before the handler returns" — does not hold, because the capture is not on that call stack.

Consequence: opt.background is true for every request, X-Ubus-No-Touch is set unconditionally, and the idle timer is never refreshed. An actively used LuCI tab is then logged out after sessiontime — the opposite of the intended behaviour, and worse than today.

The flag has to be read synchronously in request()'s own frame, before the promise:

request(target, options) {
    const background = (options && 'background' in options)
        ? options.background : Request.background;

    return Promise.resolve(target).then(url => {
        …
        opt.background = background;

---

Generated by Claude Code

[openwrt-ai]

Separately from the sampling order (see the note on line 717): the window this opens is too narrow to cover LuCI's actual user-initiated request sites, so fixing the sampling alone would not be enough.

The reset runs before the page's own handler. For browser-generated events a microtask checkpoint is performed after each listener invocation — HTML's "clean up after running script" performs one whenever the JS execution context stack becomes empty, and each listener callback is a separate script invocation. So the sequence for a real click is: document capture listener sets false and queues the reset → listener returns, stack empties → checkpoint runs the reset → Request.background is true again → only now does the handler on the target element run. The flag only stays false across listeners when the event is dispatched programmatically from another script frame (el.click(), dispatchEvent()), i.e. exactly the case that is not a user gesture.

User-initiated requests are async continuations, not synchronous handler code. ui.changes.apply() issues its XHR from inside a .then() at ui.js:5508, after an intervening checkConnectivityAffected() round trip and possibly a modal confirmation. uci.save() at uci.js:902 is likewise reached from the form's promise chain, and its follow-up load()/reorderSections() calls run from .then() blocks several ticks after the click. None of these are on the handler's synchronous stack, so all of them would still be tagged background.

Would a "last user interaction" timestamp be more robust than a stack-scoped flag — record Date.now() in these listeners and treat a request as foreground when it starts within a short window (a few seconds) of the last gesture? That survives async continuations and degrades gracefully, whereas the stack-scoped flag is all-or-nothing and currently resolves to "nothing".

---

Generated by Claude Code

[openwrt-ai]

nit: background is now a caller-settable request option — the if (!('background' in opt)) guard on line 716 deliberately lets an explicit value win, and flushRequestQueue() relies on that — but it is missing from the LuCI.request.RequestOptions typedef at luci.js:642-687, which documents every other option down to [header] and [responseProgress]. Worth adding an @property {boolean} [background=true] entry there, so call sites that want to force a request to touch the session know the knob exists.

---

Generated by Claude Code

[micpf]

Thank you for the detailed analysis — you are right on both counts.

Sampling bug: Request.background was read inside Promise.resolve(target).then(...), i.e. one microtask after the reset was already queued. Fixed in aee886a by capturing the value synchronously at the top of request(), before the return Promise.resolve(...) deferral.

Window too narrow: The microtask-checkpoint-after-each-listener behaviour means the flag would already be reset before the target element's handler runs anyway, and real user-initiated RPCs (e.g. ui.changes.apply, uci.save) are issued from async .then() continuations, not synchronous handler code.

Following your suggestion, the new revision replaces the boolean flag with a timestamp (Request.lastUserInteraction). The capture-phase listeners now simply record Date.now() on each gesture. request() computes background synchronously by comparing that timestamp against BACKGROUND_THRESHOLD (5 s). This correctly handles async continuations — an RPC fired from a .then() chain seconds after a click is still treated as foreground as long as the gesture is recent. Poll XHRs, which are driven by requestAnimationFrame on a fixed 5 s cadence with no recent user gesture, will always exceed the threshold.

Nit about @property {boolean} [background] in RequestOptions also addressed in aee886a.

[openwrt-ai]

Re-reviewed the force-pushed revision (854a69b → 014613e).

The timestamp approach resolves the previous revision's threads: background is now derived synchronously at the top of request(), ahead of the Promise.resolve(target) deferral, so the microtask-ordering problem is gone and the value survives async continuations. flushRequestQueue() writes an explicit boolean into reqopt, and since options?.background ?? … only falls through on null/undefined, the per-batch decision is no longer re-derived and overwritten. The commit message matches the diff. CI on 014613e is green.

Two new points inline: the header is also attached to cross-origin XHRs, and the new document listeners are registered non-passive.

---

Generated by Claude Code

[openwrt-ai]

Request.request() is LuCI's general-purpose HTTP client, not only the ubus transport, and several callers point it at cross-origin URLs. X-Ubus-No-Touch is not a CORS-safelisted request header, so tagging those XHRs turns them into preflighted requests that additionally require the remote server to echo x-ubus-no-touch in Access-Control-Allow-Headers; if it doesn't, the request fails outright.

Concrete site: attendedsysupgrade registers handleRequest as a 5 s poll (rebuilders, [main build](https://github.com/openwrt/luci/blob/014613e8da599eb192bbbd22f8874d2c4f41799e/applications/luci-app-attendedsysupgrade/htdocs/luci-static/resources/view/attendedsysupgrade/overview.js#L685)),`` and that callback issues the XHR against ${server}/api/v1/build on the ASU / rebuilder hosts. Being a poll, it is by definition more than `BACKGROUND_THRESHOLD` past the last gesture, so it always gets the header. [`repokeys.js:115`](https://github.com/openwrt/luci/blob/014613e8da599eb192bbbd22f8874d2c4f41799e/modules/luci-mod-system/htdocs/luci-static/resources/view/system/repokeys.js#L115)`` likewise fetches an arbitrary user-supplied https:// URL through the same path.

The header is only meaningful to the local uhttpd/rpcd, so restrict it to same-origin targets (opt.url has already been through expandURL() at this point):

Suggested change

	if (opt.background)
	opt.xhr.setRequestHeader('X-Ubus-No-Touch', '1');
	if (opt.background && new URL(opt.url, location.href).origin == location.origin)
	opt.xhr.setRequestHeader('X-Ubus-No-Touch', '1');

---

Generated by Claude Code

[openwrt-ai]

touchstart is a scroll-blocking event type: a non-passive listener on document forces the compositor to wait for this handler before it can start a scroll or fling on every touch, and Chrome emits an intervention warning for exactly this registration pattern. The handler only stores a timestamp and never calls preventDefault(), so it should be declared passive. The boolean third argument has to become an options object for that:

Suggested change

	document.addEventListener(type, () => { Request.lastUserInteraction = Date.now() }, true);
	document.addEventListener(type, () => { Request.lastUserInteraction = Date.now() }, { capture: true, passive: true });

---

Generated by Claude Code

[openwrt-ai]

nit: the PR description still documents the superseded mechanism. It describes a Request.background boolean that capture-phase listeners clear "for the synchronous duration of the handler" with "a microtask [that] resets it to true immediately after", and claims the flag "is guaranteed to be false only while a user event handler is executing". None of that exists any more — this revision records a timestamp and compares it against a 5 s threshold, which is precisely what makes the async-continuation case (ui.changes.apply, uci.save) work. The commit message is accurate; the PR body is what carries over into the merge discussion, so it is worth syncing before merge.

---

Generated by Claude Code

[openwrt-ai]

Re-reviewed the force-pushed revision (014613e → 73a7cd4).

Both points from the previous revision are addressed: the header is now gated on new URL(opt.url, location.href).origin == location.origin, so cross-origin callers (attendedsysupgrade polling the ASU/rebuilder hosts, repokeys.js fetching a user-supplied URL) no longer get a preflight-triggering header, and the document listeners are registered { capture: true, passive: true }, which clears the scroll-blocking touchstart intervention. The PR body has also been synced to the timestamp mechanism, resolving the last open nit.

The rest of the mechanism re-checks out on this revision: background is still derived synchronously ahead of the Promise.resolve(target) deferral, opt.background is assigned before the isQueueableRequest(opt) branch so every queued entry carries a boolean, and flushRequestQueue()'s batch.every(...) therefore decides per batch. options?.background ?? … only falls through on null/undefined, so the per-batch value is not re-derived. CI on 73a7cd4 is green.

Two remaining points inline, both nits — the commit message does not mention the new same-origin restriction, and the gesture list treats scrolling as idle.

---

Generated by Claude Code

[openwrt-ai]

nit: the same-origin restriction is new in this revision and is not mentioned in the commit message. The message says only "Fix this by setting an X-Ubus-No-Touch: 1 request header on XHRs that are not driven by a user gesture", which reads as unconditional; the PR body does document the CORS-preflight rationale, but the commit message is what survives into the tree. Worth one line, e.g. "The header is restricted to same-origin targets so cross-origin XHRs (attendedsysupgrade, repository key fetches) are not turned into preflighted requests."

---

Generated by Claude Code

[openwrt-ai]

The gesture list covers input-type activity but no navigation-type activity — scroll and wheel are absent. Since BACKGROUND_THRESHOLD is 5 s, a user who is watching a page rather than editing it stops counting as active almost immediately.

That is not hypothetical for LuCI: the realtime views exist to be watched. load.js:91 registers a poll.add() callback, and the same holds for bandwidth.js, connections.js and the log views — someone scrolling through Status → System Log or tracking a transfer on the bandwidth graph produces no click/input/keydown, so every RPC on those pages is tagged X-Ubus-No-Touch and the session expires under them mid-read.

Both events reach a document-level capture listener (capture runs for non-bubbling events such as scroll too), so extending the list is mechanically the same as the existing entries. Is treating scroll/wheel as idle the intended semantics of sessiontime, or just an oversight in the event list?

---

Generated by Claude Code