fix: name a remedy that works for an unreadable RELEASES

AGENTS.md

@@ -437,6 +437,17 @@ Castle's job is configuration and release management on a running node.
   `RELEASES` file names at least `kernel` and `stdlib`. The remedy the message
   names is a restart, because that is the only thing that changes the answer.
 
+  **A restart is necessary and, in one of the two cases, not sufficient — and the
+  message has to say which.** The record is synthesised when `RELEASES` was
+  missing *or* could not be read, and Forecastle's `env.sh` creates it only when
+  it is **absent** (`[ ! -f ... ]`). So a file that is present and unreadable is
+  stepped over on every start: the node comes back on a freshly synthesised
+  record, the refusal repeats, and an operator following a message that named
+  only the restart would loop forever. The message therefore splits the remedy —
+  restart if the file is absent, make it readable or remove it *first* if it is
+  present. Do not collapse that back into "restart the system": the diagnosis
+  already admits both causes, so a single remedy is wrong for one of them.
+
   It has to be asked of the node rather than of the filesystem — a file that
   appeared *after* the boot that looked for it passes a shell test and still
   leaves the node on the synthesised record — and it has to be asked *in the call

RELEASE.md

@@ -150,6 +150,15 @@
 
 ### Fixed
 
+- The refusal for a system running from a synthesised release record now names a
+  remedy that works in both of the cases it diagnoses. It said to restart, and a
+  restart is enough only when `releases/RELEASES` was *absent*: the release
+  creates that file when it is missing, so a file that is present but unreadable
+  is stepped over on every start and the system comes back on another synthesised
+  record. An operator following the old message would have restarted
+  indefinitely. It now says to make the file readable or remove it first in that
+  case.
+
 - A release built with `include_erts: false` is now refused, by name and with
   the reason, rather than quietly managing the Erlang installation it happens to
   be running on. Such a release ships no emulator, so it runs the system one, and

lib/castle/commands.ex

@@ -344,8 +344,13 @@ defmodule Castle.Commands do
            "not be read, when the system booted. An upgrade from that record reports " <>
            "success and leaves any application whose version changed, but whose code the " <>
            "upgrade does not load, running its old code. Creating the file now would not " <>
-           "change the record this node works from. Restart the system: the release " <>
-           "creates the file before it starts."}
+           "change the record this node works from, so the system has to be restarted - " <>
+           "but check the file first, because which of the two happened decides whether a " <>
+           "restart is enough. If releases/RELEASES is absent, restart: the release " <>
+           "creates it before it starts. If it is present and could not be read, make it " <>
+           "readable or remove it before restarting - the release only creates that file " <>
+           "when it is absent, so a restart on its own will step over the unreadable one " <>
+           "and come back on the same synthesised record."}
 
       nil ->
         {:error, "#{refusal}: no release is running."}

test/castle/commands_test.exs

@@ -85,7 +85,13 @@ defmodule Castle.CommandsTest do
       assert message =~ "This system cannot be upgraded: 1.2.3 is running from a release record"
       assert message =~ "names no applications"
       assert message =~ "running its old code"
-      assert message =~ "Restart the system: the release creates the file before it starts."
+      assert message =~ "If releases/RELEASES is absent, restart: the release creates it"
+
+      # The other branch of the remedy, which a restart alone does not reach: the
+      # hook that creates the file is guarded on its absence, so an unreadable
+      # file is stepped over on every start and the node comes back on the same
+      # synthesised record. A refusal naming only the restart would loop forever.
+      assert message =~ "make it readable or remove it before restarting"
     end
 
     test "asks the release the system is running, and not another one" do
@@ -136,7 +142,7 @@ defmodule Castle.CommandsTest do
 
       assert {:error, message} = Commands.unpack("sample-1.2.3", handler)
       assert message =~ "Cannot unpack sample-1.2.3: 1.2.2 is running from a release record"
-      assert message =~ "Restart the system"
+      assert message =~ "the system has to be restarted"
       assert Stub.calls(:unpack_release) == []
     end
 
@@ -193,7 +199,7 @@ defmodule Castle.CommandsTest do
       assert {:error, message} = Commands.install("1.2.3", handler)
       assert message =~ "Cannot install 1.2.3: 1.2.2 is running from a release record"
       assert message =~ "running its old code"
-      assert message =~ "Restart the system"
+      assert message =~ "the system has to be restarted"
       assert Stub.calls(:install_release) == []
     end