Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers that PR #286 wired but never wrote #289

Merged

toasterson merged 4 commits from claude/wi-019fce4c-goal-crud-over-the-acp-control-plane-imp into main 2026-08-07 17:10:09 +00:00

Conversation 2 Commits 4 Files changed 5 +1094 -25

toasterson commented 2026-08-04 23:16:21 +00:00

Owner

Copy link

Anima work item 019fce4c-d90f-7311-aa6c-50dd754f9f29.

Part 1 of 2, split from 019e480e-44e8-7213-b072-bdfacd98af68 (which is cancelled).
Part 2 is entity schema exposure — a separate work item. They share nothing but the
MCP/control-plane registration surface. Do not do Part 2 here.

⚠ Read this first: two traps that have already cost 10 sessions

1. The old work item's auto-generated triage plan pointed at the WRONG REPOSITORY. It
listed proto/anima/v1/agent.proto, crates/anima-core/…, crates/anima-db/…,
crates/anima-server/…, crates/anima-agent/…. Those are Anima paths. This is
akh-medu, a separate repo with its own crates, and anima-agent/anima-ai are retired
in Anima anyway (ADR 0018). That mis-generation is a plausible cause of the repeated
failures. Ignore it; work from the real akhomed surface in this repo.

2. If you hit the same wall twice, STOP and report it rather than burning turns. The
predecessor looped through 9 failed sessions before being parked.

Where the work already is

PR #286 (branch claude/wi-019e480e-…, head afa6b196) added the dispatch wiring and
nothing else. It does not compile.

src/acp/control.rs — the five method names are registered:

"_akh/goal/list", "_akh/goal/create", "_akh/goal/update",
"_akh/goal/cancel", "_akh/goal/prioritize",

src/bin/akhomed.rs — five match arms dispatch to handlers that do not exist:

"goal/list"       => self.goal_list(params).await,
"goal/create"     => self.goal_create(params).await,
"goal/update"     => self.goal_update(params).await,
"goal/cancel"     => self.goal_cancel(params).await,
"goal/prioritize" => self.goal_prioritize(params).await,

$ grep -cE "async fn goal_(list|create|update|cancel|prioritize)" src/bin/akhomed.rs
0

Five call sites, zero definitions. Reproducing that wiring is trivial and is written
above; the work is the handlers. Its own comment states the intended shape: "Each handler
resolves the workspace engine, initializes AgentPredicates, and calls the corresponding
goal operation" backed by src/agent/goal.rs.

What to build

Implement the five handlers against the goal domain in src/agent/goal.rs:

• goal/create — {workspace, goal_text, criteria_text, priority, parent_goal_id?},
  creates a goal with explicit criteria.
• goal/update — {workspace, goal_id, goal_text?, criteria_text?, priority?, status?}.
• goal/cancel — {workspace, goal_id}, a soft-delete (mark cancelled/archived, do
  not hard-delete).
• goal/prioritize — {workspace, goal_ids: [...], priorities: [...]}, batch
  re-prioritise.
• goal/list — extend the existing read to include status, created_at,
  cycles_spent, triples_created, and the parent_goal chain.

Lifecycle, exactly as specified — it is deliberately richer than Anima's phase axis and is
unrelated to it, so do not try to align the two:

proposed → active → dormant → completed
                  → cancelled
                  → suspended

Verify before you trust the description

