[Spec 1273] Per-session submission lock: an awaited send now means submitted

.codex/skills/afx/SKILL.md

@@ -217,7 +217,20 @@ afx cron enable <name>          # Enable
 afx cron disable <name>         # Disable
 ```
 
-There is NO `afx cron add` — create YAML files in `.af-cron/` directly.
+There is NO `afx cron add` — create YAML files in `.af-cron/` directly:
+
+```yaml
+name: Service Health Check      # required, unique per workspace
+schedule: "*/15 * * * *"        # required, cron expression (or @hourly/@daily/@startup)
+command: ./health-check.sh      # required, run via shell
+message: "Health alert: ${output}"  # required, ${output} = trimmed command output
+condition: "exitCode != 0"      # optional JS expression, see below
+target: architect               # optional, default architect
+timeout: 30                     # optional, seconds, default 30
+enabled: true                   # optional, default true
+```
+
+`condition` is a JavaScript expression with two variables in scope: `output` (string — the command's trimmed output) and `exitCode` (number — 0 on success, the command's exit code on non-zero exit, 124 on timeout, -1 on spawn failure). With a `condition`, the message is delivered exactly when it evaluates truthy — including on failed runs (e.g. `exitCode != 0` alerts when the command fails). Without a `condition`, the message is delivered only when the command exits 0.
 
 ## Other commands
 

codev-skeleton/.codex/skills/afx/SKILL.md

@@ -144,7 +144,20 @@ afx cron enable <name>          # Enable
 afx cron disable <name>         # Disable
 ```
 
-There is NO `afx cron add` — create YAML files in `.af-cron/` directly.
+There is NO `afx cron add` — create YAML files in `.af-cron/` directly:
+
+```yaml
+name: Service Health Check      # required, unique per workspace
+schedule: "*/15 * * * *"        # required, cron expression (or @hourly/@daily/@startup)
+command: ./health-check.sh      # required, run via shell
+message: "Health alert: ${output}"  # required, ${output} = trimmed command output
+condition: "exitCode != 0"      # optional JS expression, see below
+target: architect               # optional, default architect
+timeout: 30                     # optional, seconds, default 30
+enabled: true                   # optional, default true
+```
+
+`condition` is a JavaScript expression with two variables in scope: `output` (string — the command's trimmed output) and `exitCode` (number — 0 on success, the command's exit code on non-zero exit, 124 on timeout, -1 on spawn failure). With a `condition`, the message is delivered exactly when it evaluates truthy — including on failed runs (e.g. `exitCode != 0` alerts when the command fails). Without a `condition`, the message is delivered only when the command exits 0.
 
 ## Other commands
 

codev/reviews/1273-builder-context-reset-should-b.md

@@ -201,6 +201,33 @@ The e2e was the first thing to run reset against reality, and it failed instantl
 the strongest possible restatement of the hot-tier lesson: *"it compiled" / "tests pass" is not "it
 works" — verify the real user path end-to-end.*
 
+### The delivery seam — `/clear` was never executed (2026-08-01)
+
+The live e2e's forensics closed the loop on the headline path. `afx reset`'s `/clear` **never ran**: it
+arrived as literal text welded onto the front of the re-orientation, one merged user turn beginning
+`/clear### [ARCHITECT INSTRUCTION...`. The probe's context was fully intact — it could still recite a
+secret word planted before the "reset" — while every layer reported success.
+
+**Root cause.** `writeMessageToSession` schedules the Enter that submits a message 50–80ms out and
+returns that offset without awaiting it; `/api/send` responded off that. So an awaited send resolved
+*before its own message was submitted*. Reset awaited the `/clear` write, sampled output, then wrote the
+re-orientation — inside the window, into the same composer, ahead of the Enter. One Enter submitted both.
+
+This also retro-explains the phase-6 clear-confirmation defect: at the moment the check sampled output,
+the clear had not been submitted. **Two bugs, one seam.**
+
+**Fix**: a per-session submission lock (`servers/session-submit.ts`), wired to the escape and
+immediate-delivery paths. `await submitToSession(...)` means *submitted*, not *scheduled*.
+
+**Ordering is not atomicity.** `SendBuffer` already serialises within a flush, and Spec 1307's per-session
+FIFO fixes delivery *order*. Neither would have prevented this — the two writes were correctly ordered
+and still coalesced. Being second is not the same as being separate. Worth carrying: the two properties
+look interchangeable in a design discussion and are not.
+
+**Scope, stated because the first framing was overbroad**: the lock covers what takes it — the two
+`/api/send` paths. Buffer flush, cron delivery and the raw write endpoint still write directly; human
+keystrokes deliberately always will.
+
 ## Flaky Tests
 
 None encountered. No tests were skipped by this work.

codev/state/aspir-1273_thread.md

@@ -393,6 +393,60 @@ reviewed 2-way. Reported to the architect.
 
 Remaining: the live reset against the planted-context probe, for the architect to re-run after install.
 
+## The delivery seam — `/clear` was never executed (2026-08-01)
+
+The e2e's forensics closed the loop, and the answer was worse than "the fix was wrong": **the `/clear`
+never ran at all.** It arrived as literal text welded onto the front of the re-orientation — one merged
+user turn beginning `/clear### [ARCHITECT INSTRUCTION...`, no separator. The probe's context was fully
+intact; it could still recite a secret word planted before the "reset". Every layer reported success.
+
+**Root cause.** `writeMessageToSession` schedules the Enter that submits a message 50–80ms out and
+returns that offset *without awaiting it*; `/api/send` responded off that. So an awaited send resolved
+**before its own message was submitted**. Reset awaited the `/clear` write, sampled output, then wrote
+the re-orientation — inside the window, into the same composer, ahead of the Enter. One Enter submitted
+both.
+
+The channel was right. The mistake was treating an awaited send as proof of submission when it only
+proved *scheduling*. The architect's ledger has this as a family now — *an operation that reports success
+at a point earlier than the thing it claims* — and this is its third member.
+
+It also retro-explains phase 6's clear-confirmation defect: at the moment the check sampled output, the
+clear had not been submitted. **Two bugs, one seam.** I spent three CMAP rounds patching the confirmation
+regex when the thing it was checking had not happened yet.
+
+**Ordering is not atomicity.** This is the piece worth carrying furthest. `SendBuffer` already serialises
+within a flush, and 1307's per-session FIFO fixes delivery *order* — and neither would have prevented
+this, because my two writes were correctly ordered and still coalesced. **Being second is not the same as
+being separate.** In a design conversation the two properties sound interchangeable; they are not, and
+the distinction is invisible until something concatenates.
+
+**Fix**: a per-session submission lock (`servers/session-submit.ts`) — a promise chain where each
+submission waits out its own Enter before the next write to that session begins. `await
+submitToSession(...)` means *submitted*.
+
+### Working with a sibling builder
+
+The architect assigned me the primitive and 1307 adopts it. Two things came out of that exchange that
+neither of us would have got alone:
+
+- **They corrected me.** I had claimed reset's own writes bump `_lastInputAt` and so trip their own
+  buffering. False — `recordUserInput()` is called only from the websocket handler. It did not affect the
+  diagnosis, but it was load-bearing in how I was *describing* the failure, which is its own kind of
+  wrong. They told me to verify rather than take their word; I did.
+- **I corrected the scope of my own fix.** They asked whether deleting their `busyUntil` would reopen a
+  mid-flush interleave. Rather than assert the primitive covered it, I checked: the existing signature
+  already supports a whole-batch reservation, and I pinned it with a mutation-verified test so their
+  deletion rests on a failing-on-mutation test rather than my assurance. Their reply — *a test adjacent to
+  the real path is not coverage of it* — is the right line, and they are re-verifying with their own
+  flush test regardless.
+
+Codex's CMAP then caught that my framing of the primitive was overbroad: a lock only serialises writers
+that *take* it. The uncovered writers are now enumerated with reasons, including the one that should
+never be covered — human keystrokes, because the human owns their own composer.
+
+**Still unproven**: that the clear now actually clears, and what a real clear emits. The confirmation
+pattern remains an educated guess until the live re-run.
+
 ## Status
 
 - [x] Explored afx/Tower internals

packages/codev/src/agent-farm/__tests__/spec-1273-submission-lock.test.ts

