[Bugfix #1333] afx send: surface Tower's descriptive error message instead of bare NOT_FOUND

codev/projects/bugfix-1333-afx-send-builder-non-spawning-/status.yaml

@@ -0,0 +1,16 @@
+id: bugfix-1333
+title: afx-send-builder-non-spawning-
+protocol: bugfix
+phase: pr
+plan_phases: []
+current_plan_phase: null
+gates:
+  pr:
+    status: pending
+    requested_at: '2026-08-02T04:35:30.554Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-08-02T04:02:14.892Z'
+updated_at: '2026-08-02T04:35:30.554Z'
+pr_ready_for_human: true

codev/state/bugfix-1333_thread.md

@@ -0,0 +1,129 @@
+# bugfix-1333 — afx send drops descriptive NOT_FOUND reason
+
+Issue #1333 (area/tower). BUGFIX protocol, strict mode.
+
+## Investigate (complete)
+
+**Bug:** `afx send architect:<name>` from a builder to a non-spawning architect prints
+only `[error] NOT_FOUND`. The human/agent can't tell "no such architect" from "not
+authorized to address that architect" — both look identical.
+
+**Root cause (verified in code):** Tower *produces* a descriptive message; the client
+*drops* it in favor of the machine code.
+
+1. `tower-messages.ts:229-240` `resolveArchitectByName` returns a helpful `message`:
+   - spoofing: `builder <id> may only address its own spawning architect`
+   - genuinely missing: `Architect '<name>' not found in workspace '<ws>'.`
+2. `tower-routes.ts:1472` `handleSend` serializes BOTH → `{ error: code, message }`. Wire OK.
+3. **`core/src/tower-client.ts:221`** `error = json.error || json.message || text`
+   — prefers `json.error` (the code) over `json.message` (the detail). **← root cause.**
+4. `tower-client.ts:695` `sendMessage` → `{ ok:false, error: result.error }` = `'NOT_FOUND'`.
+5. `send.ts:329,334` `throw new Error(result.error)` → `fatal('NOT_FOUND')` → bare code.
+
+**Fix decision:** single point, `tower-client.ts:221`. When the response carries both a
+machine `error` code and a human `message`, surface `"<message> (<code>)"`; otherwise fall
+back to whichever is present, then `text`. This keeps the machine code visible (issue's
+"keep the machine code available") and keeps the two cases distinguishable (issue's ask).
+
+**Blast radius (checked):**
+- No client-side programmatic comparison of `.error` against code strings — the only
+  `=== 'NOT_FOUND'/'AMBIGUOUS'/'NO_CONTEXT'` checks are server-side on `result.code`.
+- No existing test exercises `request()`'s extraction: core has no tower-client test and
+  no fetch mock; `send.test.ts` mocks `sendMessage` directly; server tests assert on
+  `result.code` and the HTTP response body. All unaffected.
+- Change is strictly MORE informative for every Tower CLI error, not just send.
+
+**Fix chosen at `request()`** (global) rather than `send.ts` (local): the message is already
+collapsed by the time it reaches `send.ts`, so a send-only fix would require plumbing a new
+`message` field through `sendMessage` + `request` (3 files, return-type changes). The
+one-line-family change at the drop site is smaller and benefits every caller.
+
+Scope: ~10 LOC + regression test. Comfortably within BUGFIX.
+
+## Fix (complete)
+
+**Change:** `packages/core/src/tower-client.ts`
+- Extracted a module-private helper `extractTowerError(text)` (the previous
+  inline try/catch extraction was duplicated verbatim at TWO sites — `request()`
+  and `pasteImage()`; both had the identical drop-the-message defect). Per
+  lessons-critical "consolidate duplicates rather than syncing them", one helper
+  now feeds both sites.
+- New logic: when both a machine `error` code and a human `message` are present
+  and distinct → `"<message> (<code>)"`; else `message || code || rawText`.
+  `typeof === 'string'` guards also harden against non-string `error` fields.
+
+**Regression test:** `packages/codev/src/agent-farm/__tests__/bugfix-1333-error-surfacing.test.ts`
+- Lives in the codev package because porch's `test` check = `npm test` =
+  `pnpm --filter @cluesmith/codev test` (codev vitest only; core's own vitest is
+  NOT run by porch). It imports `TowerClient` via the codev re-export → built
+  core dist → the exact artifact the CLI consumes. So `npm run build` (rebuilds
+  core first) must precede `npm test`; porch runs them in that order.
+- 6 cases: the #1333 spoofing scenario via `sendMessage`; spoofing-vs-missing
+  distinguishability; + 4 backward-compat guards (code-only, message-only,
+  equal code==message, non-JSON body).
+
+**Proven fails-without / passes-with:** stashed the core source, rebuilt, re-ran
+→ cases 1 & 2 fail with `Received: "NOT_FOUND"` (the exact bug); 4 guards still
+pass. Restored + rebuilt → all 6 pass.
+
+Full build + full codev suite: 4047 passed, 48 skipped, 0 failed. porch check ✓.
+
+## PR (in progress)
+
+Two atomic commits (Fix, Test). Pushed to `origin` (cluesmith/codev).
+**PR #1334** → https://github.com/cluesmith/codev/pull/1334 (base main, "Fixes #1333").
+Remote topology: origin = cluesmith/codev (PR target), fork = mohidmakhdoomi/codev.
+
+CMAP (gemini/codex/claude, --protocol bugfix --type pr) run in background.
+NOTE: `consult` couldn't auto-resolve the project in this worktree — the
+disambiguator regex expects `.builders/<digits>-<suffix>` but our worktree is
+`.builders/bugfix-1333` (no suffix after the digits), so it errored "Multiple
+projects found". Worked around with `--project-id 1333` (matches `bugfix-1333-…`
+via the `bugfix-` branch in consult/index.ts:285). Possible separate bug.
+
+⚠️ BASE-SCOPE FLAG (for architect, not mine to fix): this branch was spawned
+from an architect HEAD that is 2 commits ahead of origin/main — `fef6bddf
+[Spec 1313] Initial specification draft` sits BELOW the porch-init commit. So
+`origin/main...HEAD` includes `codev/specs/1313-afx-send-mailbox-first-delivery.md`
+(+240 lines) — unrelated to #1333. Merging PR #1334 with --merge would land that
+spec in main too. Not rewriting porch-tracked history unilaterally; flagged the
+architect to decide (drop via rebase onto origin/main, or intentionally land it).
+My own diff is exactly: tower-client.ts, the new test, the thread, + porch's
+status.yaml.
+
+### CMAP verdicts (PR #1334)
+- **gemini = REQUEST_CHANGES** (HIGH): code + tests "excellent"; sole blocker =
+  the unrelated Spec 1313 file.
+- **codex = REQUEST_CHANGES** (HIGH): implementation "correct", tests "sound";
+  sole blocker = same Spec 1313 file.
+- **claude = APPROVE** (HIGH): independently verified the root-cause chain +
+  blast radius against source (confirmed no client-side `result.error===CODE`
+  consumers; the `.error?.includes` sites at tower-routes 562/621/1923/2580 are
+  server-side; typeof guards are genuine hardening). Spec 1313 flagged
+  NON-blocking.
+
+**Unanimous on the code: fix + test are correct and well-scoped.** The only
+blocker is the inherited Spec 1313 file. Key de-risking fact (from claude):
+`fef6bddf` is ALSO an ancestor of open PR #1330 (builder/spir-1313), so dropping
+it here is LOSSLESS — the spec still lands via #1330.
+
+### Escalated to architect (queued via mailbox, main busy)
+Asked for go-ahead before `git rebase --onto origin/main fef6bddf
+builder/bugfix-1333` + force-with-lease — because that force-pushes already-pushed
+history on a shared branch (outward-facing / hard-to-reverse → confirm first).
+HOLDING the porch `pr`-gate request (`porch done`) until the base-scope question
+is resolved. Awaiting architect decision: rebase-to-clean vs. merge-as-is.
+
+### Architect approved rebase-to-clean (2026-08-02)
+Verified lossless (fef6bddf ⊂ PR #1330) + conflict-free (tower-client.ts untouched
+on origin/main across 05b2f766..3f622fe6; test is a clean add). Executed:
+1. committed the thread update (clean tree for rebase)
+2. `git fetch origin` → origin/main = 3f622fe6
+3. `git rebase --onto origin/main fef6bddf builder/bugfix-1333` → 6 commits
+   replayed cleanly, NO conflicts, fef6bddf dropped
+4. `git push --force-with-lease` → e26ce59a…2164b4bc (forced)
+Pre-rebase HEAD saved: scratchpad/pre-rebase-head.txt (e26ce59a) for recovery.
+
+**PR #1334 now clean** — 4 files, no spec: status.yaml, thread, the test,
+tower-client.ts (+249/-14). Re-verifying build+test on the rebased branch (base
+moved), then re-request the `pr` gate.

packages/codev/src/agent-farm/__tests__/bugfix-1333-error-surfacing.test.ts

@@ -0,0 +1,93 @@
+/**
+ * Regression test for #1333 — `afx send` surfaced a bare `[error] NOT_FOUND`,
+ * dropping the descriptive reason Tower produced. A builder addressing a
+ * non-spawning architect (`afx send architect:<name>`) could not tell
+ * "no such architect" from "not authorized to address that architect": both
+ * showed the same opaque code.
+ *
+ * Root cause: TowerClient.request() extracted `json.error || json.message`,
+ * preferring the machine code and discarding the human `message`. The fix
+ * (packages/core/src/tower-client.ts) surfaces the descriptive `message` with
+ * the code as a parenthetical suffix, keeping both available.
+ *
+ * These tests exercise TowerClient via the codev re-export (`../lib/tower-client`)
+ * against a stubbed fetch, so they run under codev's vitest — the suite porch's
+ * `test` check runs — and validate the built core artifact the CLI consumes.
+ */
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { TowerClient } from '../lib/tower-client.js';
+
+/** Minimal fetch Response stand-in for an error body (JSON object or raw text). */
+function errorResponse(status: number, body: unknown): Response {
+  const text = typeof body === 'string' ? body : JSON.stringify(body);
+  return {
+    ok: false,
+    status,
+    text: async () => text,
+  } as unknown as Response;
+}
+
+function stubFetch(response: Response): void {
+  vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response));
+}
+
+/** getAuthKey override keeps the constructor from touching the local key file. */
+function client(): TowerClient {
+  return new TowerClient({ getAuthKey: () => null });
+}
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+// The two distinct NOT_FOUND reasons Tower generates for `architect:<name>`.
+const SPOOFING = 'builder bugfix-1333 may only address its own spawning architect';
+const MISSING = "Architect 'codex-architect' not found in workspace 'codev'.";
+
+describe('TowerClient error surfacing (#1333)', () => {
+  it('sendMessage surfaces the descriptive spoofing reason with the code, not a bare NOT_FOUND', async () => {
+    stubFetch(errorResponse(404, { error: 'NOT_FOUND', message: SPOOFING }));
+
+    const result = await client().sendMessage('architect:codex-architect', 'hi', {
+      from: 'bugfix-1333',
+    });
+
+    expect(result.ok).toBe(false);
+    // Pre-fix this was the bare code 'NOT_FOUND'; the reason is now preserved.
+    expect(result.error).toBe(`${SPOOFING} (NOT_FOUND)`);
+  });
+
+  it('keeps the spoofing and genuinely-missing NOT_FOUND reasons distinguishable', async () => {
+    stubFetch(errorResponse(404, { error: 'NOT_FOUND', message: MISSING }));
+
+    const result = await client().request('/api/send', { method: 'POST' });
+
+    expect(result.error).toBe(`${MISSING} (NOT_FOUND)`);
+    // The two NOT_FOUND cases no longer collapse to the same opaque string.
+    expect(result.error).not.toContain('may only address');
+  });
+
+  it('falls back to the bare code when the response carries no message', async () => {
+    stubFetch(errorResponse(503, { error: 'STARTING_UP' }));
+    const result = await client().request('/health');
+    expect(result.error).toBe('STARTING_UP');
+  });
+
+  it('uses the message alone when the response carries no code', async () => {
+    stubFetch(errorResponse(500, { message: 'something broke' }));
+    const result = await client().request('/x');
+    expect(result.error).toBe('something broke');
+  });
+
+  it('does not duplicate when code and message are identical', async () => {
+    stubFetch(errorResponse(500, { error: 'boom', message: 'boom' }));
+    const result = await client().request('/x');
+    expect(result.error).toBe('boom');
+  });
+
+  it('falls back to the raw body for a non-JSON error response', async () => {
+    stubFetch(errorResponse(502, 'Bad Gateway'));
+    const result = await client().request('/x');
+    expect(result.error).toBe('Bad Gateway');
+  });
+});

