Compare commits

...

1 Commits

Author SHA1 Message Date
bigmerge 45325f7391 singleton: guard the state, not the program's name
El SDK CI - dev / build-and-test (pull_request) Failing after 4m6s
The singleton lock protected a filename, not a store. It was keyed on
$EL_SINGLETON_DIR|$TMPDIR|/tmp + /el-singleton-<program>.lock — the
program's NAME and a temp directory — and never consulted the state it
claimed to protect, while its own refusal message read "Refusing to start
a second instance against the same state."

Measured, it failed in both directions. A second engram against a
DIFFERENT data dir was refused, naming the first's pid. And
TMPDIR=/tmp/other let a second engram start against the SAME data dir
with no complaint — the two-writer data-loss condition the guard exists
to prevent, defeated by one environment variable.

Both are one error: the identity of the resource had been replaced by a
label for it.

The lock now lives inside the state it guards —
<state>/.el-singleton-<id>.lock — and the program block says what that
state is. Same directory is the same file is the same inode, so it
contends and there is no TMPDIR left in the key to change. Different
directories are different files, so they don't. Different spellings of
one directory (trailing slash, x/../x, symlink) collapse in the kernel's
own path walk, so they contend without this code comparing strings;
canonicalisation is for the message, never the decision.

`guards:` is an expression so a program can point at the resolver that
already owns its path — guards: engram_resolve_data_dir() — instead of
restating that resolver's default, which is the two-owners defect spec
18.4 exists to prevent. A `singleton:` without `guards:` is now a compile
error; emitting a name-keyed lock instead would be emitting the defect.

Kept: the flock (the kernel drops it on crash and SIGKILL, so there is
still no "delete the lock file to get unstuck" ritual — a stale file
inside a copied data dir is inert), and the holder's pid in the message.
Changed: the message is true. It says "the same state" because the lock
it failed to take is in that state, and it names the state it checked.
An unguardable state (missing, read-only) now refuses rather than
starting unguarded.

