Skip to content

Commit 858b907

Browse files
committed
fix(node): Document and pin the bound on the task scan-ceiling probe
The scan-ceiling branch of collect_visible_tasks settles has_more with an un-gated LIMIT 1 probe, so a caller can learn whether any row - readable or not - trails the position the scan stopped at. Withholding the probe does not remove that bit: enumeration past a denied window longer than one scan budget requires handing back a continuation, and following that continuation returns the same terminal page one round trip later. State what the probe discloses (one bit, only at server-chosen positions a full scan budget apart, reachable only through a MAC'd cursor, never a denied row's id, payload or ucan_token) and pin it end to end. Also correct the comment above the branch, which claimed has_more never comes from an un-gated probe while the code below it did exactly that. Refs #327
1 parent 31329e3 commit 858b907

1 file changed

Lines changed: 141 additions & 5 deletions

File tree

‎crates/gitlawb-node/src/api/tasks.rs‎

Lines changed: 141 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -309,17 +309,45 @@ pub(crate) async fn collect_visible_tasks(
309309
}
310310
}
311311

312-
// Derive has_more from visible rows or scan-budget exhaustion, never from
313-
// an un-gated candidate probe that would leak the existence of trailing
314-
// denied rows (#327 review).
312+
// While the scan is running, `has_more` is derived from visible rows only:
313+
// finding a (bounded_limit + 1)-th visible row is what proves another page
314+
// exists, so trailing denied rows inside one scan can never set it (#327
315+
// review, `trailing_denied_tasks_do_not_set_has_more_or_leak_existence`).
316+
//
317+
// The scan ceiling is the one case that cannot be answered from visible
318+
// rows, and the `else` branch below settles it with an un-gated
319+
// `LIMIT 1` probe at the last examined position. That is deliberate, and
320+
// the disclosure it carries is bounded as follows (#327 review):
321+
//
322+
// * What it can tell a caller: whether *any* candidate row — readable or
323+
// not — trails the position this request stopped at. One bit.
324+
// * Where it can be asked: only at positions the server chose, which
325+
// advance a full `MAX_TASK_SCAN_CANDIDATES` per request, and only via a
326+
// MAC'd continuation token bound to the caller and filter
327+
// (`api::task_cursor`). A caller cannot aim the probe at a row of their
328+
// choosing, so the coarsest thing it yields is the candidate count to
329+
// `MAX_TASK_SCAN_CANDIDATES` granularity.
330+
// * What it never carries: no denied row's id, timestamp, payload or
331+
// `ucan_token` reaches the caller, and `get_task` stays opaquely 404.
332+
//
333+
// Withholding the probe does not remove that bit, it only defers it:
334+
// enumeration past a denied window longer than one scan budget is a hard
335+
// requirement (`denied_window_longer_than_scan_budget_is_pageable_to_the_end`),
336+
// so the ceiling has to hand back a continuation, and following that
337+
// continuation returns the same terminal page the probe predicted — one
338+
// round trip later. Paying an extra request per scan window to relocate
339+
// the same bit is not a gate. `scan_ceiling_continuation_discloses_only_a_terminal_page`
340+
// pins the bound end to end.
315341
let (has_more, next_position) = if visible.len() > bounded_limit {
316342
visible.truncate(bounded_limit);
317343
(true, resume_position_for_page)
318344
} else if stream_ended {
319345
(false, None)
320346
} else {
321-
// When scan budget was reached on an exact batch multiple, probe if any row
322-
// exists beyond the scan budget in the database.
347+
// Scan budget exhausted with the page unproven: ask whether the
348+
// candidate stream itself is finished, so an exhausted scan that
349+
// happens to land on the last row is reported as the end rather than
350+
// as a paused scan the caller would re-request forever.
323351
let more_in_db = !db
324352
.list_tasks_keyset(
325353
status,
@@ -2195,6 +2223,114 @@ mod visible_tasks_tests {
21952223
);
21962224
}
21972225

2226+
/// #327 review: the scan-ceiling probe in `collect_visible_tasks` is
2227+
/// un-gated, so a filled page that stops at the ceiling reports
2228+
/// `has_more = true` when *any* candidate row trails the scan position,
2229+
/// including rows the caller may not read. That bit is intentional and
2230+
/// bounded — this pins the bound end to end.
2231+
///
2232+
/// One newest public task, then more than a full scan budget of denied
2233+
/// rows and nothing visible beyond them. The anonymous caller gets its
2234+
/// visible row plus a continuation, and following that continuation
2235+
/// returns the terminal page the probe predicted. What the caller learns
2236+
/// is therefore exactly what one more request would have told it anyway:
2237+
/// no denied row's id, payload or `ucan_token` is disclosed at any point,
2238+
/// and the walk ends instead of spinning.
2239+
#[sqlx::test]
2240+
async fn scan_ceiling_continuation_discloses_only_a_terminal_page(pool: PgPool) {
2241+
let state = test_state(pool).await;
2242+
state
2243+
.db
2244+
.create_repo(&repo("public-repo", DELEGATOR, "public", true))
2245+
.await
2246+
.unwrap();
2247+
let mut visible_newest = task("newest-visible", Some("public-repo"), DELEGATOR);
2248+
visible_newest.created_at = "2026-01-03T00:00:00Z".into();
2249+
visible_newest.updated_at = visible_newest.created_at.clone();
2250+
state.db.create_task(&visible_newest).await.unwrap();
2251+
2252+
// More than one scan budget of unreadable rows, with no visible row
2253+
// behind them, so the first request stops at the ceiling with rows
2254+
// still trailing it.
2255+
for i in 0..(MAX_TASK_SCAN_CANDIDATES + 5) {
2256+
let mut hidden = task(&format!("hidden-{i:05}"), None, DELEGATOR);
2257+
hidden.created_at = "2026-01-02T00:00:00Z".into();
2258+
hidden.updated_at = hidden.created_at.clone();
2259+
state.db.create_task(&hidden).await.unwrap();
2260+
}
2261+
2262+
let first = list_router(state.clone())
2263+
.oneshot(anon_get("/api/v1/tasks?limit=1"))
2264+
.await
2265+
.unwrap();
2266+
assert_eq!(first.status(), StatusCode::OK);
2267+
let body = body_json(first).await;
2268+
assert_eq!(
2269+
body["tasks"][0]["id"], "newest-visible",
2270+
"the visible row must still be served: {body}"
2271+
);
2272+
assert_eq!(body["count"], 1);
2273+
assert_eq!(
2274+
body["incomplete"], false,
2275+
"a page that filled is not a paused scan: {body}"
2276+
);
2277+
assert_eq!(
2278+
body["has_more"], true,
2279+
"the ceiling hands back a continuation so a longer denied window stays pageable: {body}"
2280+
);
2281+
let mut cursor = body["next_cursor"]
2282+
.as_str()
2283+
.expect("a continuation must accompany has_more")
2284+
.to_string();
2285+
assert!(
2286+
!body.to_string().contains("hidden-"),
2287+
"no response may disclose a denied row's id: {body}"
2288+
);
2289+
assert!(
2290+
!body.to_string().contains(SECRET_UCAN),
2291+
"no response may disclose a ucan token: {body}"
2292+
);
2293+
2294+
// Following the server's own continuation reaches the end of the
2295+
// stream without ever surfacing a denied row.
2296+
let mut requests = 1;
2297+
let terminal = loop {
2298+
requests += 1;
2299+
assert!(requests <= 4, "the continuation must terminate, not spin");
2300+
let resp = list_router(state.clone())
2301+
.oneshot(anon_get(&format!("/api/v1/tasks?limit=1&cursor={cursor}")))
2302+
.await
2303+
.unwrap();
2304+
assert_eq!(resp.status(), StatusCode::OK);
2305+
let page = body_json(resp).await;
2306+
assert_eq!(
2307+
page["count"], 0,
2308+
"nothing visible trails the denied window: {page}"
2309+
);
2310+
assert!(
2311+
!page.to_string().contains("hidden-"),
2312+
"no response may disclose a denied row's id: {page}"
2313+
);
2314+
assert!(
2315+
!page.to_string().contains(SECRET_UCAN),
2316+
"no response may disclose a ucan token: {page}"
2317+
);
2318+
match page["next_cursor"].as_str() {
2319+
Some(next) => cursor = next.to_string(),
2320+
None => break page,
2321+
}
2322+
};
2323+
2324+
assert_eq!(
2325+
terminal["has_more"], false,
2326+
"the walk must end at a terminal page: {terminal}"
2327+
);
2328+
assert_eq!(
2329+
terminal["incomplete"], false,
2330+
"a terminal page is complete, not incomplete: {terminal}"
2331+
);
2332+
}
2333+
21982334
#[sqlx::test]
21992335
async fn exhausted_scan_of_exactly_ceiling_candidates_is_not_incomplete(pool: PgPool) {
22002336
let state = test_state(pool).await;

0 commit comments

Comments
 (0)