Skip to content

Commit 7a11d0f

Browse files
author
Chris Huber
committed
fix(runs): preserve legacy artifact API pagination (#11943)
AI assistance: OpenAI gpt-5.6-sol via OpenCode; used to implement mixed-version API compatibility, focused tests, and CLI reference regeneration.
1 parent e469a4e commit 7a11d0f

6 files changed

Lines changed: 160 additions & 55 deletions

File tree

crates/homeboy-cli/src/commands/runs/remote.rs

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -231,31 +231,7 @@ fn active_runner_job_run_summary_if_durable(
231231

232232
pub fn runner_artifacts(runner_id: &str, args: &RunsArtifactsArgs) -> CmdResult<RunsOutput> {
233233
let run_id = &args.run_id;
234-
let mut query = Vec::new();
235-
for (key, value) in [
236-
("token", args.token.as_deref()),
237-
("kind", args.kind.as_deref()),
238-
("mime", args.mime.as_deref()),
239-
("original_path", args.original_path.as_deref()),
240-
("path_suffix", args.path_suffix.as_deref()),
241-
("fixture", args.fixture.as_deref()),
242-
("surface", args.surface.as_deref()),
243-
("scenario", args.scenario.as_deref()),
244-
("name_glob", args.name_glob.as_deref()),
245-
] {
246-
if let Some(value) = value {
247-
query.push(format!("{key}={}", encode_uri_component(value)));
248-
}
249-
}
250-
if !args.full {
251-
query.push(format!("limit={}", args.limit.clamp(1, 1000)));
252-
query.push(format!("offset={}", args.offset.max(0)));
253-
} else {
254-
query.push("full=1".to_string());
255-
}
256-
let query = (!query.is_empty())
257-
.then(|| format!("?{}", query.join("&")))
258-
.unwrap_or_default();
234+
let query = artifact_query(args);
259235
let data = runner::daemon_api_get(
260236
runner_id,
261237
&format!("/runs/{}/artifacts{}", encode_uri_component(run_id), query),
@@ -295,6 +271,34 @@ pub fn runner_artifacts(runner_id: &str, args: &RunsArtifactsArgs) -> CmdResult<
295271
))
296272
}
297273

274+
fn artifact_query(args: &RunsArtifactsArgs) -> String {
275+
let mut query = Vec::new();
276+
for (key, value) in [
277+
("token", args.token.as_deref()),
278+
("kind", args.kind.as_deref()),
279+
("mime", args.mime.as_deref()),
280+
("original_path", args.original_path.as_deref()),
281+
("path_suffix", args.path_suffix.as_deref()),
282+
("fixture", args.fixture.as_deref()),
283+
("surface", args.surface.as_deref()),
284+
("scenario", args.scenario.as_deref()),
285+
("name_glob", args.name_glob.as_deref()),
286+
] {
287+
if let Some(value) = value {
288+
query.push(format!("{key}={}", encode_uri_component(value)));
289+
}
290+
}
291+
if !args.full {
292+
query.push(format!("limit={}", args.limit.clamp(1, 1000)));
293+
query.push(format!("offset={}", args.offset.max(0)));
294+
} else {
295+
query.push("full=1".to_string());
296+
}
297+
(!query.is_empty())
298+
.then(|| format!("?{}", query.join("&")))
299+
.unwrap_or_default()
300+
}
301+
298302
fn directory_publication_guidance_for_artifacts(
299303
artifacts: &[ArtifactRecord],
300304
) -> Vec<RunsDirectoryArtifactPublicationGuidance> {
@@ -431,6 +435,19 @@ mod tests {
431435
assert!(guide.fetch_hint.contains("--runner <runner-id>"));
432436
}
433437

438+
#[test]
439+
fn cli_artifact_listing_explicitly_requests_its_bounded_page() {
440+
#[derive(clap::Parser)]
441+
struct Wrapper {
442+
#[command(flatten)]
443+
args: RunsArtifactsArgs,
444+
}
445+
446+
let args = <Wrapper as clap::Parser>::parse_from(["homeboy", "run-123"]).args;
447+
448+
assert_eq!(artifact_query(&args), "?limit=50&offset=0");
449+
}
450+
434451
#[test]
435452
fn append_missing_run_summaries_adds_active_runner_jobs_without_duplicates() {
436453
let mut runs = vec![RunSummary {

crates/homeboy-core/src/http_api.rs

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -239,35 +239,44 @@ where
239239
HttpEndpoint::RunArtifacts { id } => {
240240
let store = ObservationStore::open_initialized()?;
241241
require_run(&store, id)?;
242-
let filter = ArtifactListFilter {
243-
token: query_value(&request.path, "token"),
244-
kind: query_value(&request.path, "kind"),
245-
mime: query_value(&request.path, "mime"),
246-
original_path: query_value(&request.path, "original_path"),
247-
path_suffix: query_value(&request.path, "path_suffix"),
248-
fixture: query_value(&request.path, "fixture"),
249-
surface: query_value(&request.path, "surface"),
250-
scenario: query_value(&request.path, "scenario"),
251-
name_glob: query_value(&request.path, "name_glob"),
252-
limit: if query_value(&request.path, "full").as_deref() == Some("1") {
253-
0
254-
} else {
255-
query_value(&request.path, "limit")
242+
// This route predates pagination. Keep an absent pagination request
243+
// exhaustive for deployed old clients; new CLI clients opt in by
244+
// sending `limit` and `offset` explicitly.
245+
if query_value(&request.path, "limit").is_none()
246+
&& query_value(&request.path, "offset").is_none()
247+
{
248+
json!({
249+
"command": "api.runs.artifacts",
250+
"run_id": id,
251+
"artifacts": store.list_artifacts(id)?,
252+
})
253+
} else {
254+
let filter = ArtifactListFilter {
255+
token: query_value(&request.path, "token"),
256+
kind: query_value(&request.path, "kind"),
257+
mime: query_value(&request.path, "mime"),
258+
original_path: query_value(&request.path, "original_path"),
259+
path_suffix: query_value(&request.path, "path_suffix"),
260+
fixture: query_value(&request.path, "fixture"),
261+
surface: query_value(&request.path, "surface"),
262+
scenario: query_value(&request.path, "scenario"),
263+
name_glob: query_value(&request.path, "name_glob"),
264+
limit: query_value(&request.path, "limit")
256265
.and_then(|value| value.parse().ok())
257-
.unwrap_or(50)
258-
},
259-
offset: query_value(&request.path, "offset")
260-
.and_then(|value| value.parse().ok())
261-
.unwrap_or(0),
262-
};
263-
let page = store.list_artifacts_page(id, &filter)?;
264-
json!({
265-
"command": "api.runs.artifacts",
266-
"run_id": id,
267-
"artifacts": page.artifacts,
268-
"page": { "total": page.total, "limit": page.limit, "offset": page.offset,
269-
"next_offset": (page.offset + page.artifacts.len() < page.total).then_some(page.offset + page.artifacts.len()) },
270-
})
266+
.unwrap_or(50),
267+
offset: query_value(&request.path, "offset")
268+
.and_then(|value| value.parse().ok())
269+
.unwrap_or(0),
270+
};
271+
let page = store.list_artifacts_page(id, &filter)?;
272+
json!({
273+
"command": "api.runs.artifacts",
274+
"run_id": id,
275+
"artifacts": page.artifacts,
276+
"page": { "total": page.total, "limit": page.limit, "offset": page.offset,
277+
"next_offset": (page.offset + page.artifacts.len() < page.total).then_some(page.offset + page.artifacts.len()) },
278+
})
279+
}
271280
}
272281
HttpEndpoint::RunArtifactContent { id, artifact_id } => artifact_content(id, artifact_id)?,
273282
HttpEndpoint::RunFindings { id } => {

docs/commands/runs.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ homeboy runs show <run-id> [--format json|--json]
1717
homeboy runs dossier <run-id> [--format json|--json]
1818
homeboy runs resume-plan <run-id>
1919
homeboy runs evidence <run-id>
20-
homeboy runs artifacts <run-id> [--runner <runner-id>] [--pull] [--pull-dir <dir>]
20+
homeboy runs artifacts <run-id> [--runner <runner-id>] [--pull] [--pull-dir <dir>] [--limit <count>] [--offset <count>] [--full]
2121
homeboy runs refs [--kind bench] [--component <id>] [--rig <id>] [--status <status>] [--since 24h] [--artifact-kind <kind>] [--aggregate-artifact-kind <kind>]
2222
homeboy runs artifact attach <run-id> --runner <runner-id> --path <runner-path> --name <artifact-name>
2323
homeboy runs artifact get <run-id> <artifact-id> [--runner <runner-id>] [--output <path>]
@@ -116,6 +116,8 @@ for generic runner, static HTML, and matrix examples.
116116

117117
`homeboy runs artifacts <run-id> --runner <runner-id>` queries a connected runner daemon for the run's artifact records from the controller machine. `homeboy runs artifact get <run-id> <artifact-id> --runner <runner-id>` pulls selected runner-side artifact bytes through that connection into the local artifact cache, or into `--output` when provided, and reports the runner id plus source content path in JSON output. The Lab-oriented wrapper form `homeboy --runner <runner-id> runs artifact get <run-id> <artifact-id>` is accepted for the same fetch path. Use these commands when a controller-side agent has a run id and artifact id but should not SSH into the runner or know runner filesystem paths.
118118

119+
`homeboy runs artifacts` requests a 50-record page by default and returns `page.total`, `page.offset`, and `page.next_offset`; use `--limit` and `--offset` to traverse a larger inventory. `--full` explicitly requests the exhaustive legacy listing and derived summaries. The daemon HTTP route remains mixed-version compatible: `GET /runs/<run-id>/artifacts` without pagination parameters keeps its original exhaustive response, while callers that send `limit` or `offset` receive the paginated response.
120+
119121
`homeboy runs artifact attach <run-id> --runner <runner-id> --path <runner-path> --name <artifact-name>` copies an existing runner-side file into the local persisted artifact store and records it against an existing run. The runner path must be absolute and under the runner's configured `workspace_root`, `policy.workspace_roots`, or `HOMEBOY_ARTIFACT_ROOT` output root. Use this for post-run evidence files that already exist on the runner; it does not promote runner exec output or infer changed files.
120122

121123
`homeboy runs artifact cleanup-downloads` plans cleanup for local runner artifact downloads under Homeboy's artifact root (`<artifact-root>/runner`). By default it is a dry run; pass `--apply` to remove the planned cache directories. Removal is per `<runner-id>/<run-id>` cache directory, never whole-root, and a cache is reclaimable only when its *newest* byte is at least 24 hours old and no non-terminal run claims it. That floor is fixed and deliberately not exposed as a flag: this cache holds bytes an operator asked for. Anything else under the root — a loose file, a symlink, an entry that is not the canonical `<runner-id>/<run-id>` shape — is reported and never removed, and an unreadable modification time or an unavailable observation store retains rather than releases. Use `--runner` and `--run-id` to narrow *which* caches are considered; they never waive the predicate. A cache directory is only reclaimable at all when the writer tagged it `internal_fetch` in its `.homeboy-download.json` marker; an operator pull, an unreadable marker, and an absent marker all retain, and each row reports which case it was in its `intent` field. This category is excluded from a bare `homeboy cleanup --apply` and requires `homeboy cleanup --include runner-downloads`. See [cleanup](cleanup.md#why-runner-downloads-is-opt-in-only).

docs/reference/cli/commands/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Hand-written narrative for these commands lives in `docs/commands/`. -->
66

77
# Homeboy CLI reference (generated)
88

9-
`homeboy` exposes 549 visible commands across 41 top-level command families. Every page below is generated from the clap command tree in `crates/homeboy-cli`, so it cannot drift from the binary.
9+
`homeboy` exposes 552 visible commands across 41 top-level command families. Every page below is generated from the clap command tree in `crates/homeboy-cli`, so it cannot drift from the binary.
1010

1111
Hand-written narrative lives in the [commands index](../../../commands/commands-index.md). Global flags are documented in [the root command reference](../homeboy-root-command.md). Machine-readable safety, docs, output, and Lab metadata come from `homeboy contract manifest`.
1212

docs/reference/cli/commands/release.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ Plan release workflows
2929
| `-p`, `--project` | `<PROJECT>` | Release all components in a project that need a release |
3030
| `--outdated` | flag | Only release components with unreleased code commits (use with --project) |
3131
| `--path` | `<PATH>` | Override local_path for version file lookup (single component only) |
32+
| `--preflight-runner` | `<RUNNER_ID>` | Run portable lint and test release gates through the existing Lab review commands before controller-owned release mutation |
33+
| `--preflight-placement` | `<PREFLIGHT_PLACEMENT>` | Placement policy for portable release preflight gates Values: `local`, `lab`. |
3234
| `--dry-run` | flag | _no help text_ |
3335
| `--apply` | flag | Confirm risky release execution modes |
3436
| `--deploy` | flag | Deploy to all projects using this component after release |
@@ -58,6 +60,7 @@ Plan release workflows
5860
| `homeboy release artifact-source-authority` | Write a source-authority manifest for assembled release artifacts |
5961
| `homeboy release contains` | Report which release first contained a commit, and whether the installed build has it |
6062
| `homeboy release gap` | Report how far the installed build is behind the newest release |
63+
| `homeboy release readiness` | Inspect retained portable release-readiness evidence |
6164

6265
## `homeboy release changes`
6366

@@ -182,3 +185,40 @@ Report how far the installed build is behind the newest release
182185
| `--component` | `<COMPONENT_ID>` | Component whose release tag namespace to search (default: the component discovered from the working directory) |
183186
| `--path` | `<PATH>` | Checkout to inspect directly. Useful for unregistered clones, CI runners, and worktrees |
184187
| `--installed` | `<VERSION>` | Version to treat as installed instead of the running binary's version |
188+
189+
## `homeboy release readiness`
190+
191+
```sh
192+
homeboy release readiness <COMMAND>
193+
```
194+
195+
Inspect retained portable release-readiness evidence
196+
197+
| Subcommand | Summary |
198+
| --- | --- |
199+
| `homeboy release readiness show` | Show one retained readiness operation by operation:// reference or ID |
200+
| `homeboy release readiness list` | List retained readiness operations for a component |
201+
202+
## `homeboy release readiness show`
203+
204+
```sh
205+
homeboy release readiness show <REFERENCE>
206+
```
207+
208+
Show one retained readiness operation by operation:// reference or ID
209+
210+
| Argument | Required | Description |
211+
| --- | --- | --- |
212+
| `<REFERENCE>` | yes | _no help text_ |
213+
214+
## `homeboy release readiness list`
215+
216+
```sh
217+
homeboy release readiness list <COMPONENT_ID>
218+
```
219+
220+
List retained readiness operations for a component
221+
222+
| Argument | Required | Description |
223+
| --- | --- | --- |
224+
| `<COMPONENT_ID>` | yes | _no help text_ |

tests/core/http_api_test.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,43 @@ fn test_handle() {
892892
});
893893
}
894894

895+
#[test]
896+
fn run_artifacts_keeps_legacy_http_clients_exhaustive_and_pages_explicit_requests() {
897+
with_isolated_home(|home| {
898+
let _xdg = XdgGuard::unset();
899+
let store = ObservationStore::open_initialized().expect("store");
900+
let run = store
901+
.start_run(sample_run("bench", "homeboy", "artifact-pagination"))
902+
.expect("run");
903+
let artifact_path = home.path().join("artifact.json");
904+
std::fs::write(&artifact_path, b"{}").expect("artifact");
905+
for index in 0..51 {
906+
store
907+
.record_artifact(&run.id, &format!("artifact-{index}"), &artifact_path)
908+
.expect("record artifact");
909+
}
910+
911+
let legacy = http_api::handle(HttpApiRequest {
912+
method: HttpMethod::Get,
913+
path: format!("/runs/{}/artifacts", run.id),
914+
body: None,
915+
})
916+
.expect("legacy artifacts");
917+
assert_eq!(legacy.body["artifacts"].as_array().unwrap().len(), 51);
918+
assert!(legacy.body.get("page").is_none());
919+
920+
let paged = http_api::handle(HttpApiRequest {
921+
method: HttpMethod::Get,
922+
path: format!("/runs/{}/artifacts?limit=50&offset=0", run.id),
923+
body: None,
924+
})
925+
.expect("paged artifacts");
926+
assert_eq!(paged.body["artifacts"].as_array().unwrap().len(), 50);
927+
assert_eq!(paged.body["page"]["total"], 51);
928+
assert_eq!(paged.body["page"]["next_offset"], 50);
929+
});
930+
}
931+
895932
#[test]
896933
fn runs_list_reconciles_old_ownerless_running_records_before_responding() {
897934
with_isolated_home(|_home| {

0 commit comments

Comments
 (0)