@@ -0,0 +1,195 @@
+/**
+ * Per-session submission lock (Spec 1273, verify phase).
+ *
+ * The regression under test is the one that reached production: two sends to
+ * one session coalescing into a single submission, because the first send's
+ * Enter was still pending when the second write landed in the composer.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import {
+  submitToSession,
+  resetSubmissionChains,
+  pendingSubmissionSessions,
+  type SubmitClock,
+} from '../servers/session-submit.js';
+
+/**
+ * A composer that models the real failure.
+ *
+ * `write` appends to a pending buffer; `enter` submits whatever has
+ * accumulated. That is the PTY's actual behaviour, and it is why an unawaited
+ * Enter is dangerous: anything written before it joins the same submission.
+ */
+function makeComposer() {
+  let pending = '';
+  const submitted: string[] = [];
+  return {
+    submitted,
+    write(text: string) {
+      pending += text;
+    },
+    enter() {
+      submitted.push(pending);
+      pending = '';
+    },
+  };
+}
+
+/**
+ * Real timers with a short delay.
+ *
+ * A virtual clock proved more fragile than the thing it was testing — the
+ * property here is "the second write does not start until the first Enter has
+ * fired", which real timers demonstrate directly and cheaply.
+ */
+const clock: SubmitClock = { sleep: ms => new Promise(r => setTimeout(r, ms)) };
+
+const ENTER_DELAY = 20;
+
+/**
+ * Yield past the chain's internal microtasks so the in-flight write has run,
+ * while staying well inside ENTER_DELAY so the NEXT one cannot have started.
+ * A bare `Promise.resolve()` is not enough: the chain adds ticks of its own.
+ */
+const afterCurrentWrite = () => new Promise(r => setTimeout(r, 0));
+
+describe('Spec 1273 — submission lock', () => {
+  beforeEach(() => resetSubmissionChains());
+
+  it('keeps two sends to one session as two separate submissions', async () => {
+    // THE PRODUCTION BUG. Without the lock the second write joins the first's
+    // pending text and one Enter submits `/clear## CONTEXT RESET…` as a single
+    // message — exactly what reached the live probe builder.
+    const composer = makeComposer();
+
+    const write = (text: string) => () => {
+      composer.write(text);
+      setTimeout(() => composer.enter(), 0);
+      return ENTER_DELAY;
+    };
+
+    const first = submitToSession('term-1', write('/clear'), clock);
+    const second = submitToSession('term-1', write('## CONTEXT RESET'), clock);
+
+    await first;
+    await second;
+    await new Promise(r => setTimeout(r, 0));
+
+    expect(composer.submitted).toEqual(['/clear', '## CONTEXT RESET']);
+    // The decisive assertion: never welded together.
+    expect(composer.submitted.some(m => m.startsWith('/clear#'))).toBe(false);
+  });
+
+  it('does not let the second write begin before the first has submitted', async () => {
+    const order: string[] = [];
+
+    const first = submitToSession('term-1', () => { order.push('first'); return ENTER_DELAY; }, clock);
+    const second = submitToSession('term-1', () => { order.push('second'); return ENTER_DELAY; }, clock);
+
+    // The first write has run; the second is held behind the pending Enter.
+    await afterCurrentWrite();
+    expect(order).toEqual(['first']);
+
+    await Promise.all([first, second]);
+
+    expect(order).toEqual(['first', 'second']);
+  });
+
+  it('resolves only after the scheduled Enter, not when the write is scheduled', async () => {
+    // `await send(...)` must mean SUBMITTED. Responding on "scheduled" is the
+    // root cause: the HTTP 200 came back ~50ms before the message existed.
+    let resolved = false;
+
+    const submission = submitToSession('term-1', () => ENTER_DELAY, clock).then(() => {
+      resolved = true;
+    });
+
+    await afterCurrentWrite();
+    expect(resolved).toBe(false);
+
+    await submission;
+    expect(resolved).toBe(true);
+  });
+
+  it('does not serialize across different sessions', async () => {
+    // The lock is per session; unrelated terminals must not queue behind a busy
+    // one, or one slow builder would stall messaging workspace-wide.
+    const order: string[] = [];
+
+    const a = submitToSession('term-a', () => { order.push('a'); return ENTER_DELAY; }, clock);
+    const b = submitToSession('term-b', () => { order.push('b'); return ENTER_DELAY; }, clock);
+
+    await afterCurrentWrite();
+    expect([...order].sort()).toEqual(['a', 'b']);
+
+    await Promise.all([a, b]);
+  });
+
+  it('returns immediately when there is no Enter to wait for', async () => {
+    // noEnter writes report 0; waiting on them would stall the chain forever.
+    await expect(submitToSession('term-1', () => 0)).resolves.toBeUndefined();
+  });
+
+  it('a throwing submission does not poison the chain', async () => {
+    // The next message is a separate submission and is still entitled to run.
+    let ran = false;
+
+    const bad = submitToSession('term-1', () => {
+      throw new Error('write failed');
+    }, clock);
+    const good = submitToSession('term-1', () => { ran = true; return ENTER_DELAY; }, clock);
+
+    await expect(bad).rejects.toThrow('write failed');
+    await good;
+    expect(ran).toBe(true);
+  });
+
+  it('reserves the session for a whole BATCH, so a direct send cannot interleave', async () => {
+    // The flush-drain case (raised by aspir-1307 for SendBuffer.busyUntil).
+    //
+    // No API extension is needed: `write` may perform MANY writes and return
+    // the final completion offset, so one submission can cover an entire
+    // buffer flush with its existing offset threading intact. A direct send
+    // arriving mid-drain queues behind the whole batch rather than writing into
+    // a partially-delivered message.
+    //
+    // Without that reservation the interleave is the same shape as the
+    // production failure one layer down: a message landing inside a
+    // half-delivered `/clear` yields `/clear<other>` on one line.
+    const composer = makeComposer();
+
+    const batch = () => {
+      // Mirrors SendBuffer.flush: several paced writes, offsets threaded.
+      composer.write('buffered-1');
+      setTimeout(() => composer.enter(), 0);
+      composer.write('buffered-2');
+      setTimeout(() => composer.enter(), 1);
+      return ENTER_DELAY;
+    };
+
+    const direct = () => {
+      composer.write('/clear');
+      setTimeout(() => composer.enter(), 0);
+      return ENTER_DELAY;
+    };
+
+    const flush = submitToSession('term-1', batch, clock);
+    const send = submitToSession('term-1', direct, clock);
+
+    await Promise.all([flush, send]);
+    await new Promise(r => setTimeout(r, 5));
+
+    // The direct send is never welded into the batch's pending text.
+    expect(composer.submitted.some(m => m.includes('buffered') && m.includes('/clear'))).toBe(
+      false,
+    );
+    expect(composer.submitted).toContain('/clear');
+  });
+
+  it('drains its bookkeeping so a long-lived Tower does not leak', async () => {
+    await submitToSession('term-1', () => 0);
+    await new Promise(r => setTimeout(r, 0));
+    expect(pendingSubmissionSessions()).toBe(0);
+  });
+});

