The original 90b31fc8 test computed reset_at = now + 3s in the test thread,
then relied on the script spawning fast enough that the rate_limit_event
arrived while reset_at was still meaningfully in the future. Under
cargo-test load the spawn could take long enough that block_until - now
clamped to 0 and the inactivity timeout killed the script before its sleep
finished. Pin reset_at to 2099-01-01 (matching the existing
rate_limit_hard_block_sends_watcher_hard_block_event test) so the
extension is essentially infinite and the assertion isolates the
extension-vs-no-extension behavior from wall-clock slack.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When claude-code emits a rate_limit_event with status != allowed_warning,
the subprocess waits internally for the limit to clear before retrying. No
PTY output flows during that window, so the inactivity timeout in the PTY
runner would fire and kill the agent — mergemaster especially, whose
15-minute inactivity window is shorter than typical rate-limit backoffs.
Track `block_until = Some(reset_at)` on hard-block events and add the
remaining time-until-reset to the per-iteration recv timeout. Once reset_at
passes (or an earlier emit arrives), the extension implicitly drops to 0
and the base inactivity timeout resumes. Turn/budget counts aren't affected
— they come from the session log and only advance when API calls actually
complete, so a stalled retry doesn't burn either.
Regression test in agents/pty/mod.rs spawns a script that emits a hard-block
with reset_at = now+3s, sleeps 3s, then exits, with inactivity_timeout_secs
= 1. Without the fix the runner kills the script at 1s; with the fix the
deadline is bumped past the sleep and the run completes cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause was not the persist channel (the test-mode channel is unbounded
and its receiver is leaked, so sends never fail). It was that `ALL_OPS` and
`VECTOR_CLOCK` were process-wide `OnceLock` globals while `CRDT_STATE` was
already thread-local — so one test thread's `apply_compaction` would prune
another test thread's freshly-written ops out of the shared journal, and
the subsequent `all_ops_json()` read in `compaction_reduces_ops` would
return fewer than the 5 it had just written.
Mirror the pattern already used for `CRDT_STATE` and `SnapshotState`: in
`cfg(test)` use thread-local `OnceLock<Mutex<...>>`s for the op journal and
vector clock, accessed via new `all_ops_lock()` / `vector_clock_lock()`
helpers. Production code path is unchanged (still the global statics set
during `init()`).
Touches ops/read/snapshot call sites to go through the helpers. Note in
passing that this overlaps backlog story 518; that story is about the
production-side persist path, this is the cfg(test)-only journal-isolation
slice.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two latent bugs in `service/gateway/io.rs::spawn_gateway_bot`, exposed
today after a long-overdue gateway rebuild:
1. The permission channel sender was bound as `_perm_tx` (the underscore
prefix signalling "unused") and dropped at function return. The
matrix bot's permission_listener task — which holds `perm_rx` for
its lifetime per story 884 — then saw the channel close immediately
and exited with "perm_rx channel closed" 1s after starting. Net
effect: the listener was effectively absent on every gateway boot,
so non-MCP tool permission requests had no destination at all
(separate from the architectural mismatch that 898 will fix; this
was a strictly worse "listener never even ran" version of the same
problem). Bind as `perm_tx` and `mem::forget` it to keep the
channel open for the gateway's lifetime, mirroring the existing
`shutdown_tx` pattern two lines below.
2. `bot_name` was hardcoded to `"Assistant"`, ignoring
`bot.toml::display_name`. So the gateway's matrix bot announced
itself as "Assistant" and treated user messages addressed to
"Timmy" (the actual configured display_name) as unaddressed,
silently dropping them. `ambient_rooms` and
`permission_timeout_secs` were similarly ignored. Load
`BotConfig::load(config_dir)` and apply the same field plumbing
the standard-mode initialisation in `main.rs:211-232` already
uses.
Symptoms seen in production today:
- gateway.log: `Sending startup announcement: Assistant is online.`
followed by repeated `Ignoring unaddressed message from
@yossarian:crashlabs.io` lines.
- gateway.log: `permission listener started` immediately followed
(same timestamp) by `permission listener exiting (perm_rx channel
closed)`.
After this lands, rebuild the gateway binary and restart so it picks
up `bot.toml` correctly and the listener stays alive for the bot's
lifetime.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to bug 903. The attach fix made run_tests retries safe, but
agents still observed the underlying MCP transport timeout as a
tool-call error and had to handle it via retry. Implement the proper
fix: MCP `notifications/progress` events keep the client's transport
timer alive so the call never errors from the agent's perspective.
What changed:
server/src/http/mcp/progress.rs (new)
- `ProgressEmitter` (progressToken + mpsc sender) installed in a
`tokio::task_local!` scope by the SSE response path.
- `emit_progress(progress, total, message)` builds a JSON-RPC
`notifications/progress` message and sends it via the channel.
No-op when no emitter is in scope (plain JSON path / tests / API
runtimes), so tool handlers can call it unconditionally.
server/src/http/mcp/mod.rs
- mcp_post_handler now detects `Accept: text/event-stream` AND a
`params._meta.progressToken` on tools/call. When both are present,
routes through `sse_tools_call` instead of the plain JSON path.
- sse_tools_call: spawns the dispatch task with the emitter installed,
builds an SSE stream that interleaves incoming progress events with
the final JSON-RPC response, with a 15s keep-alive interval as a
backstop for tools that don't emit their own progress.
- Plain JSON behaviour is unchanged for non-SSE clients and for
everything other than tools/call.
server/src/http/mcp/shell_tools/script.rs
- tool_run_tests poll loop emits `notifications/progress` every 25s
of elapsed time (well below the typical ~60s MCP transport
timeout). Attached callers (the bug 903 fix path) also emit so
their MCP socket stays alive while waiting for the in-flight job.
- Output filtering: on a passing run the response now returns a
one-line summary ("All N tests passed.") instead of the full
`cargo test` stdout, which was pure noise that burned agent
tokens. Failure output is unchanged (truncated tail with the
`failures:` section and final test_result line). CRDT entry
stores the same filtered value so attached callers see it too.
Tests (3 new):
- emit_progress_no_op_without_emitter — calling outside scope is safe
- emit_progress_sends_notification_when_emitter_installed — full path
- emit_progress_omits_optional_fields — total/message optional
Not changed: coder system_prompts still tell agents to retry on
transport-timeout errors. That advice is now belt-and-braces — if
claude-code's HTTP MCP client honours progress notifications, no agent
will ever observe the error; if not, retry is still safe post-903. We
can drop the retry advice once we've observed the SSE path working in
the field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bug 903: every `run_tests` MCP call killed the prior `cargo test` child
for the same worktree and spawned a fresh one. Combined with the
~60s MCP client-side timeout and the 896 agent prompt that told agents
to "call run_tests again — it attaches to the in-flight test job",
this produced a respawn loop: agent calls, MCP times out at 60s, agent
retries, run_tests kills the running build and starts a new one. The
test suite never reaches the finish line.
Server log evidence: "Started test job for <worktree> (pid N)" with a
new PID every ~60-90s for the same worktree.
Fix: when `run_tests` is called and a job is already in flight for that
worktree, ATTACH to it instead of killing+respawning. The original job's
poll loop already writes the final status to the CRDT `test_jobs`
collection; attached callers just poll that CRDT entry (the same
pattern `get_test_result` uses) and return the result when the
in-flight job transitions out of "running". The 896 prompt's claim is
now actually true.
Worktrees remain isolated from each other and may run `cargo test`
concurrently — there is no cross-worktree serialisation. The single
invariant is "at most one test job per worktree at a time".
New test: `tool_run_tests_concurrent_calls_attach_to_single_job`
spawns two concurrent calls on the same worktree against a 2s
`sleep`-based script and asserts total elapsed stays close to 2s
(attach) rather than 4s (respawn).
Note: the cross-worktree linker-OOM symptom Timmy reported in the
field was downstream of the respawn loop. Killed-but-not-fully-reaped
cargo invocations stack memory pressure beyond the nominal N
worktrees. With the attach fix, each worktree runs exactly one
in-flight build at a time and old builds finish cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pipeline gap: the state machine refused `move_story(... target='backlog')`
from a Blocked story, leaving stuck items with no way to be parked while
waiting on dependent fixes — operators had to either Unblock (which
re-enters the active flow) or Archive (which loses the item).
Extend the existing Demote rule so `Blocked + Demote → Backlog` is a
legal transition, alongside the existing `Coding/Qa/Merge + Demote`.
Also update `map_stage_move_to_event` in agents/lifecycle.rs so the
chat/MCP `move_story` API recognises Blocked → backlog and routes it
through `PipelineEvent::Demote`.
Tests:
- `blocked_demote_returns_to_backlog` — happy path.
- `cannot_demote_from_done` / `cannot_demote_from_upcoming` — sanity
checks that the broadened rule does NOT permit Demote from
terminal or pre-triage stages.
Pattern follows 892 (MergeFailure → Done) and 893 (MergeFailure →
Coding) — pure transition.rs extension plus matching event mapping in
lifecycle.rs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bug 901: `unblock_story` (and the chat `unblock` command) routed through
`parse_front_matter` and errored with "Missing front matter" on any
post-865 story (story content is now CRDT-only with no YAML on disk).
In `chat/commands/unblock.rs::unblock_by_story_id`:
- Drop the early `parse_front_matter` gate.
- Read story name and blocked state from the CRDT register API instead
of parsed YAML (`crdt_state::read_item`, `pipeline_state::read_typed`).
- Keep the legacy fallback cleanup, but gate it on the content actually
starting with a `---` YAML block, so CRDT-only stories don't hit a
parse error there either.
- Remove the now-unused `parse_front_matter` import.
Surfaced a second sub-bug: even when the state-machine transition
fired (`Blocked + Unblock → Coding`), the CRDT `blocked` register was
never explicitly cleared. Pre-865 the YAML-strip content_transform
cleared it as a side effect; post-865 there is no YAML to strip.
- Add `crdt_state::set_blocked(story_id, bool)` parallel to
`set_retry_count`. Wired through `crdt_state::write` and the
crate-level re-export.
- `agents::lifecycle::transition_to_unblocked` now calls
`set_blocked(story_id, false)` alongside `set_retry_count(0)` so
the legacy register stays in sync with the typed stage.
Test: `unblock_command_works_on_crdt_only_story_no_yaml` seeds a CRDT
entry with no YAML on disk, runs unblock, asserts success + cleared
blocked + retry_count=0. All 10 existing unblock tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Claude Code rejects "Bash(:*)" with "Prefix cannot be empty before :*" —
the rule is silently skipped, which since 5b48f0d0 left no Bash entry
in the allowlist at all. Every coder agent's Bash call has been
auto-denying since that commit landed (~840 of 1.4k denials in the sled
log).
The canonical form for "allow all bash commands" is the tool name alone:
"Bash" (no parens). Apply it in three places that 5b48f0d0 touched:
- .claude/settings.json (project root, inherited by new worktrees)
- server/src/io/fs/scaffold/templates.rs (huskies init template)
- server/src/io/fs/scaffold/tests.rs (assertion now checks "Bash")
The gateway settings.json at ~/Desktop/huskies/.claude/settings.json and
the four live worktrees (810, 888, 890, 894) were also corrected — not
in this commit since they live outside the repo.
Surfaced via /doctor; reported with rule "Invalid permission rule
Bash(:*) was skipped: Prefix cannot be empty before :*".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add `disallowed_tools` field to `AgentConfig` and render it as
`--disallowedTools` CLI flag in `render_agent_args`
- Set `disallowed_tools = ["ScheduleWakeup"]` on all four coder agents
(coder-1, coder-2, coder-3, coder-opus); QA and mergemaster unaffected
- Append instruction to all coder `system_prompt`s: do not use
ScheduleWakeup to wait for run_tests; if run_tests appears to time out,
call run_tests again — it attaches to the in-flight job and blocks
- Add tests: `render_agent_args_disallowed_tools` and
`coder_agents_disallow_schedule_wakeup`
The per-command allowlist (Bash(cargo:*), Bash(git:*), …) misses any tool
a coder agent reaches for outside the curated set — ./script/*, make, curl,
jq, docker, test, [, etc. Each miss hits prompt_permission, which auto-denies
on the sled because no listener holds perm_rx (the matrix bot lives in the
gateway). 1,377 such denies in the sled log over the past week, accounting
for most of the recent throughput slowdown.
Replace the curated list with a single Bash(:*) wildcard in:
- .claude/settings.json (project root, picked up on git worktree add)
- server/src/io/fs/scaffold/templates.rs (used only by huskies init when
no .claude/settings.json already exists)
Update scaffold/tests.rs to assert the wildcard rather than a fixed set
of patterns; the per-command gate offered no real safety in this trusted
single-user deployment, since the prompt was never going to reach a human
anyway (that's the bug).
Stopgap until story 898 lands the proper sled→gateway permission
forwarding — at which point the wildcard can be narrowed back if desired.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
864 changes write_item_with_content to take 4 args (ItemMeta), but the
master regression test calls the 3-arg form. After 864 squash-merges,
the merged code has the 4-arg fn AND the 3-arg call site, breaking
compile in the merge worktree.
Drop the test for now (the actual run on 864 today validated the fix
end-to-end). Re-add it in a follow-up after 864 lands, using the new
signature.
The mergemaster gates run rustfmt and rejected 864's merge because
several files I added/touched in master today had not been fmt'd.
Six files affected, mostly trivial line-wrapping nits. Fixes the
formatting gate for the next 864 merge attempt.
The bug 882 abort-respawn safeguard caps consecutive crashes at 5 then
blocks the story — but the underlying stdio abort itself stays unfixed:
each respawn calls start_agent which reads session_store.json, finds the
prior session id, passes --resume to claude-code, and re-triggers the
same crash. Five identical respawns later, the story is blocked.
Now: when an abort+no-session exit triggers respawn, we first call
session_store::remove_sessions_for_story to drop every entry for the
story. The next spawn starts cold (no --resume), which avoids the
bloated stdio replay claude-code is choking on.
The function was already implemented but #[cfg(test)] only — promoted
to a non-test pub fn. Existing remove_sessions_for_story_cleans_up test
unchanged and still green.
Net effect: instead of "5 retries, then blocked", we get "1 abort, prune,
respawn cold, agent runs normally". The story can resume work without
losing its worktree state.
After story 871 the `agent` pin lives in the typed CRDT register
(`PipelineItemView.agent`), not the YAML front matter — the YAML
mutation was removed at the same time. Both spawn-resolution paths
(`auto_assign::story_checks::read_story_front_matter_agent` and
`start::validation::read_front_matter_agent`) still read only YAML
via parse_front_matter, which returns None for any story whose pin
was set via the post-871 typed setter. The spawn then falls back to
"first available coder," silently downgrading opus-pinned stories to
the first available sonnet — which is why 855/864/866 kept hitting the
80-turn watchdog limit despite the user's explicit opus pin.
Now: both paths consult `crdt_state::read_item()` first and use
`view.agent` if non-empty. YAML parsing remains as a fallback so older
stories whose CRDT entry doesn't yet have the field still resolve.
Adds a regression test that seeds an item with empty YAML, sets the
typed CRDT register via `set_agent`, and asserts
`read_story_front_matter_agent` returns the CRDT value.
Before: tool_run_check (and run_build/run_lint via run_script_tool)
returned the entire cargo log verbatim in `output`. For runs with many
errors the response routinely exceeded the MCP token cap, was dumped
to a tool-results file, and the agent had to scrape it with python3
just to see the error list — burning many turns on file archaeology
for what should be a one-look operation. Real example: 864's coder
hit `result (143,708 characters) exceeds maximum allowed tokens` and
spent ~8 turns extracting 3 errors.
Now:
- New `service::shell::parse_diagnostics` parses `error[CODE]:` /
`warning[CODE]:` headers + their `--> file:line` markers into
structured `Diagnostic { kind, code, message, file, line }`.
- `tool_run_check` (and the run_build/run_lint shared body) returns
`{ passed, exit_code, errors: [...], warnings: [...], summary }`.
Raw `output` is dropped from the default response.
- New `verbose: bool` argument (default false) restores the raw
output for callers who actually need it.
- Updated the existing tool_run_check test to assert the new
contract (150 errors → 150 structured entries, response < 50KB).
Skipped run_tests in this pass — its parser would need to recognise
test-runner output (different format from cargo); will land separately.
Closes 886.
Before: handle_message.rs acquired services.perm_rx only while processing
one chat message and dropped it on chat_fut completion. The moment the
bot wasn't actively responding, prompt_permission auto-denied any spawned
coder bash call as "no interactive session" — making unattended coder
work impossible.
Now: a permission_listener task is spawned at bot startup and holds
perm_rx for the bot's lifetime. Permission requests are forwarded to
the first configured Matrix room, replies resolved by the existing
on_room_message handler via pending_perm_replies. Per-message acquire is
gone from handle_message.rs (chat_fut just awaits cleanly).
- New module: chat/transport/matrix/bot/permission_listener.rs.
- Wired into run_bot before BotContext construction; bot_sent_event_ids
is hoisted out so the listener and the rest of the bot share it.
- handle_message.rs no longer touches perm_rx.
- diagnostics/permission.rs comment updated to reflect the new reality.
- Regression test asserts the listener forwards a PermissionForward to
the target room and records the pending reply key — exactly the path
that was broken when no chat_fut was in flight.
Discord/Slack/WhatsApp transports still acquire perm_rx per message
(commands.rs:368 / commands/llm.rs:83 / commands/llm.rs:82). They are
not the active transport in this deployment so their per-message acquire
remains dormant; the same listener pattern should be applied to them as
follow-up work in 884 phase 2.
Claude Code 2.1.123+ honours wildcard Bash allowlist patterns only in
the canonical form `Bash(cmd:*)`. The space form `Bash(cmd *)` falls
through to prompt_permission and gets auto-denied in agent mode,
breaking spawned coders.
- Rewrite all `Bash(cmd *)` patterns in STORY_KIT_CLAUDE_SETTINGS to
the colon form.
- Replace separate `Bash(cargo build:*)` / `Bash(cargo check:*)` with
a single `Bash(cargo:*)`.
- Add commonly-needed patterns: python3, node, npm, which, sed, awk,
rg, diff, sort, uniq.
- Patch the live project-root .claude/settings.json so the running
system picks up the fix immediately (rebuilt scaffolds will match).
- Add regression test asserting no `Bash(... *)` patterns survive and
required common commands are present.
855 deleted the HTTP /mcp route and pointed agents at ws://...crdt-sync,
but Claude Code's .mcp.json doesn't speak ws:// and the rendezvous WS
never had MCP method handlers wired up — so every spawned Claude Code
agent (gateway-routed and local) booted with zero huskies tools and
died on --permission-prompt-tool=mcp__huskies__prompt_permission.
Restore mcp_post_handler / mcp_get_handler / handle_initialize, re-add
the /mcp route, and revert all three .mcp.json writers to emit
http://localhost:{port}/mcp with explicit "type": "http". Reuses the
already-extracted gateway::jsonrpc types and the surviving
dispatch_tool_call / list_tools surfaces — net add ~140 lines.
Federation work is unaffected: /crdt-sync continues to do CRDT sync,
which is what it was actually doing. MCP-over-WebSocket for cross-LAN
agents was never wired up by 855 and can be done as a proper follow-up
with a regression test that boots a real claude and verifies tool
registration.
Verified end-to-end: /mcp initialize, tools/list (74 tools incl.
prompt_permission), and tools/call all respond correctly from inside
the rebuilt container.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dave
Timmy