docs: document the command boundary and spec its public surface by ausimian · Pull Request #28 · ausimian/castle

ausimian · 2026-08-23T14:20:39Z

Refs #11. Targets release/1.0.0. Documentation and typespecs only; no behaviour changed.

@moduledoc was the mix new placeholder and nothing but customize/1 carried a @doc or @spec, so the published HexDocs reference page listed undocumented, un-spec'd functions.

The issue is stale in two ways

It lists generate/1, which castle#13 deleted along with the build.config path — not documented, not resurrected. And it predates Castle.customize/1, which castle#12 documented in full; that is left untouched and is the standard the rest was written to match.

What is published and what is not

These are not an Elixir library API, and the docs have to say so or a reader is surprised twice — by functions that print rather than return, and by ones that raise rather than answer. Castle is the command boundary.

make_releases/0 — @doc false, with a @spec. Its only caller is Forecastle's env.sh fragment invoking --eval "Castle.make_releases()" in the preboot VM, gated on releases/RELEASES being absent. There is no bin/castle subcommand, and by hand it either does nothing or does what the next start does anyway. Its contract is with a shell fragment in another project; the spec stays because a spec is the contract whether or not the page shows it.
install/2..5 — documented as the test seam they are. One @doc covers all arities of a defaulted clause, and ExDoc renders the signature with all four defaults visible, so silence would read as API. install/1 is the operator's form; the rest exist so a concurrency test can drive the public boundary, which AGENTS.md explains at length.
running/1 — documented, though it has no subcommand of its own. The bin/castle subcommands are exactly commit, install, releases, remove, unpack, upgradable; running/1's only shipped caller is the confirmation loop inside bin/castle install. It is documented anyway because it answers a read-only question and automation driving rpc directly needs precisely it — a different case from make_releases/0, which mutates bookkeeping once and is useless by hand.
The rest are documented naming the bin/castle command that reaches them, what they refuse, and what they leave behind.

Every public function carries a @spec, and they are all :: :ok (keyword() for customize/1) because that is the whole of what report!/1 returns: Enum.each/2 on success, a raise otherwise.

Seven claims the first pass got wrong

The first commit here documented the surface; a review of it found seven false or overstated claims, all fixed in the second. Worth listing, because this branch is nothing but assertions and there is no Dialyzer checking any of them:

