Skip to content

Commit f41664f

Browse files
bakeyclaude
andauthored
fix(sources): paginate github issues on the gateway raw page length (#176)
fix(sources): paginate github issues on the gateway's raw page length The OC list_repository_issues action filters pull requests out AFTER paginating, so the filtered issues array's length is not a termination signal: short-page termination silently truncated the scan on the first PR-bearing page, and even empty-page termination fails on 100 consecutive PRs. Upstream now reports the raw page length (pageInfo.fetched, oomol-lab/open-connector#228); this change consumes it. Engine: PageNumber gains raw_page_size_path, mirroring total_pages_path — when declared, the scan continues while the RAW page was full regardless of how short (or empty) the filtered rows are; a missing signal propagates as RowPathNotFound and a non-integer one fails as the new PaginationRawPageSizeInvalid (kind-only), never a silent truncation. The two authoritative signals are mutually exclusive at validate time. Loader exposes raw_page_size_path on page_number pagination; the issues table declares $.pageInfo.fetched. Contract re-captured from a live gateway carrying the upstream fix (outputSchema now declares pageInfo) and the fingerprint re-pinned, so older gateways fail issues registration at the fingerprint gate instead of truncating. Live-verified: all 11 tables register with the new pin and the issues scan reaches the credential wall. Tests: engine units (continue on full raw page with short/empty rows, terminate on short raw page, missing/invalid signal failures, total/raw mutual exclusion) and a pack e2e driving a 3-page scan whose middle page is ALL pull requests — the case that defeats every filtered-count heuristic. Stubs and the demo stub gateway now emit pageInfo. Docs and spec updated with the minimum-gateway note. 246 open_connector / 803 lib tests green. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 33033a3 commit f41664f

9 files changed

Lines changed: 280 additions & 23 deletions

File tree

crates/skardi/src/sources/providers/open_connector/error.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,19 @@ pub enum OpenConnectorError {
230230
found: String,
231231
},
232232

233+
/// The declared raw-page-size signal was present but not a non-negative
234+
/// integer. Treating it as anything else would either truncate the scan
235+
/// or loop it; carries the JSON *kind* only, never the value.
236+
#[error(
237+
"Open Connector pagination raw page size at '{path}' on page {page} is {found}, \
238+
expected a non-negative integer"
239+
)]
240+
PaginationRawPageSizeInvalid {
241+
path: String,
242+
page: usize,
243+
found: String,
244+
},
245+
233246
/// A continuation cursor was present at the declared path but was not a
234247
/// string. Treating it as end-of-collection would silently truncate the
235248
/// scan, so it fails instead. Carries the JSON *kind* only, never the

crates/skardi/src/sources/providers/open_connector/packs/fixtures/github/contracts/list_repository_issues.json

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,22 @@
123123
},
124124
"additionalProperties": true
125125
}
126+
},
127+
"pageInfo": {
128+
"type": "object",
129+
"properties": {
130+
"fetched": {
131+
"type": "integer",
132+
"minimum": 0,
133+
"description": "Number of items GitHub returned on this page before filtering. Continue paginating while this equals perPage, which defaults to 30."
134+
}
135+
},
136+
"additionalProperties": false,
137+
"required": [
138+
"fetched"
139+
],
140+
"description": "Pagination signals from the raw GitHub page, before pull requests are filtered out."
126141
}
127142
},
128143
"additionalProperties": false
129-
}
144+
}

crates/skardi/src/sources/providers/open_connector/packs/github.rs

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,15 @@
77
//! - **Page-number pagination everywhere** (`page`/`perPage`, 100 per page
88
//! — GitHub's maximum; the camelCase keys are Open Connector's strict
99
//! action-input contract, not GitHub's raw REST parameters). A short or
10-
//! empty page terminates the scan, which is GitHub's documented
11-
//! end-of-collection signal.
10+
//! empty page terminates the scan — except for `issues`, whose OC action
11+
//! filters pull requests out AFTER paginating, so a filtered page's
12+
//! length is not a termination signal: the table declares
13+
//! `raw_page_size_path: $.pageInfo.fetched` (upstream #228) and the scan
14+
//! continues while the RAW page was full, even when the filtered rows
15+
//! come back short or empty. Requires a gateway with
16+
//! oomol-lab/open-connector#228 (older gateways fail the fingerprint
17+
//! gate at registration, and their responses would fail the scan loudly
18+
//! with a missing `$.pageInfo.fetched` — never a silent truncation).
1219
//! - **Filters are allowlisted only where faithful — and every string-enum
1320
//! push is Inexact.** `issues.state` / `pull_requests.state` narrow the
1421
//! fetch but DataFusion re-applies them: the translation is faithful only
@@ -115,6 +122,49 @@ mod tests {
115122
MockResponse::ok(&discovery_ok("{}", output_schema, true, None))
116123
}
117124

