Closes#56. Everything below the run loop already existed — this is the thing that runs a model.
The build tag is gone, and doc.go says what's true
internal/agent's doc comment promised two separations: cmd/pansy not importing the package, and the tool wiring behind //go:build majordomo. Both are gone, and the comment is rewritten rather than left as a stale aspiration — the issue was explicit about not leaving it.
A tag that keeps the agent out of the binary only earns its keep if you'd ever ship a build without the agent, and the agent is the point. Keeping it meant an untagged CI that never compiled the code that matters.
majordomo is a real dependency now, resolved from the Gitea instance as a pseudo-version with no replace directive — a replace pointing at ../majordomo builds on my laptop and breaks the Docker build, which has no sibling checkout. It's stdlib-first and pure Go, so CGO_ENABLED=0 and the single static binary survive (19 MB, verified locally; the image build on this PR is the real check).
A turn is one change set
This is the whole reason acting without a confirmation prompt is defensible. "Empty the garlic bed and plant cucumbers" is one object edit and a dozen planting inserts, and it has to undo as one action, not thirteen.
The scope is opened even for a turn that turns out to be a question — a change set with no revisions is never written, so asking costs nothing and history isn't littered with empty entries.
Config
The model spec goes to majordomo.Parseverbatim. That grammar (including comma-separated failover chains) is majordomo's; re-implementing any of it here would just mean two places to update when it grows.
The key needs a bridge: majordomo's ollama-cloud preset reads OLLAMA_API_KEY, while pansy — like gadfly — is configured with OLLAMA_CLOUD_API_KEY. The provider is registered explicitly on a private registry rather than depending on ambient environment, so that mismatch is visible instead of showing up as a mysterious empty token.
Bounded, but not budgeted
Step cap, timeout, and majordomo's loop guards. This is loop safety, not cost control — spend caps are explicitly not a v2 concern.
A capped run does not fail: it keeps what it managed to do, that work is recorded and undoable, and the reply says it stopped early rather than going silent.
Streaming, because silence reads as a hang
A turn that clears a bed and replants it makes a dozen tool calls over tens of seconds. Without SSE that's a long silence followed by everything at once — which defeats a design that rests on watching the canvas change as it happens.
Conversations persist
Per (user, garden). Client-held history would be lost on refresh, which is exactly when someone reloads to check whether the agent's change landed.
Only the user/assistant text is stored, not the full transcript: continuity needs what was said and what came back, and replaying a stored tool call would replay a decision made against a garden that has since moved on. It also keeps majordomo's message shape out of the schema.
Unconfigured instances
No key → the routes aren't registered at all, so it's a 404 rather than a handler that apologizes. A configured-but-unresolvable model logs and disables the assistant rather than refusing to boot: a garden planner that won't start because of a chat feature is worse than one without chat.
Verification — one test per acceptance criterion
Driven by majordomo's scriptable fake provider, so the whole loop runs hermetically:
TestTurnIsOneChangeSet — the flagship: clear + refill produces exactly one change set, source=agent, summarised with the user's own words, and one revert puts the garlic back.
TestViewerGetsAnExplainableRefusal — a viewer's turn is refused up front, and at the tool layer a denial comes back as a readable tool result rather than killing the run.
TestRunStopsAtTheStepCap — a wedged model terminates cleanly, Truncated is set, and the reply says so.
TestReadOnlyTurnWritesNoChangeSet — questions don't litter history.
TestNewRunnerNeedsConfiguration — every unconfigured shape errors, including an unresolvable model spec.
TestAgentRoutesAbsentWithoutAKey — 404s, and the rest of the app is unaffected.
go test ./... green with no build tag. go vet, gofmt, and a CGO_ENABLED=0 static build all clean.
Closes #56. Everything below the run loop already existed — this is the thing that runs a model.
### The build tag is gone, and `doc.go` says what's true
`internal/agent`'s doc comment promised two separations: `cmd/pansy` not importing the package, and the tool wiring behind `//go:build majordomo`. **Both are gone, and the comment is rewritten rather than left as a stale aspiration** — the issue was explicit about not leaving it.
A tag that keeps the agent out of the binary only earns its keep if you'd ever ship a build *without* the agent, and the agent is the point. Keeping it meant an untagged CI that never compiled the code that matters.
majordomo is a real dependency now, resolved from the Gitea instance as a pseudo-version **with no `replace` directive** — a `replace` pointing at `../majordomo` builds on my laptop and breaks the Docker build, which has no sibling checkout. It's stdlib-first and pure Go, so `CGO_ENABLED=0` and the single static binary survive (19 MB, verified locally; the image build on this PR is the real check).
### A turn is one change set
This is the whole reason acting without a confirmation prompt is defensible. *"Empty the garlic bed and plant cucumbers"* is one object edit and a dozen planting inserts, and it has to undo as **one** action, not thirteen.
The scope is opened even for a turn that turns out to be a question — a change set with no revisions is never written, so asking costs nothing and history isn't littered with empty entries.
### Config
The model spec goes to `majordomo.Parse` **verbatim**. That grammar (including comma-separated failover chains) is majordomo's; re-implementing any of it here would just mean two places to update when it grows.
The key needs a bridge: majordomo's `ollama-cloud` preset reads `OLLAMA_API_KEY`, while pansy — like gadfly — is configured with `OLLAMA_CLOUD_API_KEY`. The provider is registered explicitly on a private registry rather than depending on ambient environment, so that mismatch is visible instead of showing up as a mysterious empty token.
### Bounded, but not budgeted
Step cap, timeout, and majordomo's loop guards. **This is loop safety, not cost control** — spend caps are explicitly not a v2 concern.
A capped run does *not* fail: it keeps what it managed to do, that work is recorded and undoable, and the reply says it stopped early rather than going silent.
### Streaming, because silence reads as a hang
A turn that clears a bed and replants it makes a dozen tool calls over tens of seconds. Without SSE that's a long silence followed by everything at once — which defeats a design that rests on watching the canvas change as it happens.
### Conversations persist
Per (user, garden). Client-held history would be lost on refresh, which is *exactly* when someone reloads to check whether the agent's change landed.
Only the user/assistant **text** is stored, not the full transcript: continuity needs what was said and what came back, and replaying a stored tool call would replay a decision made against a garden that has since moved on. It also keeps majordomo's message shape out of the schema.
### Unconfigured instances
No key → the routes aren't registered at all, so it's a 404 rather than a handler that apologizes. A configured-but-*unresolvable* model logs and disables the assistant rather than refusing to boot: a garden planner that won't start because of a chat feature is worse than one without chat.
### Verification — one test per acceptance criterion
Driven by majordomo's scriptable fake provider, so the whole loop runs hermetically:
- **`TestTurnIsOneChangeSet`** — the flagship: clear + refill produces **exactly one** change set, `source=agent`, summarised with the user's own words, and one revert puts the garlic back.
- **`TestViewerGetsAnExplainableRefusal`** — a viewer's turn is refused up front, and at the tool layer a denial comes back as a readable tool *result* rather than killing the run.
- **`TestRunStopsAtTheStepCap`** — a wedged model terminates cleanly, `Truncated` is set, and the reply says so.
- **`TestReadOnlyTurnWritesNoChangeSet`** — questions don't litter history.
- **`TestNewRunnerNeedsConfiguration`** — every unconfigured shape errors, including an unresolvable model spec.
- **`TestAgentRoutesAbsentWithoutAKey`** — 404s, and the rest of the app is unaffected.
`go test ./...` green **with no build tag**. `go vet`, `gofmt`, and a `CGO_ENABLED=0` static build all clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
Everything below the run loop already existed. This is the thing that runs a
model.
The build tag is gone, deliberately. internal/agent's doc comment promised two
separations — cmd/pansy not importing the package, and the tool wiring behind
//go:build majordomo — and both have been rewritten rather than left as a stale
aspiration. A tag that keeps the agent out of the binary only earns its keep if
you would ever ship a build without the agent, and the agent is the point;
keeping it meant an untagged CI that never compiled the code that matters.
majordomo is a real dependency now, resolved from the Gitea instance as a
pseudo-version with no replace directive, so the Docker build (which has no
sibling checkout) resolves it the same way this machine does. It is stdlib-first
and pure Go, so CGO_ENABLED=0 and the single static binary survive.
A TURN IS ONE CHANGE SET. That is the whole reason acting without a confirmation
prompt is defensible: "empty the garlic bed and plant cucumbers" is one object
edit and a dozen planting inserts, and it has to undo as one action rather than
thirteen. The scope is opened even for a turn that turns out to be a question,
because a change set with no revisions is never written — so asking costs
nothing and history isn't littered with empty entries.
The model spec goes to majordomo.Parse verbatim. That grammar, including
comma-separated failover chains, is majordomo's; re-implementing any of it here
would only mean two places to update when it grows. The key needs a bridge
though: majordomo's ollama-cloud preset reads OLLAMA_API_KEY while pansy (like
gadfly) is configured with OLLAMA_CLOUD_API_KEY, so the provider is registered
explicitly on a private registry rather than depending on ambient environment.
Runs are bounded by a step cap, a timeout and majordomo's loop guards. This is
loop safety, not cost control — pansy is a personal tool and spend caps are
explicitly not a v2 concern. A capped run does NOT fail: it kept whatever it
managed to do, that work is recorded and undoable, and the reply says it stopped
early rather than going silent.
The chat endpoint streams. A turn that clears a bed and replants it makes a
dozen tool calls over tens of seconds, and without streaming that is a long
silence followed by everything at once — which reads as a hang, and defeats a
design that rests on watching the canvas change as it happens.
Conversations persist per (user, garden). Client-held history would be lost on a
refresh, which is exactly when someone reloads to check whether the agent's
change landed. Only the user/assistant TEXT is stored, not the model's full
transcript: continuity needs what was said and what came back, and replaying a
stored tool call would replay a decision made against a garden that has since
moved on. It also keeps majordomo's message shape out of the schema.
An instance with no key starts, serves the app, and doesn't advertise the agent
— the routes aren't registered at all, the same shape as OIDC 404ing when
unconfigured. A configured-but-unresolvable model logs and disables the
assistant rather than refusing to boot: a garden planner that won't start
because of a chat feature is worse than one without chat.
Tool refusals reach the model as tool results it can explain, not 500s. The ACL
story only works if it can narrate the refusal.
Closes#56
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
The first finding breaks this PR's central promise and is the reason the review
was worth running.
The run context carries a timeout. When it fired, WithChangeSet's recovery path
tried to record what had already committed using that SAME dead context — which
fails, losing the history for changes that really happened. So a timed-out turn
left its partial work un-undoable, while the user-facing message cheerfully said
"anything I'd already changed is in History". That message was a lie in exactly
the case it was written for.
Both recovery paths (WithChangeSet and RevertChangeSet) now commit with
context.WithoutCancel. The commonest reason those paths run at all is a
cancelled or timed-out context, so using it to write the record of what it did
was self-defeating. There's a test that cancels mid-turn and asserts the partial
work is recorded, marked partial, and revertible.
turnSummary sliced bytes, so a message whose 120th byte fell inside a multibyte
character stored invalid UTF-8 in the history summary. Not hypothetical for text
people type. Trimmed by runes now, with a test using emoji.
RecordAgentExchange's failure was logged and swallowed, directly under a comment
claiming the user would see that their turn wasn't saved. The turn itself
succeeded, so Done still goes out — but with a warning saying the exchange
wasn't saved and won't survive a reload, because a clean "done" followed by a
conversation that has forgotten it is the quieter lie. It also now records with
a detached context, since the commonest reason that write fails is the client
having gone away, and the exchange is worth keeping either way.
AgentRunID was declared, documented as an executus join, and never set by
anything. It's set now, from a per-run id that's also logged at run start, so a
row in the history list has a thread back to the run that produced it — and the
comment describes that rather than a dependency this repo doesn't have.
Turn.History was populated and never read, left over from the client-held
history design that persistence replaced. Removed.
The SSE plumbing moved out of the handler into openEventStream, so agentChat
reads like the other decode/call/encode handlers in the package.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
All fixed in d7c4e1a. The first finding breaks this PR's central promise and is the reason running the review was worth it.
A timed-out turn left its work un-undoable
The run context carries a timeout. When it fired, WithChangeSet's recovery path tried to record what had already committed using that same dead context — which fails. So a timed-out turn left its partial work with no change set, while the user-facing message cheerfully said "anything I'd already changed is on the canvas, and in History."
That message was a lie in exactly the case it was written for. The one guarantee this whole feature rests on — a turn is one undoable change set — silently didn't hold on timeout, which is the failure mode most likely to actually happen in production.
Both recovery paths (WithChangeSet and RevertChangeSet) now commit with context.WithoutCancel. The commonest reason those paths run at all is a cancelled or timed-out context, so using it to write the record of what it did was self-defeating. There's a test that cancels mid-turn and asserts the partial work is recorded, marked partial, and revertible.
turnSummary sliced bytes
A message whose 120th byte fell inside a multibyte character stored invalid UTF-8 in the history summary. Not hypothetical for text people type. Trimmed by runes now, with an emoji test.
The comment promised something the code didn't do
RecordAgentExchange's failure was logged and swallowed, directly beneath a comment claiming the user would see that their turn wasn't saved. The turn itself succeeded, so Done still goes out — but now with a warning saying the exchange wasn't saved and won't survive a reload. A clean "done" followed by a conversation that has forgotten it is the quieter lie. It also records with a detached context, since the commonest reason that write fails is the client having gone away, and the exchange is worth keeping either way.
The rest
AgentRunID was declared, documented as an executus join, and never set. It's set now from a per-run id that's also logged at run start, so a row in the history list has a thread back to the run that produced it — and the comment describes that rather than a dependency this repo has just disclaimed.
Turn.History was populated and never read — left over from the client-held-history design that persistence replaced. Removed.
agentChat was a 53-line outlier. SSE plumbing moved into openEventStream, so it reads like the other decode/call/encode handlers.
go test ./... green with no build tag, go vet, gofmt, CGO_ENABLED=0 static build clean. The image build on this branch passed, which confirms the Docker stage resolves majordomo from Gitea without a replace.
All fixed in `d7c4e1a`. The first finding breaks this PR's central promise and is the reason running the review was worth it.
### A timed-out turn left its work un-undoable
The run context carries a timeout. When it fired, `WithChangeSet`'s recovery path tried to record what had already committed **using that same dead context** — which fails. So a timed-out turn left its partial work with no change set, while the user-facing message cheerfully said *"anything I'd already changed is on the canvas, and in History."*
That message was a lie **in exactly the case it was written for.** The one guarantee this whole feature rests on — a turn is one undoable change set — silently didn't hold on timeout, which is the failure mode most likely to actually happen in production.
Both recovery paths (`WithChangeSet` and `RevertChangeSet`) now commit with `context.WithoutCancel`. The commonest reason those paths run *at all* is a cancelled or timed-out context, so using it to write the record of what it did was self-defeating. There's a test that cancels mid-turn and asserts the partial work is recorded, marked partial, and revertible.
### `turnSummary` sliced bytes
A message whose 120th byte fell inside a multibyte character stored **invalid UTF-8** in the history summary. Not hypothetical for text people type. Trimmed by runes now, with an emoji test.
### The comment promised something the code didn't do
`RecordAgentExchange`'s failure was logged and swallowed, directly beneath a comment claiming the user would see that their turn wasn't saved. The turn itself succeeded, so `Done` still goes out — but now with a warning saying the exchange wasn't saved and won't survive a reload. A clean "done" followed by a conversation that has forgotten it is the quieter lie. It also records with a detached context, since the commonest reason *that* write fails is the client having gone away, and the exchange is worth keeping either way.
### The rest
- **`AgentRunID` was declared, documented as an executus join, and never set.** It's set now from a per-run id that's also logged at run start, so a row in the history list has a thread back to the run that produced it — and the comment describes that rather than a dependency this repo has just disclaimed.
- **`Turn.History` was populated and never read** — left over from the client-held-history design that persistence replaced. Removed.
- **`agentChat` was a 53-line outlier.** SSE plumbing moved into `openEventStream`, so it reads like the other decode/call/encode handlers.
`go test ./...` green with no build tag, `go vet`, `gofmt`, `CGO_ENABLED=0` static build clean. The image build on this branch passed, which confirms the Docker stage resolves majordomo from Gitea without a `replace`.
steve
merged commit 3a3ce16fce into main2026-07-21 06:22:10 +00:00
steve
deleted branch feat/agent-runtime2026-07-21 06:22:10 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #56. Everything below the run loop already existed — this is the thing that runs a model.
The build tag is gone, and
doc.gosays what's trueinternal/agent's doc comment promised two separations:cmd/pansynot importing the package, and the tool wiring behind//go:build majordomo. Both are gone, and the comment is rewritten rather than left as a stale aspiration — the issue was explicit about not leaving it.A tag that keeps the agent out of the binary only earns its keep if you'd ever ship a build without the agent, and the agent is the point. Keeping it meant an untagged CI that never compiled the code that matters.
majordomo is a real dependency now, resolved from the Gitea instance as a pseudo-version with no
replacedirective — areplacepointing at../majordomobuilds on my laptop and breaks the Docker build, which has no sibling checkout. It's stdlib-first and pure Go, soCGO_ENABLED=0and the single static binary survive (19 MB, verified locally; the image build on this PR is the real check).A turn is one change set
This is the whole reason acting without a confirmation prompt is defensible. "Empty the garlic bed and plant cucumbers" is one object edit and a dozen planting inserts, and it has to undo as one action, not thirteen.
The scope is opened even for a turn that turns out to be a question — a change set with no revisions is never written, so asking costs nothing and history isn't littered with empty entries.
Config
The model spec goes to
majordomo.Parseverbatim. That grammar (including comma-separated failover chains) is majordomo's; re-implementing any of it here would just mean two places to update when it grows.The key needs a bridge: majordomo's
ollama-cloudpreset readsOLLAMA_API_KEY, while pansy — like gadfly — is configured withOLLAMA_CLOUD_API_KEY. The provider is registered explicitly on a private registry rather than depending on ambient environment, so that mismatch is visible instead of showing up as a mysterious empty token.Bounded, but not budgeted
Step cap, timeout, and majordomo's loop guards. This is loop safety, not cost control — spend caps are explicitly not a v2 concern.
A capped run does not fail: it keeps what it managed to do, that work is recorded and undoable, and the reply says it stopped early rather than going silent.
Streaming, because silence reads as a hang
A turn that clears a bed and replants it makes a dozen tool calls over tens of seconds. Without SSE that's a long silence followed by everything at once — which defeats a design that rests on watching the canvas change as it happens.
Conversations persist
Per (user, garden). Client-held history would be lost on refresh, which is exactly when someone reloads to check whether the agent's change landed.
Only the user/assistant text is stored, not the full transcript: continuity needs what was said and what came back, and replaying a stored tool call would replay a decision made against a garden that has since moved on. It also keeps majordomo's message shape out of the schema.
Unconfigured instances
No key → the routes aren't registered at all, so it's a 404 rather than a handler that apologizes. A configured-but-unresolvable model logs and disables the assistant rather than refusing to boot: a garden planner that won't start because of a chat feature is worse than one without chat.
Verification — one test per acceptance criterion
Driven by majordomo's scriptable fake provider, so the whole loop runs hermetically:
TestTurnIsOneChangeSet— the flagship: clear + refill produces exactly one change set,source=agent, summarised with the user's own words, and one revert puts the garlic back.TestViewerGetsAnExplainableRefusal— a viewer's turn is refused up front, and at the tool layer a denial comes back as a readable tool result rather than killing the run.TestRunStopsAtTheStepCap— a wedged model terminates cleanly,Truncatedis set, and the reply says so.TestReadOnlyTurnWritesNoChangeSet— questions don't litter history.TestNewRunnerNeedsConfiguration— every unconfigured shape errors, including an unresolvable model spec.TestAgentRoutesAbsentWithoutAKey— 404s, and the rest of the app is unaffected.go test ./...green with no build tag.go vet,gofmt, and aCGO_ENABLED=0static build all clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
🪰 Gadfly — live review status
2/5 reviewers finished · updated 2026-07-21 06:21:50Z
claude-code/sonnet· claude-code — ⏳ 4/5 lensesglm-5.2:cloud· ollama-cloud — ✅ donekimi-k2.6:cloud· ollama-cloud — ⏳ 1/5 lensesopencode/glm-5.2:cloud· opencode — ✅ doneopencode/kimi-k2.6:cloud· opencode — ⏳ 0/5 lensesLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
All fixed in
d7c4e1a. The first finding breaks this PR's central promise and is the reason running the review was worth it.A timed-out turn left its work un-undoable
The run context carries a timeout. When it fired,
WithChangeSet's recovery path tried to record what had already committed using that same dead context — which fails. So a timed-out turn left its partial work with no change set, while the user-facing message cheerfully said "anything I'd already changed is on the canvas, and in History."That message was a lie in exactly the case it was written for. The one guarantee this whole feature rests on — a turn is one undoable change set — silently didn't hold on timeout, which is the failure mode most likely to actually happen in production.
Both recovery paths (
WithChangeSetandRevertChangeSet) now commit withcontext.WithoutCancel. The commonest reason those paths run at all is a cancelled or timed-out context, so using it to write the record of what it did was self-defeating. There's a test that cancels mid-turn and asserts the partial work is recorded, marked partial, and revertible.turnSummarysliced bytesA message whose 120th byte fell inside a multibyte character stored invalid UTF-8 in the history summary. Not hypothetical for text people type. Trimmed by runes now, with an emoji test.
The comment promised something the code didn't do
RecordAgentExchange's failure was logged and swallowed, directly beneath a comment claiming the user would see that their turn wasn't saved. The turn itself succeeded, soDonestill goes out — but now with a warning saying the exchange wasn't saved and won't survive a reload. A clean "done" followed by a conversation that has forgotten it is the quieter lie. It also records with a detached context, since the commonest reason that write fails is the client having gone away, and the exchange is worth keeping either way.The rest
AgentRunIDwas declared, documented as an executus join, and never set. It's set now from a per-run id that's also logged at run start, so a row in the history list has a thread back to the run that produced it — and the comment describes that rather than a dependency this repo has just disclaimed.Turn.Historywas populated and never read — left over from the client-held-history design that persistence replaced. Removed.agentChatwas a 53-line outlier. SSE plumbing moved intoopenEventStream, so it reads like the other decode/call/encode handlers.go test ./...green with no build tag,go vet,gofmt,CGO_ENABLED=0static build clean. The image build on this branch passed, which confirms the Docker stage resolves majordomo from Gitea without areplace.