Skip to content

ci(nightly): harden kiro etag, cursor resolver, no-cache comment - #1522

Merged
thepagent merged 1 commit into
mainfrom
fix/nightly-kiro-etag-multipart
Sep 3, 2026
Merged

ci(nightly): harden kiro etag, cursor resolver, no-cache comment#1522
thepagent merged 1 commit into
mainfrom
fix/nightly-kiro-etag-multipart

Conversation

@chaodu-agent

Copy link
Copy Markdown
Collaborator

Follow-up to #1519 addressing review findings from the two-reviewer pass (Sol, Fable) that landed after merge.

Findings addressed

# Reviewer Severity Fix
F1 Sol latent hard-break Kiro latest-object ETag validation now accepts multipart ETags (<md5>-<partcount>), not only single-part 32-hex. The ~586MB object sits at the S3 multipart boundary; a future re-upload flipping its ETag to the suffixed form would otherwise hard-fail every nightly-kiro build with no SHA fallback. If-Match accepts multipart ETags, so both shapes are valid preconditions.
#2 Fable flakiness Cursor resolver uses grep -m1 instead of grep … | head -1, avoiding the SIGPIPE (exit 141) that head-closing-the-pipe triggers under pipefail and which surfaced as a spurious red job.
F5 Sol comment accuracy Corrected the misleading no-cache: true comment — no-cache rebuilds every Dockerfile.package layer for the variant (base image, apt/node, CLI install), not just the CLI layer. The openab binary is not recompiled (prebuilt via the bins= build context).

Optional nits from the review (check_ver leading-dash guard, npm --retry-all-errors, immutable-tag pre-existence check, docs for the two tag families) are intentionally not in this PR to keep it minimal; they can follow separately.

Tested

  • python3 -c "import yaml; yaml.safe_load(...)" → YAML parses.
  • Verified grep -oE -m1 … | sed … extracts the first cursor version and exits 0 under set -o pipefail (no exit 141), locally reproducing the SIGPIPE-avoidance.
  • F1/F5 are a regex-widening and a comment change respectively; no behavior change on the release/pre-beta path (those pass their pinned SHAs and never hit the ETag branch).

Blocked / not verified

  • Did not run the full nightly workflow end-to-end (requires the scheduled/dispatch trigger on main and GHCR push); changes are confined to the resolve/build steps and validated by YAML parse + local shell repro.

Review Contract

Acceptance criteria

  1. Kiro ETag validation accepts both ^[0-9a-f]{32}$ and ^[0-9a-f]{32}-[0-9]+$.
  2. Cursor resolver no longer uses head in a pipe; first-match extraction preserved.
  3. no-cache comment accurately describes what is rebuilt.
  4. No change to the release (v*) or pre-beta build paths; parity check unaffected.

Out of scope (non-blocking follow-ups): check_ver leading-dash guard, npm retry-all-errors, immutable-tag pre-existence check, tag-family docs.

Stopping rule: default three-stage (full review → fix verification → final regression check).

Follow-up to #1519 addressing review findings from Sol and Fable:

- F1: accept multipart S3 ETags (<md5>-<partcount>) in the Kiro
  latest-object validation. The ~586MB object sits at the multipart
  boundary; a future re-upload flipping its ETag to the suffixed form
  would otherwise hard-fail every nightly-kiro build with no SHA
  fallback. If-Match accepts multipart ETags, so both shapes are valid.
- #2: use `grep -m1` instead of `grep | head -1` in the cursor
  resolver to avoid the SIGPIPE (exit 141) that head-closing-the-pipe
  triggers under pipefail, which surfaced as a spurious red job.
- F5: correct the misleading no-cache comment — no-cache rebuilds every
  Dockerfile.package layer for the variant, not just the CLI layer; the
  openab binary is not recompiled (prebuilt via the bins= context).
@chaodu-agent
chaodu-agent force-pushed the fix/nightly-kiro-etag-multipart branch from 434803f to 23a9e94 Compare September 3, 2026 18:34
@thepagent
thepagent enabled auto-merge (squash) September 3, 2026 18:35
@thepagent
thepagent disabled auto-merge September 3, 2026 18:35
@thepagent
thepagent merged commit 474bbad into main Sep 3, 2026
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants