Skip to content

deploy: a key S3 keeps on delete is never counted as removed - #640

Merged
jthingelstad merged 2 commits into
mainfrom
session/deploy-site-deletes
Oct 21, 2026
Merged

jthingelstad merged 2 commits into
mainfrom
session/deploy-site-deletes

Conversation

@jthingelstad

Copy link
Copy Markdown
Owner

A follow-up to #639 that answers its Codex finding and records the measured timings.

The finding (P2). DeleteObjects answers HTTP 200 even when S3 keeps some of the keys, and lists those in Errors. publish() ignored that response, counted every key as removed and dropped it from the record. The prune did the same.

The fix. deleteKeys in lib/site-publish.mjs returns the keys S3 kept.

  • A document the build no longer has that S3 kept fails the publish, just as a failed PUT does. A page may have been taken down on purpose, so it must not stay live without anyone noticing. The publish record is not written, so the next deploy checks the files again with HEAD. The next deploy's listing also lists the document for removal again.
  • An asset the prune could not delete is only a warning. It is housekeeping, and the next prune tries again. The log count now says how many were actually deleted.
  • Test: services/mcp/test/site-publish.test.mjs adds "a key S3 kept on delete is never counted as removed".

Measured. These are the site-lane deploy records before and after #639:

phase #637 (before) #639 (after)
assets 20.3 s 1.7 s
site 21.4 s 7.4 s
total before acceptance 56.9 s 22.2 s

The first deploy after #639 had no publish record yet, so it confirmed about 1,470 matching files with HEAD. From the next deploy on, the record covers them. The note (docs/notes/2026-10-21-site-publish.md) carries these numbers and the live header read-back.

Checks: npm run verify passes on Node 24. e2e-needed printed false.

🤖 Generated with Claude Code

Otto Thing and others added 2 commits October 21, 2026 03:05
DeleteObjects answers 200 when S3 keeps some keys and names them in
Errors (Codex on #639). deleteKeys returns them: a document the build
no longer has that S3 kept fails the publish, like a failed PUT; an
asset the prune could not delete is a warning, left for the next prune,
and the count says what was deleted.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@jthingelstad
jthingelstad marked this pull request as ready for review October 21, 2026 08:06
@jthingelstad
jthingelstad enabled auto-merge (rebase) October 21, 2026 08:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T08:08:13.979566Z cd9bea3 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@jthingelstad
jthingelstad merged commit fcd1e93 into main Oct 21, 2026
7 checks passed
@jthingelstad
jthingelstad deleted the session/deploy-site-deletes branch October 21, 2026 08:09
@jthingelstad

Copy link
Copy Markdown
Owner Author

Deployed to production 2026-10-21T08:15:06Z (2026-10-21 03:15:06 CDT).

  • Commit: fcd1e937 (site lane); this PR's: 86632690, fcd1e937
  • MCP contract: 11.13.3
  • JSON API: 3.2.1
  • Acceptance: passed (the whole suite)
  • Timing: started 2026-10-21T08:09:44Z (2026-10-21 03:09:44 CDT); the CI gate waited 2 s, then 5 m 21 s to this record: build 2 s, grants 2 s, card-art 2 s, site 4 s, smoke 4 s, acceptance 5 m 3 s

jthingelstad pushed a commit that referenced this pull request Oct 21, 2026
The order in infra/AGENTS.md and the ship skill left out the first
migrations (--create) and the public statistics snapshot, which stops a
platform deploy when it fails (Codex on #637). #640 had added its
measurements and the delete handling to #639's merged note; they move
to a note of their own, since a merged note is history.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant