Merge pull request 'singleton: guard the state, not the program's name' (#157) from fix/singleton-guards-the-state into dev

lang/runtime/el_runtime.c

@@ -19809,22 +19809,84 @@ void log_warn(el_val_t msg_v) {
  * become a convention. */
 static int el_singleton_fd = -1;
 static char el_singleton_path[1024];
+static char el_singleton_state[1024];
 
-static const char* el_singleton_dir(void) {
-    const char* d = getenv("EL_SINGLETON_DIR");
-    if (d && *d) return d;
-    d = getenv("TMPDIR");
-    if (d && *d) return d;
-    return "/tmp";
-}
-
-/* el_singleton_acquire — claim exclusive process identity, or refuse to start.
- * Compiler-injected as the FIRST statement of main() for any program whose
- * `program` block declares `singleton:`. */
-el_val_t el_singleton_acquire(el_val_t id_v) {
+/* el_singleton_acquire — claim exclusive use of the guarded STATE, or refuse to
+ * start. Compiler-injected as the FIRST statement of main() for any program
+ * whose `program` block declares `singleton:` (which must also declare
+ * `guards:` — see lang/spec/language.md §18.2).
+ *
+ * GUARD THE THING, NOT THE NAME.
+ *
+ * Until 2026-08-16 this lock was keyed on the program's NAME and on $TMPDIR —
+ * `$EL_SINGLETON_DIR|$TMPDIR|/tmp` + `/el-singleton-<name>.lock` — and never
+ * consulted the state it claimed to protect. Its own refusal message said
+ * "Refusing to start a second instance against the same state" while it had not
+ * looked at any state. Measured, it failed in BOTH directions:
+ *
+ *   - FALSE POSITIVE: two engrams against genuinely DIFFERENT data dirs could
+ *     not coexist. The second was refused, naming the first's pid — for sharing
+ *     a name, not a store.
+ *   - FALSE NEGATIVE (the dangerous one): `TMPDIR=/tmp/other` let a second
+ *     instance start against the SAME data dir with no complaint. That is
+ *     exactly the two-instance data-loss condition the guard exists to prevent,
+ *     and the workaround was one environment variable.
+ *
+ * Both are one error: the identity of the resource had been replaced by a label
+ * for it. The fix is to put the lock file INSIDE the state it guards:
+ *
+ *     <state>/.el-singleton-<id>.lock
+ *
+ * That placement is the whole mechanism, and it is why there is no hashing, no
+ * canonical-path registry, and no environment variable left to subvert:
+ *
+ *   - Same directory  => same file => same inode => the flock CONTENDS. There is
+ *     no TMPDIR in the key, so there is nothing to change to get past it.
+ *   - Different dirs  => different files => no contention. Two stores are two
+ *     stores; they were never in conflict and are no longer treated as if they
+ *     were.
+ *   - Different SPELLINGS of one directory — trailing slash, `x/../x`, a symlink
+ *     — resolve to the same inode in the kernel's own path walk, so they contend
+ *     without this code comparing strings at all. Path canonicalisation here is
+ *     for the human-readable message, never for the decision.
+ *
+ * Kept, deliberately, from the version this replaces: it is an flock and not a
+ * pidfile (the kernel releases it on crash and on SIGKILL, so there is no stale
+ * state and therefore no "delete the lock file to get unstuck" ritual), and it
+ * reports the HOLDER'S PID (added because a stale process survived `pkill -f`
+ * and went on answering probes; "already running" is not actionable, a pid is).
+ *
+ * Changed: the message is now TRUE. It says "the same state" because the lock it
+ * failed to take lives in that state, and it names the state it checked. */
+el_val_t el_singleton_acquire(el_val_t id_v, el_val_t state_v) {
     const char* id = EL_CSTR(id_v);
     if (!id || !*id) return EL_NULL;
 
+    /* A singleton with nothing to guard is the defect this function exists to
+     * remove; refuse rather than silently fall back to name-keying. The compiler
+     * rejects `singleton:` without `guards:`, so reaching this is a toolchain
+     * mismatch, not a user mistake — say so. */
+    const char* state = EL_CSTR(state_v);
+    if (!state || !*state) {
+        fprintf(stderr,
+            "[el] FATAL: singleton '%s' was given no state to guard.\n"
+            "[el]        A lock keyed on a program's NAME instead of on the state it\n"
+            "[el]        protects is not a guard: it refuses unrelated instances and\n"
+            "[el]        permits concurrent ones. Declare `guards: <path>` alongside\n"
+            "[el]        `singleton:` in the program block (spec §18.2).\n", id);
+        exit(1);
+    }
+
+    /* Canonicalise so the operator is told WHICH directory was checked, in one
+     * spelling, whatever spelling they typed. This is a readability measure, not
+     * the mechanism: realpath() may fail (the directory may not exist yet) and
+     * correctness must not depend on it — when it succeeds it names the same
+     * directory, and when it does not we fall back to the path as given and the
+     * kernel's own path walk still collapses the spellings at open() time. */
+    char* rp = realpath(state, NULL);
+    snprintf(el_singleton_state, sizeof(el_singleton_state), "%s", rp ? rp : state);
+    free(rp);
+
     /* Sanitise the id into a filename. */
     char safe[256];
     size_t si = 0;
@@ -19835,13 +19897,25 @@ el_val_t el_singleton_acquire(el_val_t id_v) {
         safe[si++] = (char)(ok ? c : '-');
     }
     safe[si] = '\0';
+    /* THE MECHANISM: the lock lives inside the state it guards. Two spellings of
+     * one directory name one file; two directories name two files. Note there is
+     * no $TMPDIR and no $EL_SINGLETON_DIR in this path — the escape hatch that
+     * made the guard bypassable is gone because there is nowhere left to put it. */
     snprintf(el_singleton_path, sizeof(el_singleton_path),
-             "%s/el-singleton-%s.lock", el_singleton_dir(), safe);
+             "%s/.el-singleton-%s.lock", el_singleton_state, safe);
 
     int fd = open(el_singleton_path, O_RDWR | O_CREAT, 0644);
     if (fd < 0) {
-        fprintf(stderr, "[el] FATAL: singleton '%s': cannot open lock file %s: %s\n",
-                id, el_singleton_path, strerror(errno));
+        /* Unguardable state. Refusing is the only honest option: starting anyway
+         * would mean running unguarded against exactly the store the guard is
+         * here to protect. */
+        fprintf(stderr,
+            "[el] FATAL: singleton '%s': cannot open the lock inside the state it guards.\n"
+            "[el]        state: %s\n"
+            "[el]        lock:  %s (%s)\n"
+            "[el]        The guarded directory must exist and be writable. Refusing to\n"
+            "[el]        start unguarded against it.\n",
+            id, el_singleton_state, el_singleton_path, strerror(errno));
         exit(1);
     }
     if (flock(fd, LOCK_EX | LOCK_NB) != 0) {
@@ -19857,11 +19931,14 @@ el_val_t el_singleton_acquire(el_val_t id_v) {
         fprintf(stderr, "[el] FATAL: another instance of '%s' is already running", id);
         if (holder > 0) fprintf(stderr, " (pid %ld)", holder);
         fprintf(stderr, ".\n"
-                "[el]        lock: %s\n"
+                "[el]        state: %s\n"
+                "[el]        lock:  %s\n"
                 "[el]        Refusing to start a second instance against the same\n"
-                "[el]        state. Stop the running one and VERIFY it is gone\n"
-                "[el]        (ps -p <pid>) before retrying.\n",
-                el_singleton_path);
+                "[el]        state. Two writers against one store is data loss, not a\n"
+                "[el]        warning. Stop the running one and VERIFY it is gone\n"
+                "[el]        (ps -p %ld) before retrying — or point this instance at a\n"
+                "[el]        different state, which is permitted and is not refused.\n",
+                el_singleton_state, el_singleton_path, holder > 0 ? holder : (long)0);
         close(fd);
         exit(1);
     }

lang/runtime/el_runtime.h

@@ -1091,7 +1091,7 @@ el_val_t  __env_get(el_val_t key);
  * All three are COMPILER-INJECTED at the head of main() — they are not meant to
  * be written by hand, which is the point: the guarantee cannot be forgotten at a
  * call site because there is no call site. */
-el_val_t  el_singleton_acquire(el_val_t id);                    /* §18.1 process identity */
+el_val_t  el_singleton_acquire(el_val_t id, el_val_t state);    /* §18.2 process identity — keyed on the guarded state */
 el_val_t  el_config_declare(el_val_t name, el_val_t type,
                             el_val_t deflt, el_val_t has_default,
                             el_val_t required);                 /* §18.2 config schema    */