feat: rename and retime a published page with stele amend - #5
Merged
Conversation
Follows stele-pages 570b375, which grew `PATCH /pages/:slug` — the verb that
changes the two things about a page that used to be fixed at publication: where
it lives and how long it lives.
One command mapping 1:1 to the route, the way `publish` maps to POST and
`update` to PUT:
stele amend <slug> [--slug <name>] [--ttl <days>]
Either flag alone, or both in one request — so a rename-and-retime cannot half
apply the way two commands over one route would.
The trap this change is mostly built around: an absent `--ttl` here means
*leave the deadline exactly where it is*, where the same absence on `publish`
means "take the server's default". `PageTTL?` is one optional with one absent
case and the two verbs read it in opposite directions, so nothing enforces the
difference except neither caller inventing a value to fill the silence with. A
`ttl=7` sent because seven looked reasonable would put a week's deadline on a
page published to be kept, and the 200 would look perfectly fine. Hence a test
asserting the parameter is *absent* rather than empty, and a smoke check that
renames a `--ttl never` page and watches its deadline stay null.
`SteleError.slugTaken` grows an `advice` field, taken from the route's
`Expectation` the way `notFound`'s already was. It was the same mistake found
twice: "omit `--slug` and let the server generate one" is the right next move on
a publish and a false one on an amendment, where omitting it means *do not
rename* and no slug is ever allocated — an agent following that advice would
come back with a client-side "nothing to amend" and never reach the server.
`Expectation.amend` exists for the same reason on the 404 side: `write`'s advice
names `stele update` and says to publish first, but an amendment's 404 is
usually a page that has *expired*, where the tempting next move is `--ttl never`
and it fails identically.
An amendment naming neither flag is refused before a credential is read or a
byte is sent — the server answers it with a 400, but it is a mistake in the
invocation, and this side can name the two flags that would fix it. Same bargain
`PageIO.read` strikes over an empty file.
Docs record what a rename costs, because it is the sharper half: the old name is
freed the instant it commits, with no redirect and no tombstone, so a link
already in circulation breaks — first as an ordinary 404, then, if somebody
claims the name, as a link pointing at their page. `update` keeps the URL and
replaces the page; `amend` keeps the page and replaces the URL.
0.2.0 -> 0.3.0. Also corrects a pre-existing claim that the server has no delete
route — it has had one since stele-pages c83463a; what this CLI has is no
command that reaches it.
147 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018qEtLbpPoxtUtucznPohMG
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR adds page amendment support. Users can rename pages and change deadlines without changing content. The client sends PATCH requests, the CLI exposes ChangesPage amendment support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AmendCommand
participant SteleClient
participant PagesAPI
AmendCommand->>SteleClient: Submit slug and optional amendment values
SteleClient->>PagesAPI: PATCH /pages/:slug with optional query parameters
PagesAPI-->>SteleClient: Return PageLocation or route-specific error
SteleClient-->>AmendCommand: Return result
AmendCommand-->>AmendCommand: Report URL or actionable error
Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows stele-pages
570b375, which grewPATCH /pages/:slug— the verb that changes the two things about a page that used to be fixed at publication: where it lives and how long it lives.The shape
One command mapping 1:1 to the route, the way
publishmaps to POST andupdateto PUT:Either flag alone, or both in one request — so a rename-and-retime cannot half-apply the way two commands over one route would. The alternative surfaces (separate
rename/retime, or flags bolted ontoupdate) were considered and rejected:update's "never changes the deadline" contract is worth keeping true.The positional is
<slug>and the rename flag is--slug, matchingpublish --slug. They collide in Swift, not on the command line — the option is declared.customLong("slug")against anewSlugproperty.The trap this is mostly built around
An absent
--ttlonamendmeans leave the deadline exactly where it is, where the same absence onpublishmeans "take the server's default".PageTTL?is one optional with one absent case and the two verbs read it in opposite directions, so nothing enforces the difference except neither caller inventing a value to fill the silence with.A
ttl=7sent because seven looked like a reasonable default would put a week's deadline on a page its author published to keep forever — and the200would look perfectly fine. So:--ttlwas passed (a rename with no lifetime sends no ttl at allasserts the URL contains nottlsubstring at all);--ttl never, renames with--slugonly, and assertsexpiresis still null. That check is the only thing anywhere that would notice this bug.Error advice became route-specific
SteleError.slugTakengrows anadvicefield, taken from the route'sExpectationthe waynotFound's already was. It was the same mistake found twice: "omit--slugand let the server generate one" is the right next move on a publish and a false one on an amendment, where omitting it means do not rename and no slug is ever allocated. An agent following that advice would come back with a client-side "nothing to amend" and never reach the server.Expectation.amendexists for the matching reason on the 404 side:write's advice namesstele updateand says to publish first, but an amendment's 404 is usually a page that has expired, where the tempting next move is--ttl never— which fails identically.slugTaken(String?)→slugTaken(detail:advice:)is a source-breaking change to a public enum case. Fine on a 0.x line with one consumer, noted for the record.Refusals
An amendment naming neither flag is refused before a credential is read or a byte is sent (exit 1). The server answers it with a 400 of its own, but it is a mistake in the invocation, and this side can name the two flags that would fix it — the same bargain
PageIO.readstrikes over an empty file. Taken name → exit 5, no live page (expired counts as none) → exit 7; both codes already existed.Docs
The README's "A page's deadline is fixed when it is published … republish if you need a different lifetime" is now false and is rewritten: a deadline is fixed for a page's body, not for the page.
PUT's400on?ttl=stays, and stays correct — a replacement cannot retime a page.A new section records what a rename costs, because it is the sharper half. The old name is freed the instant it commits, with no redirect and no tombstone, so a link already in circulation breaks — first as an ordinary 404, then, if somebody claims the name, as a link quietly pointing at their page.
updatekeeps the URL and replaces the page;amendkeeps the page and replaces the URL.Also corrects a pre-existing claim that the server has no delete route — it has had one since stele-pages
c83463a; what this CLI has is no command that reaches it.Testing
0.2.0→0.3.0. 147 tests pass, build clean with no warnings,bash -nclean on the smoke script.Smoke coverage (needs a live server, not run here): rename serves the bytes at the new name and 404s the old one at once; the freed name is claimable again; retime alone leaves the name alone; rename alone leaves a
neverdeadline null; and the three refusals — the exit-1 one with the credential file moved aside, which is how the script proves the refusal happened before anything was sent.Follow-up, not in this PR
The server's
PublishSkill.swift— served atGET /skilland proxied verbatim bystele skill— currently states that "stelehas no rename command and no retime command" and tells agents a lifetime is chosen once, at publication. True when570b375shipped; false the moment this merges. That's a companion PR instele-pages.🤖 Generated with Claude Code
https://claude.ai/code/session_018qEtLbpPoxtUtucznPohMG
Summary by CodeRabbit
New Features
stele amendwith validation, TTL options, and updated URL reporting.Bug Fixes
Documentation
Tests
Chores