Also corrects lang/AGENTS.md's compiler rebuild line, which had gone
stale: linking el_runtime.c alone no longer resolves.
2026-08-16 16:08:40 -05:00
8 changed files with 213 additions and 49 deletions
+17 -7
View File
@@ -23,16 +23,26 @@
// warning. The runtime takes an exclusive flock at startup and a second start
// is refused loudly with the holder's pid.
//
// NOT declared here, on purpose: ENGRAM_DATA_DIR. Its resolution is owned by
// engram_resolve_data_dir() (el_runtime.c), which defaults to $HOME/.neuron/engram
// and fails LOUD rather than silently persisting to an ephemeral directory.
// Declaring a default for it here as well would put the data dir's fallback in
// two places which is precisely the defect this migration removes (until
// 2026-08-15 the reseed backup path carried its own "/tmp/engram" default that
// disagreed with the resolver, so the pre-destructive safety copy landed in /tmp).
// guards: names WHAT the singleton protects this program's data directory. The
// lock lives inside it, so the guard is keyed on the store and not on the word
// "engram": two engrams against the same store cannot both run no matter how the
// environment is spelled, and two engrams against DIFFERENT stores are not each
// other's business and are not refused. Until 2026-08-16 the lock was keyed on
// the program name and $TMPDIR, and both of those sentences were false.
//
// It names the resolver rather than restating its path, for the same reason
// ENGRAM_DATA_DIR is NOT declared as an `env` entry below: engram_resolve_data_dir()
// (el_runtime.c) owns that path it defaults to $HOME/.neuron/engram and fails
// LOUD rather than silently persisting to an ephemeral directory. Restating the
// default here would give the data dir two owners that can disagree, which is
// precisely the defect this migration removes (until 2026-08-15 the reseed backup
// path carried its own "/tmp/engram" default that disagreed with the resolver, so
// the pre-destructive safety copy landed in /tmp). A guard that resolved the path
// its own way could guard a directory the program never writes to.
// HOME is likewise not declared: it is a genuine environment read, not a knob.
program "engram" {
singleton: "engram"
guards: engram_resolve_data_dir()
// Core server
env ENGRAM_BIND: String = ":8742"
+11 -7
View File
@@ -111,14 +111,18 @@ After changing any `.el` source in `el-compiler/src/` (run from the `lang/` dir)
```bash
# 1. Stage2: current elc compiles the (modified) compiler to C
./dist/platform/elc elc-cli.el > elc-new.c
# 2. Build the new compiler. The C link target is el_runtime.c — it holds the
# engram store + http/json/state impls the compiler output calls. el_runtime.c
# self-hosts elc on its own; el_seed.c is the (aspirational) seed layer and does
# NOT compile standalone under clang (missing prototypes for the el_runtime.c
# symbols it wraps — see caveat below), so link el_runtime.c here.
cc -std=c11 -I runtime -lcurl -lpthread \
# 2. Build the new compiler. Link the WHOLE runtime set, not el_runtime.c alone:
# el_runtime.c calls into engram_store / engram_vindex / eg_cosine_batch and
# wraps el_seed.c, so a one-file link fails at `ld` with undefined symbols
# (verified 2026-08-16 — the previous single-file line in this doc is stale).
cc -std=c11 -O2 -I runtime -I$(brew --prefix openssl@3)/include \
-L$(brew --prefix openssl@3)/lib \
-o dist/platform/elc-new \
elc-new.c runtime/el_runtime.c
elc-new.c runtime/el_runtime.c runtime/el_seed.c \
runtime/engram_cognition.c runtime/engram_geometry.c runtime/engram_reason.c \
runtime/engram_store.c runtime/engram_verify.c runtime/engram_vindex.c \
runtime/eg_cosine_batch.c runtime/eg_cosine_batch_strategy_cpu.c \
-lcurl -lssl -lcrypto -lpthread -lm
# 3. Verify self-hosting FIXPOINT (stage3 == stage2 output, byte-identical):
./dist/platform/elc-new elc-cli.el > elc-verify.c
diff elc-new.c elc-verify.c # must be identical
BIN
View File
Binary file not shown.
+16 -1
View File
@@ -3295,6 +3295,12 @@ fn cgi_arg(value: String, has_value: Bool) -> String {
// exit before touching configuration, ports, or any data directory.
// 2. config declarations resolve env-or-default, one declaration per entry.
// 3. validate LAST report EVERY missing/ill-typed entry at once, then exit.
//
// `singleton:` carries its `guards:` expression as its SECOND argument the
// state the lock protects, evaluated here at the process boundary. A singleton
// without one does not compile (see below): a lock keyed on the program's name
// rather than on its state refuses unrelated instances and permits concurrent
// ones, which is not a weaker guard but a wrong one.
fn el_bool_arg(b: Bool) -> String {
if b { return "EL_INT(1)" }
return "EL_INT(0)"
@@ -3306,7 +3312,16 @@ fn emit_program_init(stmt: Map<String, Any>) -> Void {
let has_singleton: Bool = stmt["has_singleton"]
if has_singleton {
let sid: String = stmt["singleton"]
emit_line(" el_singleton_acquire(EL_STR(" + c_str_lit(sid) + "));")
let has_guards: Bool = stmt["has_guards"]
if has_guards {
let guards_c: String = cg_expr(stmt["guards"])
emit_line(" el_singleton_acquire(EL_STR(" + c_str_lit(sid) + "), " + guards_c + ");")
} else {
// Refuse at COMPILE time. The alternative emitting a name-keyed
// lock is the defect itself, and it fails silently in the direction
// that loses data.
emit_line("#error \"singleton '" + sid + "' declares no `guards:` — a singleton must name the state it protects, e.g. `guards: engram_resolve_data_dir()` (spec 18.2)\"")
}
}
let entries = stmt["entries"]
let n: Int = native_list_len(entries)
+36 -7
View File
@@ -1976,6 +1976,18 @@ fn parse_stmt(tokens: [Any], pos: Int) -> Map<String, Any> {
// singleton: "id" process identity. The runtime takes an exclusive
// lock at startup; a SECOND start is refused, loudly,
// instead of two processes sharing one data dir.
// guards: <expr> WHAT that singleton protects: an expression yielding
// the path of the guarded state directory, evaluated at
// startup. MANDATORY with `singleton:`, because a lock
// keyed on a program's NAME rather than on its STATE is
// not a guard measured 2026-08-16, the name-keyed
// version refused unrelated instances (different data
// dirs) AND permitted concurrent ones (same data dir,
// different $TMPDIR). It is an expression and not a
// string so a program can point at the resolver that
// already OWNS the path (§18.4) instead of restating
// its default here, which would give the path two
// owners that can disagree.
// env NAME: T = "d" one configuration entry. Its type and its default
// are declared ONCE, here, and resolved+validated
// before main() body runs.
@@ -1993,6 +2005,8 @@ fn parse_stmt(tokens: [Any], pos: Int) -> Map<String, Any> {
let p = expect(tokens, p, "LBrace")
let singleton = ""
let has_singleton = false
let guards_node = { "expr": "Str", "value": "" }
let has_guards = false
let entries = native_list_empty()
// Entry-scratch declared at loop-body level (not inside the branch) so
// that inner `let` forms compile to assignment rather than a C-scoped
@@ -2048,13 +2062,26 @@ fn parse_stmt(tokens: [Any], pos: Int) -> Map<String, Any> {
"required": erequired
})
} else {
// scalar field: `name: "value"`
let p = expect(tokens, p, "Colon")
let fval = tok_value(tokens, p)
let p = p + 1
if str_eq(fname, "singleton") {
let singleton = fval
let has_singleton = true
if str_eq(fname, "guards") {
// guards: <expr> the STATE the singleton protects.
// Parsed as a full expression, not a string literal, so
// it can name the resolver that owns the path
// (`guards: engram_resolve_data_dir()`) rather than
// duplicating that resolver's default here.
let p = expect(tokens, p, "Colon")
let g_r = parse_expr(tokens, p)
let guards_node = g_r["node"]
let p = g_r["pos"]
let has_guards = true
} else {
// scalar field: `name: "value"`
let p = expect(tokens, p, "Colon")
let fval = tok_value(tokens, p)
let p = p + 1
if str_eq(fname, "singleton") {
let singleton = fval
let has_singleton = true
}
}
}
let k5 = tok_kind(tokens, p)
@@ -2070,6 +2097,8 @@ fn parse_stmt(tokens: [Any], pos: Int) -> Map<String, Any> {
"name": name,
"singleton": singleton,
"has_singleton": has_singleton,
"guards": guards_node,
"has_guards": has_guards,
"entries": entries
}, p)
}
+96 -19
View File
@@ -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);
}
+1 -1
View File
@@ -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 */
+36 -7
View File
@@ -1133,6 +1133,7 @@ The `program` block is where a concern of this shape is declared once and enforc
```
program "engram" {
singleton: "engram"
guards: engram_resolve_data_dir()
env ENGRAM_BIND: String = ":8742"
env GUIDE_PORT: Int = "8771"
env ENGRAM_API_KEY: String required
@@ -1145,24 +1146,50 @@ Grammar:
```ebnf
program_block = "program" string "{" { program_field } "}" ;
program_field = singleton_field | env_field ;
program_field = singleton_field | guards_field | env_field ;
singleton_field = "singleton" ":" string [ "," ] ;
guards_field = "guards" ":" expr [ "," ] ;
env_field = "env" ident ":" type
[ "=" string ] [ "required" ] [ "," ] ;
```
`singleton` and `env` are **not** reserved words. They are read as identifier token values by the block's own parse loop, so they remain usable as ordinary identifiers everywhere else. `program` is the only keyword this section adds.
`singleton`, `guards` and `env` are **not** reserved words. They are read as identifier token values by the block's own parse loop, so they remain usable as ordinary identifiers everywhere else. `program` is the only keyword this section adds.
### 18.2 Process identity — `singleton`
### 18.2 Process identity — `singleton` and `guards`
`singleton: "id"` compiles to an `el_singleton_acquire("id")` call injected as the **first statement of `main()`**, before any user statement runs.
`singleton: "id"` with `guards: <expr>` compiles to `el_singleton_acquire("id", <expr>)`, injected as the **first statement of `main()`**, before any user statement runs. `<expr>` evaluates to the path of the **state** the singleton protects.
The runtime takes an exclusive non-blocking `flock` on `<dir>/el-singleton-<id>.lock`, where `<dir>` is `$EL_SINGLETON_DIR`, else `$TMPDIR`, else `/tmp`. On success it writes its pid and holds the descriptor open for the life of the process. On contention it **refuses to start**: it reports the holder's pid, names the lock file, and exits 1.
**`guards:` is mandatory.** A `singleton:` without one is a compile error. This is not defensive strictness; it is the correction of a defect measured in this tree on 2026-08-16, and the rule the rest of this section exists to state:
Two properties are deliberate:
> **Guard the thing, not the name.** A lock that protects state must be keyed on the state.
- **It is a lock, not a pidfile.** The kernel releases an `flock` when the owning process dies — including on `SIGKILL` and on crash. There is therefore no stale-lock state, and so no "delete the lock file to get unstuck" recovery ritual. Such a ritual would itself be a convention, which is the thing this section exists to remove.
Until that date the lock was `<dir>/el-singleton-<id>.lock` where `<dir>` was `$EL_SINGLETON_DIR`, else `$TMPDIR`, else `/tmp`. It was keyed on the program's **name** and on a temp directory, and it never consulted the state it claimed to protect — while its own refusal message read *"Refusing to start a second instance against the same state."* Measured, it failed in **both** directions:
| Situation | Correct answer | Name-keyed lock gave |
|---|---|---|
| same data dir, same `$TMPDIR` | refuse | refuse ✅ |
| same data dir, different `$TMPDIR` | refuse | **started** ❌ — the two-writer data-loss condition, defeated by one environment variable |
| different data dirs, same `$TMPDIR` | both start | **refused**, naming an unrelated pid ❌ |
| same dir spelled differently, different `$TMPDIR` | refuse | **started** ❌ |
Both failure directions are one error: the identity of a resource had been replaced by a label for it. The false negative is the dangerous one — a guard whose bypass is `TMPDIR=/tmp/other` is not a guard.
**The mechanism.** The lock file lives **inside the guarded directory**: `<state>/.el-singleton-<id>.lock`. The runtime takes an exclusive non-blocking `flock` on it, writes its pid, and holds the descriptor open for the life of the process.
That single placement decision is the whole fix, 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. `$TMPDIR` is not in the key, so there is nothing to change to get past it. `$EL_SINGLETON_DIR` no longer exists.
- **Different directories** ⇒ 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 during the kernel's own path walk, so they contend without this code comparing strings. Path canonicalisation happens only to make the diagnostic name one directory in one spelling; the *decision* never depends on it.
- **An unguardable state** — the directory is missing, or read-only — is a **refusal**, not a fallback. Starting unguarded against the store the guard exists to protect is the failure being removed.
**Why `guards:` is an expression and not a string.** The runtime cannot know, generically, which environment variable holds an arbitrary program's state; and a program whose state path already has an owner must not restate it. The engram's data dir is resolved by `engram_resolve_data_dir()`, which owns both the `$ENGRAM_DATA_DIR` read and the `$HOME/.neuron/engram` fallback (§18.4). Writing `guards: engram_resolve_data_dir()` points the guard at that owner. A `guards:` that took a string would force the path's default to be written down twice, and a guard that resolved the path its own way could end up locking a directory the program never writes to — the same two-owners defect §18.4 exists to prevent.
Three properties are deliberate:
- **It is a lock, not a pidfile.** The kernel releases an `flock` when the owning process dies — including on `SIGKILL` and on crash. There is therefore no stale-lock state, and so no "delete the lock file to get unstuck" recovery ritual. Such a ritual would itself be a convention, which is the thing this section exists to remove. (A lock file left behind inside a copied data directory — `cp -Rc` and friends — is inert: it carries no lock, only a stale pid string that the next holder overwrites.)
- **It reports the holder's pid.** "Already running" is not actionable. A pid is. This is the direct answer to the observed failure where a stale process survived a `pkill` and went on answering probes.
- **The message is true.** It names the state it checked and the lock it failed to take, and it says "the same state" only because the lock it contended for is *in* that state. A diagnostic that asserts a check that did not happen is worse than no diagnostic: it is what let the name-keyed version read as correct for as long as it did.
Refusal is loud and total. It is not a warning, and the program does not continue degraded. This matters more than it looks: today a second engram whose `bind()` fails merely *returns* from `http_serve` — after it has already replayed the WAL and written boot-time backup files — and then exits **0**, indistinguishable from a clean run. `singleton` refuses before the first side effect.
@@ -1184,6 +1211,8 @@ Some values look like configuration and are not. `ENGRAM_DATA_DIR` already has a
The rule: **a variable belongs in the program block when the block would be its only owner.** If a resolver already owns it, leave it there.
This is also why `guards:` (§18.2) takes an expression: it lets the block *reference* the existing owner — `guards: engram_resolve_data_dir()` — rather than become a second one.
`HOME` is likewise not configuration. It is an environment fact, and stays a raw `env()` read.
---