fix(engine): restore multi-turn crisis escalation on the agentic path (P0, closes #129) #130

Merged
tim.lingo merged 2 commits from fix/129-history-amplification into main 2026-08-07 15:54:44 +00:00
Member

P0 SAFETY. Closes #129. Multi-turn crisis escalation has been inert on the agentic path since ff421d3 (2026-08-05). This restores it, and adds the gate that would have caught it.

What was broken

The crisis score has two halves: the message just sent, and the distress accumulated across the conversation. The second half exists for the case where no single message trips the bell on its own — distress that builds over several turns.

ff421d3 correctly moved conversation history to a per-session key via conv_hist_key(session_id). One consumer did not move with it: the agentic path's L1 screen kept reading the anonymous conv_history bucket. The desktop app always mints a session id (DaemonClient.kt:706), so history was always written under session_hist_<id> and that read always returned "". The escalation half scored 0 on every real conversation.

Single-message hard bell was never affected. The plain path (soul.el:398) was always correct.

The comment on that line documented this exact bug being fixed once already, under issue #9. The fix was right then; the rename re-broke it and the comment went on describing a repair that no longer held. A comment is not a gate — which is most of why this PR is bigger than one line.

The fix

One line: state_get("conv_history")state_get(conv_hist_key(session_id)). Same thing the thread-anchoring read thirty lines below it in the same handler already does.

The rest is structure so it cannot happen quietly again:

  • agentic_safety_screen() now owns the two decisions that were inline — which window the screen sees, and the screen call. Inline safety inputs are untestable safety inputs.
  • The call-site comment states the invariant (read window == written window) rather than naming a key that can be renamed out from under it.

Two-leg proof — one variable

Both runs are the same tree and the same command; the only difference is that one line.

result exit
before 3. REGRESSION #129 … FAIL got: soft_bell expected: hard_bell — 8 passed, 1 failed 1
after 9 passed, 0 failed 0

The test also pins the specificity leg (a calm history must NOT escalate), the isolation leg (one session's distress must not score another session), and the anonymous-session fallback, so it cannot pass by hard-belling everything.

Full engine rebuild from these sources is clean: gen-soul-amalgam.sh → 1,164,103 bytes / 1226 inlined bodies (gate wants ≥ 1200); cc-brain.sh → 903,096 bytes, 0 errors. nm confirms T _agentic_safety_screen and T _conv_hist_key in the built binary.

Why there is a test runner in this PR

tests/ has held 14 test programs for months with no way to run them. CI does not run them. The convention printed in their own headers — elc soul.el && ./soul --test tests/x.el — refers to a --test flag the El runtime does not implement. The tests were documentation, not gates. That is how a P0 safety regression shipped with a test directory sitting right there.

scripts/run-el-test.sh compiles and runs one test program, reusing the gen-soul-amalgam.sh discovery (elc emits an extern prototype for a module with a .elh beside it and inlines the bodies when it does not, so a test importing ../chat.el must compile in a scratch tree with headers removed). Scratch copy on purpose — the worktree is shared. It runs under a throwaway HOME so a test can never reach the live engram. Exit status is the gate: these tests print failures and still exit 0, so the runner greps for FAIL lines and for a zero assertion count too.

Two traps it deliberately does not inherit from the older suites, worth knowing before anyone writes another test:

  • let pass_count = pass_count + 1 inside an assert function declares a local that dies with the call. Every existing suite prints "0 passed, 0 failed" regardless of what happened.
  • A test program without a cgi block compiles as a 'utility', which may not reference the self-formation primitives (llm_call_system, llm_vision) that chat.el's agentic loop calls — it fails to build on a capability violation it never triggers at runtime.

Rung reached — stated plainly

BUILT + RUNS (discriminating test). NOT in a DMG, NOT verified in the app a human opens. Neither is claimed here.

Not fixed by this PR

  1. feat/soul-openai-tools-v2 carries the same defect independently at chat.el:2937. It needs this change or a merge before it ships.
  2. The defect class is still invisible to every gate we have. A read of a state key that no producer writes is a silent empty string, not an error. #129 proposes making it a build-time failure; the same class produced three other findings in the 08-06 audit. That is the follow-on, and the compiler-level version of it is Will's call.
  3. Production is a separate exposure. CI compiles the committed dist/soul.c, which was last regenerated 2026-08-03 and is now 8 El-commits behind — it does not contain conv_hist_key at all. Merging this fixes the sources and the desktop build; the cloud engine stays stale until that amalgam is regenerated. Tracked in #111 (with today's measurements) and neuron-ui#209.

🤖 Generated with Claude Code

**P0 SAFETY. Closes #129.** Multi-turn crisis escalation has been inert on the agentic path since `ff421d3` (2026-08-05). This restores it, and adds the gate that would have caught it. ## What was broken The crisis score has two halves: the message just sent, and the distress accumulated across the conversation. The second half exists for the case where no single message trips the bell on its own — distress that builds over several turns. `ff421d3` correctly moved conversation history to a per-session key via `conv_hist_key(session_id)`. One consumer did not move with it: the agentic path's L1 screen kept reading the anonymous `conv_history` bucket. The desktop app always mints a session id (`DaemonClient.kt:706`), so history was always written under `session_hist_<id>` and that read always returned `""`. The escalation half scored 0 on every real conversation. Single-message hard bell was never affected. The plain path (`soul.el:398`) was always correct. **The comment on that line documented this exact bug being fixed once already, under issue #9.** The fix was right then; the rename re-broke it and the comment went on describing a repair that no longer held. A comment is not a gate — which is most of why this PR is bigger than one line. ## The fix One line: `state_get("conv_history")` → `state_get(conv_hist_key(session_id))`. Same thing the thread-anchoring read thirty lines below it in the same handler already does. The rest is structure so it cannot happen quietly again: - **`agentic_safety_screen()`** now owns the two decisions that were inline — which window the screen sees, and the screen call. Inline safety inputs are untestable safety inputs. - The call-site comment states the **invariant** (read window == written window) rather than naming a key that can be renamed out from under it. ## Two-leg proof — one variable Both runs are the same tree and the same command; the only difference is that one line. | | result | exit | |---|---|---| | before | `3. REGRESSION #129 … FAIL got: soft_bell expected: hard_bell` — 8 passed, **1 failed** | 1 | | after | 9 passed, **0 failed** | 0 | The test also pins the specificity leg (a calm history must NOT escalate), the isolation leg (one session's distress must not score another session), and the anonymous-session fallback, so it cannot pass by hard-belling everything. Full engine rebuild from these sources is clean: `gen-soul-amalgam.sh` → 1,164,103 bytes / 1226 inlined bodies (gate wants ≥ 1200); `cc-brain.sh` → 903,096 bytes, 0 errors. `nm` confirms `T _agentic_safety_screen` and `T _conv_hist_key` in the built binary. ## Why there is a test runner in this PR `tests/` has held 14 test programs for months **with no way to run them.** CI does not run them. The convention printed in their own headers — `elc soul.el && ./soul --test tests/x.el` — refers to a `--test` flag the El runtime does not implement. The tests were documentation, not gates. That is how a P0 safety regression shipped with a test directory sitting right there. `scripts/run-el-test.sh` compiles and runs one test program, reusing the `gen-soul-amalgam.sh` discovery (elc emits an extern prototype for a module with a `.elh` beside it and inlines the bodies when it does not, so a test importing `../chat.el` must compile in a scratch tree with headers removed). Scratch copy on purpose — the worktree is shared. It runs under a throwaway `HOME` so a test can never reach the live engram. Exit status is the gate: these tests print failures and still exit 0, so the runner greps for `FAIL` lines and for a zero assertion count too. Two traps it deliberately does not inherit from the older suites, worth knowing before anyone writes another test: - `let pass_count = pass_count + 1` inside an assert function declares a **local** that dies with the call. Every existing suite prints "0 passed, 0 failed" regardless of what happened. - A test program without a `cgi` block compiles as a `'utility'`, which may not reference the self-formation primitives (`llm_call_system`, `llm_vision`) that `chat.el`'s agentic loop calls — it fails to build on a capability violation it never triggers at runtime. ## Rung reached — stated plainly **BUILT + RUNS (discriminating test).** NOT in a DMG, NOT verified in the app a human opens. Neither is claimed here. ## Not fixed by this PR 1. **`feat/soul-openai-tools-v2` carries the same defect independently** at `chat.el:2937`. It needs this change or a merge before it ships. 2. **The defect class is still invisible to every gate we have.** A read of a state key that no producer writes is a silent empty string, not an error. #129 proposes making it a build-time failure; the same class produced three other findings in the 08-06 audit. That is the follow-on, and the compiler-level version of it is Will's call. 3. **Production is a separate exposure.** CI compiles the committed `dist/soul.c`, which was last regenerated 2026-08-03 and is now 8 El-commits behind — it does not contain `conv_hist_key` at all. Merging this fixes the sources and the desktop build; the cloud engine stays stale until that amalgam is regenerated. Tracked in #111 (with today's measurements) and neuron-ui#209. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
tim.lingo changed target branch from fix/liveness-engine-91 to main 2026-08-07 15:54:30 +00:00
tim.lingo added 2 commits 2026-08-07 15:54:31 +00:00
tests/ has held 14 test programs for months with no way to run them. CI does
not run them. The convention printed in their own headers
(`elc soul.el && ./soul --test tests/x.el`) refers to a --test flag the El
runtime does not implement. So the tests were documentation, not gates — which
is how a P0 safety regression shipped with a test directory sitting right
there.

scripts/run-el-test.sh compiles and runs one test program. It reuses the
gen-soul-amalgam.sh discovery: elc emits only an extern prototype for a module
that has a .elh beside it, and inlines the bodies when it does not, so a test
importing ../chat.el must be compiled in a scratch tree with the headers
removed. Scratch copy on purpose — the worktree is shared. It runs the binary
under a throwaway HOME so a test can never reach the live engram.

Exit status is the gate: the El tests print failures and still exit 0, so the
runner greps for FAIL lines and for a zero assertion count as well.

tests/test_history_amplification.el pins the invariant #129 violated: the
window the safety screen READS must be the window conv_history_record WRITES.
Not "must be called conv_history" — must AGREE.

THIS COMMIT IS RED BY DESIGN. On this tree the test fails one assertion:

  3. REGRESSION #129 — agentic screen reads the session's own window
    FAIL: distress history escalates the agentic screen to hard_bell
      got:      soft_bell
      expected: hard_bell
  history amplification tests: 8 passed, 1 failed   (runner exit 1)

The next commit turns it green by changing one line. Two legs, one variable —
that is the whole point of committing the test first.

Two flaws in the older harness that this one does not copy: the idiom
`let pass_count = pass_count + 1` inside an assert function declares a local
that dies with the call, so every existing suite prints "0 passed, 0 failed"
regardless of outcome; and a test program without a `cgi` block compiles as a
'utility', which may not reference the self-formation primitives chat.el's
agentic loop calls — it fails to build on a capability violation it never
triggers at runtime.

Refs #129

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
P0 SAFETY. Closes the regression we introduced in ff421d3 (2026-08-05).

ff421d3 correctly moved conversation history to a per-session key via
conv_hist_key(session_id). One consumer did not move with it: the agentic
path's L1 safety screen kept reading the anonymous "conv_history" bucket. The
desktop app always mints a session id (DaemonClient.kt:706), so history was
always written under session_hist_<id> and that read always returned "".

The half of the crisis score that receives history is the escalation half — the
one that exists for distress building across several turns, where no single
message trips the bell on its own. It scored 0 on every real conversation for
two days. Single-message hard bell was never affected.

The bitter part: the comment that line carried documented this exact bug being
fixed once already, under issue #9. The fix was right then. The rename
re-broke it, and the comment went on describing a repair that no longer held.
A comment is not a gate.

The read now goes through conv_hist_key like every other consumer, including
the plain path at soul.el:398 and the thread-anchoring read thirty lines below
it in this same handler. It is one line. The rest of this commit is structure
so it cannot happen quietly again:

  - agentic_safety_screen() owns the two decisions that were inline — which
    window the screen sees, and the screen call. Inline safety inputs are
    untestable safety inputs; that is what let a rename starve this one with
    nothing failing and nothing logging.
  - the comment above the call site now states the invariant (read window ==
    written window) instead of naming a key that can be renamed out from under
    it.

TWO-LEG PROOF, one variable — the single line state_get("conv_history") ->
state_get(conv_hist_key(session_id)):

  before  scripts/run-el-test.sh tests/test_history_amplification.el
          3. REGRESSION #129 ... FAIL  got: soft_bell  expected: hard_bell
          8 passed, 1 failed          runner exit 1
  after   same command, same tree, that one line changed
          9 passed, 0 failed          runner exit 0

Full engine rebuild from these sources is clean: gen-soul-amalgam.sh ->
1,164,103 bytes / 1226 inlined bodies (gate wants >= 1200), cc-brain.sh ->
903,096 bytes, 0 errors. agentic_safety_screen and conv_hist_key both present
in the built binary (nm: T _agentic_safety_screen, T _conv_hist_key).

Rung reached: BUILT + RUNS (discriminating test). NOT yet in a DMG and not yet
verified in the app a human opens — those are the next two rungs and neither is
claimed here.

Known and NOT fixed by this commit:
  - feat/soul-openai-tools-v2 carries the same defect independently at
    chat.el:2937 and needs the same change or a merge.
  - the defect CLASS (a read of a state key no producer writes) is still
    invisible to every gate we have. Issue #129 proposes making it a build
    error; that is the follow-on.

Closes #129

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tim.lingo merged commit 18714e6142 into main 2026-08-07 15:54:44 +00:00
Sign in to join this conversation.