packages/codev/src/agent-farm/__tests__/tower-routes.test.ts

@@ -1555,11 +1555,24 @@ describe('tower-routes', () => {
       const { res } = makeRes();
 
       await handleRequest(req, res, ctx);
-      // Message is written first, then \r is sent separately after a 50ms delay
-      // so the PTY processes the multi-line paste before receiving Enter (Bugfix #492).
-      const writeCalls = mockWrite.mock.calls;
-      expect(writeCalls.length).toBe(1); // Initial write (message only)
-      expect(writeCalls[0][0]).not.toMatch(/\r$/); // No \r in initial write
+      // Message is written first, then \r is sent SEPARATELY after a delay, so
+      // the PTY processes the paste before receiving Enter (Bugfix #492/#481).
+      // That separation is the property this test exists to protect.
+      const writeCalls = mockWrite.mock.calls.map(c => c[0] as string);
+      expect(writeCalls[0]).toContain('hello');
+      expect(writeCalls[0]).not.toMatch(/\r$/); // Enter is never appended
+
+      // UPDATED (Spec 1273 verify): this previously asserted `length === 1` —
+      // i.e. that the route returned BEFORE the Enter was written. That was the
+      // bug, not the contract: an awaited send resolving before its own
+      // submission is how `afx reset` got `/clear` welded onto the front of the
+      // next message and never cleared anything. `/api/send` now awaits the
+      // submission, so by the time the request resolves the Enter HAS landed.
+      //
+      // Asserted as properties rather than an exact count, because the
+      // formatted message may be paced line-by-line (Bugfix #584).
+      expect(writeCalls.length).toBeGreaterThan(1);
+      expect(writeCalls.at(-1)).toBe('\r');
     });
 
     it('delivers message without Enter when noEnter is set (Bugfix #481)', async () => {