Skip to content

Commit 8a83d12

Browse files
ausimianclaude
andcommitted
fix: ask for a RELEASES file the handler accepts, not one that parses
Fifth correction to one sentence, and the last, because it stops naming a property of the file. Each earlier version gave a condition that was necessary and not sufficient, so each admitted a narrower counterexample: restart missed a present-but-unreadable file; absent-or-present missed one created readably after boot; readable missed malformed terms, since init/1 reads it with file:consult/1; consultable missed a file of two valid terms, since init/1 accepts only {ok, [Term]}. The hook leaves an existing file alone whatever is in it, so every one of those states loops. The remedy now asks for a file release_handler accepts, and says that no single property of the file is the test. It cannot be narrowed again because it claims no mechanism, and it is what an operator needs regardless: the handler is the thing that has to take the file. AGENTS.md records the series, so the next reader does not substitute whichever internal criterion is current - the move that was wrong five times. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent fdfa7f5 commit 8a83d12

4 files changed

Lines changed: 35 additions & 24 deletions

File tree

AGENTS.md

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -457,13 +457,22 @@ Castle's job is configuration and release management on a running node.
457457
than the branch it replaced. Do not turn it back into a case analysis of the
458458
cause, and do not collapse it into a bare "restart the system" either.
459459

460-
**"Consultable", not "readable".** `init/1` reads the file with
461-
`file:consult/1`, so a file whose permissions are fine but whose terms are
462-
malformed fails in exactly the same way, and the hook leaves an existing file
463-
alone whatever is in it. An earlier version of this message said "readable" and
464-
so sent an operator with a corrupt `RELEASES` round the same restart loop it was
465-
written to end. Permissions are not the whole of the condition; being
466-
consultable by `:release_handler` is.
460+
**Name the authority, not the mechanism — and this is the lesson of five
461+
successive corrections to one sentence.** Each named a property of the file and
462+
each was necessary but not sufficient, so each admitted a narrower
463+
counterexample: "restart" missed a file that was present and unreadable;
464+
"present or absent" missed one created readably since boot; "readable" missed
465+
malformed terms, because `init/1` reads it with `file:consult/1`; "consultable"
466+
missed a file of two valid terms, because `init/1` accepts only `{ok, [Term]}`.
467+
There is no reason to think that series had ended, and the hook leaves an
468+
existing file alone whatever is in it, so every one of those states loops.
469+
470+
The message therefore asks for a file **`:release_handler` accepts**, and says
471+
that no single property of the file is the test. That cannot be narrowed further
472+
because it does not claim a mechanism, and it is what an operator needs anyway:
473+
the handler is the thing that has to take the file. Do not "improve" it by
474+
substituting whichever internal criterion is current — that is the move that was
475+
wrong five times.
467476

468477
**And it must not name `releases/RELEASES` unqualified**, because that is the
469478
file the *release* creates, not necessarily the one the handler reads — see

RELEASE.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,16 @@
4747
- `Castle.unpack/1` and `Castle.install/1` now refuse a system that cannot be
4848
upgraded from, and refuse it in the same call that would otherwise have done
4949
the work. `:release_handler` reads `releases/RELEASES` once, as it starts, and
50-
when it cannot — the file absent, or there but not consultable — it makes a
51-
release record up out of the boot script's name and version — a record that
52-
names no applications at all.
50+
when it cannot — the file absent, or there but not something it accepts — it
51+
makes a release record up out of the boot script's name and version — a record
52+
that names no applications at all.
5353
Upgrading a system in that state is worse than being stopped: the install
5454
reports success, and every application whose version changed but whose code the
5555
upgrade does not explicitly load goes on running its old code out of the
5656
directory of the release that was just replaced, until a later `remove` deletes
5757
it. Nothing can repair the running system afterwards, because creating the file
5858
changes no record the node holds — so what the refusal says is to restart, with
59-
the file either absent or consultable first. See *Fixed* below for what that
59+
the file either absent or accepted first. See *Fixed* below for what that
6060
condition is and why a bare restart is not always enough.
6161

6262
The question is asked of the node's own records rather than of the filesystem,
@@ -153,14 +153,14 @@
153153

154154
- The refusal for a system running from a synthesised release record now names a
155155
remedy that works. It said to restart, and a restart alone is enough only when
156-
the `RELEASES` file `:release_handler` reads is absent or consultable: it reads
157-
that file with `file:consult/1`, so a malformed one fails just as an unreadable
158-
one does, and the release creates the file only when it is *absent* so
159-
anything left in place that cannot be consulted is stepped over on every start
160-
and the system comes back on another synthesised record. An operator following the old message would have restarted
156+
the `RELEASES` file `:release_handler` reads is absent or accepted by the handler
157+
itself: present, readable and parsing as Erlang terms are each necessary and none
158+
of them sufficient. The release creates the file only when it is *absent*, so
159+
anything left in place that the handler will not accept is stepped over on every
160+
start and the system comes back on another synthesised record. An operator following the old message would have restarted
161161
indefinitely.
162162

163-
The refusal now asks for that file to be absent or consultable before the restart,
163+
The refusal now asks for that file to be absent or accepted before the restart,
164164
and identifies it rather than assuming: `releases/RELEASES` under the release
165165
root, unless `RELDIR` or the `sasl` `releases_dir` parameter points elsewhere.
166166
Where one of those does, the two are different files and a restart cannot fix it

lib/castle/commands.ex

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,14 @@ defmodule Castle.Commands do
346346
"upgrade does not load, running its old code. Creating the file now would not " <>
347347
"change the record this node works from, so the system has to be restarted. " <>
348348
"Before restarting, make sure the RELEASES file :release_handler reads is " <>
349-
"either absent or one it can consult - it reads that file with file:consult/1, " <>
350-
"so a malformed one fails exactly as an unreadable one does, and permissions " <>
351-
"are not the whole of the condition. The release creates that file only when " <>
352-
"it is absent, so anything left in place that cannot be consulted is stepped " <>
353-
"over on every start and the system comes back on the same synthesised " <>
354-
"record. Absent, or consultable, is what a restart needs. That file is " <>
349+
"either absent or one that :release_handler itself accepts - being present, " <>
350+
"being readable and parsing as Erlang terms are each necessary and none of " <>
351+
"them sufficient, so the test that matters is whether the handler takes it, " <>
352+
"not any one property of the file. The release creates that file only when it " <>
353+
"is absent, so anything left in place that the handler will not accept is " <>
354+
"stepped over on every start and the system comes back on the same " <>
355+
"synthesised record. Absent, or accepted, is what a restart needs. That file " <>
356+
"is " <>
355357
"releases/RELEASES under the release root unless RELDIR or the sasl " <>
356358
"releases_dir parameter points elsewhere; where one of those does, the release " <>
357359
"creates a file at the root that the handler will not read, so the one it does " <>

test/castle/commands_test.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ defmodule Castle.CommandsTest do
9393
# absence - and a case analysis of the cause, which is what this said first,
9494
# has no advice at all for a file that was absent at boot and has been
9595
# created readably since, where a plain restart is all that is needed.
96-
assert message =~ "Absent, or consultable, is what a restart needs."
96+
assert message =~ "Absent, or accepted, is what a restart needs."
9797
end
9898

9999
test "asks the release the system is running, and not another one" do

0 commit comments

Comments
 (0)