dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: support unitless slow durations (#2450)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-08-09 11:54:09 +01:00
committed by GitHub
parent 0472eb1c73
commit bbdd426776
7 changed files with 240 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/cmd/mock-agent/background_test.go
+8 -6
View File
@@ -7,10 +7,10 @@ import (
acp "github.com/coder/acp-go-sdk"
)
// TestParseBackgroundDuration pins the /background argument parsing, including
// the regression Copilot flagged on PR #3: a unit-bearing value like "1m" must
// not be mangled into "1ms" by the bare-seconds fallback.
func TestParseBackgroundDuration(t *testing.T) {
// TestParseCommandDuration pins command argument parsing, including the
// regression Copilot flagged on PR #3: a unit-bearing value like "1m" must not
// be mangled into "1ms" by the bare-seconds fallback.
func TestParseCommandDuration(t *testing.T) {
const def = 8 * time.Second
cases := []struct {
name string
@@ -19,17 +19,19 @@ func TestParseBackgroundDuration(t *testing.T) {
}{
{"no argument uses default", "/background", def},
{"bare number is seconds", "/background 12", 12 * time.Second},
{"slow bare number is seconds", "/slow 60", 60 * time.Second},
{"explicit seconds", "/background 20s", 20 * time.Second},
{"explicit minutes (regression: not 1ms)", "/background 1m", time.Minute},
{"explicit hours", "/background 2h", 2 * time.Hour},
{"explicit milliseconds", "/background 500ms", 500 * time.Millisecond},
{"unparseable falls back to default", "/background soon", def},
{"zero falls back to default", "/background 0", def},
{"negative falls back to default", "/background -1s", def},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := parseBackgroundDuration(tc.cmd, def); got != tc.want {
t.Fatalf("parseBackgroundDuration(%q) = %v, want %v", tc.cmd, got, tc.want)
if got := parseCommandDuration(tc.cmd, def); got != tc.want {
t.Fatalf("parseCommandDuration(%q) = %v, want %v", tc.cmd, got, tc.want)
}
})
}
apps/backend/cmd/mock-agent/handler.go
+9 -17
View File
@@ -322,7 +322,7 @@ const asyncSubagentAgentID = "agent_e2e_async_lifecycle"
// The teardown variant intentionally omits step 5 so execution termination is
// the only evidence available to reconcile the registration.
func emitAsyncSubagentLifecycle(e *emitter, cmd string, emitCompletion bool) {
d := parseBackgroundDuration(cmd, 20*time.Second)
d := parseCommandDuration(cmd, 20*time.Second)
toolCallID := nextToolID()
e.launchAsyncSubagentTool(
toolCallID,
@@ -350,7 +350,7 @@ func emitAsyncSubagentLifecycle(e *emitter, cmd string, emitCompletion bool) {
// It is intentionally different from /background, whose foreground prompt stays
// open for the entire delay, so E2E can cover settled+background explicitly.
func emitDetachedBackgroundWork(e *emitter, cmd string) {
d := parseBackgroundDuration(cmd, 8*time.Second)
d := parseCommandDuration(cmd, 8*time.Second)
e.text("Launching detached background work; this foreground turn is complete.")
taskToolID := nextToolID()
@@ -390,7 +390,7 @@ func emitSleep(e *emitter, cmd string) {
// while the session still reads RUNNING. When the hold elapses the subagent
// completes, the foreground resumes, and the turn ends (→ done).
func emitBackgroundWork(e *emitter, cmd string) {
d := parseBackgroundDuration(cmd, 8*time.Second)
d := parseCommandDuration(cmd, 8*time.Second)
e.text("Kicking off background work; I'll keep going in the background.")
@@ -416,13 +416,11 @@ func emitBackgroundWork(e *emitter, cmd string) {
e.text("Background work complete.")
}
// parseBackgroundDuration reads the optional duration argument of a /background
// command, returning def when it is absent or unparseable. A value carrying an
// explicit unit is honored as-is (`1m`, `500ms`, `2h`); a bare number is treated
// as seconds (`8` → 8s). The explicit-unit parse is tried FIRST: appending "s"
// to a unit-bearing value like `1m` would otherwise parse as the valid-but-wrong
// "1ms" (1 millisecond) and never reach the correct interpretation.
func parseBackgroundDuration(cmd string, def time.Duration) time.Duration {
// parseCommandDuration reads an optional command duration, returning def when
// it is absent or unparseable. Explicit units are honored as-is; bare numbers
// are treated as seconds. The explicit-unit parse is tried first so a value
// like `1m` is not misread as the valid-but-wrong `1ms`.
func parseCommandDuration(cmd string, def time.Duration) time.Duration {
parts := strings.Fields(cmd)
if len(parts) < 2 {
return def
@@ -456,13 +454,7 @@ func emitCrash(e *emitter, model string) {
// emitSlowResponse generates a response with configurable total duration.
func emitSlowResponse(e *emitter, prompt, model string) {
totalDuration := 5 * time.Second
parts := strings.Fields(prompt)
if len(parts) >= 2 {
if d, err := time.ParseDuration(parts[1]); err == nil && d > 0 {
totalDuration = d
}
}
totalDuration := parseCommandDuration(prompt, 5*time.Second)
steps := 5
stepDelay := totalDuration / time.Duration(steps)
apps/backend/cmd/mock-agent/slow_test.go
+16
View File
@@ -0,0 +1,16 @@
package main
import "testing"
func TestSlowResponseBareNumberUsesSeconds(t *testing.T) {
e, updates := newTestEmitter()
emitSlowResponse(e, "/slow 50ms", "mock-fast")
const want = "Running slow response (50ms total)..."
for _, update := range updates.getUpdates() {
if getTextContent(update) == want {
return
}
}
t.Fatalf("slow response did not report %q", want)
}
docs/plans/mock-agent-slow-duration/plan.md
+75
View File
@@ -0,0 +1,75 @@
---
spec: docs/specs/mock-agent-slow-duration/spec.md
created: 2026-08-08
status: done
---
# Implementation Plan: Mock-agent slow command duration syntax
## Overview
Extend the mock agent's existing duration parsing so `/slow 60` resolves to
60 seconds while preserving explicit-unit durations and the five-second
fallback. Reuse the already-tested explicit-unit-first parsing behavior used by
the background commands, then add focused parser and handler coverage without
changing the ACP command advertisement or any frontend code.
## Backend
### Shared duration parsing
- `apps/backend/cmd/mock-agent/handler.go`
- Generalize the existing background-duration parser, or extract its argument
parsing into a clearly command-independent helper.
- Try an explicit Go duration first (`60s`, `500ms`, `2m`) so unit-bearing
values are not altered.
- Treat a positive bare numeric value as seconds (`60` → `60s`).
- Keep the caller-provided default for missing, zero, negative, or malformed
values.
- Route `emitSlowResponse` through this parser with a five-second default and
preserve the existing background-command behavior.
- Preserve the existing `/background 12` behavior already covered by the
parser table; the new behavior is limited to the `/slow` caller.
## Tests
- **What:** Duration argument resolution for `/slow` and existing background
commands, including bare seconds, explicit units, defaults, and invalid
values.
**File:** `apps/backend/cmd/mock-agent/background_test.go` (rename or
generalize the existing parser test as needed).
**How:** Table-driven unit test; the `/slow 60` case must fail before the
parser change because it currently resolves to the supplied eight-second
default rather than 60 seconds.
- **What:** The `/slow` handler uses the resolved duration in its simulated
response path.
**File:** `apps/backend/cmd/mock-agent/slow_test.go` (new) or the nearest
existing mock-agent handler test file.
**How:** Invoke the handler with a short explicit duration to avoid a
60-second test, and assert that the response reports that resolved duration;
the parser table separately covers the 60-second bare-number contract.
No frontend or E2E changes are planned: the ACP command transport and existing
slash-command UI flow are unchanged, and this repair is confined to the mock
agent's backend fixture behavior.
## Verification Results
- `cd apps/backend && go test -run '^TestSlowResponseBareNumberUsesSeconds$' -count=1 ./cmd/mock-agent` failed before the production change with the expected five-second marker, then passed after the change.
- `cd apps/backend && go test -run 'Test(ParseCommandDuration|Slow)' ./cmd/mock-agent` — passed (`12 passed`).
- `cd apps/backend && go test ./cmd/mock-agent` — passed (`193 passed`).
- `make -C apps/backend build-mock-agent` — passed; the host mock-agent binary
was rebuilt. The containers E2E project was not run, so the Linux binary was
not required.
- `gofmt -w cmd/mock-agent/handler.go cmd/mock-agent/background_test.go cmd/mock-agent/slow_test.go` — passed.
- `git diff --check` — passed.
## Implementation Waves And Parallel Candidates
Wave 1 (sequential):
- [x] [task-01-slow-duration-parser](task-01-slow-duration-parser.md)
## Open Questions
None.
docs/plans/mock-agent-slow-duration/task-01-slow-duration-parser.md
+78
View File
@@ -0,0 +1,78 @@
---
id: "01-slow-duration-parser"
title: "Support unitless slow durations"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/mock-agent-slow-duration/spec.md"
---
# Task 01: Support unitless slow durations
Implement the `/slow` duration parsing repair after adding the regression
coverage.
## Acceptance
- `/slow 60` resolves to 60 seconds, while `/slow 60s`, `/slow 500ms`, and
`/slow 2m` retain their explicit durations.
- `/slow` and invalid, zero, or negative arguments retain the five-second
default; existing `/background` duration behavior remains unchanged.
- The mock-agent response path uses the resolved duration and reports it using
the existing output format.
## Verification
Run the focused parser/handler tests first, then the complete mock-agent
package:
```bash
cd apps/backend
go test -run 'Test(ParseCommandDuration|Slow)' ./cmd/mock-agent
go test ./cmd/mock-agent
```
## Files likely touched
- `apps/backend/cmd/mock-agent/handler.go`
- `apps/backend/cmd/mock-agent/background_test.go`
- `apps/backend/cmd/mock-agent/slow_test.go`
## Dependencies
None.
## Parallelism
Sequential. The parser and its tests share the same behavior and should land as
one focused change.
## Inputs
- `docs/specs/mock-agent-slow-duration/spec.md`
- `docs/plans/mock-agent-slow-duration/plan.md`
- Existing command-duration parser coverage in
`apps/backend/cmd/mock-agent/background_test.go`
- `apps/backend/cmd/mock-agent/AGENTS.md`
## Output contract
Report the final files changed, the red and green targeted test results, the
complete mock-agent package result, any risks, and synchronized task/plan
statuses. Do not change the ACP command advertisement, frontend code, or
unrelated mock-agent commands.
## Results
- RED: `cd apps/backend && go test -run '^TestSlowResponseBareNumberUsesSeconds$' -count=1 ./cmd/mock-agent` — failed as expected because `/slow 1` reported the five-second path (`0 passed, 1 failed`).
- GREEN: `cd apps/backend && go test -run '^TestSlowResponseBareNumberUsesSeconds$' -count=1 ./cmd/mock-agent` — passed (`1 passed`).
- Focused: `cd apps/backend && go test -run 'Test(ParseCommandDuration|Slow)' ./cmd/mock-agent` — passed (`12 passed`).
- Package: `cd apps/backend && go test ./cmd/mock-agent` — passed (`193 passed`).
- Formatting: `gofmt -w cmd/mock-agent/handler.go cmd/mock-agent/background_test.go cmd/mock-agent/slow_test.go` — passed.
- Diff hygiene: `git diff --check` — passed.
- `make -C apps/backend build-mock-agent` — passed; the host mock-agent binary
was rebuilt. The containers E2E project was not run, so the Linux binary was
not required.
- Review follow-up: the handler-path test uses `/slow 50ms` to keep the suite
fast while the parser table covers `/slow 60`.
docs/specs/INDEX.md
+1
View File
@@ -221,6 +221,7 @@ System pages (Radarr/Sonarr-style) for status, disk usage, database maintenance,
| Spec | Status |
|---|---|
| [mock-agent-slow-duration](mock-agent-slow-duration/spec.md) | shipped |
| [session-subscription-recovery](session-subscription-recovery/spec.md) | draft |
| [npm-nightly-channel](npm-nightly-channel/spec.md) | shipped |
| [agent-resume-runtime-recovery](agent-resume-runtime-recovery/spec.md) | shipped |
docs/specs/mock-agent-slow-duration/spec.md
+53
View File
@@ -0,0 +1,53 @@
---
status: shipped
created: 2026-08-08
owner: codex
---
# Mock-agent slow command duration syntax
## Why
The mock agent's `/slow` command is used to keep a deterministic agent turn
active while testing queueing, cancellation, and session behavior. Users expect
an argument such as `/slow 60` to mean 60 seconds, but the current command
silently falls back to its five-second default because the parser requires an
explicit duration unit.
## What
- `/slow` with no duration keeps its existing five-second default.
- `/slow <positive number>` interprets the bare number as seconds. For example,
`/slow 60` holds the turn for 60 seconds.
- `/slow <duration-with-unit>` continues to accept explicit Go duration forms,
including `/slow 60s`, `/slow 500ms`, and `/slow 2m`.
- Missing, zero, negative, or malformed duration arguments continue to use the
five-second default rather than failing the prompt.
## Command contract
The command is exposed through the mock agent's ACP available-command
advertisement as `slow`. Its prompt form is `/slow [duration]`. The resolved
duration controls the total simulated turn delay, and the response reports the
resolved duration using the existing mock-agent output format.
## Scenarios
- **GIVEN** the mock agent is ready, **WHEN** it receives `/slow 60`, **THEN**
it simulates a 60-second turn rather than using the five-second default.
- **GIVEN** the mock agent is ready, **WHEN** it receives `/slow 60s`, **THEN**
it simulates the same 60-second turn.
- **GIVEN** the mock agent is ready, **WHEN** it receives `/slow 500ms`, **THEN**
it preserves the explicit 500-millisecond duration.
- **GIVEN** the mock agent is ready, **WHEN** it receives `/slow`, **THEN** it
uses the five-second default.
- **GIVEN** the mock agent is ready, **WHEN** it receives `/slow 0`, a negative
value, or malformed text, **THEN** it uses the five-second default.
## Out of scope
- Adding a `/send` command; `/send 60` is not a mock-agent command.
- Changing the default duration or the behavior of `/sleep`, `/background`, or
other mock-agent commands.
- Changing the existing ACP command advertisement or its input hint.
- Adding persistence, configuration, or a feature flag for the duration.