125+
#[tokio::test]
126+
async fn filtered_issue_pages_do_not_truncate_the_scan() {
127+
// The OC action filters pull requests out AFTER paginating, so a
128+
// filtered page can be short — or entirely empty — while more pages
129+
// exist. The raw signal (pageInfo.fetched, upstream #228) must
130+
// drive continuation: page 1 returns 2 issues of a full raw page,
131+
// page 2 is ALL pull requests (0 issues, raw full), page 3 is the
132+
// genuine final page. Short-page termination would stop after page
133+
// 1 and lose everything after it.
134+
let gateway = MockGateway::start(|req| {
135+
if req.method == "GET" && req.path == "/v1/health" {
136+
return MockResponse::ok("{}");
137+
}
138+
if req.method == "GET" && req.path.starts_with("/v1/actions/") {
139+
return github_discovery(&req.path);
140+
}
141+
if req.method == "POST" && req.path == "/v1/actions/github.list_repository_issues" {
142+
let body: Value = serde_json::from_str(&req.body).unwrap_or_default();
143+
let page = body["input"]["page"].as_u64().unwrap_or(1);
144+
let response = match page {
145+
1 => json!({"issues": [issue(1, "open", "2026-01-01T00:00:00Z"),
146+
issue(2, "open", "2026-01-01T00:00:00Z")],
147+
"pageInfo": {"fetched": 100}}),
148+
2 => json!({"issues": [], "pageInfo": {"fetched": 100}}),
149+
3 => json!({"issues": [issue(3, "open", "2026-01-02T00:00:00Z")],
150+
"pageInfo": {"fetched": 1}}),
151+
other => panic!("unexpected page {other}"),
152+
};
153+
return MockResponse::ok(&envelope_ok(&response.to_string()));
154+
}
155+
MockResponse::new(404, "{}")
156+
})
157+
.await;
158+
let (_gw, ctx) = setup_with_gateway(gateway, "SKARDI_TEST_OC_GITHUB_RAW_PAGE").await;
159+
160+
let batches = collect(&ctx, "SELECT number FROM saas.gh.issues ORDER BY number").await;
161+
assert_eq!(
162+
rows_of(&batches),
163+
3,
164+
"all three pages were scanned: the short and the all-PR page did not terminate"
165+
);
166+
}
167+
118168
#[test]
119169
fn fingerprint_coverage_gap_is_pinned() {
120170
// The gate protects only what upstream DECLARES; these mapped
@@ -623,16 +673,24 @@ bindings:
623673
.and_then(Value::as_str)
624674
.unwrap_or("open")
625675
.to_string();
626-
let slice: Vec<_> = rows
676+
let matching: Vec<_> = rows
627677
.iter()
628678
.filter(|row| {
629679
state == "all" || row.get("state").and_then(Value::as_str) == Some(&state)
630680
})
681+
.collect();
682+
let slice: Vec<_> = matching
683+
.iter()
631684
.skip((page - 1) * per_page)
632685
.take(per_page)
633-
.cloned()
686+
.map(|row| (*row).clone())
634687
.collect();
635-
return MockResponse::ok(&envelope_ok(&json!({"issues": slice}).to_string()));
688+
// The raw page length before any post-pagination filtering; the
689+
// stub does none, so it equals the slice length.
690+
let fetched = slice.len();
691+
return MockResponse::ok(&envelope_ok(
692+
&json!({"issues": slice, "pageInfo": {"fetched": fetched}}).to_string(),
693+
));
636694
}
637695
MockResponse::new(404, "{}")
638696
}
@@ -658,6 +716,14 @@ bindings:
658716
let served = std::sync::Arc::clone(&served);
659717
MockGateway::start(move |req| issues_handler(req, &served)).await
660718
};
719+
setup_with_gateway(gateway, token_env).await
720+
}
721+
722+
/// Register the issues binding against an arbitrary gateway stub.
723+
async fn setup_with_gateway(
724+
gateway: MockGateway,
725+
token_env: &str,
726+
) -> (MockGateway, SessionContext) {
661727
unsafe {
662728
std::env::set_var(token_env, "test-token");
663729
}
@@ -1356,7 +1422,9 @@ bindings:
13561422
return MockResponse::ok(&envelope_ok(r#"{"repositories": []}"#));
13571423
}
13581424
if req.method == "POST" && req.path == "/v1/actions/github.list_repository_issues" {
1359-
return MockResponse::ok(&envelope_ok(r#"{"issues": []}"#));
1425+
return MockResponse::ok(&envelope_ok(
1426+
r#"{"issues": [], "pageInfo": {"fetched": 0}}"#,
1427+
));
13601428
}
13611429
MockResponse::new(404, "{}")
13621430
})

crates/skardi/src/sources/providers/open_connector/packs/github.yaml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,17 @@ tables:
3535

3636
issues:
3737
action: github.list_repository_issues
38-
fingerprint: 45fbef8622d90702e536b9b2e0ade6bcc7da61f7cb654d62a90b2e36df13c64b
38+
fingerprint: 0530ff93d700f3aba2754160a76cf69919a7753fe14b26835463fdb88cfad8e3
3939
row_path: "$.issues"
4040
pagination:
4141
strategy: page_number
4242
page_input: page
4343
page_size_input: perPage
4444
page_size: 100
45+
# The OC action filters pull requests out AFTER paginating; the raw
46+
# page length (oomol-lab/open-connector#228) is the only sound
47+
# termination signal — a short or empty issues array is not.
48+
raw_page_size_path: "$.pageInfo.fetched"
4549
resources:
4650
required: [owner, repo]
4751
fixed_inputs:

crates/skardi/src/sources/providers/open_connector/packs/loader.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,8 @@ enum PaginationDoc {
400400
page_size: u32,
401401
#[serde(default)]
402402
total_pages_path: Option<String>,
403+
#[serde(default)]
404+
raw_page_size_path: Option<String>,
403405
},
404406
Cursor {
405407
cursor_input: String,
@@ -418,11 +420,13 @@ impl PaginationDoc {
418420
page_size_input,
419421
page_size,
420422
total_pages_path,
423+
raw_page_size_path,
421424
} => PaginationStrategy::PageNumber {
422425
page_param: leak_str(page_input),
423426
per_page_param: leak_str(page_size_input),
424427
per_page: page_size,
425428
total_pages_path: total_pages_path.map(leak_str),
429+
raw_page_size_path: raw_page_size_path.map(leak_str),
426430
},
427431
Self::Cursor {
428432
cursor_input,

0 commit comments

Comments
 (0)