It was written 2026-05-21. Check the live tool surface first — what agent_goals and
list_workspaces actually expose today may differ. The motivating numbers ("16 active
goals, 53 total, 44 cancelled", observed 2026-05-20) are stale; re-measure if any cleanup
semantics depend on them.

Acceptance

• cargo build --features server succeeds — i.e. every dispatched handler exists.
• Each of the five verbs is reachable over the ACP control plane and returns a sane result
  against a real workspace.
• goal/cancel soft-deletes: the goal is still retrievable with status = cancelled.
• Cancelling a goal does not wedge an in-flight OODA loop.
• Tests covering create → update → cancel and the batch prioritise path.

Anima work item `019fce4c-d90f-7311-aa6c-50dd754f9f29`. Part 1 of 2, split from `019e480e-44e8-7213-b072-bdfacd98af68` (which is cancelled). Part 2 is `entity schema exposure` — a separate work item. They share nothing but the MCP/control-plane registration surface. **Do not do Part 2 here.** ## ⚠ Read this first: two traps that have already cost 10 sessions **1. The old work item's auto-generated triage plan pointed at the WRONG REPOSITORY.** It listed `proto/anima/v1/agent.proto`, `crates/anima-core/…`, `crates/anima-db/…`, `crates/anima-server/…`, `crates/anima-agent/…`. Those are **Anima** paths. This is **akh-medu**, a separate repo with its own crates, and `anima-agent`/`anima-ai` are retired in Anima anyway (ADR 0018). That mis-generation is a plausible cause of the repeated failures. Ignore it; work from the real akhomed surface in this repo. **2. If you hit the same wall twice, STOP and report it** rather than burning turns. The predecessor looped through 9 failed sessions before being parked. ## Where the work already is PR #286 (branch `claude/wi-019e480e-…`, head `afa6b196`) added the **dispatch wiring and nothing else**. It does not compile. `src/acp/control.rs` — the five method names are registered: ``` "_akh/goal/list", "_akh/goal/create", "_akh/goal/update", "_akh/goal/cancel", "_akh/goal/prioritize", ``` `src/bin/akhomed.rs` — five match arms dispatch to handlers that do not exist: ```rust "goal/list" => self.goal_list(params).await, "goal/create" => self.goal_create(params).await, "goal/update" => self.goal_update(params).await, "goal/cancel" => self.goal_cancel(params).await, "goal/prioritize" => self.goal_prioritize(params).await, ``` ``` $ grep -cE "async fn goal_(list|create|update|cancel|prioritize)" src/bin/akhomed.rs 0 ``` **Five call sites, zero definitions.** Reproducing that wiring is trivial and is written above; the work is the handlers. Its own comment states the intended shape: *"Each handler resolves the workspace engine, initializes `AgentPredicates`, and calls the corresponding goal operation"* backed by `src/agent/goal.rs`. ## What to build Implement the five handlers against the goal domain in `src/agent/goal.rs`: - **`goal/create`** — `{workspace, goal_text, criteria_text, priority, parent_goal_id?}`, creates a goal with explicit criteria. - **`goal/update`** — `{workspace, goal_id, goal_text?, criteria_text?, priority?, status?}`. - **`goal/cancel`** — `{workspace, goal_id}`, a soft-delete (mark cancelled/archived, do not hard-delete). - **`goal/prioritize`** — `{workspace, goal_ids: [...], priorities: [...]}`, batch re-prioritise. - **`goal/list`** — extend the existing read to include `status`, `created_at`, `cycles_spent`, `triples_created`, and the `parent_goal` chain. Lifecycle, exactly as specified — it is deliberately richer than Anima's phase axis and is unrelated to it, so do not try to align the two: ``` proposed → active → dormant → completed → cancelled → suspended ``` ## Verify before you trust the description It was written 2026-05-21. **Check the live tool surface first** — what `agent_goals` and `list_workspaces` actually expose today may differ. The motivating numbers ("16 active goals, 53 total, 44 cancelled", observed 2026-05-20) are stale; re-measure if any cleanup semantics depend on them. ## Acceptance - `cargo build --features server` succeeds — i.e. every dispatched handler exists. - Each of the five verbs is reachable over the ACP control plane and returns a sane result against a real workspace. - `goal/cancel` soft-deletes: the goal is still retrievable with `status = cancelled`. - Cancelling a goal does not wedge an in-flight OODA loop. - Tests covering create → update → cancel and the batch prioritise path.

toasterson added 1 commit 2026-08-04 23:16:30 +00:00

Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers that PR #286 wired but never wrote ... fc6500ad56

Anima implementation session.

toasterson added 1 commit 2026-08-05 12:38:38 +00:00

Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers that PR #286 wired but never wrote ... dae0c6ef34

Anima implementation session.

toasterson reviewed 2026-08-05 19:11:33 +00:00

toasterson left a comment

Copy link

Review verdict: BLOCK — the handlers write goals into a store the running agent never reads.

Blocker 1 — handlers bypass the live Agent (src/bin/akhomed.rs:1237-1500): all five handlers (goal_create/goal_update/goal_cancel/goal_prioritize/goal_list) call self.get_engine(ws) only, never self.get_agent(ws). Mutations never touch the cached Arc<Mutex<Agent>>'s in-memory goals: Vec<Goal> (src/agent/agent.rs:167), so a running OODA loop operates on stale goal state indefinitely with no re-sync path.

Blocker 2 — two divergent goal stores, last writer wins (src/agent/agent.rs:773-774 + persist_goals at :3705,:3722): the Agent's restore path prefers a bincode blob only the Agent itself writes, not the KG triples these handlers write. The agent's next persist_goals overwrites that blob from its stale in-memory list — silently reverting any control-plane create/update/cancel/prioritize after a snapshot or restart. There is no reconciliation.

Fix direction for 1+2: route the handlers through the Agent (lock get_agent(ws), mutate its goal list via the Agent's own goal APIs, let the Agent persist), or add an explicit invalidate/re-sync so Engine-side writes reach the live Agent before its next persist. One authoritative store, not two.

Blocker 3 — doesn't build on #286: merge-base 2822e95 predates PR #286 (afa6b19, unmerged); this diff independently re-adds identical CONTROL_METHODS entries and dispatch arms in src/acp/control.rs. If #286 lands first this conflicts; rebase onto #286 or subsume/close it explicitly.

Minor 4 — goal_prioritize (akhomed.rs:1415-1462): ? on the first bad goal_id mid-batch leaves earlier KG writes committed with no rollback — validate all ids first, or report per-item results.

Minor 5: handlers flatten AgentError diagnostics into format! strings (pre-existing ServerControl signature limitation, but it compounds).

Minor 6: no test exercises the Postgres/WAL path (temp_state() has no PG pool, wire_wal no-ops), so the persistence claim is untested.

**Review verdict: BLOCK — the handlers write goals into a store the running agent never reads.** **Blocker 1 — handlers bypass the live Agent (`src/bin/akhomed.rs:1237-1500`):** all five handlers (`goal_create`/`goal_update`/`goal_cancel`/`goal_prioritize`/`goal_list`) call `self.get_engine(ws)` only, never `self.get_agent(ws)`. Mutations never touch the cached `Arc<Mutex<Agent>>`'s in-memory `goals: Vec<Goal>` (`src/agent/agent.rs:167`), so a running OODA loop operates on stale goal state indefinitely with no re-sync path. **Blocker 2 — two divergent goal stores, last writer wins (`src/agent/agent.rs:773-774` + `persist_goals` at `:3705,:3722`):** the Agent's restore path prefers a bincode blob only the Agent itself writes, not the KG triples these handlers write. The agent's next `persist_goals` overwrites that blob from its stale in-memory list — silently reverting any control-plane create/update/cancel/prioritize after a snapshot or restart. There is no reconciliation. **Fix direction for 1+2:** route the handlers through the Agent (lock `get_agent(ws)`, mutate its goal list via the Agent's own goal APIs, let the Agent persist), or add an explicit invalidate/re-sync so Engine-side writes reach the live Agent before its next persist. One authoritative store, not two. **Blocker 3 — doesn't build on #286:** merge-base `2822e95` predates PR #286 (`afa6b19`, unmerged); this diff independently re-adds identical `CONTROL_METHODS` entries and dispatch arms in `src/acp/control.rs`. If #286 lands first this conflicts; rebase onto #286 or subsume/close it explicitly. **Minor 4 — `goal_prioritize` (`akhomed.rs:1415-1462`):** `?` on the first bad `goal_id` mid-batch leaves earlier KG writes committed with no rollback — validate all ids first, or report per-item results. **Minor 5:** handlers flatten `AgentError` diagnostics into `format!` strings (pre-existing `ServerControl` signature limitation, but it compounds). **Minor 6:** no test exercises the Postgres/WAL path (`temp_state()` has no PG pool, `wire_wal` no-ops), so the persistence claim is untested.

toasterson referenced this pull request from toasterson/Anima 2026-08-06 16:45:40 +00:00

MCP surface wedges permanently while gRPC stays healthy — suspected unbounded outbound Forgejo call under MCP handler serialization (resync_pr_state) #504

toasterson force-pushed claude/wi-019fce4c-goal-crud-over-the-acp-control-plane-imp from dae0c6ef34 to 4950ae4a17 2026-08-06 17:36:54 +00:00 Compare

toasterson reviewed 2026-08-06 18:37:39 +00:00

toasterson left a comment

Copy link

Review verdict: BLOCK (head 4950ae4, round 3, 2026-08-06 — human review after gate-needs-human)

Round-1 blockers (a) and (b) are both still unaddressed:

(a) Live-Agent bypass — UNFIXED. All five handlers in src/bin/akhomed.rs (goal_create/update/cancel/prioritize/list) call get_engine(ws) exclusively; get_agent never appears in any handler body. Agent.goals: Vec<Goal> (src/agent/agent.rs:167) is populated once at construction and never reloaded, so _akh/goal/cancel on a goal a resident Agent is actively working does not stop it — the KG triples change, the live agent's in-memory status doesn't, until process restart.

(b) Dual goal stores — UNFIXED and lossy. persist_goals (src/agent/goal.rs:655) writes the bincode blob that Agent::new/resume (agent.rs:773) treats as primary: restore_goals_from_store(...) short-circuits the KG-triple fallback whenever the blob exists. The handlers write triples only, never the blob — so once any Agent session has persisted the blob, goals created/edited via _akh/goal/* become invisible on the next resume, and a later persist_goals from the resumed (API-blind) agent overwrites and buries them. Round 3's only new code (update_goal_description) adds a third triple-based path without crossing the blob/KG boundary.

Fix shape (either):

1. Route through get_agent(ws) when an agent is resident — mutate live Agent.goals + call persist_goals — falling back to KG-only writes when none is cached; or
2. Collapse to one source of truth: drop the blob priority, always resume goals from KG, and make a resident Agent re-derive its goals view from the engine.
   Plus a regression test that proves a control-plane write is visible to a resident Agent (none of the six new tests construct one via get_agent).

Secondary:

• (c) CONTROL_METHODS + dispatch arms duplicate still-open PR #286's wiring instead of building on it — if #286 lands later this produces duplicate entries; reconcile or close #286 as superseded.
• GoalStatus::from_label silently maps unknown status strings to Pending — a bogus "status" value should be a client error.
• let _ = engine.add_triple(...) swallows write failures in update_goal_description and parent-wiring.

Tests are decent CRUD coverage of the engine-only path but never exercise the live-Agent scenario that is the crux of this WI.

**Review verdict: BLOCK** (head `4950ae4`, round 3, 2026-08-06 — human review after gate-needs-human) Round-1 blockers (a) and (b) are both still unaddressed: **(a) Live-Agent bypass — UNFIXED.** All five handlers in `src/bin/akhomed.rs` (`goal_create/update/cancel/prioritize/list`) call `get_engine(ws)` exclusively; `get_agent` never appears in any handler body. `Agent.goals: Vec<Goal>` (src/agent/agent.rs:167) is populated once at construction and never reloaded, so `_akh/goal/cancel` on a goal a resident Agent is actively working does not stop it — the KG triples change, the live agent's in-memory status doesn't, until process restart. **(b) Dual goal stores — UNFIXED and lossy.** `persist_goals` (src/agent/goal.rs:655) writes the bincode blob that `Agent::new`/resume (agent.rs:773) treats as *primary*: `restore_goals_from_store(...)` short-circuits the KG-triple fallback whenever the blob exists. The handlers write triples only, never the blob — so once any Agent session has persisted the blob, goals created/edited via `_akh/goal/*` become invisible on the next resume, and a later `persist_goals` from the resumed (API-blind) agent overwrites and buries them. Round 3's only new code (`update_goal_description`) adds a third triple-based path without crossing the blob/KG boundary. **Fix shape (either):** 1. Route through `get_agent(ws)` when an agent is resident — mutate live `Agent.goals` + call `persist_goals` — falling back to KG-only writes when none is cached; **or** 2. Collapse to one source of truth: drop the blob priority, always resume goals from KG, and make a resident Agent re-derive its goals view from the engine. Plus a regression test that proves a control-plane write is visible to a resident Agent (none of the six new tests construct one via `get_agent`). **Secondary:** - (c) `CONTROL_METHODS` + dispatch arms duplicate still-open PR #286's wiring instead of building on it — if #286 lands later this produces duplicate entries; reconcile or close #286 as superseded. - `GoalStatus::from_label` silently maps unknown status strings to `Pending` — a bogus `"status"` value should be a client error. - `let _ = engine.add_triple(...)` swallows write failures in `update_goal_description` and parent-wiring. Tests are decent CRUD coverage of the engine-only path but never exercise the live-Agent scenario that is the crux of this WI.

toasterson force-pushed claude/wi-019fce4c-goal-crud-over-the-acp-control-plane-imp from 4950ae4a17 to 1bd8ae1f28 2026-08-06 20:47:37 +00:00 Compare

toasterson added 1 commit 2026-08-07 17:09:00 +00:00

Route goal CRUD writes through the resident Agent, not just the KG ... d5b96fed06

The five _akh/goal/* control-plane handlers only ever called get_engine,
never get_agent, so a goal created/updated/cancelled over ACP landed in
the KG but never touched the running Agent's in-memory `goals: Vec<Goal>`
— invisible to the OODA loop until the process restarted. Worse, since
Agent::resume tries the bincode `agent:goals` blob before falling back to
KG triples, a workspace that had ever called persist_goals would silently
discard any KG-only control-plane write on the next resume.

Fix: add Agent::add_goal_with_parent/cancel_goal and extend Agent::update_goal
(unused outside agent.rs, so its signature was free to grow) with a
`description` parameter. The five handlers now peek for a resident agent via
a new ServerState::resident_agent (read-only cache lookup, no lazy
construction) and, when one exists, mutate Agent.goals directly under its
std::sync::Mutex (no .await held across the lock) and re-serialize the
agent:goals blob via goal::persist_goals so the blob and KG stay coherent.
Only when no agent is cached does a handler fall back to the original
KG-only path. goal_list reads from the resident agent too, so reads and
writes agree within one process.

Also:
- GoalStatus::from_label now returns Option instead of silently defaulting
  unrecognized labels to Pending. The client-facing paths (goal_update's
  fallback branch and the new Agent::update_goal) turn None into a hard
  error (a new AgentError::InvalidGoalStatus, with miette diagnostic) before
  mutating anything; the KG-restore paths keep the old lenient
  unwrap_or(Pending) behavior since there's no caller there to report to.
- Propagate the previously-swallowed `let _ = engine.add_triple/remove_triple`
  results in update_goal_description and goal_create's parent-wiring block
  instead of silently ignoring partial failures.
- Add a regression test (goal_create_cancel_routes_through_resident_agent)
  that forces a resident Agent before any goal call, exercises create+cancel
  through dispatch, then re-reads the same in-memory Agent directly and
  asserts it reflects both. Verified it fails against the bypass by
  temporarily forcing resident_agent to always return None, then restored.

toasterson changed title from WIP: Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers that PR #286 wired but never wrote to Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers that PR #286 wired but never wrote 2026-08-07 17:09:50 +00:00

toasterson merged commit 738a85eaf4 into main 2026-08-07 17:10:09 +00:00

toasterson referenced this pull request from a commit 2026-08-07 17:10:10 +00:00

Goal CRUD over the ACP control plane: implement the five _akh/goal/* handlers (#289)

toasterson deleted branch claude/wi-019fce4c-goal-crud-over-the-acp-control-plane-imp 2026-08-07 17:10:17 +00:00

toasterson referenced this pull request from a commit 2026-08-08 13:46:50 +00:00

Concurrent ACP sessions: per-session SessionContext + Agent Mutex->RwLock, rebased onto main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

toasterson/akh-medu!289

No description provided.