Castle.Error is not the only failure. Commands.installed/5 deliberately re-raises an exception, throw or exit out of install_release/1 once the marker is settled. Automation rescuing only Castle.Error misses a command that blew up. AGENTS.md carried the same unqualified claim and is corrected too.
upgradable/0 had reintroduced a claim this repo corrected five times — that a synthesised record means releases/RELEASES was unreadable. It says what the refusal message says: the file was not one :release_handler accepted, no single property of the file is the test, and the path is qualified with RELDIR / {sasl, releases_dir} (castle#23).
The install preflight was documented in the wrong order, and as four steps rather than five. Arming is also before install_release/1 and can also refuse — with the configuration already written, which is exactly what occupied/3 and stale/4 tell an operator. The boundary is install_release/1, not the first call into :release_handler: the release-record lookup is such a call.
Rollback was overstated. After a restart transition the reboot the install asked for is already accounted for — the launcher consumes the marker pair and boots the target. It is the restarts after that one, before commit, which read start_erl.data and return. RELEASE.md repeated the unqualified version.
Commit is not a no-op for an already-permanent release. OTP's promotion step is, but Commands.commit/5 materialises first since castle#14, so an explicit commit rewrites sys.config from current provider inputs and a failing provider now fails the command. Separately the argumentless form selects the release awaiting commit — which_releases(:current) — not "the version running now".
Three of five install arities had no spec, contradicting this branch's own claim that every public function carries one. Added, and verified with Code.Typespec.fetch_specs/1 rather than assumed — ExDoc renders only the widest arity's spec for a defaulted definition, so the page looked identical with two specs as with five and the gap was undetectable from mix docs.
:unpacked does not mean "never installed". A failed or rolled-back continuation leaves a version in that state; it is now described as staged and not currently installed, including that case.

Also corrected, found while checking claims

Three internal notes left stale by castle#14's commit serialisation: materialise/3's @doc said the ERTS guard "is not redundant for Castle.commit/1, which still composes this in front of commit/3" — commit materialises inside its own serialised region now and there is no commit/3; two more commit/3 references in install/5's @doc; and a serialised/2 comment pointing at a nonexistent Castle.materialise/1.

`AGENTS.md`

The upgradable/0 bullet's "whether it belongs in the documented API surface is #11's to settle" is settled in place. The Known limitations bullet for the undocumented API is replaced by "Nothing checks the @specs" — no Dialyzer, so a spec or a refusal-claim that stops being true fails nothing, and both are read against Castle.Commands by hand. It also records that a per-arity spec gap is invisible to mix docs and must be counted against Castle.__info__(:functions).

Verification

mix precommit green — 154 tests, credo --strict clean. mix compile --warnings-as-errors --force clean under Elixir 1.20.3/OTP 28.4.1, which matters because this branch is nothing but specs. mix docs clean: two ex_doc warnings about Kernel.CLI being hidden (it is @moduledoc false, so it can be named but not linked) were fixed by naming elixir --rpc-eval instead, as Castle.Error's own moduledoc already did.

No tests added. The claims the docs make are already pinned — test/castle_test.exs asserts the raises, the silent successes and the printing; test/castle/erts_guard_test.exs pins that every gated command raises while upgradable/0 and releases/0 still answer, which is the moduledoc's closing paragraph. Asserting on documentation prose would be worse than nothing.

Out of scope, and one thing it turned up

castle#9 (README) and castle#8 (coverage) are untouched. Castle.Commands has the same per-arity spec gap across 18 arities; left alone deliberately, since it is @moduledoc false and no claim in the repo covers it.

Finding 5 also existed on the shell side — bin/castle --help said commit [VSN] "Defaults to the version that is running now", contradicting the code twenty lines above it. Fixed separately on Forecastle's release/1.0.0 in a684dae, along with a test whose name implied the same wrong thing.

@moduledoc

The @moduledoc was still the placeholder `mix new` writes and none of the commands carried a @doc or a @SPEC, so the published page said nothing about the one thing a reader has to know before calling any of it: these are not an Elixir API. Every function but `customize/1` is a command entry point that `bin/castle` reaches over `rpc` - it prints its report and returns a bare `:ok`, and it raises `Castle.Error` on failure so that the calling VM exits non-zero. A reader who takes them for ordinary functions is surprised by both halves, so the moduledoc says what a command boundary is and every @doc names the `bin/castle` command that reaches it. castle#11. The distinction the issue asked to be settled, settled deliberately: * `make_releases/0` is @doc false. Its only caller is the launcher's `env.sh` fragment, in the preboot VM of a start whose deployment has no `RELEASES` yet, and by hand it either does nothing or does what the next start would do anyway. It keeps its @SPEC: the spec is the contract whether or not the function is published. * `install/2..5` is documented as what it is. One @doc covers every arity of a clause with defaults, so silence about the extra four would leave a test seam reading as an API. * Everything else is a command an operator invokes, and is documented with that framing rather than hidden - `upgradable/0` included, since the state it reports is invisible otherwise and a diagnostic nobody is told about is one nobody thinks to ask. The specs say `:: :ok` and nothing more, because that is the whole of what `report!/1` returns. `mix docs` is clean; its one complaint was a reference to `Kernel.CLI`, which is hidden, so `Castle.Error`'s own wording is used instead. Three internal notes had gone stale and are corrected with it: two named a `commit/3` that is now `commit/5`, and one claimed `commit` still composes materialisation outside the serialised region, which castle#14 ended. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C4oaMvbR1cbxrZBj8qwkqN

@doc

Review of 01ccbf2 found seven inaccuracies, all of them assertions nothing in this project checks. Two were regressions of rules this repo had already settled rather than ordinary staleness, and both are now written down in AGENTS.md so the next draft does not repeat them. The two regressions: * `upgradable/0` said the record is synthesised when the system started without a `releases/RELEASES` file "it could read". That is the *readable* phrasing five successive corrections to the refusal message had already discarded - it misses malformed terms - and it named the file unqualified, which castle#23 forbids because RELDIR and `{sasl, releases_dir}` relocate it. The @doc now says what the refusal says: the file `:release_handler` accepts, no one property of the file being the test, and the path qualified. * `commit/1` said committing an already-permanent version "succeeds and changes nothing". That was true before castle#14 moved materialisation inside `commit`'s serialised region. `make_permanent/1` is still a no-op for such a version, but the target's `sys.config` has been rewritten from current provider inputs by then, and a provider that fails now fails the command. The five others: * `report!/1` does not make every failure a `Castle.Error`. `installed/5` settles the marker and re-raises what `install_release/1` raised, threw or exited with, so automation rescuing only `Castle.Error` would read a blown-up command as a success. The moduledoc now says which failures become which. * The install preflight is documented in the order it happens: the ERTS guard, the release-record lookup and its refusal, the pending-marker refusal, materialisation, arming - five steps, before `install_release/1` rather than before any `:release_handler` call, since the lookup is one. The pending-marker refusal was missing entirely, and it is what stops a refused caller replacing the configuration a pending install is about to boot. * "Anything that takes the system down brings the previous version back" is false for a restart install. The launcher consumes the marker pair on the next start and boots the target; it is the restarts after that one which return to the previous permanent version. RELEASE.md said the same thing and is corrected with it. * `bin/castle commit` with no version selects a `:current` release and exits non-zero when there is none - not "the version running now", which a system on its permanent release also has. * `unpacked` is staged and not currently installed, a version a rollback returned to that state included. `running/1`'s own @doc already said so. And the specs: two @specs on a five-arity defaulted definition left `install/2..4` unspecced, contradicting this branch's own claim that the whole public surface carries one. All five are now present, checked with `Code.Typespec.fetch_specs(Castle)` against `Castle.__info__(:functions)` rather than by reading the source - ExDoc renders only the widest arity's spec, so the page could not have shown the gap. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C4oaMvbR1cbxrZBj8qwkqN

ausimian · 2026-08-23T14:20:41Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0f7c073a5

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Two claims a review caught, both the same shape as the seven before them: a qualification applied in one place and not the neighbouring one. unpack/1 said the tarball has to be copied into the deployment's releases directory. release_handler reads whichever directory RELDIR or the sasl releases_dir parameter names - do_unpack_release joins RelDir, and RelDir is that override when it is set - so an operator following it on such a deployment copies the archive somewhere unpack will not look. The release-record file already carried this caveat two @docs away. customize/1's :tar warning repeated the same unqualified path and now points at unpack/1 for it. And AGENTS.md still said a failure raises Castle.Error three paragraphs after the rule correcting exactly that. installed/5 re-raises an exception, throw or exit out of install_release/1 unchanged, so a rescue narrowed to Castle.Error misses a command that blew up rather than refused. Said as refusal rather than failure, with a paragraph on why the blanket version keeps coming back. The module comment on the boundary had it too. Co-Authored-By: Claude <noreply@anthropic.com>

ausimian · 2026-08-23T14:31:06Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e152efc4f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

remove/1's doc said the command deletes erts-<vsn>, using the same vsn as its release-version argument. do_remove_release/4 reads erts_vsn out of the release record and removes erts-<EVsn> only when no remaining release refers to that emulator - and the two numbers are unrelated, normally different ones: a release at 0.1.1 may be carrying erts-16.2. So the doc named a directory that generally does not exist. The same placeholder was reused in five other places where it sits beside lib/<app>-<vsn> and so reads as the same version, including the ERTS guard's own refusal message, which ships. All now say erts-<erts_vsn>. The synthetic release fixture's erts-<vsn>/bin is left alone: there is no adjacent lib/<app>-<vsn> and the version genuinely is the emulator's. The whole-message assertion in erts_guard_test failed on the reword, which is what it is for - the message has been wrong three times about what it can claim, so changing it deliberately means editing that test deliberately. Co-Authored-By: Claude <noreply@anthropic.com>

ausimian · 2026-08-23T14:39:39Z

@codex review

@doc

AGENTS.md already quoted do_remove_release/4 deleting filename:join(Root, "erts-" ++ EVsn) correctly; the drift was in the placeholder prose around it, which reused <vsn> for both namespaces and so named a directory that generally does not exist. Note it at the authority, with where it had already got to - remove/1's @doc and the ERTS guard's refusal message, which ships. Co-Authored-By: Claude <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8bba5a049

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@doc

…laim Two claims a review found, both asserting more than the code does. The unpack doc offered RELDIR and the sasl releases_dir parameter as ways to name another release directory. rel_dir/0 is Path.join(Deployment.root_dir(), "releases") and consults neither, so setting one does not merely go unread: it moves the directory the handler unpacks into and keeps its records in, while the ERTS guard reading the target .rel (commands.ex:128) and the step writing the target configuration (commands.ex:743) both still resolve <root>/releases/<vsn>. The doc was directing operators into an upgrade that cannot complete. It now says so. AGENTS.md already had this right and cites issue #23; the omission was only in the public doc. The moduledoc said the commands that are questions "succeed with nothing to say at all". releases/0 is a question and prints a line per release through report!/1, which its own @doc promises, so automation was given the wrong stdout contract. What gets printed varies by command, not by kind: upgradable/0 and running/1 return {:ok, []}, releases/0 returns the table. Co-Authored-By: Claude <noreply@anthropic.com>

ausimian · 2026-08-23T14:48:07Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 131ef59217

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@doc

The passage explaining what is published and what is hidden named two exceptions and then said every other function is a command an operator invokes. running/1 is not: bin/castle dispatches exactly releases, upgradable, unpack, install, remove and commit, and running/1 has no subcommand - its only shipped caller is the confirmation loop inside bin/castle install. Its own new @doc says so, so AGENTS.md contradicted the file it is describing. It stays published, but for a different reason than the other two: it answers a read-only question and mutates nothing, and automation driving rpc rather than bin/castle needs it to know when an install has finished booting. That is the distinction from make_releases/0, which mutates bookkeeping once and is useless by hand. Co-Authored-By: Claude <noreply@anthropic.com>

ausimian · 2026-08-23T14:55:41Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fec90e98fa

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The previous commit overcorrected. Fixing a paragraph that offered RELDIR and the sasl releases_dir parameter as supported, it also asserted that <root>/releases is the only release directory Castle resolves - which is wrong about the tarball, the one thing the paragraph exists to locate. unpack/1 hands the name straight to :release_handler.unpack_release/1 and joins nothing; do_unpack_release/4 does Tar = filename:join(RelDir, ReleaseName ++ ".tar.gz") against the handler own releases directory, which the overrides move. An operator following the text would have failed at the unpack step. The text now separates the two halves: the tarball goes where the handler reads it, and the overrides move that; everything Castle resolves for itself is joined onto code:root_dir(), so an override still leaves the halves disagreeing and an unpack that succeeds is followed by an install that cannot. The unsupported warning and the #23 reference stay. Also corrects a claim the previous commit made in passing, in its message and its prose: commands.ex:128 is in create_releases/3, on the make_releases/0 path, not in the ERTS guard. The install-side consumers of Castle rel_dir are restart_planned?/3, unclaimed/3, materialise/3, arm/4 and installed/5. Co-Authored-By: Claude <noreply@anthropic.com>

ausimian · 2026-08-23T15:02:36Z

@codex review

chatgpt-codex-connector · 2026-08-23T15:06:53Z

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: edadd5411f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ausimian and others added 2 commits August 23, 2026 23:44

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread lib/castle.ex Outdated

Comment thread AGENTS.md Outdated

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread lib/castle.ex Outdated

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread lib/castle.ex Outdated

Comment thread lib/castle.ex Outdated

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread AGENTS.md Outdated

chatgpt-codex-connector Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread lib/castle.ex Outdated

ausimian merged commit bc19bf5 into release/1.0.0 Aug 23, 2026
7 checks passed

ausimian deleted the issue/11-docs branch August 23, 2026 15:13

Conversation

ausimian commented Aug 23, 2026

The issue is stale in two ways

What is published and what is not

Seven claims the first pass got wrong

Also corrected, found while checking claims

AGENTS.md

Verification

Out of scope, and one thing it turned up

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ausimian commented Aug 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`AGENTS.md`