Scope the S3 cache prefix by BK_CACHE_LANE

[buildpulser]

When the runner sets BK_CACHE_LANE, the post step commits under s3://<bucket>/<ns>/<lane>/ and the hydrate reads the same prefix; without the env nothing changes.

Why: a buildkitd running rootless (uid-shifted user namespace) and one running as root store the same layer under different on-disk ownership, so a snapshot committed by one and hydrated by the other can leave a non-root build stage unable to write into its own files (EACCES). Scoping the prefix keeps the two caches apart; switching modes becomes a cold start instead of a broken build.

Also: parseS3ListAges strips the listed key prefix by string so scoped keys still match the refresh --include patterns (tests added). dist/ rebuilt.

Backward compatible: @v2 users without the env see no change.

[buildpulse-v2]

Agent code review

Walkthrough

This PR fixes a bug where parseS3ListAges used a hard-coded two-segment path strip (slice(2).join('/')) that was correct for unscoped caches (<ns>/<prefix>/) but produced wrong keys for lane-scoped caches (<ns>/<lane>/<prefix>/). The fix adds an optional keyPrefix parameter to parseS3ListAges that, when provided, filters listing lines to those under the prefix and strips it precisely; the two-segment fallback is kept for callers that don't pass a prefix. commitToS3 in src/post.js is refactored into cacheBase + commitToS3(lane) so that lane-aware S3 URIs and the correct keyPrefix value flow through to the age-listing call. Two regression tests for the keyPrefix behaviour are added, and both dist/ bundles are rebuilt.

Findings

No issues found.

Verdict

The fix is mechanically correct: cacheBase produces the right base and keyPrefix for both scoped and unscoped lanes, backward-compatibility is preserved (empty lane restores the previous s3://<bucket>/<ns> path exactly), the SAFE-regex guard is applied to lane before it reaches any URI, region validation is still performed first in commitToS3, and the two new tests directly capture the previously-broken key-extraction behaviour. No auto-merge red flags are present. APPROVE.

Confidence

HIGH — every changed file was read in full; key logic was traced end-to-end against the before/after diff.

💡 Tips for working with the BuildPulse reviewer

• Re-review on demand — comment @buildpulse review on the PR (/buildpulse review also works). The fresh review bills the requester's seat.
• Teach it by replying to a finding — reply to any inline comment: "we never do X here" becomes a standing rule for this repository (acknowledged in-thread, not raised again); "not in this PR" mutes the topic for this PR only. Replies are free. Org admins can view or remove learned rules on the AI Features page.
• Every push is re-reviewed automatically — smaller PRs get a fresh full review, larger ones get a delta review of what changed since the last look; an "Update branch" merge with an unchanged diff carries the previous verdict forward without a new billed review.
• Draft PRs are skipped until marked ready for review (repositories can opt in to draft reviews on the AI Features page).

[buildpulse-v2]

Agent code review

Walkthrough

This PR scopes the S3 cache prefix by BK_CACHE_LANE when the runner sets it. It extracts a cacheBase(bucket, ns, lane) helper to compute both base (S3 URI) and keyPrefix (bucket-relative path with trailing slash), threads lane from the environment through commitToS3, and passes the fully-qualified keyPrefix+pfx/ to parseS3ListAges so the age-listing parser can strip the correct prefix from S3 --recursive output. The src/index.js log line is updated to surface the active lane. Two new tests cover the lane-scoped and unscoped keyPrefix paths in parseS3ListAges, and dist/ is rebuilt.

Findings

No issues found.

Verdict

The change is internally consistent: cacheBase constructs keyPrefix as ns/lane/ (lane-scoped) or ns/ (legacy), commitToS3 lists s3://bucket/ns/lane/snap-ca/ (or without lane segment) with --recursive, and passes ns/lane/snap-ca/ as the keyPrefix to parseS3ListAges, so stripping that prefix from the full S3 object key yields bare keys like aaa.tar.zst that match the --include patterns used in the refresh step. The SAFE regex guard on lane prevents injection the same way it guards bucket and ns. The fail-open behavior (nearExpiry returns true on null/missing) is unchanged. The two new tests directly exercise the corrected prefix-stripping path for both lane-scoped and unscoped cases. The dist rebuild is present. No bugs, security issues, weakened tests, or scope drift found.

Confidence

HIGH — all 6 changed files read in full; critical caller chain (cacheBase → commitToS3 → objectAgesDays → parseS3ListAges → nearExpiry → refresh) traced end-to-end; no prior KB context or learned rules to apply.

💡 Tips for working with the BuildPulse reviewer

• Re-review on demand — comment @buildpulse review on the PR (/buildpulse review also works). The fresh review bills the requester's seat.
• Teach it by replying to a finding — reply to any inline comment: "we never do X here" becomes a standing rule for this repository (acknowledged in-thread, not raised again); "not in this PR" mutes the topic for this PR only. Replies are free. Org admins can view or remove learned rules on the AI Features page.
• Every push is re-reviewed automatically — smaller PRs get a fresh full review, larger ones get a delta review of what changed since the last look; an "Update branch" merge with an unchanged diff carries the previous verdict forward without a new billed review.
• Draft PRs are skipped until marked ready for review (repositories can opt in to draft reviews on the AI Features page).