packages/core/src/tower-client.ts

@@ -179,6 +179,31 @@ export interface TowerClientOptions {
 
 import { encodeWorkspacePath } from './workspace.js';
 
+/**
+ * Extract a human-facing error string from a Tower error response body (#1333).
+ *
+ * Tower error responses carry a machine `error` code and, for many cases, a
+ * human-readable `message` explaining *why* (e.g. the builder spoofing guard:
+ * "builder <id> may only address its own spawning architect"). The previous
+ * extraction preferred the bare code and discarded the message, so the CLI
+ * surfaced an opaque `NOT_FOUND` with no reason. Surface the descriptive
+ * message when present, keeping the code as a parenthetical suffix so both the
+ * human reason and the machine code reach the caller. Falls back to the code
+ * alone, then to the raw (non-JSON) body text.
+ */
+function extractTowerError(text: string): string {
+  try {
+    const json = JSON.parse(text) as { error?: unknown; message?: unknown };
+    const code = typeof json.error === 'string' ? json.error : undefined;
+    const detail = typeof json.message === 'string' ? json.message : undefined;
+    return detail && code && detail !== code
+      ? `${detail} (${code})`
+      : detail || code || text;
+  } catch {
+    return text;
+  }
+}
+
 export class TowerClient {
   private readonly baseUrl: string;
   private readonly getAuthKey: () => string | null;
@@ -215,13 +240,7 @@ export class TowerClient {
 
       if (!response.ok) {
         const text = await response.text();
-        let error: string;
-        try {
-          const json = JSON.parse(text);
-          error = json.error || json.message || text;
-        } catch {
-          error = text;
-        }
+        const error = extractTowerError(text);
         return { ok: false, status: response.status, error };
       }
 
@@ -624,13 +643,7 @@ export class TowerClient {
       });
       if (!response.ok) {
         const text = await response.text();
-        let error: string;
-        try {
-          const json = JSON.parse(text);
-          error = json.error || json.message || text;
-        } catch {
-          error = text;
-        }
+        const error = extractTowerError(text);
         return { ok: false, error };
       }
       const data = (await response.json()) as { path: string };