test(office): pin taskless run sessions' launch, fallback and failure-safety guarantees (#3831)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -2,6 +2,7 @@ package backendapp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -11,13 +12,19 @@ import (
|
||||
"github.com/kandev/kandev/internal/agent/agents"
|
||||
client "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
officecosts "github.com/kandev/kandev/internal/office/costs"
|
||||
officemodels "github.com/kandev/kandev/internal/office/models"
|
||||
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
officeroutines "github.com/kandev/kandev/internal/office/routines"
|
||||
officeservice "github.com/kandev/kandev/internal/office/service"
|
||||
"github.com/kandev/kandev/internal/office/shared"
|
||||
officewakeup "github.com/kandev/kandev/internal/office/wakeup"
|
||||
"github.com/kandev/kandev/internal/orchestrator"
|
||||
"github.com/kandev/kandev/internal/orchestrator/executor"
|
||||
taskrepo "github.com/kandev/kandev/internal/task/repository"
|
||||
@@ -43,6 +50,16 @@ type routineCronHarness struct {
|
||||
orchestrator *orchestrator.Service
|
||||
agentMgr *stubAgentManager
|
||||
workspaceID string
|
||||
db *sqlx.DB
|
||||
// eventBus is the single bus the harness's real taskSvc, workflowSvc,
|
||||
// and orchestrator subscribe to. A test needing to observe how the
|
||||
// real orchestrator reacts to a run-owned lifecycle event must wire
|
||||
// its own office/service.Service onto this same bus rather than
|
||||
// standing up a private, disconnected one — TestRoutine_CronFire_LightweightReachesSession
|
||||
// builds its own local bus for exactly that reason, so it never
|
||||
// actually exercises what the real orchestrator does with a taskless
|
||||
// event; this field exists so a different test can.
|
||||
eventBus bus.EventBus
|
||||
}
|
||||
|
||||
func newRoutineCronHarness(t *testing.T) *routineCronHarness {
|
||||
@@ -66,6 +83,13 @@ func newRoutineCronHarness(t *testing.T) *routineCronHarness {
|
||||
if err != nil {
|
||||
t.Fatalf("workflow repository: %v", err)
|
||||
}
|
||||
// Office agent CRUD reads/writes the merged agent_profiles table (ADR
|
||||
// 0005 Wave C), which only the settings store schema creates —
|
||||
// officesqlite.NewWithDB does not create it. See base_test.go's
|
||||
// initSharedAgentProfilesSchema in internal/office/service.
|
||||
if _, _, err := settingsstore.Provide(sqlxDB, sqlxDB, nil); err != nil {
|
||||
t.Fatalf("settings store init: %v", err)
|
||||
}
|
||||
officeRepo, err := officesqlite.NewWithDB(sqlxDB, sqlxDB, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("office repository: %v", err)
|
||||
@@ -142,9 +166,23 @@ func newRoutineCronHarness(t *testing.T) *routineCronHarness {
|
||||
orchestrator: orchestratorSvc,
|
||||
agentMgr: agentMgr,
|
||||
workspaceID: workspaces[0].ID,
|
||||
db: sqlxDB,
|
||||
eventBus: eventBus,
|
||||
}
|
||||
}
|
||||
|
||||
// countRows returns COUNT(*) from table, used to assert the negative half of
|
||||
// AC-OFFICE-TASKLESS-001.1: a taskless routine fire must create no tasks row
|
||||
// and no task_sessions row.
|
||||
func (h *routineCronHarness) countRows(t *testing.T, table string) int {
|
||||
t.Helper()
|
||||
var count int
|
||||
if err := h.db.Get(&count, "SELECT COUNT(*) FROM "+table); err != nil {
|
||||
t.Fatalf("count %s: %v", table, err)
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
// awaitTaskInProgress polls until the task reaches IN_PROGRESS, the last DB
|
||||
// write in executor.runAgentProcessAsync's post-LaunchAgent goroutine (see
|
||||
// its callers for why that goroutine outlives the LaunchAgent call this test
|
||||
@@ -285,26 +323,154 @@ func TestRoutine_CronFire_HeavyRoutineReachesSession(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoutine_CronFire_LightweightReachesSession is deliberately skipped:
|
||||
// the lightweight (taskless) routine path has no session to reach at all
|
||||
// today (see card 49894d63 — gap 1, the taskless execution model). Writing
|
||||
// it out in full, rather than leaving the gap unrepresented, means the skip
|
||||
// carries the assertion that will need to pass once that card lands instead
|
||||
// of silently having no coverage for either shape.
|
||||
func TestRoutine_CronFire_LightweightReachesSession(t *testing.T) {
|
||||
t.Skip("blocked on card 49894d63: the lightweight (taskless) routine flow has no task_sessions row to assert on yet")
|
||||
// lightweightRunLauncher is a test double for officeservice.RunSessionLauncher,
|
||||
// mirroring service_test.tasklessTestLauncher (internal/office/service/
|
||||
// taskless_lifecycle_test.go): it reserves and starts a real
|
||||
// office_run_sessions row through the same repo the scheduler under test
|
||||
// reads, rather than merely recording the call.
|
||||
type lightweightRunLauncher struct {
|
||||
svc *officeservice.Service
|
||||
calls []officeservice.LaunchContext
|
||||
}
|
||||
|
||||
func (l *lightweightRunLauncher) StartRunSession(
|
||||
ctx context.Context, run *officemodels.Run, agent *officemodels.AgentInstance,
|
||||
launch officeservice.LaunchContext, _ *officeservice.RouteOverride,
|
||||
) (officeservice.RunSessionLaunch, error) {
|
||||
l.calls = append(l.calls, launch)
|
||||
attempt := len(l.calls)
|
||||
id := fmt.Sprintf("lightweight-run-session-%s-%d", run.ID, attempt)
|
||||
now := time.Now().UTC()
|
||||
session := &officemodels.RunSession{
|
||||
ID: id, WorkspaceID: agent.WorkspaceID, AgentProfileID: agent.ID,
|
||||
RunID: run.ID, Attempt: attempt, State: officemodels.RunSessionStatePreparing,
|
||||
CreatedAt: now, Version: 1,
|
||||
}
|
||||
reserved, err := l.svc.RepoForTest().ReserveRunSession(ctx, session)
|
||||
if err != nil || !reserved {
|
||||
return officeservice.RunSessionLaunch{}, fmt.Errorf(
|
||||
"reserve test run session: reserved=%v err=%w", reserved, err)
|
||||
}
|
||||
acpSessionID := "acp-" + id
|
||||
if _, err := l.svc.RepoForTest().BindRunSessionExecution(
|
||||
ctx, id, "execution-"+id, launch.ProfileID, "test-adapter", "test-model", acpSessionID,
|
||||
); err != nil {
|
||||
return officeservice.RunSessionLaunch{}, err
|
||||
}
|
||||
if _, err := l.svc.RepoForTest().MarkRunSessionStarted(
|
||||
ctx, id, "execution-"+id, launch.ProfileID, "test-adapter", "test-model", acpSessionID,
|
||||
); err != nil {
|
||||
return officeservice.RunSessionLaunch{}, err
|
||||
}
|
||||
return officeservice.RunSessionLaunch{
|
||||
SessionID: id, ExecutionID: "execution-" + id,
|
||||
ExecutionProfileID: launch.ProfileID, Adapter: "test-adapter", Model: "test-model",
|
||||
ACPSessionID: acpSessionID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// awaitLightweightLaunchCount polls RunSchedulerTick until the launcher has
|
||||
// been called at least `want` times, bounded by a deadline. TickScheduledTriggers
|
||||
// and RunSchedulerTick are both synchronous in this harness (no detached
|
||||
// goroutine, unlike the heavy-routine launch path above), so a single tick
|
||||
// is expected to suffice; the poll loop exists only to absorb any queue
|
||||
// interaction it would otherwise be flaky to assume away.
|
||||
func awaitLightweightLaunchCount(
|
||||
t *testing.T, ctx context.Context, officeSvc *officeservice.Service, launcher *lightweightRunLauncher, want int,
|
||||
) {
|
||||
t.Helper()
|
||||
deadline := time.Now().Add(5 * time.Second)
|
||||
for {
|
||||
officeservice.RunSchedulerTick(officeSvc, ctx)
|
||||
if len(launcher.calls) >= want {
|
||||
return
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
t.Fatalf("timed out waiting for %d lightweight launch(es), got %d", want, len(launcher.calls))
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoutine_CronFire_LightweightReachesSession proves
|
||||
// AC-OFFICE-TASKLESS-001.1 and the first clause of .2 end-to-end: a cron
|
||||
// trigger firing a lightweight routine (empty task_template) must reach a
|
||||
// real office_run_sessions row with runs.session_id bound to it — never a
|
||||
// task_sessions row, which the design forbids for a taskless run — and two
|
||||
// separate fires must produce two distinct sessions rather than reusing one.
|
||||
//
|
||||
// This exercises the exact production wiring
|
||||
// (routines.RoutineService -> routineWakeupAdapter -> wakeup.Dispatcher ->
|
||||
// service.Service's scheduler), substituting only the run-session launcher
|
||||
// (RunSessionLauncher is the seam that boundary exists for) and the budget
|
||||
// checker (a real costs.CostService, because routine_dispatch_cron
|
||||
// classifies as unattended provenance and would otherwise be cancelled at
|
||||
// the no-budget-evaluator gate).
|
||||
func TestRoutine_CronFire_LightweightReachesSession(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
h := newRoutineCronHarness(t)
|
||||
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"})
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
eventBus := bus.NewMemoryEventBus(log)
|
||||
officeSvc := officeservice.NewService(officeservice.ServiceOptions{
|
||||
Repo: h.officeRepo, Logger: log, EventBus: eventBus,
|
||||
})
|
||||
// Synchronous handlers + a real event bus let this test simulate the
|
||||
// first attempt's completion (AgentCompleted) between the two fires
|
||||
// below, the same way an agent process reporting done would free the
|
||||
// agent for its next run — ClaimNextEligibleRun refuses to claim a
|
||||
// second run for an agent that still has one in status=claimed.
|
||||
officeSvc.SetSyncHandlers(true)
|
||||
if err := officeSvc.RegisterEventSubscribers(eventBus); err != nil {
|
||||
t.Fatalf("register event subscribers: %v", err)
|
||||
}
|
||||
activity := shared.NewActivityLogger(h.officeRepo, log)
|
||||
officeSvc.SetBudgetChecker(officecosts.NewCostService(h.officeRepo, log, activity, officeSvc, officeSvc))
|
||||
launcher := &lightweightRunLauncher{svc: officeSvc}
|
||||
officeSvc.SetRunSessionLauncher(launcher)
|
||||
|
||||
wakeupDispatcher := officewakeup.NewDispatcher(h.officeRepo, h.officeRepo, log)
|
||||
wakeupDispatcher.SetRoutineLookup(h.officeRepo)
|
||||
h.routineSvc.SetWakeupEnqueuer(&routineWakeupAdapter{repo: h.officeRepo, dispatcher: wakeupDispatcher})
|
||||
|
||||
// agent_profiles.agent_id is a NOT NULL FK to agents (CLI tool
|
||||
// registrations); newRoutineCronHarness opens its DB with FK
|
||||
// enforcement on (db.OpenSQLite), unlike office/service's base_test.go
|
||||
// (":memory:" with no _foreign_keys pragma). With no CLI tool rows
|
||||
// seeded, CreateAgentInstance's DefaultAgentID fallback resolves to ""
|
||||
// and the insert violates the FK, so seed one row here.
|
||||
if _, err := h.db.Exec(
|
||||
`INSERT INTO agents (id, name, created_at, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
|
||||
"cli-agent-lightweight", "CLI Agent",
|
||||
); err != nil {
|
||||
t.Fatalf("seed agents row: %v", err)
|
||||
}
|
||||
|
||||
agent := &officemodels.AgentInstance{
|
||||
WorkspaceID: h.workspaceID, Name: "lightweight-assignee",
|
||||
Role: officemodels.AgentRoleCEO, Status: officemodels.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := officeSvc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
|
||||
routine := &officeroutines.Routine{
|
||||
ID: "routine-light-1",
|
||||
WorkspaceID: h.workspaceID,
|
||||
Name: "Lightweight routine",
|
||||
TaskTemplate: "",
|
||||
AssigneeAgentProfileID: "routine-assignee",
|
||||
AssigneeAgentProfileID: agent.ID,
|
||||
Status: "active",
|
||||
Variables: "{}",
|
||||
// always_create (normalised to wakeup.PolicyAlwaysEnqueue) so the
|
||||
// second fire below creates its own run instead of coalescing into
|
||||
// the first fire's still-active one — this test's whole point is
|
||||
// asserting two fires produce two distinct sessions.
|
||||
ConcurrencyPolicy: "always_create",
|
||||
}
|
||||
if err := h.routineSvc.CreateRoutine(ctx, routine); err != nil {
|
||||
t.Fatalf("create routine: %v", err)
|
||||
@@ -320,19 +486,107 @@ func TestRoutine_CronFire_LightweightReachesSession(t *testing.T) {
|
||||
if err := h.routineSvc.CreateRoutineTrigger(ctx, trigger); err != nil {
|
||||
t.Fatalf("create trigger: %v", err)
|
||||
}
|
||||
|
||||
// First fire.
|
||||
triggers, err := h.officeRepo.ListTriggersByRoutineID(ctx, routine.ID)
|
||||
if err != nil || len(triggers) == 0 || triggers[0].NextRunAt == nil {
|
||||
t.Fatalf("list triggers: triggers=%d err=%v", len(triggers), err)
|
||||
}
|
||||
fireTime := *triggers[0].NextRunAt
|
||||
|
||||
if err := h.routineSvc.TickScheduledTriggers(ctx, fireTime.Add(time.Second)); err != nil {
|
||||
t.Fatalf("tick scheduled triggers: %v", err)
|
||||
}
|
||||
awaitLightweightLaunchCount(t, ctx, officeSvc, launcher, 1)
|
||||
|
||||
// Once card 49894d63 lands, assert a task_sessions row exists for the
|
||||
// agent the lightweight wakeup dispatched to.
|
||||
t.Fatal("unreachable: update this assertion when the lightweight path reaches a session")
|
||||
runs, err := officeSvc.ListRuns(ctx, h.workspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list runs after first fire: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
firstRun := runs[0]
|
||||
if firstRun.SessionID == "" {
|
||||
t.Fatalf("run.session_id is empty after launch, want it bound to an office_run_sessions row")
|
||||
}
|
||||
firstSession, err := officeSvc.RepoForTest().GetRunSession(ctx, firstRun.SessionID)
|
||||
if err != nil || firstSession == nil {
|
||||
t.Fatalf("get run session %q: %v", firstRun.SessionID, err)
|
||||
}
|
||||
if firstSession.RunID != firstRun.ID {
|
||||
t.Errorf("session.run_id = %q, want %q", firstSession.RunID, firstRun.ID)
|
||||
}
|
||||
|
||||
// Negative half of AC-OFFICE-TASKLESS-001.1: the lightweight fire must
|
||||
// create no tasks row and no task_sessions row — that is what
|
||||
// distinguishes it from the heavy-routine path above.
|
||||
if got := h.countRows(t, "tasks"); got != 0 {
|
||||
t.Errorf("tasks row count = %d, want 0 (lightweight fire must not create a task)", got)
|
||||
}
|
||||
if got := h.countRows(t, "task_sessions"); got != 0 {
|
||||
t.Errorf("task_sessions row count = %d, want 0 (taskless run must never create one)", got)
|
||||
}
|
||||
|
||||
// Simulate the first attempt completing, the same way a real agent
|
||||
// process finishing would: this both frees the agent for the second
|
||||
// fire's claim (ClaimNextEligibleRun skips an agent with any run still
|
||||
// status=claimed) and mirrors what an actual taskless run's lifecycle
|
||||
// looks like end to end.
|
||||
if err := eventBus.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: firstSession.ExecutionID, AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: firstRun.ID, RunSessionID: firstRun.SessionID, RunAttempt: 1, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: h.workspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish first attempt completion: %v", err)
|
||||
}
|
||||
|
||||
// Second fire: AC-OFFICE-TASKLESS-001.2's first clause — two fires
|
||||
// produce two distinct sessions, with no ACP session id reused.
|
||||
triggers, err = h.officeRepo.ListTriggersByRoutineID(ctx, routine.ID)
|
||||
if err != nil || len(triggers) == 0 || triggers[0].NextRunAt == nil {
|
||||
t.Fatalf("list triggers before second fire: triggers=%d err=%v", len(triggers), err)
|
||||
}
|
||||
secondFireTime := *triggers[0].NextRunAt
|
||||
if !secondFireTime.After(fireTime) {
|
||||
t.Fatalf("second fire's next_run_at = %v, want after first fire's %v", secondFireTime, fireTime)
|
||||
}
|
||||
if err := h.routineSvc.TickScheduledTriggers(ctx, secondFireTime.Add(time.Second)); err != nil {
|
||||
t.Fatalf("tick scheduled triggers (second fire): %v", err)
|
||||
}
|
||||
awaitLightweightLaunchCount(t, ctx, officeSvc, launcher, 2)
|
||||
|
||||
runs, err = officeSvc.ListRuns(ctx, h.workspaceID)
|
||||
if err != nil || len(runs) != 2 {
|
||||
t.Fatalf("list runs after second fire: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
var secondRun *officemodels.Run
|
||||
for _, r := range runs {
|
||||
if r.ID != firstRun.ID {
|
||||
secondRun = r
|
||||
break
|
||||
}
|
||||
}
|
||||
if secondRun == nil {
|
||||
t.Fatalf("expected a second, distinct run; runs=%#v", runs)
|
||||
}
|
||||
if secondRun.SessionID == "" {
|
||||
t.Fatalf("second run.session_id is empty, want it bound to an office_run_sessions row")
|
||||
}
|
||||
if secondRun.SessionID == firstRun.SessionID {
|
||||
t.Fatalf("second fire reused session id %q from the first fire, want a distinct session", secondRun.SessionID)
|
||||
}
|
||||
secondSession, err := officeSvc.RepoForTest().GetRunSession(ctx, secondRun.SessionID)
|
||||
if err != nil || secondSession == nil {
|
||||
t.Fatalf("get run session %q: %v", secondRun.SessionID, err)
|
||||
}
|
||||
if secondSession.ACPSessionID == firstSession.ACPSessionID {
|
||||
t.Errorf("second fire reused ACP session id %q from the first fire, want distinct",
|
||||
secondSession.ACPSessionID)
|
||||
}
|
||||
|
||||
if got := h.countRows(t, "tasks"); got != 0 {
|
||||
t.Errorf("tasks row count after second fire = %d, want 0", got)
|
||||
}
|
||||
if got := h.countRows(t, "task_sessions"); got != 0 {
|
||||
t.Errorf("task_sessions row count after second fire = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// stubAgentManager implements executor.AgentManagerClient with just enough
|
||||
|
||||
@@ -0,0 +1,261 @@
|
||||
package backendapp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
officecosts "github.com/kandev/kandev/internal/office/costs"
|
||||
officemodels "github.com/kandev/kandev/internal/office/models"
|
||||
officeservice "github.com/kandev/kandev/internal/office/service"
|
||||
"github.com/kandev/kandev/internal/office/shared"
|
||||
taskmodels "github.com/kandev/kandev/internal/task/models"
|
||||
taskservice "github.com/kandev/kandev/internal/task/service"
|
||||
)
|
||||
|
||||
// runOwnedEventHarness bundles a routineCronHarness (real taskSvc/workflowSvc/
|
||||
// orchestrator.Service on one bus) with a real office/service.Service wired
|
||||
// onto that SAME bus (h.eventBus), so a lifecycle event this test publishes
|
||||
// is observed by the real orchestrator exactly as it would be in production —
|
||||
// unlike TestRoutine_CronFire_LightweightReachesSession, which stands up its
|
||||
// own private, disconnected bus and therefore never exercises the
|
||||
// orchestrator's reaction to a taskless event at all.
|
||||
type runOwnedEventHarness struct {
|
||||
h *routineCronHarness
|
||||
officeSvc *officeservice.Service
|
||||
launcher *lightweightRunLauncher
|
||||
agent *officemodels.AgentInstance
|
||||
}
|
||||
|
||||
func newRunOwnedEventHarness(t *testing.T) *runOwnedEventHarness {
|
||||
t.Helper()
|
||||
h := newRoutineCronHarness(t)
|
||||
ctx := context.Background()
|
||||
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"})
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
officeSvc := officeservice.NewService(officeservice.ServiceOptions{
|
||||
Repo: h.officeRepo, Logger: log, EventBus: h.eventBus,
|
||||
})
|
||||
officeSvc.SetSyncHandlers(true)
|
||||
if err := officeSvc.RegisterEventSubscribers(h.eventBus); err != nil {
|
||||
t.Fatalf("register event subscribers: %v", err)
|
||||
}
|
||||
activity := shared.NewActivityLogger(h.officeRepo, log)
|
||||
officeSvc.SetBudgetChecker(officecosts.NewCostService(h.officeRepo, log, activity, officeSvc, officeSvc))
|
||||
launcher := &lightweightRunLauncher{svc: officeSvc}
|
||||
officeSvc.SetRunSessionLauncher(launcher)
|
||||
|
||||
// agent_profiles.agent_id is a NOT NULL FK to agents (CLI tool
|
||||
// registrations); newRoutineCronHarness opens its DB with FK enforcement
|
||||
// on, so CreateAgentInstance's DefaultAgentID fallback needs a seeded row.
|
||||
if _, err := h.db.Exec(
|
||||
`INSERT INTO agents (id, name, created_at, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
|
||||
"cli-agent-run-owned", "CLI Agent",
|
||||
); err != nil {
|
||||
t.Fatalf("seed agents row: %v", err)
|
||||
}
|
||||
|
||||
agent := &officemodels.AgentInstance{
|
||||
WorkspaceID: h.workspaceID, Name: "run-owned-event-assignee",
|
||||
// CEO defaults to SkipIdleRuns=false, sidestepping the idle-skip gate
|
||||
// entirely; this test is about the SessionID=="" guard, not idle-skip.
|
||||
Role: officemodels.AgentRoleCEO, Status: officemodels.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := officeSvc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
|
||||
return &runOwnedEventHarness{h: h, officeSvc: officeSvc, launcher: launcher, agent: agent}
|
||||
}
|
||||
|
||||
// launchTasklessRun queues and launches one real taskless run through the
|
||||
// production scheduler path, returning the run and the office_run_sessions
|
||||
// row the launcher bound to it — genuine run-owned identity, not a
|
||||
// hand-crafted one.
|
||||
func (r *runOwnedEventHarness) launchTasklessRun(
|
||||
t *testing.T, ctx context.Context,
|
||||
) (*officemodels.Run, *officemodels.RunSession) {
|
||||
t.Helper()
|
||||
wantCalls := len(r.launcher.calls) + 1
|
||||
if _, err := r.officeSvc.QueueRun(ctx, r.agent.ID, shared.RunReasonRoutineDispatchEvent, `{}`, ""); err != nil {
|
||||
t.Fatalf("queue run: %v", err)
|
||||
}
|
||||
awaitLightweightLaunchCount(t, ctx, r.officeSvc, r.launcher, wantCalls)
|
||||
|
||||
runs, err := r.officeSvc.ListRuns(ctx, r.h.workspaceID)
|
||||
if err != nil {
|
||||
t.Fatalf("list runs: %v", err)
|
||||
}
|
||||
var run *officemodels.Run
|
||||
for _, candidate := range runs {
|
||||
if candidate.SessionID != "" {
|
||||
session, sessErr := r.officeSvc.RepoForTest().GetRunSession(ctx, candidate.SessionID)
|
||||
if sessErr == nil && session != nil && session.State != officemodels.RunSessionStateFinished &&
|
||||
session.State != officemodels.RunSessionStateInterrupted {
|
||||
run = candidate
|
||||
}
|
||||
}
|
||||
}
|
||||
if run == nil {
|
||||
t.Fatalf("no launched, still-open run found among %#v", runs)
|
||||
}
|
||||
session, err := r.officeSvc.RepoForTest().GetRunSession(ctx, run.SessionID)
|
||||
if err != nil || session == nil {
|
||||
t.Fatalf("get run session %q: %v", run.SessionID, err)
|
||||
}
|
||||
return run, session
|
||||
}
|
||||
|
||||
// decoyTaskSession creates a real task and a real, hand-inserted RUNNING
|
||||
// task_session under it — a row that WOULD be read or mutated by
|
||||
// handleAgentCompletedLocked/handleAgentFailedLocked if either ever
|
||||
// resolved a task session for a run-owned (SessionID=="") lifecycle event.
|
||||
func decoyTaskSession(t *testing.T, ctx context.Context, h *routineCronHarness) *taskmodels.TaskSession {
|
||||
t.Helper()
|
||||
workflows, err := h.taskSvc.ListWorkflows(ctx, h.workspaceID, true)
|
||||
if err != nil || len(workflows) == 0 {
|
||||
t.Fatalf("ListWorkflows: workflows=%d err=%v", len(workflows), err)
|
||||
}
|
||||
steps, err := h.workflowSvc.ListStepsByWorkflow(ctx, workflows[0].ID)
|
||||
if err != nil || len(steps) == 0 {
|
||||
t.Fatalf("ListStepsByWorkflow: steps=%d err=%v", len(steps), err)
|
||||
}
|
||||
taskResult, err := h.taskSvc.CreateTask(ctx, &taskservice.CreateTaskRequest{
|
||||
WorkspaceID: h.workspaceID, WorkflowID: workflows[0].ID,
|
||||
WorkflowStepID: steps[0].ID, Title: "Decoy task for run-owned event guard",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateTask: %v", err)
|
||||
}
|
||||
|
||||
now := time.Now().UTC()
|
||||
session := &taskmodels.TaskSession{
|
||||
ID: "decoy-session-" + taskResult.Task.ID, TaskID: taskResult.Task.ID,
|
||||
AgentExecutionID: "decoy-execution", State: taskmodels.TaskSessionStateRunning,
|
||||
StartedAt: now, UpdatedAt: now,
|
||||
}
|
||||
if err := h.taskRepo.CreateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("create decoy task session: %v", err)
|
||||
}
|
||||
stored, err := h.taskRepo.GetTaskSession(ctx, session.ID)
|
||||
if err != nil || stored == nil {
|
||||
t.Fatalf("get decoy task session: %v", err)
|
||||
}
|
||||
return stored
|
||||
}
|
||||
|
||||
// subscribeTaskMutationEvents counts task.updated/task.moved publications on
|
||||
// bus for the duration of the calling test, per the repo's own rule
|
||||
// (apps/backend/CLAUDE.md "Task lifecycle events") that any task-row
|
||||
// mutation must publish one of these — so a zero count after the event under
|
||||
// test is a repo-convention-aligned proxy for "no task mutation occurred".
|
||||
func subscribeTaskMutationEvents(t *testing.T, eb bus.EventBus) *int32 {
|
||||
t.Helper()
|
||||
var count int32
|
||||
handler := func(_ context.Context, _ *bus.Event) error {
|
||||
atomic.AddInt32(&count, 1)
|
||||
return nil
|
||||
}
|
||||
for _, subject := range []string{events.TaskUpdated, events.TaskMoved} {
|
||||
sub, err := eb.Subscribe(subject, handler)
|
||||
if err != nil {
|
||||
t.Fatalf("subscribe %q: %v", subject, err)
|
||||
}
|
||||
t.Cleanup(func() { _ = sub.Unsubscribe() })
|
||||
}
|
||||
return &count
|
||||
}
|
||||
|
||||
// TestRunOwnedLifecycleEvent_AgentCompleted_NoTaskSessionEffect pins item 3 of
|
||||
// task-06's completion half: a run-owned AgentCompleted event (SessionID=="",
|
||||
// carrying real RunID/RunSessionID/RunAttempt from a genuine taskless launch)
|
||||
// must leave every existing task_session untouched and trigger no
|
||||
// task.updated/task.moved workflow-transition event. handleAgentCompleted's
|
||||
// `if data.SessionID == ""` branch (internal/orchestrator/event_handlers_agent.go)
|
||||
// routes straight to handleAgentCompletedLocked with a nil guard, which then
|
||||
// fails to resolve any task session for SessionID=="" and returns before any
|
||||
// workflow evaluation — this test proves that behavior against the real
|
||||
// orchestrator wired on the same bus the real Office service publishes on,
|
||||
// not just by reading the source.
|
||||
func TestRunOwnedLifecycleEvent_AgentCompleted_NoTaskSessionEffect(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
r := newRunOwnedEventHarness(t)
|
||||
decoy := decoyTaskSession(t, ctx, r.h)
|
||||
run, session := r.launchTasklessRun(t, ctx)
|
||||
|
||||
mutationCount := subscribeTaskMutationEvents(t, r.h.eventBus)
|
||||
|
||||
if err := r.h.eventBus.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test",
|
||||
lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: session.ExecutionID, AgentID: "test-adapter", AgentProfileID: r.agent.ID,
|
||||
RunID: run.ID, RunSessionID: session.ID, RunAttempt: session.Attempt,
|
||||
OwnerKind: lifecycle.ExecutionOwnerRun, WorkspaceID: r.h.workspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish run-owned agent completed: %v", err)
|
||||
}
|
||||
|
||||
after, err := r.h.taskRepo.GetTaskSession(ctx, decoy.ID)
|
||||
if err != nil || after == nil {
|
||||
t.Fatalf("get decoy task session after event: %v", err)
|
||||
}
|
||||
if after.State != decoy.State {
|
||||
t.Errorf("decoy session state = %q, want unchanged %q", after.State, decoy.State)
|
||||
}
|
||||
if !after.UpdatedAt.Equal(decoy.UpdatedAt) {
|
||||
t.Errorf("decoy session updated_at = %v, want unchanged %v", after.UpdatedAt, decoy.UpdatedAt)
|
||||
}
|
||||
if got := atomic.LoadInt32(mutationCount); got != 0 {
|
||||
t.Errorf("task.updated/task.moved publications = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunOwnedLifecycleEvent_AgentFailed_NoTaskSessionEffect is the failure
|
||||
// counterpart: handleAgentFailed's `if data.SessionID == ""` branch dispatches
|
||||
// straight to handleAgentFailedLocked, whose only session/task-scoped
|
||||
// branches (shouldDropSessionFailure, handleTransientFailure,
|
||||
// routeDynamicAgentFailure, handleRecoverableFailureLockedState) all guard on
|
||||
// data.SessionID != "" and are skipped; the no-session fallback tail
|
||||
// (scheduler.HandleTaskCompleted/RetryTask/writeTaskReviewState) operates on
|
||||
// data.TaskID, which is likewise "" for a run-owned event, so it resolves no
|
||||
// real task either.
|
||||
func TestRunOwnedLifecycleEvent_AgentFailed_NoTaskSessionEffect(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
r := newRunOwnedEventHarness(t)
|
||||
decoy := decoyTaskSession(t, ctx, r.h)
|
||||
run, session := r.launchTasklessRun(t, ctx)
|
||||
|
||||
mutationCount := subscribeTaskMutationEvents(t, r.h.eventBus)
|
||||
|
||||
if err := r.h.eventBus.Publish(ctx, events.AgentFailed, bus.NewEvent(events.AgentFailed, "test",
|
||||
lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: session.ExecutionID, AgentID: "test-adapter", AgentProfileID: r.agent.ID,
|
||||
RunID: run.ID, RunSessionID: session.ID, RunAttempt: session.Attempt,
|
||||
OwnerKind: lifecycle.ExecutionOwnerRun, WorkspaceID: r.h.workspaceID, Status: "FAILED",
|
||||
ErrorMessage: "run-owned provider failure",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish run-owned agent failed: %v", err)
|
||||
}
|
||||
|
||||
after, err := r.h.taskRepo.GetTaskSession(ctx, decoy.ID)
|
||||
if err != nil || after == nil {
|
||||
t.Fatalf("get decoy task session after event: %v", err)
|
||||
}
|
||||
if after.State != decoy.State {
|
||||
t.Errorf("decoy session state = %q, want unchanged %q", after.State, decoy.State)
|
||||
}
|
||||
if !after.UpdatedAt.Equal(decoy.UpdatedAt) {
|
||||
t.Errorf("decoy session updated_at = %v, want unchanged %v", after.UpdatedAt, decoy.UpdatedAt)
|
||||
}
|
||||
if got := atomic.LoadInt32(mutationCount); got != 0 {
|
||||
t.Errorf("task.updated/task.moved publications = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,315 @@
|
||||
package backendapp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/jmoiron/sqlx"
|
||||
|
||||
runtimeapi "github.com/kandev/kandev/internal/agent/runtime"
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
officecosts "github.com/kandev/kandev/internal/office/costs"
|
||||
officemodels "github.com/kandev/kandev/internal/office/models"
|
||||
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
officeservice "github.com/kandev/kandev/internal/office/service"
|
||||
"github.com/kandev/kandev/internal/office/shared"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
)
|
||||
|
||||
// fakeReconcileBackend is a runtimeapi.Backend + runtimeapi.RunOwnerRecovery
|
||||
// test double standing in for the lifecycle Manager. Launch records a live
|
||||
// execution carrying the owner officeRunSessionLauncher.AdmitExecution
|
||||
// admitted, so StartRunSession's real reserve/bind/admit/start sequence runs
|
||||
// unmodified. evict then simulates a backend restart losing that execution
|
||||
// from the recovered runtime inventory — the exact condition
|
||||
// ReconcileRunSessions' "not found after restart" branch exists to detect.
|
||||
type fakeReconcileBackend struct {
|
||||
mu sync.Mutex
|
||||
executions map[string]*lifecycle.AgentExecution
|
||||
nextID int
|
||||
stoppedOwners []runtimeapi.ExecutionOwner
|
||||
}
|
||||
|
||||
func newFakeReconcileBackend() *fakeReconcileBackend {
|
||||
return &fakeReconcileBackend{executions: map[string]*lifecycle.AgentExecution{}}
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) Launch(_ context.Context, req *lifecycle.LaunchRequest) (*lifecycle.AgentExecution, error) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.nextID++
|
||||
id := fmt.Sprintf("fake-exec-%d", f.nextID)
|
||||
exec := &lifecycle.AgentExecution{
|
||||
ID: id,
|
||||
Owner: req.Owner,
|
||||
OwnerAdmission: req.OwnerAdmission,
|
||||
AgentProfileID: req.AgentProfileID,
|
||||
WorkspaceID: req.WorkspaceID,
|
||||
Status: v1.AgentStatusRunning,
|
||||
StartedAt: time.Now().UTC(),
|
||||
ACPSessionID: "acp-" + id,
|
||||
}
|
||||
f.executions[id] = exec
|
||||
return exec, nil
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) StartAgentProcess(_ context.Context, _ string) error { return nil }
|
||||
|
||||
func (f *fakeReconcileBackend) PromptAgent(
|
||||
_ context.Context, _ string, _ string, _ []v1.MessageAttachment, _ bool,
|
||||
) (*lifecycle.PromptResult, error) {
|
||||
return &lifecycle.PromptResult{}, nil
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) StopAgentWithReason(_ context.Context, _ string, _ string, _ bool) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) GetExecution(executionID string) (*lifecycle.AgentExecution, bool) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
exec, ok := f.executions[executionID]
|
||||
return exec, ok
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) SetMcpMode(_ context.Context, _ string, _ string) error { return nil }
|
||||
|
||||
// evict removes an execution, simulating a backend restart that lost the
|
||||
// runtime's record of it.
|
||||
func (f *fakeReconcileBackend) evict(executionID string) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
delete(f.executions, executionID)
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) StopRunOwnerForRecovery(_ context.Context, owner runtimeapi.ExecutionOwner) error {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.stoppedOwners = append(f.stoppedOwners, owner)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeReconcileBackend) stopCallCount() int {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
return len(f.stoppedOwners)
|
||||
}
|
||||
|
||||
var _ runtimeapi.Backend = (*fakeReconcileBackend)(nil)
|
||||
var _ runtimeapi.RunOwnerRecovery = (*fakeReconcileBackend)(nil)
|
||||
|
||||
// reconcileHarness wires a real office/service.Service, a real
|
||||
// officesqlite.Repository, and a real officeRunSessionLauncher over a fake
|
||||
// runtime backend, so a test can drive officeRunSessionLauncher.StartRunSession
|
||||
// and ReconcileRunSessions directly instead of reproducing their effects.
|
||||
type reconcileHarness struct {
|
||||
svc *officeservice.Service
|
||||
repo *officesqlite.Repository
|
||||
eb bus.EventBus
|
||||
launcher *officeRunSessionLauncher
|
||||
backend *fakeReconcileBackend
|
||||
}
|
||||
|
||||
func newReconcileHarness(t *testing.T) *reconcileHarness {
|
||||
t.Helper()
|
||||
dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "reconcile.db"))
|
||||
if err != nil {
|
||||
t.Fatalf("OpenSQLite: %v", err)
|
||||
}
|
||||
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
|
||||
t.Cleanup(func() { _ = sqlxDB.Close() })
|
||||
|
||||
// Office agent CRUD reads/writes the merged agent_profiles table (ADR
|
||||
// 0005 Wave C), which only the settings store schema creates.
|
||||
if _, _, err := settingsstore.Provide(sqlxDB, sqlxDB, nil); err != nil {
|
||||
t.Fatalf("settings store init: %v", err)
|
||||
}
|
||||
officeRepo, err := officesqlite.NewWithDB(sqlxDB, sqlxDB, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("office repository: %v", err)
|
||||
}
|
||||
|
||||
// agent_profiles.agent_id is a NOT NULL FK to agents (CLI tool
|
||||
// registrations); db.OpenSQLite enforces foreign keys, so seed one row
|
||||
// for CreateAgentInstance's DefaultAgentID fallback to resolve to.
|
||||
if _, err := sqlxDB.Exec(
|
||||
`INSERT INTO agents (id, name, created_at, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
|
||||
"cli-agent-reconcile", "CLI Agent",
|
||||
); err != nil {
|
||||
t.Fatalf("seed agents row: %v", err)
|
||||
}
|
||||
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"})
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
eventBus := bus.NewMemoryEventBus(log)
|
||||
|
||||
svc := officeservice.NewService(officeservice.ServiceOptions{
|
||||
Repo: officeRepo, Logger: log, EventBus: eventBus,
|
||||
})
|
||||
svc.SetSyncHandlers(true)
|
||||
if err := svc.RegisterEventSubscribers(eventBus); err != nil {
|
||||
t.Fatalf("register event subscribers: %v", err)
|
||||
}
|
||||
activity := shared.NewActivityLogger(officeRepo, log)
|
||||
svc.SetBudgetChecker(officecosts.NewCostService(officeRepo, log, activity, svc, svc))
|
||||
|
||||
backend := newFakeReconcileBackend()
|
||||
launcher := newOfficeRunSessionLauncher(officeRepo, backend, log)
|
||||
svc.SetRunSessionLauncher(launcher)
|
||||
|
||||
return &reconcileHarness{svc: svc, repo: officeRepo, eb: eventBus, launcher: launcher, backend: backend}
|
||||
}
|
||||
|
||||
// TestContinuationSummary_InterruptedAttemptRecoveredAtStartup_PreservesPriorSummary
|
||||
// pins the third failure mode: an attempt interrupted by a backend restart.
|
||||
// Unlike a hand-simulated version, this drives the real
|
||||
// officeRunSessionLauncher.ReconcileRunSessions against a real
|
||||
// office_run_sessions row reserved, bound, and started by the real
|
||||
// StartRunSession path, with only the runtime backend faked — proving the
|
||||
// actual startup selection, liveness, and requeue branches, not a
|
||||
// reproduction of their presumed effects.
|
||||
func TestContinuationSummary_InterruptedAttemptRecoveredAtStartup_PreservesPriorSummary(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
h := newReconcileHarness(t)
|
||||
|
||||
agent := &officemodels.AgentInstance{
|
||||
ID: "taskless-interrupt-agent", WorkspaceID: "ws-1", Name: "taskless-interrupt-agent",
|
||||
Role: officemodels.AgentRoleCEO, Status: officemodels.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := h.svc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
|
||||
// First attempt: launch and complete normally, producing the baseline
|
||||
// continuation summary that recovery must not disturb.
|
||||
if _, err := h.svc.QueueRun(ctx, agent.ID, officeservice.RunReasonRoutineTrigger, `{}`, "interrupt-first"); err != nil {
|
||||
t.Fatalf("queue first: %v", err)
|
||||
}
|
||||
officeservice.RunSchedulerTick(h.svc, ctx)
|
||||
runs, err := h.svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list first run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
first := runs[0]
|
||||
firstSession, err := h.repo.GetRunSession(ctx, first.SessionID)
|
||||
if err != nil || firstSession == nil {
|
||||
t.Fatalf("get first run session %q: %v", first.SessionID, err)
|
||||
}
|
||||
if err := h.eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: firstSession.ExecutionID, AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: first.ID, RunSessionID: first.SessionID, RunAttempt: firstSession.Attempt, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish first completion: %v", err)
|
||||
}
|
||||
before, err := h.svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || before == nil || before.Content == "" {
|
||||
t.Fatalf("writer did not produce a baseline summary: %v (row=%v)", err, before)
|
||||
}
|
||||
|
||||
// Second attempt: launch normally through the real StartRunSession path,
|
||||
// then never complete it — this is the interrupted attempt, still
|
||||
// carrying a live (per the fake backend) execution.
|
||||
if _, err := h.svc.QueueRun(ctx, agent.ID, officeservice.RunReasonRoutineTrigger, `{}`, "interrupt-second"); err != nil {
|
||||
t.Fatalf("queue second: %v", err)
|
||||
}
|
||||
officeservice.RunSchedulerTick(h.svc, ctx)
|
||||
runs, err = h.svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 2 {
|
||||
t.Fatalf("list second run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
var second *officemodels.Run
|
||||
for _, r := range runs {
|
||||
if r.ID != first.ID {
|
||||
second = r
|
||||
}
|
||||
}
|
||||
if second == nil || second.SessionID == "" {
|
||||
t.Fatalf("expected a second run with a bound session, got %#v", runs)
|
||||
}
|
||||
secondSession, err := h.repo.GetRunSession(ctx, second.SessionID)
|
||||
if err != nil || secondSession == nil || secondSession.ExecutionID == "" {
|
||||
t.Fatalf("get second run session %q: %v (session=%v)", second.SessionID, err, secondSession)
|
||||
}
|
||||
|
||||
// Simulate a backend restart: the runtime's recovered inventory no
|
||||
// longer contains this execution.
|
||||
h.backend.evict(secondSession.ExecutionID)
|
||||
|
||||
if err := h.launcher.ReconcileRunSessions(ctx); err != nil {
|
||||
t.Fatalf("reconcile run sessions: %v", err)
|
||||
}
|
||||
|
||||
if got := h.backend.stopCallCount(); got != 1 {
|
||||
t.Fatalf("StopRunOwnerForRecovery calls = %d, want 1", got)
|
||||
}
|
||||
recovered, err := h.repo.GetRunSession(ctx, secondSession.ID)
|
||||
if err != nil || recovered == nil {
|
||||
t.Fatalf("get recovered session %q: %v", secondSession.ID, err)
|
||||
}
|
||||
if recovered.State != officemodels.RunSessionStateInterrupted || recovered.ErrorMessage == "" {
|
||||
t.Fatalf("recovered session = %#v, want state=interrupted with an error message", recovered)
|
||||
}
|
||||
runsAfterReconcile, err := h.svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil {
|
||||
t.Fatalf("list runs after reconcile: %v", err)
|
||||
}
|
||||
var secondAfterReconcile *officemodels.Run
|
||||
for _, r := range runsAfterReconcile {
|
||||
if r.ID == second.ID {
|
||||
secondAfterReconcile = r
|
||||
}
|
||||
}
|
||||
if secondAfterReconcile == nil || secondAfterReconcile.Status != officeservice.RunStatusQueued {
|
||||
t.Fatalf("second run after reconcile = %#v, want status %q", secondAfterReconcile, officeservice.RunStatusQueued)
|
||||
}
|
||||
|
||||
// The recovery calls themselves must not have touched the summary.
|
||||
afterRecovery, err := h.svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || afterRecovery == nil {
|
||||
t.Fatalf("read summary after interrupted-attempt recovery: %v (row=%v)", err, afterRecovery)
|
||||
}
|
||||
if afterRecovery.Content != before.Content || afterRecovery.UpdatedByRunID != before.UpdatedByRunID {
|
||||
t.Fatalf("interrupted-attempt recovery clobbered continuation summary: before=%#v after=%#v", before, afterRecovery)
|
||||
}
|
||||
|
||||
// The requeued run must still be able to relaunch and complete
|
||||
// normally, attributing a fresh summary write to itself without
|
||||
// disturbing the untouched baseline row's identity mid-flight.
|
||||
officeservice.RunSchedulerTick(h.svc, ctx)
|
||||
relaunched, err := h.repo.GetRun(ctx, second.ID)
|
||||
if err != nil || relaunched == nil || relaunched.SessionID == "" || relaunched.SessionID == second.SessionID {
|
||||
t.Fatalf("relaunched run = %#v, err=%v, want a new bound session distinct from %q", relaunched, err, second.SessionID)
|
||||
}
|
||||
relaunchedSession, err := h.repo.GetRunSession(ctx, relaunched.SessionID)
|
||||
if err != nil || relaunchedSession == nil {
|
||||
t.Fatalf("get relaunched session %q: %v", relaunched.SessionID, err)
|
||||
}
|
||||
if err := h.eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: relaunchedSession.ExecutionID, AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: second.ID, RunSessionID: relaunched.SessionID, RunAttempt: relaunchedSession.Attempt, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish relaunch completion: %v", err)
|
||||
}
|
||||
final, err := h.svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || final == nil {
|
||||
t.Fatalf("read summary after relaunch completion: %v (row=%v)", err, final)
|
||||
}
|
||||
if final.UpdatedByRunID != second.ID {
|
||||
t.Fatalf("summary after relaunch completion attributed to %q, want the recovered run %q",
|
||||
final.UpdatedByRunID, second.ID)
|
||||
}
|
||||
}
|
||||
@@ -75,24 +75,39 @@ func (s *Service) AppendRunEvent(
|
||||
runID, eventType, level string,
|
||||
payload map[string]interface{},
|
||||
) {
|
||||
if runID == "" {
|
||||
return
|
||||
}
|
||||
body := "{}"
|
||||
if len(payload) > 0 {
|
||||
if b, err := json.Marshal(payload); err == nil {
|
||||
body = string(b)
|
||||
}
|
||||
}
|
||||
evt, err := s.repo.AppendRunEvent(ctx, runID, eventType, level, body)
|
||||
if err != nil {
|
||||
if err := s.appendRunEventStrict(ctx, runID, eventType, level, payload); err != nil {
|
||||
s.logger.Warn("append run event failed",
|
||||
zap.String("run_id", runID),
|
||||
zap.String("event_type", eventType),
|
||||
zap.Error(err))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// appendRunEventStrict persists a run event and returns persistence or payload
|
||||
// errors to the caller. Event-bus publication remains best effort because the
|
||||
// database row is the durable record and the bus only notifies live clients.
|
||||
func (s *Service) appendRunEventStrict(
|
||||
ctx context.Context,
|
||||
runID, eventType, level string,
|
||||
payload map[string]interface{},
|
||||
) error {
|
||||
if runID == "" {
|
||||
return nil
|
||||
}
|
||||
body := "{}"
|
||||
if len(payload) > 0 {
|
||||
b, err := json.Marshal(payload)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
body = string(b)
|
||||
}
|
||||
evt, err := s.repo.AppendRunEvent(ctx, runID, eventType, level, body)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
s.publishRunEventAppended(ctx, evt)
|
||||
return nil
|
||||
}
|
||||
|
||||
// publishRunEventAppended emits the per-run notification on the event
|
||||
|
||||
@@ -0,0 +1,293 @@
|
||||
package service_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
"github.com/kandev/kandev/internal/office/service"
|
||||
)
|
||||
|
||||
// lastLaunchedSession resolves the run session tasklessTestLauncher most
|
||||
// recently reserved, by its own naming convention ("run-session-<n>" where n
|
||||
// is the shared call counter across every run this launcher instance has
|
||||
// launched), and reads back its real attempt number. The launcher's attempt
|
||||
// counter is global to the launcher instance rather than per-run, so a
|
||||
// second launch — whether for a second run or a relaunch of a recovered one
|
||||
// — always carries attempt 2, never 1; a lifecycle event that hardcodes
|
||||
// RunAttempt: 1 for that second launch silently fails to resolve to a
|
||||
// claimed run (resolveLifecycleRun's exact-run-session-event guard treats
|
||||
// it as a stale predecessor and no-ops), which would make a "did the event
|
||||
// clobber anything" assertion vacuously true instead of meaningful.
|
||||
func lastLaunchedSession(
|
||||
t *testing.T, ctx context.Context, svc *service.Service, launcher *tasklessTestLauncher,
|
||||
) (sessionID string, attempt int) {
|
||||
t.Helper()
|
||||
sessionID = fmt.Sprintf("run-session-%d", len(launcher.calls))
|
||||
session, err := svc.RepoForTest().GetRunSession(ctx, sessionID)
|
||||
if err != nil || session == nil {
|
||||
t.Fatalf("get launched run session %s: %v (session=%v)", sessionID, err, session)
|
||||
}
|
||||
return sessionID, session.Attempt
|
||||
}
|
||||
|
||||
// TestContinuationSummary_FailedTasklessAttempt_PreservesPriorSummary_AgentScope
|
||||
// pins REQ-OFFICE-TASKLESS-001's event-path guarantee: a failed/interrupted
|
||||
// attempt must not clobber the last successful continuation summary.
|
||||
// refreshContinuationSummary is called only from
|
||||
// handleTasklessAgentCompleted; handleTasklessAgentFailed never calls it,
|
||||
// so a failed attempt structurally cannot reach the writer. This drives a
|
||||
// real success (which writes the summary through the production writer,
|
||||
// never by hand) followed by a real failure for a second run under the
|
||||
// same agent:<id> scope, and asserts the row is byte-identical afterward.
|
||||
func TestContinuationSummary_FailedTasklessAttempt_PreservesPriorSummary_AgentScope(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
launcher := &tasklessTestLauncher{svc: svc}
|
||||
svc.SetRunSessionLauncher(launcher)
|
||||
|
||||
agent := &models.AgentInstance{
|
||||
ID: "taskless-fail-agent", WorkspaceID: "ws-1", Name: "taskless-fail-agent",
|
||||
Role: models.AgentRoleCEO, Status: models.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "fail-first"); err != nil {
|
||||
t.Fatalf("queue first: %v", err)
|
||||
}
|
||||
runs, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list first run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
first := runs[0]
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
if err := eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-run-session-1", AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: first.ID, RunSessionID: "run-session-1", RunAttempt: 1, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish first completion: %v", err)
|
||||
}
|
||||
before, err := svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || before == nil || before.Content == "" {
|
||||
t.Fatalf("writer did not produce a baseline summary: %v (row=%v)", err, before)
|
||||
}
|
||||
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "fail-second"); err != nil {
|
||||
t.Fatalf("queue second: %v", err)
|
||||
}
|
||||
runs, err = svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 2 {
|
||||
t.Fatalf("list second run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
second := runs[0]
|
||||
if second.ID == first.ID {
|
||||
second = runs[1]
|
||||
}
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
sessionID2, attempt2 := lastLaunchedSession(t, ctx, svc, launcher)
|
||||
if err := eb.Publish(ctx, events.AgentFailed, bus.NewEvent(events.AgentFailed, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-" + sessionID2, AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: second.ID, RunSessionID: sessionID2, RunAttempt: attempt2, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "FAILED", ErrorMessage: "agent crashed",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish second failure: %v", err)
|
||||
}
|
||||
|
||||
after, err := svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || after == nil {
|
||||
t.Fatalf("read summary after failed attempt: %v (row=%v)", err, after)
|
||||
}
|
||||
if after.Content != before.Content || after.UpdatedByRunID != before.UpdatedByRunID {
|
||||
t.Fatalf("failed attempt clobbered continuation summary: before=%#v after=%#v", before, after)
|
||||
}
|
||||
}
|
||||
|
||||
// TestContinuationSummary_FailedTasklessAttempt_PreservesPriorSummary_RoutineScope
|
||||
// is the routine:<id> counterpart. QueueRun has no context_snapshot
|
||||
// parameter, so a routine-scoped run is seeded via raw SQL exactly as
|
||||
// CreateRun would persist it (continuation_scope decided once, at
|
||||
// creation) — mirroring continuation_summary_reader_test.go's established
|
||||
// pattern, including that pattern's choice to carry only RunID (no
|
||||
// RunSessionID) on the lifecycle event — while the summary itself is
|
||||
// always produced by the real completion-event writer, never written by
|
||||
// hand.
|
||||
func TestContinuationSummary_FailedTasklessAttempt_PreservesPriorSummary_RoutineScope(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
createTestAgent(t, svc, "ws-1", "agent-scope-r-fail")
|
||||
|
||||
claimedAt := time.Now().UTC()
|
||||
svc.ExecSQL(t, `
|
||||
INSERT INTO runs (
|
||||
id, agent_profile_id, reason, payload, status, coalesced_count,
|
||||
context_snapshot, continuation_scope, requested_at, claimed_at
|
||||
) VALUES (
|
||||
'run-r-fail-1', 'agent-scope-r-fail', 'routine_trigger', '{}',
|
||||
'claimed', 1, '{"routine_id":"rt-fail"}', 'routine:rt-fail', ?, ?
|
||||
)
|
||||
`, claimedAt, claimedAt)
|
||||
|
||||
if err := eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-r-fail-1", AgentID: "test-adapter", AgentProfileID: "agent-scope-r-fail",
|
||||
RunID: "run-r-fail-1", WorkspaceID: "ws-1", Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish completion: %v", err)
|
||||
}
|
||||
before, err := svc.GetContinuationSummaryForTest(ctx, "agent-scope-r-fail", "routine:rt-fail")
|
||||
if err != nil || before == nil || before.Content == "" {
|
||||
t.Fatalf("writer did not produce a summary under routine:rt-fail: %v (row=%v)", err, before)
|
||||
}
|
||||
|
||||
claimedAt2 := claimedAt.Add(time.Minute)
|
||||
svc.ExecSQL(t, `
|
||||
INSERT INTO runs (
|
||||
id, agent_profile_id, reason, payload, status, coalesced_count,
|
||||
context_snapshot, continuation_scope, requested_at, claimed_at
|
||||
) VALUES (
|
||||
'run-r-fail-2', 'agent-scope-r-fail', 'routine_trigger', '{}',
|
||||
'claimed', 1, '{"routine_id":"rt-fail"}', 'routine:rt-fail', ?, ?
|
||||
)
|
||||
`, claimedAt2, claimedAt2)
|
||||
|
||||
if err := eb.Publish(ctx, events.AgentFailed, bus.NewEvent(events.AgentFailed, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-r-fail-2", AgentID: "test-adapter", AgentProfileID: "agent-scope-r-fail",
|
||||
RunID: "run-r-fail-2", WorkspaceID: "ws-1", Status: "FAILED", ErrorMessage: "boom",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish failure: %v", err)
|
||||
}
|
||||
|
||||
after, err := svc.GetContinuationSummaryForTest(ctx, "agent-scope-r-fail", "routine:rt-fail")
|
||||
if err != nil || after == nil {
|
||||
t.Fatalf("read summary after failure: %v (row=%v)", err, after)
|
||||
}
|
||||
if after.Content != before.Content || after.UpdatedByRunID != before.UpdatedByRunID {
|
||||
t.Fatalf("failed attempt clobbered continuation summary: before=%#v after=%#v", before, after)
|
||||
}
|
||||
}
|
||||
|
||||
// TestContinuationSummary_InterruptedAttemptRecoveredAtStartup_PreservesPriorSummary
|
||||
// (the third failure mode: an attempt interrupted by a backend restart) now
|
||||
// lives in internal/backendapp/office_run_session_launcher_reconcile_test.go,
|
||||
// where it can drive the real officeRunSessionLauncher.ReconcileRunSessions
|
||||
// against a real office_run_sessions row instead of reproducing its effects
|
||||
// by hand — that function lives outside this package's scope.
|
||||
|
||||
// TestContinuationSummary_UpsertFailure_PreservesRunCompletionAndRecordsRunEvent
|
||||
// pins the plan's one allowed production change: when the continuation
|
||||
// summary upsert itself fails, refreshContinuationSummary records a new run
|
||||
// event naming the scope, does not roll anything back, and the run still
|
||||
// reaches its terminal state. The table is renamed away so the write fails
|
||||
// with a genuine SQL error; summary.LoadInputs's own sub-helpers swallow
|
||||
// their errors internally (confirmed: no path in
|
||||
// internal/office/summary/inputs.go returns non-nil to its caller), so this
|
||||
// is the only reachable failure surface today — the load-error branch added
|
||||
// alongside it is defensive, not currently triggerable through the public
|
||||
// API.
|
||||
func TestContinuationSummary_UpsertFailure_PreservesRunCompletionAndRecordsRunEvent(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
launcher := &tasklessTestLauncher{svc: svc}
|
||||
svc.SetRunSessionLauncher(launcher)
|
||||
|
||||
agent := &models.AgentInstance{
|
||||
ID: "taskless-upsert-fail-agent", WorkspaceID: "ws-1", Name: "taskless-upsert-fail-agent",
|
||||
Role: models.AgentRoleCEO, Status: models.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "upsert-fail-first"); err != nil {
|
||||
t.Fatalf("queue first: %v", err)
|
||||
}
|
||||
runs, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list first run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
first := runs[0]
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
if err := eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-run-session-1", AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: first.ID, RunSessionID: "run-session-1", RunAttempt: 1, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish first completion: %v", err)
|
||||
}
|
||||
before, err := svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || before == nil || before.Content == "" {
|
||||
t.Fatalf("writer did not produce a baseline summary: %v (row=%v)", err, before)
|
||||
}
|
||||
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "upsert-fail-second"); err != nil {
|
||||
t.Fatalf("queue second: %v", err)
|
||||
}
|
||||
runs, err = svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 2 {
|
||||
t.Fatalf("list second run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
second := runs[0]
|
||||
if second.ID == first.ID {
|
||||
second = runs[1]
|
||||
}
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
sessionID2, attempt2 := lastLaunchedSession(t, ctx, svc, launcher)
|
||||
|
||||
svc.ExecSQL(t, `ALTER TABLE agent_continuation_summaries RENAME TO agent_continuation_summaries_bak`)
|
||||
|
||||
if err := eb.Publish(ctx, events.AgentCompleted, bus.NewEvent(events.AgentCompleted, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: "execution-" + sessionID2, AgentID: "test-adapter", AgentProfileID: agent.ID,
|
||||
RunID: second.ID, RunSessionID: sessionID2, RunAttempt: attempt2, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "COMPLETED",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish second completion: %v", err)
|
||||
}
|
||||
|
||||
runsAfter, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runsAfter) != 2 {
|
||||
t.Fatalf("list runs after upsert failure: runs=%#v err=%v", runsAfter, err)
|
||||
}
|
||||
var secondAfter *models.Run
|
||||
for _, r := range runsAfter {
|
||||
if r.ID == second.ID {
|
||||
secondAfter = r
|
||||
}
|
||||
}
|
||||
if secondAfter == nil || secondAfter.Status != service.RunStatusFinished {
|
||||
t.Fatalf("second run after upsert failure = %#v, want status %q", secondAfter, service.RunStatusFinished)
|
||||
}
|
||||
|
||||
runEvents, err := svc.ListRunEventsForTest(ctx, second.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("list run events: %v", err)
|
||||
}
|
||||
found := false
|
||||
for _, e := range runEvents {
|
||||
if string(e.EventType) == "continuation_summary.upsert_failed" &&
|
||||
string(e.Level) == "warn" && strings.Contains(e.Payload, `"scope":"agent:`+agent.ID+`"`) {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("expected a continuation_summary.upsert_failed run event naming the scope, got %#v", runEvents)
|
||||
}
|
||||
|
||||
svc.ExecSQL(t, `ALTER TABLE agent_continuation_summaries_bak RENAME TO agent_continuation_summaries`)
|
||||
after, err := svc.GetContinuationSummaryForTest(ctx, agent.ID, "agent:"+agent.ID)
|
||||
if err != nil || after == nil {
|
||||
t.Fatalf("read summary after restoring table: %v (row=%v)", err, after)
|
||||
}
|
||||
if after.Content != before.Content || after.UpdatedByRunID != before.UpdatedByRunID {
|
||||
t.Fatalf("failed upsert wrote a partial row despite the INSERT erroring: before=%#v after=%#v", before, after)
|
||||
}
|
||||
}
|
||||
@@ -582,6 +582,12 @@ func (s *Service) markRoutingSuccess(ctx context.Context, run *models.Run) {
|
||||
// uses are otherwise unrelated to each other.
|
||||
const runEventFieldAgentID = "agent_id"
|
||||
|
||||
// runEventFieldErrorMessage is the run-event/activity payload key for the
|
||||
// underlying error text. Shared package-wide (event_subscribers.go,
|
||||
// scheduler_runs.go, scheduler_integration.go, failure.go) for the same
|
||||
// duplicate-literal reason as runEventFieldAgentID above.
|
||||
const runEventFieldErrorMessage = "error_message"
|
||||
|
||||
// handleTasklessAgentCompleted attributes a taskless run completion,
|
||||
// finishes the run, and refreshes the per-agent, per-scope continuation
|
||||
// summary so the next fire has bridge context. The
|
||||
@@ -693,8 +699,10 @@ func (s *Service) finishRunSession(
|
||||
// given agent and upserts it under run.ContinuationScope — the scope key
|
||||
// models.ContinuationScopeForRun computed once, at run-creation time, and
|
||||
// persisted onto the row (see runs/repository/sqlite.CreateRun). Errors
|
||||
// are logged at warn — the prior row stays intact (last-good wins) and
|
||||
// the run completion proceeds.
|
||||
// are logged at warn and recorded on the run's own event stream, naming
|
||||
// the scope that failed to write — the prior row stays intact (last-good
|
||||
// wins), the run's terminal state is not rolled back, and completion
|
||||
// proceeds either way.
|
||||
//
|
||||
// This deliberately reads the persisted field rather than recomputing it:
|
||||
// run here comes from resolveLifecycleRun's fresh DB fetch, which can
|
||||
@@ -715,6 +723,7 @@ func (s *Service) refreshContinuationSummary(
|
||||
if err != nil {
|
||||
s.logger.Warn("continuation-summary load inputs failed",
|
||||
zap.String("run_id", run.ID), zap.Error(err))
|
||||
s.appendContinuationSummaryFailureEvent(ctx, run.ID, "continuation_summary.load_failed", scope, err)
|
||||
return
|
||||
}
|
||||
body := summaryBuild(inputs)
|
||||
@@ -728,6 +737,19 @@ func (s *Service) refreshContinuationSummary(
|
||||
if upsertErr != nil {
|
||||
s.logger.Warn("continuation-summary upsert failed",
|
||||
zap.String("run_id", run.ID), zap.Error(upsertErr))
|
||||
s.appendContinuationSummaryFailureEvent(ctx, run.ID, "continuation_summary.upsert_failed", scope, upsertErr)
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) appendContinuationSummaryFailureEvent(
|
||||
ctx context.Context, runID, eventType, scope string, cause error,
|
||||
) {
|
||||
if err := s.appendRunEventStrict(ctx, runID, eventType, string(models.RunEventLevelWarn),
|
||||
map[string]interface{}{"scope": scope, runEventFieldErrorMessage: cause.Error()}); err != nil {
|
||||
s.logger.Warn("continuation-summary failure event append failed",
|
||||
zap.String("run_id", runID),
|
||||
zap.String("event_type", eventType),
|
||||
zap.Error(err))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -827,9 +849,9 @@ func (s *Service) handleAgentFailed(ctx context.Context, event *bus.Event) error
|
||||
}
|
||||
// Lifecycle: terminal "error" event for the run detail Events log.
|
||||
s.AppendRunEvent(ctx, run.ID, "error", "error", map[string]interface{}{
|
||||
"task_id": data.TaskID,
|
||||
"session_id": data.SessionID,
|
||||
"error_message": data.ErrorMessage,
|
||||
"task_id": data.TaskID,
|
||||
"session_id": data.SessionID,
|
||||
runEventFieldErrorMessage: data.ErrorMessage,
|
||||
})
|
||||
// Clear before routing can make the run claimable again. This prevents
|
||||
// cleanup from this attempt from matching a relaunch that reuses its run ID.
|
||||
@@ -879,9 +901,9 @@ func (s *Service) handleTasklessAgentFailed(
|
||||
return s.handleTasklessAgentLookupError(ctx, data, err)
|
||||
}
|
||||
s.AppendRunEvent(ctx, run.ID, "error", "error", map[string]interface{}{
|
||||
"session_id": data.SessionID,
|
||||
"run_session_id": data.RunSessionID,
|
||||
"error_message": data.ErrorMessage,
|
||||
"session_id": data.SessionID,
|
||||
"run_session_id": data.RunSessionID,
|
||||
runEventFieldErrorMessage: data.ErrorMessage,
|
||||
})
|
||||
s.clearAgentWorking(ctx, run.AgentProfileID, run.ID)
|
||||
if err := s.finishRunSession(ctx, data, models.RunSessionStateFailed, data.ErrorMessage); err != nil {
|
||||
|
||||
@@ -747,11 +747,11 @@ func (s *Service) publishRunFailed(
|
||||
"run_id": run.ID,
|
||||
"agent_profile_id": run.AgentProfileID,
|
||||
"task_id": taskIDFromRunPayload(run.Payload),
|
||||
"error_message": errorMessage,
|
||||
"consecutive_failures": count,
|
||||
"threshold": threshold,
|
||||
"finished_at": time.Now().UTC().Format(time.RFC3339),
|
||||
}
|
||||
data[runEventFieldErrorMessage] = errorMessage
|
||||
_ = s.eb.Publish(ctx, "office.run.failed",
|
||||
bus.NewEvent("office.run.failed", "office-failure", data))
|
||||
}
|
||||
|
||||
@@ -741,8 +741,8 @@ func (si *SchedulerIntegration) launchAgent(
|
||||
si.logger.Error("agent launch failed",
|
||||
zap.String("run_id", runID), zap.Error(err))
|
||||
si.svc.AppendRunEvent(ctx, runID, "error", "error", map[string]interface{}{
|
||||
"phase": "adapter.invoke",
|
||||
"error_message": err.Error(),
|
||||
"phase": "adapter.invoke",
|
||||
runEventFieldErrorMessage: err.Error(),
|
||||
})
|
||||
si.releaseCheckoutIfNeeded(ctx, run)
|
||||
_ = si.svc.HandleRunFailure(ctx, run, err)
|
||||
@@ -853,8 +853,8 @@ func (si *SchedulerIntegration) failTasklessRun(
|
||||
return
|
||||
}
|
||||
si.svc.AppendRunEvent(ctx, run.ID, "error", "error", map[string]interface{}{
|
||||
"phase": "scheduler.launch",
|
||||
"error_message": msg,
|
||||
"phase": "scheduler.launch",
|
||||
runEventFieldErrorMessage: msg,
|
||||
})
|
||||
si.svc.recordTerminalShape(ctx, run, RunStatusFailed, nil)
|
||||
run.ErrorMessage = msg
|
||||
@@ -906,8 +906,8 @@ func (si *SchedulerIntegration) failUnlaunchableRun(
|
||||
ctx context.Context, run *models.Run, agent *models.AgentInstance, msg string,
|
||||
) {
|
||||
si.svc.AppendRunEvent(ctx, run.ID, "error", "error", map[string]interface{}{
|
||||
"phase": "scheduler.launch",
|
||||
"error_message": msg,
|
||||
"phase": "scheduler.launch",
|
||||
runEventFieldErrorMessage: msg,
|
||||
})
|
||||
si.releaseCheckoutIfNeeded(ctx, run)
|
||||
// No lifecycle event backs a wiring fault, so there is no agent id to
|
||||
@@ -947,8 +947,8 @@ func (si *SchedulerIntegration) tryRoutingDispatch(
|
||||
si.logger.Error("routing dispatch failed",
|
||||
zap.String("run_id", run.ID), zap.Error(err))
|
||||
si.svc.AppendRunEvent(ctx, run.ID, "error", "error", map[string]interface{}{
|
||||
"phase": "routing.dispatch",
|
||||
"error_message": err.Error(),
|
||||
"phase": "routing.dispatch",
|
||||
runEventFieldErrorMessage: err.Error(),
|
||||
})
|
||||
si.releaseCheckoutIfNeeded(ctx, run)
|
||||
_ = si.svc.HandleRunFailure(ctx, run, err)
|
||||
|
||||
@@ -244,7 +244,7 @@ func (s *Service) publishRunProcessedForWorkspace(
|
||||
data["task_id"] = taskID
|
||||
data["comment_id"] = commentID
|
||||
if run.ErrorMessage != "" {
|
||||
data["error_message"] = run.ErrorMessage
|
||||
data[runEventFieldErrorMessage] = run.ErrorMessage
|
||||
}
|
||||
}
|
||||
event := bus.NewEvent(events.OfficeRunProcessed, "office-service", data)
|
||||
|
||||
@@ -0,0 +1,550 @@
|
||||
package service_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
"github.com/kandev/kandev/internal/office/routing"
|
||||
"github.com/kandev/kandev/internal/office/scheduler"
|
||||
"github.com/kandev/kandev/internal/office/service"
|
||||
)
|
||||
|
||||
// routedTestLauncher is service.RunSessionLauncher wired directly onto a
|
||||
// real scheduler.SchedulerService (not the fake captureDispatcher used by
|
||||
// the task-bound routing tests), so DispatchWithRouting / tryCandidates /
|
||||
// HandlePostStartFailure run unmodified and persist real
|
||||
// office_run_route_attempts / office_run_sessions rows. Session ids and
|
||||
// attempts are keyed off run.CurrentRouteAttemptSeq, which
|
||||
// dispatch_routing.go's recordAttemptStart sets on the run pointer before
|
||||
// launchCandidate calls StartRunSession — so a failed candidate's seq is
|
||||
// never reused by the candidate that actually launches.
|
||||
type routedTestLauncher struct {
|
||||
svc *service.Service
|
||||
|
||||
mu sync.Mutex
|
||||
calls []service.LaunchContext
|
||||
failFor map[string]error
|
||||
}
|
||||
|
||||
func (l *routedTestLauncher) StartRunSession(
|
||||
ctx context.Context, run *models.Run, agent *models.AgentInstance,
|
||||
launch service.LaunchContext, route *service.RouteOverride,
|
||||
) (service.RunSessionLaunch, error) {
|
||||
l.mu.Lock()
|
||||
l.calls = append(l.calls, launch)
|
||||
l.mu.Unlock()
|
||||
seq := run.CurrentRouteAttemptSeq
|
||||
id := fmt.Sprintf("route-session-%d", seq)
|
||||
now := time.Now().UTC()
|
||||
session := &models.RunSession{
|
||||
ID: id, WorkspaceID: agent.WorkspaceID, AgentProfileID: agent.ID,
|
||||
RunID: run.ID, Attempt: seq, State: models.RunSessionStatePreparing,
|
||||
CreatedAt: now, Version: 1,
|
||||
}
|
||||
reserved, err := l.svc.RepoForTest().ReserveRunSession(ctx, session)
|
||||
if err != nil || !reserved {
|
||||
return service.RunSessionLaunch{}, fmt.Errorf("reserve test run session: reserved=%v err=%w", reserved, err)
|
||||
}
|
||||
// Mirrors officeRunSessionLauncher.StartRunSession's real order: the
|
||||
// session is always reserved before the runtime launch can fail, so a
|
||||
// pre-launch-fallback candidate still leaves its own terminal session
|
||||
// row rather than none at all.
|
||||
if failErr, ok := l.failFor[route.ProviderID]; ok {
|
||||
if _, err := l.svc.RepoForTest().FinishRunSession(ctx, id, models.RunSessionStateFailed, failErr.Error()); err != nil {
|
||||
return service.RunSessionLaunch{}, err
|
||||
}
|
||||
return service.RunSessionLaunch{}, failErr
|
||||
}
|
||||
execID := "execution-" + id
|
||||
acpID := "acp-" + id
|
||||
if _, err := l.svc.RepoForTest().BindRunSessionExecution(
|
||||
ctx, id, execID, route.ExecutionProfileID, route.ProviderID, route.Model, acpID,
|
||||
); err != nil {
|
||||
return service.RunSessionLaunch{}, err
|
||||
}
|
||||
if _, err := l.svc.RepoForTest().MarkRunSessionStarted(
|
||||
ctx, id, execID, route.ExecutionProfileID, route.ProviderID, route.Model, acpID,
|
||||
); err != nil {
|
||||
return service.RunSessionLaunch{}, err
|
||||
}
|
||||
return service.RunSessionLaunch{
|
||||
SessionID: id, ExecutionID: execID,
|
||||
ExecutionProfileID: route.ExecutionProfileID, Adapter: route.ProviderID, Model: route.Model,
|
||||
ACPSessionID: acpID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (l *routedTestLauncher) callCount() int {
|
||||
l.mu.Lock()
|
||||
defer l.mu.Unlock()
|
||||
return len(l.calls)
|
||||
}
|
||||
|
||||
// seedRoutedWorkspaceConfig mirrors scheduler_test's seedRoutingConfig
|
||||
// (internal/office/scheduler/dispatch_routing_test.go), reimplemented here
|
||||
// because that helper lives in an external test package this file cannot
|
||||
// import. Provider ids must be real rules-catalogue keys ("claude-acp",
|
||||
// "codex-acp", ...) for routingerr.Classify's per-provider rules (used by
|
||||
// the proven "anthropic_quota_exceeded" fallback string) to match.
|
||||
func seedRoutedWorkspaceConfig(
|
||||
t *testing.T, repo *officesqlite.Repository, workspaceID string, providers []routing.ProviderID,
|
||||
) {
|
||||
t.Helper()
|
||||
profiles := map[routing.ProviderID]routing.ProviderProfile{}
|
||||
for _, p := range providers {
|
||||
profiles[p] = routing.ProviderProfile{
|
||||
TierMap: routing.TierMap{Balanced: string(p) + "-bal"},
|
||||
ExecutionProfileIDs: routing.ExecutionProfileIDs{Balanced: string(p) + "-profile"},
|
||||
Mode: "default",
|
||||
}
|
||||
}
|
||||
cfg := &routing.WorkspaceConfig{
|
||||
Enabled: true, DefaultTier: routing.TierBalanced,
|
||||
ProviderOrder: providers, ProviderProfiles: profiles,
|
||||
}
|
||||
if err := repo.UpsertWorkspaceRouting(context.Background(), workspaceID, cfg); err != nil {
|
||||
t.Fatalf("upsert routing: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// buildRoutedScheduler wires a real scheduler.SchedulerService the same
|
||||
// way internal/backendapp/main.go wires production: repo + resolver +
|
||||
// run-session launcher, then handed to Service.SetRoutingDispatcher so
|
||||
// launchAgent's taskless branch reaches DispatchWithRouting instead of
|
||||
// the direct RunSessionLauncher path.
|
||||
func buildRoutedScheduler(t *testing.T, svc *service.Service) *scheduler.SchedulerService {
|
||||
t.Helper()
|
||||
sched := scheduler.NewSchedulerService(svc.RepoForTest(), logger.Default(), svc)
|
||||
sched.SetResolver(routing.NewResolver(svc.RepoForTest(), nil))
|
||||
return sched
|
||||
}
|
||||
|
||||
func createRoutedTestAgent(t *testing.T, svc *service.Service, id string) *models.AgentInstance {
|
||||
t.Helper()
|
||||
agent := &models.AgentInstance{
|
||||
ID: id, WorkspaceID: "ws-1", Name: id,
|
||||
Role: models.AgentRoleCEO, Status: models.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"local_pc"}`,
|
||||
}
|
||||
if err := svc.CreateAgentInstance(context.Background(), agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
return agent
|
||||
}
|
||||
|
||||
func countTaskRows(t *testing.T, svc *service.Service, workspaceID string) (tasks, taskSessions int) {
|
||||
t.Helper()
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&tasks,
|
||||
`SELECT COUNT(*) FROM tasks WHERE workspace_id = ?`, workspaceID); err != nil {
|
||||
t.Fatalf("count task rows: %v", err)
|
||||
}
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&taskSessions,
|
||||
`SELECT COUNT(*) FROM task_sessions`); err != nil {
|
||||
t.Fatalf("count task_session rows: %v", err)
|
||||
}
|
||||
return tasks, taskSessions
|
||||
}
|
||||
|
||||
// TestRoutedTaskless_FirstCandidateSucceeds covers the one-attempt-succeeds
|
||||
// half of AC-OFFICE-TASKLESS-001.3's routed-profile requirement: exactly
|
||||
// one attempt row carrying the candidate's provider/model/execution
|
||||
// profile, the Office-built prompt reaching the launcher, runs.session_id
|
||||
// bound to the single session, and no task/task_session rows created.
|
||||
func TestRoutedTaskless_FirstCandidateSucceeds(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
sched := buildRoutedScheduler(t, svc)
|
||||
launcher := &routedTestLauncher{svc: svc, failFor: map[string]error{}}
|
||||
sched.SetRunSessionLauncher(launcher)
|
||||
svc.SetRoutingDispatcher(sched)
|
||||
seedRoutedWorkspaceConfig(t, svc.RepoForTest(), "ws-1",
|
||||
[]routing.ProviderID{"claude-acp", "codex-acp"})
|
||||
|
||||
agent := createRoutedTestAgent(t, svc, "routed-agent-solo")
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger,
|
||||
`{"one_time_instructions":"ROUTED_TASKLESS_PROMPT_SENTINEL"}`, "routed-solo"); err != nil {
|
||||
t.Fatalf("queue: %v", err)
|
||||
}
|
||||
runs, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list queued run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
run := runs[0]
|
||||
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
|
||||
if launcher.callCount() != 1 || launcher.calls[0].Prompt == "" {
|
||||
t.Fatalf("routed launch = %#v, want exactly one real-prompt call", launcher.calls)
|
||||
}
|
||||
persistedRun, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil || persistedRun == nil {
|
||||
t.Fatalf("reload dispatched run: %v (run=%v)", err, persistedRun)
|
||||
}
|
||||
if launcher.calls[0].Prompt != persistedRun.AssembledPrompt {
|
||||
t.Fatalf("routed prompt = %q, want persisted Office prompt %q", launcher.calls[0].Prompt, persistedRun.AssembledPrompt)
|
||||
}
|
||||
if !strings.Contains(launcher.calls[0].Prompt, "ROUTED_TASKLESS_PROMPT_SENTINEL") {
|
||||
t.Fatalf("routed prompt lost the known Office instruction: %q", launcher.calls[0].Prompt)
|
||||
}
|
||||
if len(launcher.calls[0].AdditionalSkillSlugs) != 0 {
|
||||
t.Fatalf("taskless launch received task-only skills: %v", launcher.calls[0].AdditionalSkillSlugs)
|
||||
}
|
||||
|
||||
attempts, err := svc.RepoForTest().ListRouteAttempts(ctx, run.ID)
|
||||
if err != nil || len(attempts) != 1 {
|
||||
t.Fatalf("route attempts = %#v, err=%v, want exactly one", attempts, err)
|
||||
}
|
||||
if attempts[0].ProviderID != "claude-acp" || attempts[0].ExecutionProfileID != "claude-acp-profile" {
|
||||
t.Fatalf("attempt candidate = %#v, want claude-acp/claude-acp-profile", attempts[0])
|
||||
}
|
||||
|
||||
sessions, err := svc.RepoForTest().ListRunSessions(ctx, run.ID)
|
||||
if err != nil || len(sessions) != 1 {
|
||||
t.Fatalf("run sessions = %#v, err=%v, want exactly one", sessions, err)
|
||||
}
|
||||
session := sessions[0]
|
||||
if session.State != models.RunSessionStateRunning {
|
||||
t.Fatalf("session state = %q, want running", session.State)
|
||||
}
|
||||
if session.Adapter != "claude-acp" || session.Model != "claude-acp-bal" ||
|
||||
session.ExecutionProfileID != "claude-acp-profile" {
|
||||
t.Fatalf("session route fields = %#v, want claude-acp/claude-acp-bal/claude-acp-profile", session)
|
||||
}
|
||||
|
||||
runs, err = svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 || runs[0].SessionID != session.ID {
|
||||
t.Fatalf("run projection = %#v, err=%v, want session_id=%q", runs, err, session.ID)
|
||||
}
|
||||
|
||||
tasks, taskSessions := countTaskRows(t, svc, agent.WorkspaceID)
|
||||
if tasks != 0 || taskSessions != 0 {
|
||||
t.Fatalf("routed taskless launch created tasks=%d task_sessions=%d, want 0/0", tasks, taskSessions)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoutedTaskless_PreLaunchFallback_TwoAttemptsTwoSessions covers a
|
||||
// routed profile whose first candidate fails during the launch call itself
|
||||
// (a provider error returned from StartRunSession, not a post-start event):
|
||||
// two office_run_route_attempts rows (seq 1 failed, seq 2 launched) and two
|
||||
// office_run_sessions rows, since StartRunSession always reserves its
|
||||
// session before it can fail — the failed candidate's row lands Failed, the
|
||||
// winning candidate's row lands Running. Also covers usage attribution
|
||||
// landing on the launched attempt's own session, not the failed candidate's
|
||||
// (AC-OFFICE-TASKLESS-001.4).
|
||||
func TestRoutedTaskless_PreLaunchFallback_TwoAttemptsTwoSessions(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
sched := buildRoutedScheduler(t, svc)
|
||||
launcher := &routedTestLauncher{svc: svc, failFor: map[string]error{
|
||||
"claude-acp": fmt.Errorf("anthropic_quota_exceeded: please try again"),
|
||||
}}
|
||||
sched.SetRunSessionLauncher(launcher)
|
||||
svc.SetRoutingDispatcher(sched)
|
||||
seedRoutedWorkspaceConfig(t, svc.RepoForTest(), "ws-1",
|
||||
[]routing.ProviderID{"claude-acp", "codex-acp"})
|
||||
|
||||
agent := createRoutedTestAgent(t, svc, "routed-agent-prelaunch")
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "routed-prelaunch"); err != nil {
|
||||
t.Fatalf("queue: %v", err)
|
||||
}
|
||||
runs, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list queued run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
run := runs[0]
|
||||
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
|
||||
// Ordering: both candidates were tried in the same synchronous
|
||||
// dispatch (not just a final count), so the launcher recorded the
|
||||
// failing claude-acp call before the succeeding codex-acp call.
|
||||
if launcher.callCount() != 2 {
|
||||
t.Fatalf("launcher calls = %d, want 2 (one failed, one succeeded)", launcher.callCount())
|
||||
}
|
||||
|
||||
attempts, err := svc.RepoForTest().ListRouteAttempts(ctx, run.ID)
|
||||
if err != nil || len(attempts) != 2 {
|
||||
t.Fatalf("route attempts = %#v, err=%v, want exactly two", attempts, err)
|
||||
}
|
||||
if attempts[0].Seq != 1 || attempts[0].ProviderID != "claude-acp" ||
|
||||
attempts[0].Outcome != scheduler.RouteAttemptOutcomeFailedProviderUnavail ||
|
||||
attempts[0].FinishedAt == nil {
|
||||
t.Fatalf("first attempt = %#v, want seq=1 claude-acp failed_provider_unavailable with FinishedAt set", attempts[0])
|
||||
}
|
||||
if attempts[1].Seq != 2 || attempts[1].ProviderID != "codex-acp" ||
|
||||
attempts[1].Outcome != scheduler.RouteAttemptOutcomeLaunched {
|
||||
t.Fatalf("second attempt = %#v, want seq=2 codex-acp launched", attempts[1])
|
||||
}
|
||||
|
||||
// A session was reserved for BOTH candidates: StartRunSession always
|
||||
// reserves before it can fail, so the failed claude-acp candidate still
|
||||
// leaves its own terminal row rather than none at all.
|
||||
sessions, err := svc.RepoForTest().ListRunSessions(ctx, run.ID)
|
||||
if err != nil || len(sessions) != 2 {
|
||||
t.Fatalf("run sessions = %#v, err=%v, want exactly two (one per candidate)", sessions, err)
|
||||
}
|
||||
var failedSession, session *models.RunSession
|
||||
for i := range sessions {
|
||||
switch sessions[i].Attempt {
|
||||
case 1:
|
||||
failedSession = &sessions[i]
|
||||
case 2:
|
||||
session = &sessions[i]
|
||||
}
|
||||
}
|
||||
// Production's BindRunSessionExecution (which sets Adapter/Model) only
|
||||
// runs after a successful runtime.Launch, so a candidate that fails
|
||||
// inside StartRunSession is reserved but never adapter-bound — its
|
||||
// identity is the run/attempt pair and the recorded error, not Adapter.
|
||||
if failedSession == nil || failedSession.State != models.RunSessionStateFailed ||
|
||||
failedSession.ErrorMessage == "" {
|
||||
t.Fatalf("failed candidate's session = %#v, want attempt=1 failed with an error message", failedSession)
|
||||
}
|
||||
if session == nil || session.Adapter != "codex-acp" || session.Attempt != 2 ||
|
||||
session.State != models.RunSessionStateRunning {
|
||||
t.Fatalf("launched session = %#v, want codex-acp attempt=2 running", session)
|
||||
}
|
||||
|
||||
runs, err = svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 || runs[0].SessionID != session.ID {
|
||||
t.Fatalf("run projection = %#v, err=%v, want session_id=%q", runs, err, session.ID)
|
||||
}
|
||||
|
||||
tasks, taskSessions := countTaskRows(t, svc, agent.WorkspaceID)
|
||||
if tasks != 0 || taskSessions != 0 {
|
||||
t.Fatalf("routed fallback launch created tasks=%d task_sessions=%d, want 0/0", tasks, taskSessions)
|
||||
}
|
||||
|
||||
// Usage attribution reaches the cost ledger joined to the attempt
|
||||
// that actually launched (codex-acp / session), not a first-attempt
|
||||
// session that never existed.
|
||||
usagePayload := lifecycle.AgentStreamEventPayload{
|
||||
ExecutionID: session.ExecutionID, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, RunSessionID: session.ID, RunAttempt: 2,
|
||||
AgentProfileID: agent.ID, AgentType: "claude-acp",
|
||||
Data: &lifecycle.AgentStreamEventData{
|
||||
PromptGeneration: 1, TurnID: "turn-1", CurrentModelID: session.Model,
|
||||
Usage: &streams.PromptUsage{
|
||||
InputTokens: 5, OutputTokens: 2, OutputTokensPresent: true, TotalTokens: 7,
|
||||
ProviderReportedCostSubcents: 42, ProviderReportedCostPresent: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
if err := eb.Publish(ctx, events.BuildAgentStreamSubject(session.ID),
|
||||
bus.NewEvent(events.AgentStream, "test", usagePayload)); err != nil {
|
||||
t.Fatalf("publish usage: %v", err)
|
||||
}
|
||||
var costCount int
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&costCount,
|
||||
`SELECT COUNT(*) FROM office_cost_events WHERE session_id = ?`, session.ID); err != nil {
|
||||
t.Fatalf("count costs for launched session: %v", err)
|
||||
}
|
||||
if costCount != 1 {
|
||||
t.Fatalf("cost events joined to launched session = %d, want 1", costCount)
|
||||
}
|
||||
var totalCosts int
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&totalCosts, `SELECT COUNT(*) FROM office_cost_events`); err != nil {
|
||||
t.Fatalf("count total costs: %v", err)
|
||||
}
|
||||
if totalCosts != 1 {
|
||||
t.Fatalf("total cost events = %d, want 1 (none misattributed to the failed first candidate)", totalCosts)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoutedTaskless_PostStartFallback_RetryAndDelayedDuplicateAreSafe
|
||||
// covers the retry half of AC-OFFICE-TASKLESS-001.2 (a post-start failure
|
||||
// produces a genuinely distinct second session/ACP session on the same
|
||||
// run, driven through the real HandlePostStartFailure -> RequeueForNextCandidate
|
||||
// -> next scheduler tick chain, not a synthetic two-session setup) and the
|
||||
// same-run half of AC-OFFICE-TASKLESS-001.4: once attempt 2 is live, a
|
||||
// delayed/duplicate terminal event naming the now-stale attempt 1 session
|
||||
// must be a safe no-op — no run completion, no second cost-ledger row, no
|
||||
// cleared agent-working state — because resolveLifecycleRun's session-state
|
||||
// gate excludes an already-terminal session from an exact-id lookup.
|
||||
func TestRoutedTaskless_PostStartFallback_RetryAndDelayedDuplicateAreSafe(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
sched := buildRoutedScheduler(t, svc)
|
||||
launcher := &routedTestLauncher{svc: svc, failFor: map[string]error{}}
|
||||
sched.SetRunSessionLauncher(launcher)
|
||||
svc.SetRoutingDispatcher(sched)
|
||||
seedRoutedWorkspaceConfig(t, svc.RepoForTest(), "ws-1",
|
||||
[]routing.ProviderID{"claude-acp", "codex-acp"})
|
||||
|
||||
agent := createRoutedTestAgent(t, svc, "routed-agent-poststart")
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, "routed-poststart"); err != nil {
|
||||
t.Fatalf("queue: %v", err)
|
||||
}
|
||||
runs, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runs) != 1 {
|
||||
t.Fatalf("list queued run: runs=%#v err=%v", runs, err)
|
||||
}
|
||||
run := runs[0]
|
||||
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
if launcher.callCount() != 1 {
|
||||
t.Fatalf("launcher calls after first tick = %d, want 1", launcher.callCount())
|
||||
}
|
||||
session1ID := "route-session-1"
|
||||
session1, err := svc.RepoForTest().GetRunSession(ctx, session1ID)
|
||||
if err != nil || session1 == nil || session1.State != models.RunSessionStateRunning {
|
||||
t.Fatalf("session1 after launch = %#v, err=%v, want running", session1, err)
|
||||
}
|
||||
|
||||
// Real post-start failure event for attempt 1: this is the same
|
||||
// production event a live agent process publishes, not a bypass of
|
||||
// the event-subscriber machinery.
|
||||
if err := eb.Publish(ctx, events.AgentFailed, bus.NewEvent(events.AgentFailed, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: session1.ExecutionID, AgentID: "claude-acp", AgentProfileID: agent.ID,
|
||||
RunID: run.ID, RunSessionID: session1ID, RunAttempt: 1, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "FAILED",
|
||||
ErrorMessage: "anthropic_quota_exceeded: please try again",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish attempt1 failure: %v", err)
|
||||
}
|
||||
|
||||
session1, err = svc.RepoForTest().GetRunSession(ctx, session1ID)
|
||||
if err != nil || session1 == nil || session1.State != models.RunSessionStateFailed {
|
||||
t.Fatalf("session1 after post-start failure = %#v, err=%v, want failed", session1, err)
|
||||
}
|
||||
// Ordering: attempt 1's session reached its terminal state before
|
||||
// attempt 2 was ever reserved — assert this directly, not just the
|
||||
// eventual row count, by checking no second session exists yet.
|
||||
sessionsAfterFailure, err := svc.RepoForTest().ListRunSessions(ctx, run.ID)
|
||||
if err != nil || len(sessionsAfterFailure) != 1 {
|
||||
t.Fatalf("run sessions right after attempt1 failure = %#v, err=%v, want exactly one (attempt2 not reserved yet)",
|
||||
sessionsAfterFailure, err)
|
||||
}
|
||||
queuedRun, err := svc.RepoForTest().GetRun(ctx, run.ID)
|
||||
if err != nil || queuedRun.Status != models.RunStatusQueued {
|
||||
t.Fatalf("run status after post-start fallback = %#v, err=%v, want queued", queuedRun, err)
|
||||
}
|
||||
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
if launcher.callCount() != 2 {
|
||||
t.Fatalf("launcher calls after second tick = %d, want 2 (retry to the next candidate)", launcher.callCount())
|
||||
}
|
||||
if got := launcher.calls[1]; got.Prompt == "" {
|
||||
t.Fatalf("retry launch context missing prompt: %#v", got)
|
||||
}
|
||||
|
||||
session2ID := "route-session-2"
|
||||
session2, err := svc.RepoForTest().GetRunSession(ctx, session2ID)
|
||||
if err != nil || session2 == nil || session2.State != models.RunSessionStateRunning {
|
||||
t.Fatalf("session2 after retry = %#v, err=%v, want running", session2, err)
|
||||
}
|
||||
if session2.ID == session1.ID || session2.ExecutionID == session1.ExecutionID ||
|
||||
session2.ACPSessionID == session1.ACPSessionID {
|
||||
t.Fatalf("retry reused attempt1's session/execution/ACP identity: session1=%#v session2=%#v", session1, session2)
|
||||
}
|
||||
if session2.Adapter != "codex-acp" {
|
||||
t.Fatalf("retry candidate = %q, want codex-acp (claude-acp excluded after its failure)", session2.Adapter)
|
||||
}
|
||||
|
||||
sessions, err := svc.RepoForTest().ListRunSessions(ctx, run.ID)
|
||||
if err != nil || len(sessions) != 2 {
|
||||
t.Fatalf("run sessions after retry = %#v, err=%v, want exactly two", sessions, err)
|
||||
}
|
||||
runsAfterRetry, err := svc.ListRuns(ctx, agent.WorkspaceID)
|
||||
if err != nil || len(runsAfterRetry) != 1 || runsAfterRetry[0].SessionID != session2.ID {
|
||||
t.Fatalf("run projection after retry = %#v, err=%v, want session_id=%q", runsAfterRetry, err, session2.ID)
|
||||
}
|
||||
|
||||
var workingRunID string
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&workingRunID,
|
||||
`SELECT working_run_id FROM agent_profiles WHERE id = ?`, agent.ID); err != nil {
|
||||
t.Fatalf("read working run: %v", err)
|
||||
}
|
||||
if workingRunID != run.ID {
|
||||
t.Fatalf("agent working_run_id after retry = %q, want %q", workingRunID, run.ID)
|
||||
}
|
||||
|
||||
// Attempt 2's own usage lands on its own session.
|
||||
if err := eb.Publish(ctx, events.BuildAgentStreamSubject(session2ID),
|
||||
bus.NewEvent(events.AgentStream, "test", lifecycle.AgentStreamEventPayload{
|
||||
ExecutionID: session2.ExecutionID, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, RunSessionID: session2ID, RunAttempt: 2,
|
||||
AgentProfileID: agent.ID, AgentType: "codex-acp",
|
||||
Data: &lifecycle.AgentStreamEventData{
|
||||
PromptGeneration: 1, TurnID: "turn-1", CurrentModelID: session2.Model,
|
||||
Usage: &streams.PromptUsage{
|
||||
InputTokens: 4, OutputTokens: 1, OutputTokensPresent: true, TotalTokens: 5,
|
||||
ProviderReportedCostSubcents: 77, ProviderReportedCostPresent: true,
|
||||
},
|
||||
},
|
||||
})); err != nil {
|
||||
t.Fatalf("publish attempt2 usage: %v", err)
|
||||
}
|
||||
var costCountBefore int
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&costCountBefore, `SELECT COUNT(*) FROM office_cost_events`); err != nil {
|
||||
t.Fatalf("count costs before duplicate: %v", err)
|
||||
}
|
||||
if costCountBefore != 1 {
|
||||
t.Fatalf("cost events after attempt2 usage = %d, want 1", costCountBefore)
|
||||
}
|
||||
|
||||
// Delayed/duplicate terminal event for the now-stale attempt 1,
|
||||
// arriving after attempt 2 is live on the same run. Must be a
|
||||
// no-op: no completion, no second ledger row, no cleared
|
||||
// agent-working state, attempt2 and the run untouched.
|
||||
if err := eb.Publish(ctx, events.AgentFailed, bus.NewEvent(events.AgentFailed, "test", lifecycle.AgentEventPayload{
|
||||
AgentExecutionID: session1.ExecutionID, AgentID: "claude-acp", AgentProfileID: agent.ID,
|
||||
RunID: run.ID, RunSessionID: session1ID, RunAttempt: 1, OwnerKind: lifecycle.ExecutionOwnerRun,
|
||||
WorkspaceID: agent.WorkspaceID, Status: "FAILED",
|
||||
ErrorMessage: "anthropic_quota_exceeded: please try again",
|
||||
})); err != nil {
|
||||
t.Fatalf("publish delayed duplicate attempt1 failure: %v", err)
|
||||
}
|
||||
|
||||
runAfterDuplicate, err := svc.RepoForTest().GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run after duplicate: %v", err)
|
||||
}
|
||||
if runAfterDuplicate.Status == models.RunStatusFinished || runAfterDuplicate.Status == models.RunStatusFailed {
|
||||
t.Fatalf("run status after delayed duplicate = %q, want no completion", runAfterDuplicate.Status)
|
||||
}
|
||||
session2Unchanged, err := svc.RepoForTest().GetRunSession(ctx, session2ID)
|
||||
if err != nil || session2Unchanged.State != models.RunSessionStateRunning || session2Unchanged.Version != session2.Version {
|
||||
t.Fatalf("session2 after delayed duplicate = %#v, err=%v, want unchanged running version=%d",
|
||||
session2Unchanged, err, session2.Version)
|
||||
}
|
||||
session1Unchanged, err := svc.RepoForTest().GetRunSession(ctx, session1ID)
|
||||
if err != nil || session1Unchanged.State != models.RunSessionStateFailed {
|
||||
t.Fatalf("session1 after delayed duplicate = %#v, err=%v, want unchanged failed", session1Unchanged, err)
|
||||
}
|
||||
attemptsAfterDuplicate, err := svc.RepoForTest().ListRouteAttempts(ctx, run.ID)
|
||||
if err != nil || len(attemptsAfterDuplicate) != 2 {
|
||||
t.Fatalf("route attempts after delayed duplicate = %#v, err=%v, want still exactly two", attemptsAfterDuplicate, err)
|
||||
}
|
||||
var costCountAfter int
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&costCountAfter, `SELECT COUNT(*) FROM office_cost_events`); err != nil {
|
||||
t.Fatalf("count costs after duplicate: %v", err)
|
||||
}
|
||||
if costCountAfter != costCountBefore {
|
||||
t.Fatalf("cost events after delayed duplicate = %d, want unchanged %d", costCountAfter, costCountBefore)
|
||||
}
|
||||
var workingRunIDAfter string
|
||||
if err := svc.RepoForTest().ReaderDB().Get(&workingRunIDAfter,
|
||||
`SELECT working_run_id FROM agent_profiles WHERE id = ?`, agent.ID); err != nil {
|
||||
t.Fatalf("read working run after duplicate: %v", err)
|
||||
}
|
||||
if workingRunIDAfter != run.ID {
|
||||
t.Fatalf("agent working_run_id after delayed duplicate = %q, want unchanged %q", workingRunIDAfter, run.ID)
|
||||
}
|
||||
|
||||
tasks, taskSessions := countTaskRows(t, svc, agent.WorkspaceID)
|
||||
if tasks != 0 || taskSessions != 0 {
|
||||
t.Fatalf("routed post-start fallback created tasks=%d task_sessions=%d, want 0/0", tasks, taskSessions)
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ package service_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
@@ -70,6 +71,61 @@ func TestIdleSkip_RoutineDispatchNoTasks_Skipped(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestIdleSkip_RoutineDispatchEventWithSkipIdleRuns_NotSkipped pins
|
||||
// AC-OFFICE-TASKLESS-001.3's second sentence from the idle-skip gate's
|
||||
// side: a manual or webhook fire (reason=routine_dispatch_event) must not
|
||||
// be idle-skipped even when the assignee has SkipIdleRuns=true and zero
|
||||
// actionable tasks — the same shape that idle-skips a periodic
|
||||
// routine_dispatch_cron fire in
|
||||
// TestIdleSkip_RoutineDispatchNoTasks_Skipped above. checkIdleSkip's
|
||||
// IsPeriodicTasklessWake guard is what should distinguish them; nothing
|
||||
// before this test drove RunReasonRoutineDispatchEvent through the
|
||||
// scheduler to prove it.
|
||||
func TestIdleSkip_RoutineDispatchEventWithSkipIdleRuns_NotSkipped(t *testing.T) {
|
||||
svc := newTestService(t)
|
||||
ctx := context.Background()
|
||||
launcher := &tasklessTestLauncher{svc: svc}
|
||||
svc.SetRunSessionLauncher(launcher)
|
||||
|
||||
agent := &models.AgentInstance{
|
||||
WorkspaceID: "ws-3",
|
||||
Name: "idle-worker-routine-dispatch-event",
|
||||
Role: models.AgentRoleWorker,
|
||||
Status: models.AgentStatusIdle,
|
||||
ExecutorPreference: `{"type":"worktree"}`,
|
||||
}
|
||||
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
if !agent.SkipIdleRuns {
|
||||
t.Fatalf("worker role should default to SkipIdleRuns=true")
|
||||
}
|
||||
// No tasks assigned: this is the exact condition that idle-skips a
|
||||
// routine_dispatch_cron fire for this same agent shape.
|
||||
|
||||
if _, err := svc.QueueRun(ctx, agent.ID, shared.RunReasonRoutineDispatchEvent, `{}`, ""); err != nil {
|
||||
t.Fatalf("queue: %v", err)
|
||||
}
|
||||
|
||||
service.RunSchedulerTick(svc, ctx)
|
||||
if len(launcher.calls) != 1 {
|
||||
t.Fatalf("manual/webhook taskless run launcher calls = %d, want 1", len(launcher.calls))
|
||||
}
|
||||
if !strings.Contains(launcher.calls[0].Prompt, shared.RunReasonRoutineDispatchEvent) {
|
||||
t.Fatalf("manual/webhook launch prompt = %q, want reason %q", launcher.calls[0].Prompt, shared.RunReasonRoutineDispatchEvent)
|
||||
}
|
||||
|
||||
entries, err := svc.ListActivity(ctx, "ws-3", 50)
|
||||
if err != nil {
|
||||
t.Fatalf("list activity: %v", err)
|
||||
}
|
||||
for _, e := range entries {
|
||||
if e.Action == "run_idle_skipped" {
|
||||
t.Error("manual/webhook fire (routine_dispatch_event) must not be idle-skipped")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIdleSkip_RoutineDispatch_CoordinatorNotSkipped is the complementary
|
||||
// case to TestIdleSkip_RoutineDispatchNoTasks_Skipped (PR #2973 review
|
||||
// round 1, github-actions suggestion): a coordinator (CEO role) defaults to
|
||||
|
||||
@@ -31,6 +31,21 @@ func TestIsPeriodicTasklessWake_CronConstantIsSkippable(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsPeriodicTasklessWake_RoutineDispatchEventIsNonSkippable pins
|
||||
// AC-OFFICE-TASKLESS-001.3's second sentence: a manual or webhook fire
|
||||
// (RunReasonRoutineDispatchEvent) must never be treated as a periodic wake,
|
||||
// so periodic idle-skipping can never consume it. Nothing pinned this
|
||||
// value before — it falls through IsPeriodicTasklessWake's switch to the
|
||||
// default case today, so deleting that branch or moving the constant into
|
||||
// the "true" case would be a silent behavior change.
|
||||
func TestIsPeriodicTasklessWake_RoutineDispatchEventIsNonSkippable(t *testing.T) {
|
||||
if shared.IsPeriodicTasklessWake(shared.RunReasonRoutineDispatchEvent) {
|
||||
t.Errorf("IsPeriodicTasklessWake(%q) = true, want false — manual/webhook "+
|
||||
"fires must not be treated as a periodic wake",
|
||||
shared.RunReasonRoutineDispatchEvent)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoutineDispatchReason_SourceMapping pins RoutineDispatchReason's
|
||||
// source→reason mapping so RunReasonRoutineDispatchCron and
|
||||
// RunReasonRoutineDispatchEvent cannot silently drift apart from
|
||||
|
||||
@@ -47,7 +47,7 @@ function runEvent(seq: number, eventType: string): RunEvent {
|
||||
type Props = {
|
||||
runId: string;
|
||||
initialEvents: RunEvent[];
|
||||
initialStatus: "claimed";
|
||||
initialStatus: "claimed" | "finished" | "failed" | "cancelled";
|
||||
};
|
||||
|
||||
function renderLiveSync(initialProps: Props) {
|
||||
@@ -164,4 +164,35 @@ describe("useRunLiveSync", () => {
|
||||
expect(result.current.outputSummary).toBe("final answer");
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps listening for events written after a terminal status", () => {
|
||||
const { result } = renderLiveSync({
|
||||
...initialProps,
|
||||
initialStatus: "finished",
|
||||
});
|
||||
|
||||
expect(clients.active.subscribeRun).toHaveBeenCalledWith("run-1");
|
||||
|
||||
const delayedWarning = runEvent(1, "continuation_summary.upsert_failed");
|
||||
act(() => {
|
||||
handlers.listener!({ run_id: "run-1", event: delayedWarning });
|
||||
});
|
||||
|
||||
expect(result.current.status).toBe("finished");
|
||||
expect(result.current.events).toEqual([delayedWarning]);
|
||||
});
|
||||
|
||||
it("keeps listening when a terminal event arrives before a delayed warning", () => {
|
||||
const { result } = renderLiveSync(initialProps);
|
||||
|
||||
const complete = runEvent(1, "complete");
|
||||
const delayedWarning = runEvent(2, "continuation_summary.load_failed");
|
||||
act(() => {
|
||||
handlers.listener!({ run_id: "run-1", event: complete });
|
||||
handlers.listener!({ run_id: "run-1", event: delayedWarning });
|
||||
});
|
||||
|
||||
expect(result.current.status).toBe("finished");
|
||||
expect(result.current.events).toEqual([complete, delayedWarning]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -24,20 +24,20 @@ type Options = {
|
||||
|
||||
/**
|
||||
* Returns the live-merged events list and an observed status that
|
||||
* follows terminal events emitted on the bus. While `initialStatus`
|
||||
* is `claimed` (the running state), the hook subscribes to
|
||||
* `run.subscribe` over the WS and appends `run.event.appended`
|
||||
* payloads to the events list. Terminal events (`complete` /
|
||||
* `finished` / `error` / `failed`) update the local status so the
|
||||
* header reflects the new state without a snapshot refetch.
|
||||
* follows terminal events emitted on the bus. While the hook is mounted,
|
||||
* it subscribes to `run.subscribe` over the WS and
|
||||
* appends `run.event.appended` payloads to the events list. Terminal
|
||||
* events (`complete` / `finished` / `error` / `failed`) update the
|
||||
* local status so the header reflects the new state without a snapshot
|
||||
* refetch. The subscription stays open until unmount so a diagnostic
|
||||
* event written after the terminal event is still visible.
|
||||
*
|
||||
* Idempotency: dedupes by event seq (Wave 1 enforces monotonic seq
|
||||
* per run id) so duplicate notifications from a reconnect or a race
|
||||
* with the snapshot fetch don't double-render rows.
|
||||
*
|
||||
* Cleanup: unsubscribes on unmount AND when the run reaches a
|
||||
* terminal status — there's no point holding the bus subscription
|
||||
* open for runs that can no longer emit events.
|
||||
* Cleanup: unsubscribes on unmount or when the hook switches to a
|
||||
* different run or WebSocket client.
|
||||
*/
|
||||
export function useRunLiveSync(
|
||||
runId: string,
|
||||
@@ -127,9 +127,6 @@ export function useRunLiveSync(
|
||||
}, [agentId, runId, status]);
|
||||
|
||||
useEffect(() => {
|
||||
if (status !== "claimed") return;
|
||||
if (TERMINAL_STATUSES.has(status)) return;
|
||||
|
||||
if (!client) return;
|
||||
const unsubscribeWs = client.subscribeRun(runId);
|
||||
const unsubscribeListener = subscribeRunEvents(runId, (payload) => {
|
||||
@@ -146,7 +143,7 @@ export function useRunLiveSync(
|
||||
unsubscribeListener();
|
||||
unsubscribeWs();
|
||||
};
|
||||
}, [client, runId, status]);
|
||||
}, [client, runId]);
|
||||
|
||||
return { events, status, outputSummary };
|
||||
}
|
||||
|
||||
@@ -43,6 +43,11 @@ must express owner identity without importing Office packages. The Office design
|
||||
owns the vertical behavior; shared runtime code implements the admission seam.
|
||||
Existing task tables, task-session foreign keys and task-only APIs stay strict.
|
||||
|
||||
The current coverage change delivers the launch and observation path. Pause,
|
||||
cleanup controls and restart reconciliation remain requirements for a follow-up;
|
||||
the implementation plan records them as pending work and does not claim them as
|
||||
delivered here.
|
||||
|
||||
A successful taskless turn ends that session. The next fire or retry starts a new
|
||||
session and consumes only the existing bounded continuation summary. Persisted
|
||||
failed runs from the old unsupported-launch path remain history; they are not
|
||||
|
||||
@@ -0,0 +1,332 @@
|
||||
---
|
||||
created: 2026-09-19
|
||||
status: in_progress
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Taskless Office Run Sessions
|
||||
|
||||
## Overview
|
||||
|
||||
`REQ-OFFICE-TASKLESS-001` requires a lightweight (taskless) Office wake to start
|
||||
a real agent session and reach a visible terminal outcome **without creating any
|
||||
task or task session**. The run-owned session mechanism that satisfies it has
|
||||
landed: `office_run_sessions` (Office repository, unique `(run_id, attempt)`),
|
||||
the `RunSessionLauncher` seam, its `officeRunSessionLauncher` implementation,
|
||||
`runs.session_id` binding, and startup reconciliation via `ReconcileRunSessions`.
|
||||
|
||||
**Coverage scope is limited to the launch and observation path.**
|
||||
`AC-OFFICE-TASKLESS-001.5` (operator stop controls) and `.6` (restart
|
||||
reconciliation) remain requirements. Their four work orders — Tasks 02, 03, 04
|
||||
and 07 — are pending follow-up work and are not implemented by this coverage
|
||||
change. Their current gaps and follow-up constraints are recorded under
|
||||
[Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
|
||||
**Three results remain, and all three are coverage.** No production change is in
|
||||
scope beyond the run event and strict append handling Task 06 uses for a failed
|
||||
continuation-summary write. The pending lifecycle work remains outside this
|
||||
change.
|
||||
|
||||
The first: the end-to-end proof that an **armed cron trigger** reaches a launched
|
||||
taskless session is still skipped. `TestRoutine_CronFire_LightweightReachesSession`
|
||||
carries `t.Skip("blocked on card 49894d63: the lightweight (taskless) routine
|
||||
flow has no task_sessions row to assert on yet")`, and its intended assertion
|
||||
targets the wrong table: the shipped design deliberately writes no
|
||||
`task_sessions` row. Its heavy-routine sibling in the same file already passes.
|
||||
This is the clause the card owes Beta. Task 01 also closes the manual/webhook
|
||||
half of `AC-OFFICE-TASKLESS-001.3` and the two-fires half of `.2`.
|
||||
|
||||
The second: the provider-routed taskless path has no test at all — not for the
|
||||
routed launch itself, not for the actual adapter/model recorded on that path,
|
||||
and not for a delayed event from an older attempt of the same run, which
|
||||
provider fallback produces routinely. Task 05.
|
||||
|
||||
The third: two event-path guarantees are asserted by nothing — `.2`'s "a failed
|
||||
attempt shall not replace the last successful summary", and task consumers not
|
||||
acting on run-owned events. Task 06 also adds the one run event `.2` gained in
|
||||
Spec round 4 for a failed summary write, which the current code swallows
|
||||
silently.
|
||||
|
||||
## Prior art
|
||||
|
||||
### Our own prior reasoning (compiled wiki)
|
||||
|
||||
Searched the vault at `/Users/henry/Documents/henry/wiki` (pinned `@henry`) via the
|
||||
QMD MCP `wiki` collection, 578 documents, index last updated 2026-09-18. The
|
||||
`obsidian-wiki` CLI is not installed on this machine, so the GraphRAG pre-pass
|
||||
was skipped and retrieval was QMD lexical + semantic + HyDE. Direct filesystem
|
||||
reads of the vault are sandbox-blocked here, so QMD was the only transport; the
|
||||
skill's `log.md` append could not be performed for the same reason.
|
||||
|
||||
Two pages answer this initiative's design fork directly and outrank fresh
|
||||
reasoning:
|
||||
|
||||
- `[[agent-session-object]]` (score 0.76, `lifecycle: draft`, updated
|
||||
2026-09-10) — a session is "the addressable unit of agent work: one durable,
|
||||
server-side record with an identity, a lifecycle, an owner, attached
|
||||
artefacts, consumption, and derived analytics", and explicitly **not** a loop
|
||||
iteration. Names "conflating session with conversation" as an anti-pattern.
|
||||
This is the case for the session being first-class in its own right rather
|
||||
than a child of a work item.
|
||||
- `[[event-triggered-agent-activation]]` (score 0.83) — "a trigger is a standing
|
||||
order that opens a session with no human in the initiating position". Notes
|
||||
that filtering must happen **before** session creation so non-matching events
|
||||
cost nothing, and warns that "a daily cron on a no-op still opens a session;
|
||||
cost scales with firings, not with useful work". That is the rationale for the
|
||||
idle-skip gate this design preserves.
|
||||
- `[[kandev]]` — records that the legacy `heartbeat` wakeup source is retired
|
||||
and periodic wakes now flow through a per-coordinator cron routine, which
|
||||
matches the dispatch chain under test.
|
||||
|
||||
The wiki has **no** page on the specific storage question (nullable foreign key
|
||||
versus synthetic placeholder row), so that fork was resolved from the code and
|
||||
the system design rather than from prior reasoning.
|
||||
|
||||
### What other products shipped (saas-kb)
|
||||
|
||||
Queried the `saas-kb` MCP with `category: "ai_sdlc"`, per platform rather than
|
||||
once, because retrieval collapses onto whichever vendor documents best.
|
||||
|
||||
- **Warp — Scheduled Agents.** A cron schedule "starts a fresh agent session for
|
||||
every run"; "no state is carried over between runs". The session is the unit;
|
||||
a task appears only as an *output* the run links to. Also documents the
|
||||
identity trap this repo handles separately: a CLI-created schedule "runs as
|
||||
its creator and opens pull requests under that person's GitHub account"
|
||||
unless an agent identity is set.
|
||||
- **Augment Code — Automations/Schedules.** "If a Session is one conversation, an
|
||||
automation is the standing order that starts those conversations without you
|
||||
in the loop."
|
||||
- **Paperclip — Heartbeats & Routines.** The outlier: there, a routine fire
|
||||
"creates (or reopens) a task" and the assignment wakes the agent, so every
|
||||
scheduled fire has a visible execution issue. Its stated motivation is
|
||||
traceability ("why did this agent run?"). It also ships the
|
||||
concurrency (coalesce / skip / always-enqueue) and catch-up (skip-missed /
|
||||
capped-batch) policies this repo already has.
|
||||
|
||||
Treat all of the above as vendor claims, not evidence of correctness.
|
||||
|
||||
### What we are doing differently, and why
|
||||
|
||||
Warp and Augment converge on the session being the unit and a task not being a
|
||||
prerequisite, which supports this design. Paperclip materialises a task per fire;
|
||||
we deliberately do not, because Kandev already has a dedicated
|
||||
`office_routine_runs` record that supplies the traceability Paperclip gets from
|
||||
its execution issue, so a task per fire would add a board row no human asked for
|
||||
and pollute task counts.
|
||||
|
||||
Where we differ from **both** shapes: rather than reusing the task-bound session
|
||||
table with an optional task, this design adds a separate Office-owned
|
||||
`office_run_sessions` record. The system design forbids the two options this
|
||||
initiative was originally filed to weigh: "No changes to task-session
|
||||
nullability or synthetic task creation are permitted." That keeps every existing
|
||||
task-session invariant and every reader that assumes a task intact, at the cost
|
||||
of a second session table, whose reconciliation and retention are specified
|
||||
alongside it.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Un-skip `TestRoutine_CronFire_LightweightReachesSession`
|
||||
(`apps/backend/internal/backendapp/office_routine_cron_to_session_test.go`)
|
||||
and rewrite its assertion against the storage seam that actually shipped:
|
||||
an `office_run_sessions` row for the dispatched run, and `runs.session_id`
|
||||
bound to it.
|
||||
- Assert the negative half of `AC-OFFICE-TASKLESS-001.1` explicitly: the fire
|
||||
creates **no** `tasks` row and **no** `task_sessions` row.
|
||||
- Correct the stale skip comment above the test, which describes a
|
||||
`task_sessions` row the design forbids.
|
||||
- Assert the manual/webhook half of `AC-OFFICE-TASKLESS-001.3` and the
|
||||
two-distinct-sessions half of `.2` (Task 01).
|
||||
- Cover the provider-routed taskless launch, which has no test (Task 05).
|
||||
- Pin the two event-path guarantees nothing asserts, and the new failed-write
|
||||
outcome on `AC-OFFICE-TASKLESS-001.2` (Task 06).
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **Operator stop controls and restart reconciliation.** These remain required
|
||||
by `AC-OFFICE-TASKLESS-001.5` and `.6` but are not implemented in this
|
||||
coverage change. Three current gaps are named rather than left silent: run
|
||||
cancellation, agent disable and agent removal do not stop a live run-owned
|
||||
execution; a crash between an attempt's terminal write and its run's terminal
|
||||
write leaves a run claimed behind a terminal attempt; and the launch-failure
|
||||
path discards the `runtime.Stop` error and writes the attempt terminal anyway.
|
||||
All three, and the two problems a follow-up must resolve first, are recorded
|
||||
under [Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
- Any production change beyond the one run event Task 06 adds for a failed
|
||||
summary write. Every remaining result is coverage. If a test cannot be written
|
||||
without changing production behavior, that is a finding against the
|
||||
requirement, not a licence to edit the code under test.
|
||||
- Pruning run-session rows when run-history retention deletes their run. Named
|
||||
out of scope in the requirement, with what a follow-up would need.
|
||||
- Storing a route-attempt-to-session association. The route ledger has no
|
||||
session column and no criterion needs one.
|
||||
- The heavy-routine path and its test, which already pass.
|
||||
- `REQ-OFFICE-COORDINATOR-AUTHORITY-*` (the taskless run's board-read and
|
||||
annotation authority). Delivered separately and already `implemented` under
|
||||
[`office-taskless-coordinator-authority`](../office-taskless-coordinator-authority/plan.md).
|
||||
- Interactive taskless chat, synthetic tasks, and cross-fire ACP resumption,
|
||||
which the requirement lists as out of scope.
|
||||
|
||||
## Technical approach
|
||||
|
||||
The dispatch chain under test already exists end to end:
|
||||
|
||||
```text
|
||||
TickScheduledTriggers
|
||||
-> processCronTrigger (claims the armed next_run_at slot)
|
||||
-> dispatchRoutineRun (task_template == "" => lightweight branch)
|
||||
-> agent_wakeup_requests row
|
||||
-> wakeup dispatcher -> fresh taskless run
|
||||
-> scheduler processRun -> launchAgent (taskID == "")
|
||||
-> RunSessionLauncher.StartRunSession
|
||||
-> office_run_sessions row + persistLaunchedSession -> runs.session_id
|
||||
```
|
||||
|
||||
The test harness in the file (`newRoutineCronHarness`) already stands up the
|
||||
SQLite store, office repository, orchestrator and a `stubAgentManager` whose
|
||||
`LaunchAgent` captures the request, which is what the passing heavy-routine test
|
||||
uses. The work is to drive the same harness down the lightweight branch and
|
||||
assert on the Office-owned session record instead of the task-owned one.
|
||||
|
||||
## Tests
|
||||
|
||||
Rebuilt from assertions rather than filenames. Spec Review round 2 found the
|
||||
previous table cited neighbouring files for six criteria: a test in the right
|
||||
area is not evidence for the criterion. Every entry below names the test
|
||||
function that asserts the clause, and a clause with no such test is listed as a
|
||||
gap with the work order that closes it.
|
||||
|
||||
| Acceptance criteria | Evidence |
|
||||
| --- | --- |
|
||||
| `.1` starts a real session, no task or task session | `office/service/taskless_lifecycle_test.go` — `TestTasklessRoutineRunLaunchesRunSessionAndCompletes` (run session reaches a terminal outcome; asserts zero `tasks` rows). Cron-fire end to end: Task 01 |
|
||||
| `.2` continuation scope is chosen once and round-trips | `office/service/continuation_summary_reader_test.go` — `TestLoadContinuationSummary_RoutineScope_RoundTripsThroughRealWriterAndReader`, `..._AgentScope_...`, `..._ScopeSurvivesCoalesceAfterClaim` |
|
||||
| `.2` every fire and retry uses a fresh, never-reused session | **Gap.** The scope tests above assert selection and round-trip, not session identity. Nothing asserts that two fires and a retry get distinct sessions. Two fires: Task 01. A retry on one run: Task 05 |
|
||||
| `.2` a failed summary write leaves the last successful summary intact | **Gap.** The write failure is swallowed today. Task 06 |
|
||||
| `.2` a failed attempt does not replace the last successful summary | **Gap.** No test drives a failure and reads the summary back. Task 06 |
|
||||
| `.3` periodic idle-skip admission on a cron wake | `office/service/wo46_idle_skip_routine_dispatch_test.go` — `TestIdleSkip_RoutineDispatchNoTasks_Skipped`, `TestIdleSkip_RoutineDispatch_CoordinatorNotSkipped`. Both queue `shared.RunReasonRoutineDispatchCron`, so they cover the periodic half of the clause and only that half |
|
||||
| `.3` idle skipping does not consume a manual or webhook wake | **Partly covered; the remaining half is a gap.** `office/shared/runreasons_test.go` — `TestRoutineDispatchReason_SourceMapping` asserts a manual or webhook fire carries `RunReasonRoutineDispatchEvent`, and `TestIsPeriodicTasklessWake_LegacyRoutineDispatchIsNonSkippable` pins the ambiguous legacy literal non-skippable. Nothing asserts `IsPeriodicTasklessWake(RunReasonRoutineDispatchEvent)` is false — that value reaches the `default` branch untested — and nothing drives `checkIdleSkip` with that reason and `SkipIdleRuns` on. Task 01 |
|
||||
| `.3` capabilities apply to a taskless run | `office/runtime/tasks_list_test.go` — `TestListTasks_TasklessRunWithCapabilitySucceeds`; `office/repository/sqlite/failure_test.go` — `TestHasPriorTasklessFailedRun`, `TestHasPriorTasklessFailedRun_IsolatedByContinuationScope` |
|
||||
| `.3` coalescing applies to a taskless run | `office/service/continuation_summary_reader_test.go` — `TestLoadContinuationSummary_ScopeSurvivesCoalesceAfterClaim` (coalesce into a claimed taskless run); `office/routines/service_reconciliation_test.go` — `TestDispatch_LightweightRoutine_SourceKeysAreUnique`, `TestDispatch_LightweightRoutine_WebhookRequestKeyIsStable` |
|
||||
| `.3` budget admission applies | **Inherited structurally, not cited.** `admitRun` runs before the task/taskless branch in `launchAgent`, so a taskless run cannot reach launch without passing the same five gates. Recorded here rather than cited, because no taskless-specific assertion exists or is owed. |
|
||||
| `.3` retry and backoff apply | **Inherited structurally, not cited.** A failed taskless launch settles through the shared run-failure path (`failTasklessRun` → `MarkRunFailed`) and the shared routing backoff, neither of which has a task predicate. Recorded rather than cited for the same reason as budget admission. |
|
||||
| `.3` provider-routed profiles work | **Gap.** The routed taskless path has no test. Task 05 |
|
||||
| `.4` run history shows the exact session and runtime outcome; usage attributed once | `office/service/event_subscribers_run_output_test.go` — `TestHandleTasklessAgentCompleted_RecordsOutputSummaryAndKeepsContinuationSummary`; `office/service/taskless_lifecycle_test.go` — `TestTasklessUsageDuplicate` |
|
||||
| `.4` run history shows the **actual** adapter/model | `office/dashboard/run_detail_test.go` — `TestGetRunDetailActualInvocation` (reads the recorded adapter and model back off the record), `TestGetRunDetailUsesNewestRunSessionInvocation` (two attempts on one run: detail shows the newest attempt's session, adapter and model). Previously uncited; the three tests on the row above only *supply* an adapter and model on a mock launch result and never read them back. The routed path, where the recorded pair can differ from the profile's, is covered by Task 05's first bullet |
|
||||
| `.4` a delayed event from an older *run* does not clear a newer run | `office/service/taskless_lifecycle_test.go` — `TestTasklessLateCompletionDoesNotFinishSuccessor` (two runs, attempt 1 each) |
|
||||
| `.4` a delayed event from an older *attempt of the same run* does not clear the newer attempt | **Gap.** Provider fallback produces exactly this shape routinely — three candidates, three attempts on one run — and no test injects a delayed predecessor event into it. Task 05 |
|
||||
| `.7` task-decision tools reject a taskless run; failed lookup fails closed | `office/runtime/handler_decision_test.go` — `TestRuntimeHandler_RecordAgentDecisionRejectsTasklessRun`; `office/runtime/scope_derivation_test.go` — `TestBuild_TasklessRunMaterializesRunnerSet`, `TestBuild_RunnerSetQueryErrorFailsClosed`, `TestBuild_WhitespaceOnlyPayloadTaskIDIsTaskless` |
|
||||
| `.7` workspace boundary retained | `office/runtime/actions_annotation_test.go` — `TestPostComment_TasklessRunRefusesCrossWorkspaceTarget` (a taskless run in `ws-1` targeting a task in `ws-2` gets `ErrTaskOutOfScope` and no comment is written) and `TestPostComment_TasklessRunRefusesNonexistentTargetWithSameSentinelAsCrossWorkspace` (the anti-oracle: a missing task returns the same sentinel, so the refusal leaks no existence). The permitted-success case is `TestRuntimeHandler_TasklessRunAnnotatesWorkspaceTaskOverHTTP` in `office/runtime/handler_taskless_annotation_test.go`, which on its own is not evidence for this clause — it asserts only a 201 |
|
||||
| `.7` capability boundary retained | `office/runtime/tasks_list_test.go` — `TestListTasks_TasklessRunWithCapabilitySucceeds` shows a taskless run traversing the capability gate, and `TestListTasks_WithoutCapabilityDenied` shows that gate returning 403 with the lister never called. The pair is the evidence: the denial test is not itself taskless-specific, and it is cited for the gate a taskless run is proven to pass through, not as a neighbouring test |
|
||||
| `.8` task-bound launches and their task/session lifecycle unchanged | `office/service/scheduler_taskless_launch_test.go` — `TestSchedulerTick_TaskBoundRunStillLaunches` |
|
||||
| `.8` a taskless failure does not auto-pause the agent or re-enter the inbox | same file — `TestSchedulerTick_TasklessRunsDoNotAutoPauseAgent`, `TestSchedulerTick_RepeatTasklessFailures_OnlyFirstStaysInInbox` |
|
||||
| `.8` historical unsupported taskless failures are not automatically replayed | **Satisfied structurally; no work order owes it.** Nothing in scope can replay a recorded failure: the unfinished-session query selects `preparing` and `running` only, so a `failed` run is in no population any code path here inspects, and no change in scope adds a channel that relaunches from run-session state. The system design states this. It stops being free when the outstanding lifecycle work lands, which is why the follow-up owes it a case |
|
||||
| Reservation CAS and schema | `office/repository/sqlite/run_sessions_test.go` — `TestRunSessionReservationCAS`, `TestRunSessionSchemaExists` |
|
||||
| Run-owned events ignored by task consumers | **Gap.** Enforced structurally by an empty task-session identity; nothing asserts it. Task 06 |
|
||||
|
||||
## E2E tests
|
||||
|
||||
None. `git diff --stat -- apps/web` is empty for this change: there is no
|
||||
`apps/web` change, no new user-visible surface, and no UI behavior to drive.
|
||||
The taskless run's existing surfaces (run detail, inbox, costs) are unchanged.
|
||||
The three surviving work orders are coverage only — the earlier form of this
|
||||
decision reasoned about Tasks 03, 04 and 07 adding backend-only behavior. Those
|
||||
pending tasks do not add a user-facing surface, so the conclusion still holds
|
||||
for this coverage change.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [completed] [Task 01: Prove the lightweight cron fire reaches a session](task-01-prove-lightweight-cron-to-session.md)
|
||||
- [completed] [Task 05: Cover routed (provider fallback) taskless launch](task-05-cover-routed-taskless-launch.md)
|
||||
- [completed] [Task 06: Pin the taskless event-path guarantees no test asserts](task-06-pin-event-path-guarantees.md)
|
||||
|
||||
Pending follow-up work for `AC-OFFICE-TASKLESS-001.5` and `.6`. These tasks are
|
||||
not part of this coverage change:
|
||||
|
||||
- [pending] [Task 02: Cover restart reconciliation of unfinished taskless attempts](task-02-cover-restart-reconciliation.md)
|
||||
- [pending] [Task 03: Stop live run sessions on run cancel and agent disable/removal](task-03-stop-run-sessions-on-cancel-and-agent-change.md)
|
||||
- [pending] [Task 04: Fail closed on discarded requeue and stop errors](task-04-fail-closed-on-recovery-and-stop-errors.md)
|
||||
- [pending] [Task 07: Settle a claimed run from its terminal attempt instead of relaunching](task-07-settle-terminal-attempt-before-relaunch.md)
|
||||
|
||||
## Dependency order
|
||||
|
||||
```text
|
||||
Task 01 Task 05 Task 06
|
||||
|
||||
Task 02 -> Task 03 -> Task 04 -> Task 07 (pending lifecycle follow-up)
|
||||
```
|
||||
|
||||
The three coverage tasks are wave 1, declare `depends_on: []`, and touch
|
||||
disjoint test files. The pending lifecycle tasks retain their dependency order
|
||||
and must be planned separately from this coverage change.
|
||||
|
||||
## Verification results
|
||||
|
||||
All three work orders (Task 01, Task 05, Task 06) are implemented and
|
||||
committed on `feature/office-beta-reconcil-t74`: `026280e61` (all Go
|
||||
production and test changes) and `f234ea888` (spec docs + this plan
|
||||
directory). Build round 1 (`fed21f008`) fixed two test-fidelity findings from
|
||||
Review round 1 (routed-launcher reserve-before-failFor ordering; the
|
||||
interrupted-attempt-recovery test now drives the real
|
||||
`officeRunSessionLauncher.ReconcileRunSessions`). Testing round 2
|
||||
independently re-verified that fixup with no genuine defect found.
|
||||
|
||||
```text
|
||||
$ go test ./internal/office/... ./internal/backendapp/... -count=1
|
||||
[... every internal/office/* subpackage ok, 4 correctly report [no test files] ...]
|
||||
ok github.com/kandev/kandev/internal/office/service 6.548s
|
||||
ok github.com/kandev/kandev/internal/backendapp 23.008s
|
||||
ok github.com/kandev/kandev/internal/backendapp/ownershiplock 0.320s
|
||||
|
||||
$ go test ./internal/office/service/... ./internal/backendapp/... -race -count=1
|
||||
ok github.com/kandev/kandev/internal/office/service 10.873s
|
||||
ok github.com/kandev/kandev/internal/backendapp 43.678s
|
||||
ok github.com/kandev/kandev/internal/backendapp/ownershiplock 4.839s
|
||||
|
||||
$ golangci-lint run ./internal/office/... ./internal/backendapp/... --new-from-rev=d355a672b1db624048f8ff87bc456a1729f78cb0 --timeout=5m
|
||||
0 issues.
|
||||
$ golangci-lint run ./internal/office/... ./internal/backendapp/... --timeout=5m
|
||||
0 issues.
|
||||
|
||||
$ git diff --name-only d355a672b...HEAD -- '*.go' | xargs gofmt -l
|
||||
(no output — clean)
|
||||
```
|
||||
|
||||
`make typecheck`, `make lint` (repo-wide `golangci-lint run ./...`), `make
|
||||
lint-format`, and `cd apps/web && pnpm run i18n:ratchet` all passed clean
|
||||
during Build (docs/backend-only change, no UI source touched). `git diff
|
||||
--stat -- apps/web` is empty on this branch, confirmed both at Build and at
|
||||
Testing round 2 — no E2E required.
|
||||
|
||||
Five accepted gaps remain from the Round-6 Spec Review decision
|
||||
(`## Decision (human, round 6)` in the task's Kandev plan): AC `.3`
|
||||
retry/backoff parity does not hold on the direct taskless path (deliberate,
|
||||
WO-35 incident, not changed); no cross-run continuation-scope winner rule
|
||||
(Task 06 tests ordering within one run's attempts only); AC `.1`'s
|
||||
prompt/tools delivery clause stays an uncovered gap (Task 01 asserts the
|
||||
session seam only); the AC `.8` replay justification was corrected, no
|
||||
behavior change; the failed-summary-write run event is best-effort, matching
|
||||
`AppendRunEvent`'s existing log-and-swallow contract. None of these
|
||||
triggered a route-back to Spec or Spec Review.
|
||||
|
||||
## Risks
|
||||
|
||||
- The test drives a multi-stage asynchronous chain (cron tick to wakeup row to
|
||||
run claim to launch). It must wait on an observable condition rather than a
|
||||
fixed sleep, matching the heavy-routine test's polling shape, or it will be
|
||||
flaky on a loaded CI shard.
|
||||
- If the lightweight chain turns out to break somewhere between the wakeup row
|
||||
and `launchAgent` when driven by this harness, the correct response is to
|
||||
report that as a behavior finding against `REQ-OFFICE-TASKLESS-001`, not to
|
||||
weaken the assertion or re-skip the test.
|
||||
|
||||
## Open questions
|
||||
|
||||
None outstanding. The storage-seam question this initiative was filed to decide
|
||||
is settled in the system design: neither a nullable `task_sessions.task_id` nor
|
||||
a synthetic task is permitted; the record is a separate Office-owned
|
||||
`office_run_sessions` row.
|
||||
@@ -0,0 +1,175 @@
|
||||
---
|
||||
id: "01-prove-lightweight-cron-to-session"
|
||||
title: "Prove the lightweight cron fire reaches a taskless session"
|
||||
status: completed
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.1
|
||||
- AC-OFFICE-TASKLESS-001.2
|
||||
- AC-OFFICE-TASKLESS-001.3
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 01: Prove the lightweight cron fire reaches a taskless session
|
||||
|
||||
## Summary
|
||||
|
||||
`AC-OFFICE-TASKLESS-001.1` requires that an eligible lightweight wake with no
|
||||
task starts a real agent session and reaches a visible terminal outcome
|
||||
**without creating any task or task session**. The run-owned session mechanism
|
||||
that satisfies this has shipped, but the end-to-end proof from an armed cron
|
||||
trigger is still skipped, and the assertion it was written to make targets a
|
||||
`task_sessions` row the design explicitly forbids.
|
||||
|
||||
Replace that skip with a real assertion against `office_run_sessions`.
|
||||
|
||||
## In scope
|
||||
|
||||
- `apps/backend/internal/backendapp/office_routine_cron_to_session_test.go`:
|
||||
- Remove the `t.Skip("blocked on card 49894d63: ...")` from
|
||||
`TestRoutine_CronFire_LightweightReachesSession` and the now-unreachable
|
||||
trailing `t.Fatal("unreachable: update this assertion when the lightweight
|
||||
path reaches a session")`.
|
||||
- Rewrite the doc comment above the test. It currently states that the
|
||||
lightweight path "has no session to reach at all today" and that the
|
||||
missing assertion is a `task_sessions` row. Both are wrong: the path
|
||||
launches through `RunSessionLauncher`, and the design forbids a
|
||||
`task_sessions` row for a taskless run.
|
||||
- Assert the positive half: after `TickScheduledTriggers` fires the armed
|
||||
trigger, an `office_run_sessions` row exists for the dispatched run, and
|
||||
`runs.session_id` is bound to that row's id.
|
||||
- Assert the negative half: the fire created no `tasks` row and no
|
||||
`task_sessions` row. This is the part of the criterion that distinguishes
|
||||
the lightweight path from the heavy one, so it must be asserted, not
|
||||
implied.
|
||||
- Wait on an observable condition (poll until the run reaches a launched
|
||||
state, bounded by a deadline), mirroring the polling the passing
|
||||
heavy-routine test in the same file already uses. Do not add a fixed sleep.
|
||||
- Assert the first clause of `AC-OFFICE-TASKLESS-001.2` across fires: arm and
|
||||
fire the trigger twice, and assert the two fires produce two distinct
|
||||
`office_run_sessions` ids with no ACP session reused between them. The
|
||||
existing continuation-summary tests assert scope selection and round-trip,
|
||||
not session identity, so nothing covers this today. The retry half of the
|
||||
same clause (a second attempt on one run) belongs to Task 05.
|
||||
|
||||
- `AC-OFFICE-TASKLESS-001.3`'s second sentence — "periodic idle skipping shall
|
||||
not consume a manual or webhook wake" — has no assertion anywhere. The two
|
||||
idle-skip tests both queue `shared.RunReasonRoutineDispatchCron`, so they cover
|
||||
only the periodic half. Close it beside the existing tests rather than in the
|
||||
cron-to-session file:
|
||||
- `apps/backend/internal/office/shared/runreasons_test.go`: assert
|
||||
`IsPeriodicTasklessWake(shared.RunReasonRoutineDispatchEvent)` is false. That
|
||||
value reaches the `default` branch of the switch today with nothing pinning
|
||||
it, so deleting the branch or adding the constant to the `true` case is a
|
||||
silent behavior change.
|
||||
- `apps/backend/internal/office/service/wo46_idle_skip_routine_dispatch_test.go`:
|
||||
a run queued with `RunReasonRoutineDispatchEvent`, an agent with
|
||||
`SkipIdleRuns` **on** and zero actionable tasks, is **not** skipped. This is
|
||||
the assertion that names the criterion: the mapping test in `shared` proves a
|
||||
manual or webhook fire carries that reason, and this one proves that reason
|
||||
survives the idle gate.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **Any production change.** This work order adds no production behavior; it
|
||||
only proves behavior that already exists. Concretely: no non-test file under
|
||||
`apps/backend/**` may change, and in particular nothing under
|
||||
`apps/backend/internal/office/**` or
|
||||
`apps/backend/internal/agent/runtime/**` outside the three `_test.go` files
|
||||
named in In scope.
|
||||
- `TestRoutine_CronFire_HeavyRoutineReachesSession`, which passes and whose
|
||||
`task_sessions` assertion is correct for the heavy path.
|
||||
- Any change to `REQ-OFFICE-TASKLESS-001` or its system design. If the test
|
||||
cannot be made to pass without a production change, stop and report a
|
||||
behavior finding rather than editing the contract or weakening the assertion.
|
||||
|
||||
## Acceptance conditions
|
||||
|
||||
1. `TestRoutine_CronFire_LightweightReachesSession` runs (is not skipped) and
|
||||
passes, asserting both an `office_run_sessions` row for the dispatched run
|
||||
and `runs.session_id` bound to it.
|
||||
2. The same test asserts that neither a `tasks` row nor a `task_sessions` row
|
||||
was created by the lightweight fire.
|
||||
3. `AC-OFFICE-TASKLESS-001.3`'s manual/webhook clause is asserted in both
|
||||
places In scope names: `IsPeriodicTasklessWake(RunReasonRoutineDispatchEvent)`
|
||||
is pinned false, and a run queued with that reason under `SkipIdleRuns` with
|
||||
zero actionable tasks is shown not to be skipped.
|
||||
4. Exactly three source files change, all of them tests:
|
||||
`apps/backend/internal/backendapp/office_routine_cron_to_session_test.go`,
|
||||
`apps/backend/internal/office/shared/runreasons_test.go`, and
|
||||
`apps/backend/internal/office/service/wo46_idle_skip_routine_dispatch_test.go`
|
||||
— plus this work order and `plan.md`. No other file changes.
|
||||
|
||||
## Verification commands
|
||||
|
||||
```bash
|
||||
cd apps/backend
|
||||
|
||||
# The target test, and its heavy sibling, must both pass and neither may skip.
|
||||
go test ./internal/backendapp/ -run 'TestRoutine_CronFire' -v -count=1
|
||||
|
||||
# Repeat under the race detector: this drives an asynchronous dispatch chain.
|
||||
go test -race ./internal/backendapp/ -run 'TestRoutine_CronFire' -count=1
|
||||
|
||||
# Flake check on the new polling assertion.
|
||||
go test ./internal/backendapp/ -run 'TestRoutine_CronFire_LightweightReachesSession' -count=5
|
||||
|
||||
# Unchanged neighbours must stay green.
|
||||
go test -count=1 ./internal/office/... ./internal/backendapp/...
|
||||
|
||||
gofmt -l internal/backendapp/office_routine_cron_to_session_test.go \
|
||||
internal/office/shared/runreasons_test.go \
|
||||
internal/office/service/wo46_idle_skip_routine_dispatch_test.go
|
||||
make lint
|
||||
```
|
||||
|
||||
From the repository root:
|
||||
|
||||
```bash
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
python3 scripts/list-docs.py validate
|
||||
```
|
||||
|
||||
## Likely files
|
||||
|
||||
- `apps/backend/internal/backendapp/office_routine_cron_to_session_test.go`
|
||||
- `apps/backend/internal/office/shared/runreasons_test.go`
|
||||
- `apps/backend/internal/office/service/wo46_idle_skip_routine_dispatch_test.go`
|
||||
|
||||
Those three test files are the only source files this work order changes.
|
||||
|
||||
Read-only references:
|
||||
|
||||
- `apps/backend/internal/office/repository/sqlite/base.go` —
|
||||
`office_run_sessions` schema, and the `ListRunSessions` shape the assertion
|
||||
should use
|
||||
- `apps/backend/internal/office/service/scheduler_integration.go` —
|
||||
`launchAgent`'s `taskID == ""` branch and `persistLaunchedSession`
|
||||
- `apps/backend/internal/office/routines/service.go` —
|
||||
`dispatchRoutineRun`'s lightweight branch
|
||||
- `apps/backend/internal/office/service/taskless_lifecycle_test.go` — existing
|
||||
taskless lifecycle assertions to match in style
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. The production behavior under test is already on `main`
|
||||
(`d355a672b1db624048f8ff87bc456a1729f78cb0`), and the sibling authority work
|
||||
(`REQ-OFFICE-COORDINATOR-AUTHORITY-*`) is `implemented`.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Flake.** The chain from cron tick to launched session is asynchronous across
|
||||
several components. Poll an observable condition with a deadline; a fixed
|
||||
sleep will be flaky under parallel shards. The `-count=5` run above is the
|
||||
check for this.
|
||||
- **A real behavior gap.** If the lightweight chain does not actually reach
|
||||
`launchAgent` under this harness, that is a genuine finding against
|
||||
`AC-OFFICE-TASKLESS-001.1`. Report it; do not re-skip the test, do not
|
||||
soften the assertion to whatever the code happens to do, and do not assert on
|
||||
an intermediate row (a wakeup request or a run claim) as a substitute for the
|
||||
session the criterion actually requires.
|
||||
@@ -0,0 +1,142 @@
|
||||
---
|
||||
id: "02-cover-restart-reconciliation"
|
||||
title: "Cover restart reconciliation of unfinished taskless attempts"
|
||||
status: pending
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.6
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 02: Cover restart reconciliation of unfinished taskless attempts
|
||||
|
||||
> **PENDING FOLLOW-UP — 2026-09-19.** This work order is not part of the current
|
||||
> coverage change. `AC-OFFICE-TASKLESS-001.6` remains an outstanding requirement,
|
||||
> and this order covers its restart reconciliation behavior.
|
||||
>
|
||||
> The file is kept, unedited below this banner, because a follow-up should start
|
||||
> from it rather than re-derive it. The current gaps and the two problems a
|
||||
> follow-up must resolve first are recorded under
|
||||
> [Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
> References below to `taskless-run-recovery.md` point at a design file superseded in the same
|
||||
> change; its content is summarized in that section.
|
||||
|
||||
## Summary
|
||||
|
||||
`AC-OFFICE-TASKLESS-001.6` is the restart criterion: unfinished attempts are
|
||||
reconciled against runtime evidence before a replacement launch, a possibly live
|
||||
predecessor blocks a replacement until it is stopped or proven absent,
|
||||
interrupted work gets an explicit outcome, and no taskless attempt stays claimed
|
||||
indefinitely just because it has no task. The system design's verification
|
||||
section requires testing "restart with a live/absent/unknown predecessor".
|
||||
|
||||
The behavior has shipped in
|
||||
`apps/backend/internal/backendapp/office_run_session_launcher.go`
|
||||
(`ReconcileRunSessions`, `sameRunOwner`, `isLiveExecution`), and it is on the
|
||||
startup path: `internal/backendapp/main.go:1134` aborts backend startup when it
|
||||
returns an error. None of it has a test. `office_run_session_launcher.go` has no
|
||||
`_test.go` sibling, and the taskless tests that exist
|
||||
(`office/service/taskless_lifecycle_test.go`,
|
||||
`office/repository/sqlite/run_sessions_test.go`) cover launch, late completion,
|
||||
usage deduplication and reservation CAS, not restart recovery.
|
||||
|
||||
This work order adds that coverage. It adds no production behavior. Task 04
|
||||
changes two discarded error results in the same file; where a case below names
|
||||
behavior Task 04 introduces, this work order asserts the behavior Task 04
|
||||
specifies rather than the current discard.
|
||||
|
||||
## In scope
|
||||
|
||||
Add `apps/backend/internal/backendapp/office_run_session_launcher_test.go`
|
||||
covering every branch `ReconcileRunSessions` can take for a durable attempt left
|
||||
in `preparing` or `running`:
|
||||
|
||||
- **Never registered** — `execution_id` empty. Expect the attempt finished as
|
||||
`interrupted` with `"runtime execution was not registered"` and its run
|
||||
requeued.
|
||||
- **Absent predecessor** — `GetExecution` returns a not-found error. Expect any
|
||||
retained runtime resource stopped, the attempt finished as `interrupted` with
|
||||
`"runtime execution was not found after restart"`, and the run requeued.
|
||||
- **Live predecessor** — status `pending`, `starting`, `running` or `ready`.
|
||||
Expect the attempt left non-terminal and the run *not* requeued, so no
|
||||
replacement can launch beside it. Cover all four statuses; a table test is
|
||||
fine.
|
||||
- **Ended while down** — status `completed` finishes the attempt and the run;
|
||||
`failed` records the runtime error message and requeues; any other terminal
|
||||
status is `interrupted` and requeues.
|
||||
- **Unknown owner** — a returned execution whose `ExecutionOwner` disagrees with
|
||||
the durable record on any of kind, workspace, run, run session, attempt or
|
||||
agent profile. Expect an error, not a silent skip and not a replacement.
|
||||
Assert per-field, not only on a wholesale mismatch, because `sameRunOwner` is
|
||||
a six-way conjunction and a single dropped term would still pass a single
|
||||
blunt case.
|
||||
- **Inspect failure** — a non-not-found error from `GetExecution` returns an
|
||||
error rather than treating the predecessor as absent.
|
||||
- **Requeue failure** — the store's requeue errors, and separately matches no
|
||||
row. Expect `ReconcileRunSessions` to return an error rather than reporting
|
||||
success with the run left claimed behind a terminal attempt. This is the case
|
||||
the current code cannot pass, because both results are discarded; Task 04 is
|
||||
the production change that makes it passable, and re-skipping or softening it
|
||||
is not an option.
|
||||
|
||||
Also assert the startup consequence once, at the seam that owns it: a
|
||||
reconciliation error must fail backend startup rather than being logged and
|
||||
stepped past. If driving `main.go`'s path is impractical, assert on the
|
||||
`ReconcileRunSessions` error being returned through
|
||||
`office/service/service.go:105` and record in the test why the startup caller is
|
||||
covered by inspection instead.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Any production change to `ReconcileRunSessions` or its helpers **beyond the
|
||||
two discarded results Task 04 owns**. If some other test cannot be written
|
||||
without changing production behavior, that is a finding against
|
||||
`REQ-OFFICE-TASKLESS-001`, not a licence to edit the code under test.
|
||||
- Cancellation sweeps (`office/pause`), which serve `AC-OFFICE-TASKLESS-001.5`
|
||||
and already have `office/pause/service_test.go`.
|
||||
- Postgres parity for the reconciliation query itself; `ListUnfinishedRunSessions`
|
||||
is covered by the repository's existing migration harness.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/ -run 'TestOfficeRunSessionLauncher|TestReconcile' -v -count=1
|
||||
go test ./internal/backendapp/ -run 'TestOfficeRunSessionLauncher|TestReconcile' -count=5
|
||||
make lint
|
||||
```
|
||||
|
||||
Goroutine ownership applies: if the test stands up anything that starts a
|
||||
goroutine, instrument it with goleak per `apps/backend/AGENTS.md`.
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/backendapp/office_run_session_launcher.go:218` —
|
||||
`ReconcileRunSessions`, and `:280`/`:286` for `sameRunOwner` / `isLiveExecution`
|
||||
- `apps/backend/internal/backendapp/main.go:1134` — the startup caller that
|
||||
aborts on error
|
||||
- `apps/backend/internal/office/repository/sqlite/run_sessions.go:200` —
|
||||
`ListUnfinishedRunSessions`, the input to reconciliation
|
||||
- `apps/backend/internal/office/service/taskless_lifecycle_test.go` — existing
|
||||
taskless test style to match
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Independent of Task 01: different file, different criterion. Both may run
|
||||
in wave 1.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Fake-passing through an over-stubbed runtime.** The interesting branches are
|
||||
distinguished by what `GetExecution` returns. A stub that returns the same
|
||||
shape everywhere will make every case look correct. Drive each branch from a
|
||||
distinct stub response and assert the durable row, not just the absence of an
|
||||
error.
|
||||
- **Asserting the message instead of the outcome.** The reason strings are
|
||||
useful evidence but are not the criterion. Assert the attempt's terminal state
|
||||
and whether the run was requeued; check the message as a secondary assertion.
|
||||
+145
@@ -0,0 +1,145 @@
|
||||
---
|
||||
id: "03-stop-run-sessions-on-cancel-and-agent-change"
|
||||
title: "Stop live run sessions on run cancel and agent disable/removal"
|
||||
status: pending
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.5
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 03: Stop live run sessions on run cancel and agent disable/removal
|
||||
|
||||
> **PENDING FOLLOW-UP — 2026-09-19.** This work order is not part of the
|
||||
> current coverage change. `AC-OFFICE-TASKLESS-001.5` remains an outstanding
|
||||
> requirement, and this order gives run cancellation, agent disable and agent
|
||||
> removal the run-session stop seam it requires. The current control gaps remain
|
||||
> open until this work is delivered.
|
||||
>
|
||||
> The file is kept, unedited below this banner, because a follow-up should start
|
||||
> from it rather than re-derive it. The current gaps and the two problems a
|
||||
> follow-up must resolve first are recorded under
|
||||
> [Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
> References below to `taskless-run-recovery.md` point at a design file superseded in the same
|
||||
> change; its content is summarized in that section.
|
||||
|
||||
## Summary
|
||||
|
||||
`AC-OFFICE-TASKLESS-001.5` names four controls that must prevent new taskless
|
||||
launches and stop live taskless executions: workspace pause, explicit run
|
||||
cancellation, agent disable/removal, and workspace removal. Two of them do.
|
||||
|
||||
- Workspace pause reaches run sessions through
|
||||
`office/pause/sweep.go:cancelRunSessions`, which lists live sessions with
|
||||
`ListLiveRunSessionsForWorkspace`, calls `RequestRunSessionCancellation` and
|
||||
stops by execution ID.
|
||||
- Workspace deletion does the same in
|
||||
`office/service/workspace_deletion.go`.
|
||||
- **Explicit run cancellation** is a status write on `runs`
|
||||
(`CancelRunsWhere` and its callers). Nothing consults
|
||||
`office_run_sessions`, so the process keeps running after the run is
|
||||
cancelled.
|
||||
- **Agent disable** (`office/agents/service.go:UpdateAgentStatus`) writes the
|
||||
status row only.
|
||||
- **Agent removal** (`office/agents/service.go:DeleteAgentInstance`) calls
|
||||
`SessionTerminator.TerminateAllForAgent`, which is
|
||||
`orchestrator/office_session_terminator.go`:
|
||||
`ListNonTerminalSessionsByAgentInstance` plus `updateTaskSessionState` keyed
|
||||
by `sess.TaskID`. That is the **task**-session store. A run-owned execution is
|
||||
never listed and never stopped.
|
||||
|
||||
This is production work, not coverage. The criterion is unchanged; the code owes
|
||||
it the seam.
|
||||
|
||||
## In scope
|
||||
|
||||
Give explicit run cancellation, agent disable and agent removal the same
|
||||
run-session stop seam workspace pause already has:
|
||||
|
||||
- Persist cancellation intent on the affected live run sessions
|
||||
(`RequestRunSessionCancellation`) **before** asking the runtime to stop, so a
|
||||
launch racing the control is rejected at the registration admission gate
|
||||
rather than starting behind it.
|
||||
- Stop each live execution by its recorded `execution_id`.
|
||||
- Count partial failures. A stop that fails leaves the session live with its
|
||||
execution ID intact so the next sweep re-lists it, and is **not** recorded as
|
||||
terminal — the rule the system design states under cancellation and recovery.
|
||||
Report the failure alongside the executions that stopped successfully.
|
||||
- Reuse the existing sweep helper rather than writing a third copy of the
|
||||
list-intent-stop-count loop. If the current helper is too workspace-shaped to
|
||||
reuse, extract the inner loop and keep one implementation.
|
||||
|
||||
Assert the **retryable** half of `AC-OFFICE-TASKLESS-001.5`, which nothing
|
||||
covers today. `TestPauseStopsMixedRunSessionInventory` runs a single sweep and
|
||||
asserts counts, so an implementation that reports a partial failure once and
|
||||
then loses the session passes it. Add, for each of the three controls and for
|
||||
workspace pause:
|
||||
|
||||
- after a stop that fails, the session is still in a live state and still
|
||||
carries its `execution_id` — assert the row, not only the returned count;
|
||||
- a **second** sweep over the same workspace re-lists that session and attempts
|
||||
the stop again. This is the design's own definition of retryable, and one
|
||||
sweep cannot demonstrate it.
|
||||
|
||||
Selection differs per control and must be exact:
|
||||
|
||||
- Run cancellation: the live sessions of that run (`ListRunSessions`, filtered
|
||||
to non-terminal, or a targeted query).
|
||||
- Agent disable/removal: the live sessions whose `agent_profile_id` is that
|
||||
agent, across the workspace.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Any change to `AC-OFFICE-TASKLESS-001.5` itself. The criterion is correct.
|
||||
- Task-session termination behavior. `TerminateAllForAgent` keeps doing what it
|
||||
does for task sessions; this adds a run-owned path beside it, and does not
|
||||
reroute task sessions through Office.
|
||||
- Workspace pause and workspace deletion, which already satisfy the criterion.
|
||||
- Retention of terminal run-session rows (named out of scope in the
|
||||
requirement).
|
||||
|
||||
## Tests
|
||||
|
||||
- Run cancel with a live run session: intent stamped, runtime stop called with
|
||||
the recorded execution ID, session not left live-and-unstopped.
|
||||
- Agent disable and agent removal, each with one live run session and one live
|
||||
**task** session for the same agent: both are stopped, and the task path keeps
|
||||
its existing behavior.
|
||||
- Partial failure: two live run sessions, one stop fails. Assert the failure is
|
||||
counted and surfaced, the failed session is still live with its execution ID,
|
||||
and the successful one is terminal. This is the case the criterion calls out
|
||||
by name ("including when another execution stopped successfully").
|
||||
- An agent with no live run session is a no-op, not an error.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/office/... -run 'Cancel|AgentRemoval|AgentDisable|RunSession' -count=1
|
||||
go test ./internal/orchestrator/ -count=1
|
||||
make lint
|
||||
```
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/office/pause/sweep.go` — the seam to reuse
|
||||
- `apps/backend/internal/office/repository/sqlite/run_sessions.go` —
|
||||
`RequestRunSessionCancellation`, `ListLiveRunSessionsForWorkspace`,
|
||||
`ListRunSessions`
|
||||
- `apps/backend/internal/orchestrator/office_session_terminator.go` —
|
||||
the task-only cascade this work order sits beside
|
||||
- `apps/backend/internal/office/service/workspace_deletion.go` — the other
|
||||
control that already does it
|
||||
|
||||
## Risks
|
||||
|
||||
- **Stopping too much.** Agent-scoped selection must not reach another agent's
|
||||
sessions in the same workspace, and run-scoped selection must not reach
|
||||
another attempt of another run. Assert the negative in each test.
|
||||
- **Ordering.** Intent before stop is not stylistic: the reverse order leaves a
|
||||
window where a preparing attempt registers behind the stop.
|
||||
+132
@@ -0,0 +1,132 @@
|
||||
---
|
||||
id: "04-fail-closed-on-recovery-and-stop-errors"
|
||||
title: "Fail closed on discarded requeue and stop errors"
|
||||
status: pending
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.5
|
||||
- AC-OFFICE-TASKLESS-001.6
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 04: Fail closed on discarded requeue and stop errors
|
||||
|
||||
> **PENDING FOLLOW-UP — 2026-09-19.** This work order is not part of the
|
||||
> current coverage change. It makes the discarded requeue result and the
|
||||
> discarded `runtime.Stop` error fail closed for the still-outstanding
|
||||
> `AC-OFFICE-TASKLESS-001.5` and `.6` requirements. The current stop error is
|
||||
> recorded as an open gap in the system design's scheduling and routing section.
|
||||
>
|
||||
> The file is kept, unedited below this banner, because a follow-up should start
|
||||
> from it rather than re-derive it. The current gaps and the two problems a
|
||||
> follow-up must resolve first are recorded under
|
||||
> [Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
> References below to `taskless-run-recovery.md` point at a design file superseded in the same
|
||||
> change; its content is summarized in that section.
|
||||
|
||||
## Summary
|
||||
|
||||
Two error results are discarded on the run-session launcher's failure paths, and
|
||||
each one silently produces a state its acceptance criterion forbids.
|
||||
|
||||
**Requeue (`AC-OFFICE-TASKLESS-001.6`).** `ReconcileRunSessions`
|
||||
(`internal/backendapp/office_run_session_launcher.go`) calls
|
||||
`_, _ = l.repo.RequeueClaimedRun(ctx, session.RunID)` on all three recovery
|
||||
branches — never-registered, absent-after-restart, and ended-while-down with a
|
||||
non-finished outcome. Both results are dropped. If the requeue errors or matches
|
||||
no row, the attempt is already terminal, the run stays `claimed`, and no
|
||||
execution exists behind it: an attempt claimed indefinitely, which is exactly
|
||||
what the criterion forbids. Startup still reports success, so nothing surfaces
|
||||
it.
|
||||
|
||||
**Stop (`AC-OFFICE-TASKLESS-001.5` and the design's no-two-live-executions
|
||||
rule).** `StartRunSession`'s failure paths call
|
||||
`_ = l.runtime.Stop(...)` and then, on several of them,
|
||||
`FinishRunSession(..., Failed, ...)`. A stop that failed followed by a terminal
|
||||
row is the case the system design rules out in as many words: the row leaves the
|
||||
non-terminal set, so `ListLiveRunSessionsForWorkspace` and
|
||||
`ListUnfinishedRunSessions` never list it again while the process may still be
|
||||
running. It is the forbidden two-live-executions window in its least visible
|
||||
form.
|
||||
|
||||
## In scope
|
||||
|
||||
- Treat a failed or no-op requeue in `ReconcileRunSessions` the same way an
|
||||
unconfirmable owner is treated: return an error, so backend startup fails
|
||||
rather than proceeding with a run claimed behind a terminal attempt. The
|
||||
boolean matters as much as the error — a requeue that matched no row is the
|
||||
same bad state as one that errored, and both must be distinguishable in the
|
||||
returned message.
|
||||
- On `StartRunSession`'s failure paths, stop discarding the `runtime.Stop`
|
||||
error. When the stop fails, do **not** write a terminal session state: leave
|
||||
the row live with its execution ID so the next sweep and the next startup
|
||||
reconciliation re-list it, record the stop failure on the session
|
||||
(`error_message`) and in the returned error, and log it at error level with
|
||||
run, session, attempt and execution identity.
|
||||
- Keep the existing behavior when the stop succeeds: terminal state, existing
|
||||
message, existing return.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- The reconciliation branch structure, `sameRunOwner` and `isLiveExecution`.
|
||||
Their logic is correct; only the discarded results change.
|
||||
- Retry or backoff policy for a failed requeue. Failing startup is the
|
||||
specified outcome; a retry loop is a different decision and is not taken here.
|
||||
- Task 03's cancellation seam, which is the same rule applied to a different
|
||||
set of callers.
|
||||
|
||||
## Tests
|
||||
|
||||
- Requeue returns an error: `ReconcileRunSessions` returns an error naming the
|
||||
run, and the attempt's terminal state is still written (the attempt is
|
||||
genuinely over; it is the run that must not be left claimed silently).
|
||||
- Requeue returns `(false, nil)`: same, with a message that distinguishes "no
|
||||
row matched" from a store error.
|
||||
- Requeue succeeds: no error, run requeued — the existing happy path stays.
|
||||
- `StartRunSession` with a failing `runtime.Stop` on the bind-failure path and
|
||||
on the start-failure path: the session row is **not** terminal, still carries
|
||||
its execution ID, records the stop failure, and the returned error names it.
|
||||
- `StartRunSession` with a succeeding stop: unchanged terminal behavior.
|
||||
|
||||
Assert on the durable row, not only on the returned error. A stub runtime that
|
||||
returns the same shape everywhere will make every branch look correct.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/ -run 'TestOfficeRunSessionLauncher|TestReconcile|TestStartRunSession' -v -count=1
|
||||
go test ./internal/backendapp/ -run 'TestOfficeRunSessionLauncher|TestReconcile|TestStartRunSession' -count=5
|
||||
make lint
|
||||
```
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/backendapp/office_run_session_launcher.go` —
|
||||
`StartRunSession` failure paths and `ReconcileRunSessions`
|
||||
- `apps/backend/internal/office/repository/sqlite/run_sessions.go` —
|
||||
`FinishRunSession`, `ListUnfinishedRunSessions`,
|
||||
`ListLiveRunSessionsForWorkspace`
|
||||
- `apps/backend/internal/backendapp/main.go` — the startup caller that aborts
|
||||
on a reconciliation error
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Shares a file with Task 02, which is test-only: if both run in the same
|
||||
tree, land Task 02's test file first so this task's behavior changes have
|
||||
assertions to break.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Failing startup too eagerly.** Only a requeue that did not take may abort
|
||||
startup. A run that was already terminal or already queued is not a failure;
|
||||
check what `RequeueClaimedRun` returns for those cases before choosing the
|
||||
error condition.
|
||||
- **Leaking a live row forever.** Leaving a row live on stop failure is
|
||||
deliberate, and it is only correct because startup reconciliation and the pause
|
||||
sweep both re-list non-terminal rows. Confirm both still reach it.
|
||||
@@ -0,0 +1,104 @@
|
||||
---
|
||||
id: "05-cover-routed-taskless-launch"
|
||||
title: "Cover routed (provider fallback) taskless launch"
|
||||
status: completed
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.3
|
||||
- AC-OFFICE-TASKLESS-001.4
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 05: Cover routed (provider fallback) taskless launch
|
||||
|
||||
## Summary
|
||||
|
||||
`AC-OFFICE-TASKLESS-001.3` requires concrete **and** provider-routed profiles to
|
||||
work for taskless runs. The routed path has no test. `launchAgent`'s taskless
|
||||
branch calls `tryRoutingDispatch` before the direct launcher, and
|
||||
`office/scheduler/dispatch_routing.go:launchCandidate` calls the same
|
||||
`StartRunSession` per candidate and then `SetRunSessionID`, but every existing
|
||||
taskless test drives the direct branch.
|
||||
|
||||
The system design states two rules about this path that nothing currently
|
||||
asserts: each candidate reserves its own attempt row (three candidates leave
|
||||
three rows, not one row rewritten three times), and no window exists in which
|
||||
two candidates of the same run hold live executions.
|
||||
|
||||
## In scope
|
||||
|
||||
Add coverage for the routed taskless launch:
|
||||
|
||||
- A routed profile whose first candidate succeeds: one attempt row, the
|
||||
candidate's provider and model recorded on it (`adapter`, `model`,
|
||||
`execution_profile_id`), and the Office prompt and skills delivered to the
|
||||
launcher unchanged.
|
||||
- A routed profile that falls back once: two attempt rows with attempt numbers
|
||||
1 and 2, the first terminal before the second is launched, and
|
||||
`runs.session_id` naming the attempt that actually launched.
|
||||
- The negative half of `AC-OFFICE-TASKLESS-001.1` on this path too: no `tasks`
|
||||
row, no `task_sessions` row.
|
||||
- Usage attribution for a routed attempt reaches the cost ledger joined to its
|
||||
own run session (`AC-OFFICE-TASKLESS-001.4`), not to the first attempt.
|
||||
- The retry half of `AC-OFFICE-TASKLESS-001.2`: the second attempt's session is
|
||||
distinct from the first's and neither reuses the other's ACP session. The fire
|
||||
half (two fires, two runs) belongs to Task 01.
|
||||
- The same-run half of `AC-OFFICE-TASKLESS-001.4`: inject a delayed terminal
|
||||
event naming attempt 1 **after** attempt 2 is live on the same run, and assert
|
||||
it applies a terminal state to attempt 1's own session while leaving attempt 2
|
||||
and the run untouched — no completion, no second ledger row, no cleared
|
||||
agent-working state. The existing
|
||||
`TestTasklessLateCompletionDoesNotFinishSuccessor` builds two separate runs at
|
||||
attempt 1 each, so it does not reach this shape, and fallback produces it
|
||||
routinely.
|
||||
|
||||
Assert the ordering directly — the predecessor's terminal state before the
|
||||
successor's reservation — rather than only the final row count. A count-only
|
||||
assertion passes even if both candidates were live at once.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Provider classification, parking and backoff policy, which
|
||||
`office/scheduler` already covers for task-bound runs and which this path
|
||||
reuses unchanged.
|
||||
- Any production change to the routing dispatcher, without exception. If the
|
||||
routed path cannot be driven without changing production behavior, that is a
|
||||
finding against `REQ-OFFICE-TASKLESS-001`, not a licence to edit the code
|
||||
under test. The launch-failure stop-error gap is already known and is
|
||||
deliberately pending follow-up work — see the system design's scheduling and routing
|
||||
section — so encountering it is not a new finding and not this order's work.
|
||||
- Task 01's cron-to-session chain, which drives the direct branch.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/office/... -run 'Routed|Fallback|Taskless' -v -count=1
|
||||
make lint
|
||||
```
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/office/scheduler/dispatch_routing.go` —
|
||||
`launchCandidate`, and the `SetRunSessionID` write per candidate
|
||||
- `apps/backend/internal/office/service/scheduler_integration.go` —
|
||||
`launchAgent`'s taskless branch and `tryRoutingDispatch`
|
||||
- `apps/backend/internal/office/service/taskless_lifecycle_test.go` — the
|
||||
direct-branch test style to match
|
||||
- `apps/backend/internal/office/repository/sqlite/costs.go` — the run-session
|
||||
join the usage assertion reads
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Independent of Tasks 01 and 02.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Asserting on the stub instead of the record.** A routing test can pass by
|
||||
checking what the launcher was called with. Assert the durable
|
||||
`office_run_sessions` rows as well.
|
||||
@@ -0,0 +1,104 @@
|
||||
---
|
||||
id: "06-pin-event-path-guarantees"
|
||||
title: "Pin the taskless event-path guarantees no test asserts"
|
||||
status: completed
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.2
|
||||
- AC-OFFICE-TASKLESS-001.4
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 06: Pin the taskless event-path guarantees no test asserts
|
||||
|
||||
## Summary
|
||||
|
||||
Two guarantees on the taskless event path hold today and are asserted by
|
||||
nothing. Both are the kind that stay true until an unrelated refactor quietly
|
||||
ends them. A third case does not hold today at all: a continuation-summary write
|
||||
that fails is swallowed, and `AC-OFFICE-TASKLESS-001.2` now states an outcome
|
||||
for it. That case carries this work order's only production change, one run
|
||||
event.
|
||||
|
||||
**A failed attempt must not replace the last successful continuation summary**
|
||||
(`AC-OFFICE-TASKLESS-001.2`, second clause). The summary is refreshed from the
|
||||
completion handler after the run's terminal compare-and-set succeeds, so a
|
||||
failed attempt never reaches it. `continuation_summary_reader_test.go` covers
|
||||
scope selection and round-tripping, and
|
||||
`event_subscribers_run_output_test.go` covers what a *completion* writes.
|
||||
Neither drives a failure and then reads the summary back.
|
||||
|
||||
**Task consumers must not act on run-owned events** (system design, events and
|
||||
observation). What enforces this is structural: a run-owned `AgentExecution`
|
||||
carries an empty `SessionID`, so a task consumer resolves no task session and
|
||||
falls out. Nothing asserts it, so the guarantee currently rests on an empty
|
||||
string continuing to behave that way.
|
||||
|
||||
## In scope
|
||||
|
||||
- Drive a taskless attempt to a failed terminal outcome with a prior successful
|
||||
summary in place for the same scope, and assert the stored summary is
|
||||
byte-identical to the successful one afterwards. Cover both scopes
|
||||
(`routine:<id>` and `agent:<id>`), since scope is chosen at run creation and
|
||||
a regression could hit one and not the other.
|
||||
- Assert the same for an interrupted attempt recovered at startup, which is the
|
||||
other way an attempt ends without success.
|
||||
- Cover the third case `AC-OFFICE-TASKLESS-001.2` now names explicitly: a
|
||||
summary **write** that itself fails. Today both the load error and the upsert
|
||||
error are swallowed as a log warning after the run's completion has already
|
||||
committed, so a run completes carrying a stale summary with no durable trace.
|
||||
The criterion's outcome is preserve-and-complete with visibility: the run stays
|
||||
complete, the stored summary stays byte-identical, and the failure is recorded
|
||||
on the run's own event stream naming the scope that was being written. The
|
||||
public summary loader currently handles its own dependency errors internally,
|
||||
so this work order asserts the reachable upsert failure; the load-error branch
|
||||
remains defensive code and is not claimed as independently tested. Emitting
|
||||
that run event is the one production change this work order carries; it adds
|
||||
no rollback and changes no terminal state.
|
||||
- Assert that a run-owned lifecycle event reaching the task-side consumers
|
||||
produces no task-session read or write and no workflow transition. Prefer an
|
||||
assertion on the observable effect (no task session mutated, no workflow
|
||||
event published) over asserting the internal branch taken.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Any rollback or retry of the failed summary write, or any change to the run's
|
||||
terminal state because of it. The criterion says the completed run stays
|
||||
complete.
|
||||
- Adding an explicit owner-kind guard to the task consumers. The rule is being
|
||||
pinned, not reimplemented; if the test shows the structural guard is
|
||||
insufficient, that is a finding against `REQ-OFFICE-TASKLESS-001`.
|
||||
- Continuation scope *selection*, already covered by
|
||||
`continuation_summary_reader_test.go`.
|
||||
- Usage deduplication, already covered by `TestTasklessUsageDuplicate`.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/office/service/ -run 'Continuation|Taskless' -v -count=1
|
||||
make lint
|
||||
```
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/office/service/event_subscribers.go` —
|
||||
`handleTasklessAgentCompleted` and the summary refresh it gates
|
||||
- `apps/backend/internal/office/service/continuation_summary_reader_test.go`
|
||||
- `apps/backend/internal/office/service/taskless_lifecycle_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Asserting a summary that was never written.** Seed the successful summary
|
||||
through the real writer, not by hand: a test that hand-writes the row proves
|
||||
the reader works, not that the failure path left it alone. The existing
|
||||
`office/wakeup/routine_e2e_test.go` is the cautionary example.
|
||||
+259
@@ -0,0 +1,259 @@
|
||||
---
|
||||
id: "07-settle-terminal-attempt-before-relaunch"
|
||||
title: "Settle a claimed run from its terminal attempt instead of relaunching"
|
||||
status: pending
|
||||
wave: 1
|
||||
depends_on:
|
||||
- "04-fail-closed-on-recovery-and-stop-errors"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-TASKLESS-001.6
|
||||
system_design:
|
||||
- ../../specs/office/system-design/taskless-run-sessions.md
|
||||
---
|
||||
|
||||
# Task 07: Settle a claimed run from its terminal attempt instead of relaunching
|
||||
|
||||
> **PENDING FOLLOW-UP — 2026-09-19.** This work order is not part of the
|
||||
> current coverage change. It settles a claimed run from an already-terminal
|
||||
> attempt for the still-outstanding `AC-OFFICE-TASKLESS-001.6` requirement and
|
||||
> carries `.8`'s replay clause. `.8` remains in scope and is satisfied
|
||||
> structurally for the current launch path.
|
||||
>
|
||||
> The file is kept, unedited below this banner, because a follow-up should start
|
||||
> from it rather than re-derive it. The current gaps and the two problems a
|
||||
> follow-up must resolve first are recorded under
|
||||
> [Outstanding: stop controls and restart recovery](../../specs/office/requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
> References below to `taskless-run-recovery.md` point at a design file superseded in the same
|
||||
> change; its content is summarized in that section.
|
||||
|
||||
## Summary
|
||||
|
||||
Taskless completion persists the attempt terminal and completes the run in two
|
||||
separate writes: `finishRunSession(..., finished, ...)` then `FinishRun`
|
||||
(`apps/backend/internal/office/service/event_subscribers.go`). Anything that
|
||||
interrupts between them — a crash, or a `FinishRun` that simply errors — leaves
|
||||
a `finished` attempt whose run is still `claimed`.
|
||||
|
||||
Nothing settles that run.
|
||||
`ListUnfinishedRunSessions` selects `state IN ('preparing','running')`, so the
|
||||
attempt is not in the reconciliation input, and `ReconcileRunSessions` consumes
|
||||
no other listing. The run instead ages into the generic stale-claim sweep
|
||||
(`RecoverStale` in `apps/backend/internal/runs/repository/sqlite/runs.go`, an
|
||||
unconditional `UPDATE runs SET status='queued' WHERE status='claimed' AND
|
||||
claimed_at < ?` driven at `staleClaimedRunAge = 30 * time.Minute`), which is
|
||||
blind to run-session state. Thirty minutes later the run is requeued and a fresh
|
||||
attempt launches beside an attempt that already completed successfully: a
|
||||
duplicate agent execution, charged again, for one wake.
|
||||
|
||||
That is the outcome `AC-OFFICE-TASKLESS-001.6` forbids — interrupted work with
|
||||
no explicit outcome — and it defeats `.4`'s "shall not complete, charge twice"
|
||||
by repeating the wake rather than by replaying its events.
|
||||
|
||||
The system design closes it in two places under `## Cancellation and recovery`,
|
||||
because two channels can notice first and only one of them requires a restart.
|
||||
This work order implements both, plus the deterministic ordering the same
|
||||
section now requires.
|
||||
|
||||
## In scope
|
||||
|
||||
**1. Startup reconciliation takes a second population, and decides per run.**
|
||||
`ReconcileRunSessions` must also inspect attempts already in a terminal state
|
||||
whose run is still claimed. The runtime is not consulted for these: the attempt
|
||||
already holds a durable outcome. Add the repository query this needs; do not
|
||||
widen `ListUnfinishedRunSessions` itself, which has a caller and a meaning of
|
||||
its own.
|
||||
|
||||
The pass then runs in the two phases the system design states, and the second
|
||||
one is the part that is easy to get wrong. **Do not act per attempt row.** A
|
||||
provider fallback routinely leaves several terminal attempts on one run, so an
|
||||
oldest-first per-row rule requeues a run from an old failure while a later
|
||||
attempt is live or has already succeeded. Implement it as:
|
||||
|
||||
- **Phase 1, per attempt, unchanged.** Reconcile every `preparing`/`running`
|
||||
attempt against runtime evidence as today. An attempt that reattached on proof
|
||||
stays live; every other one is left terminal. Either way phase 1 decides no
|
||||
run: the requeue that an interrupted predecessor calls for moves to phase 2,
|
||||
so it happens once per run rather than once per attempt.
|
||||
- **Phase 2, per run.** Group the combined input by `run_id` and decide each
|
||||
still-claimed run once, from the complete set of its attempt rows:
|
||||
1. any attempt still `preparing`/`running` → skip the run, no error;
|
||||
2. else any `finished` attempt → complete the run as processed, settled from
|
||||
the **highest-numbered `finished`** attempt (more than one `finished`:
|
||||
highest settles, record the duplicate as an error on the run);
|
||||
3. else → requeue the run to release its claim, and decide nothing else.
|
||||
|
||||
Step 2 wins over step 3 by outcome, not by attempt number: one `finished`
|
||||
attempt beats any number of higher-numbered failures. Step 3 must **not** apply
|
||||
failure or retry policy of its own — the attempt recorded its own outcome when
|
||||
it failed, and the scheduler's admission gates decide the replacement. This is
|
||||
what keeps an attempt a workspace pause deliberately stopped
|
||||
(`cancelled`/`interrupted`, `cancel_requested_at` set, `runs.status` untouched)
|
||||
out of the retry pipeline.
|
||||
|
||||
**1b. Settlement side effects are named, not inferred.** Completing a run from
|
||||
the record writes four of the five effects `handleTasklessAgentCompleted`
|
||||
performs: the run's `"complete"` event, `refreshContinuationSummary`,
|
||||
`releaseTaskCheckoutForRun` and `stampRunFinished`. Each needs only the run row
|
||||
and the agent profile ID, both of which the record holds. **The continuation
|
||||
summary is not optional** — the crash happens before the normal path wrote it,
|
||||
and `AC-OFFICE-TASKLESS-001.2` requires it to carry context to the next fire.
|
||||
`recordRunOutputSummary` is **excluded**: it resolves the final agent message
|
||||
from `AgentLifecycleData.TurnID`, or from `SessionID`, which is empty for a run
|
||||
owner by design, so there is no startup path to it. A settled run completes with
|
||||
an empty `output_summary`; that is the stated outcome, not something to
|
||||
reconstruct. The reuse named under `## Reference` performs only
|
||||
`FinishRunSession` + `FinishRun`, so this work order must extend it or introduce
|
||||
a shared completion helper that both it and `handleTasklessAgentCompleted` call.
|
||||
Do not copy the five-call block into a second place.
|
||||
|
||||
**2. Reservation refuses to allocate beside a `finished` attempt.**
|
||||
Before allocating the next attempt number, ask the same question phase 2 step 2
|
||||
asks: does the run already carry an attempt in `finished`? If so the run's work
|
||||
is done — refuse the allocation, settle the run from that attempt exactly as (1)
|
||||
does, and launch nothing. A run whose attempts are all terminal with no
|
||||
`finished` among them is an ordinary retry and allocates normally. This is the
|
||||
half that holds when the completion write failed without the process dying and
|
||||
no restart is coming.
|
||||
|
||||
**2b. The two refusals are distinguishable at the caller.** Reservation now
|
||||
refuses for two unrelated reasons and `launchAgent` must handle them oppositely:
|
||||
|
||||
- **`(run_id, attempt)` conflict** (a rival caller won the row) — the loser
|
||||
fails its own launch and the run stays in the existing failure and retry
|
||||
policy. Unchanged behavior.
|
||||
- **Settlement refusal** (the work is already done) — abandon the launch with
|
||||
**no** launch failure recorded against the run, no retry scheduled, no attempt
|
||||
row created, and the run left complete rather than claimed.
|
||||
|
||||
Return these as two distinguishable outcomes, not one error value.
|
||||
|
||||
A settlement whose own write fails differs by channel. On the startup pass it
|
||||
fails startup, exactly as Task 04 makes a failed requeue fail startup. On the
|
||||
reservation path there is no startup to fail: return the error, abandon the
|
||||
launch, leave the run claimed, and record the failure on the run's event stream.
|
||||
**Never fall through to allocating an attempt after a failed settlement** — that
|
||||
is the duplicate execution this work order exists to prevent. The next pass of
|
||||
either channel settles it; both are idempotent.
|
||||
|
||||
**3. Deterministic ordering.** `ListUnfinishedRunSessions` and
|
||||
`ListLiveRunSessionsForWorkspace` both order by `created_at ASC` with no
|
||||
secondary column, so two attempts reserved inside one clock tick are returned in
|
||||
an arbitrary order. Order by `created_at, run_id, attempt` ascending. `(run_id,
|
||||
attempt)` is unique, so the three together are a total order. This is
|
||||
load-bearing, not tidiness: the reconciliation pass returns on the first error
|
||||
it cannot resolve, so which attempts it reached before stopping must not vary
|
||||
between two runs over the same rows.
|
||||
|
||||
**4. Tests**, in the file Task 02 creates:
|
||||
|
||||
- A run claimed behind a `finished` attempt is settled by the startup pass: the
|
||||
run reaches its terminal state, and **no second attempt row is created and no
|
||||
execution is launched**. The negative half is the assertion that matters.
|
||||
- The same run settled through reservation with no restart in between.
|
||||
- Both reaching the same run: the second settlement is a no-op and its side
|
||||
effects do not run a second time.
|
||||
- A run claimed behind a `failed` / `interrupted` / `cancelled` attempt is
|
||||
requeued and not completed — this change must not turn every terminal attempt
|
||||
into a completion — and the settlement itself records no failure and schedules
|
||||
no retry beyond releasing the claim.
|
||||
- **Mixed terminal outcomes, both orders.** A run with attempt 1 `finished` and
|
||||
attempt 2 `failed` completes; a run with attempt 1 `failed` and attempt 2
|
||||
`finished` completes. Both must reach the same outcome, and neither may
|
||||
relaunch. This is the case a per-attempt-row rule gets wrong, so assert the
|
||||
attempt count as well as the run status.
|
||||
- **A live successor blocks settlement.** A run with attempt 1 `failed` and
|
||||
attempt 2 `running` is skipped without an error: the run stays claimed, no
|
||||
requeue is written, and attempt 2 is untouched.
|
||||
- **A deliberately stopped attempt is not retried by the settlement.** An
|
||||
attempt finished `cancelled` with `cancel_requested_at` set, on a still-claimed
|
||||
run, is requeued to release the claim, with no failure recorded against the run
|
||||
by the settlement itself.
|
||||
- **The four settlement side effects run, and the fifth does not.** A run settled
|
||||
from a `finished` attempt has its continuation summary written (the assertion
|
||||
that matters — `AC-OFFICE-TASKLESS-001.2` depends on it), its checkout
|
||||
released, its finished stamp applied, and a `"complete"` run event recorded,
|
||||
and its `output_summary` is empty.
|
||||
- **Reservation's two refusals are distinguishable.** A settlement refusal
|
||||
abandons the launch with no launch failure recorded on the run; a
|
||||
`(run_id, attempt)` conflict still fails the launch the way it does today.
|
||||
- **A failed settlement never allocates.** With the settlement write forced to
|
||||
fail on the reservation path, no attempt row is created, the run stays claimed,
|
||||
and the error reaches the caller.
|
||||
- `AC-OFFICE-TASKLESS-001.8`'s replay clause: a historical taskless run already
|
||||
recorded as failed stays history. Neither the startup pass, nor the reservation
|
||||
guard, nor the stale-claim sweep may turn it back into a launch. This clause
|
||||
currently has no test at all.
|
||||
- Ordering determinism: two attempts sharing a `created_at` are returned in
|
||||
`(run_id, attempt)` order across repeated reads.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Making completion a single transaction. The two writes cross a repository
|
||||
boundary and an event-bus publish; collapsing them is a larger change than this
|
||||
criterion needs, and the settlement rule above makes the window recoverable
|
||||
rather than invisible, which is what AC .6 asks for.
|
||||
- Changing `RecoverStale` or `staleClaimedRunAge`. It is a generic run-level
|
||||
safety net shared with task-bound runs; a run-session-aware predicate there
|
||||
would couple the runs repository to Office storage. This work order removes the
|
||||
reason the sweep ever sees this shape, rather than teaching it about it.
|
||||
- A periodic run-session sweep. Named out of scope in the system design, for
|
||||
parity with task sessions.
|
||||
|
||||
## Validation
|
||||
|
||||
```text
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp/ -run 'TestOfficeRunSessionLauncher|TestReconcile' -v -count=1
|
||||
go test ./internal/office/repository/sqlite/ -run 'TestRunSession' -v -count=1
|
||||
go test ./internal/office/service/ -run 'Taskless' -v -count=1
|
||||
make lint
|
||||
```
|
||||
|
||||
Run the repository tests against the Postgres harness as well: this adds a query
|
||||
and changes two `ORDER BY` clauses, which is dialect-sensitive surface.
|
||||
|
||||
## Reference
|
||||
|
||||
- `apps/backend/internal/backendapp/office_run_session_launcher.go` —
|
||||
`ReconcileRunSessions`; its `AgentStatusCompleted` branch is the nearest
|
||||
existing settlement, but it performs only `FinishRunSession` + `FinishRun`.
|
||||
Item 1b above is what it is missing: extend it, or introduce the shared
|
||||
completion helper both it and `handleTasklessAgentCompleted` call
|
||||
- `apps/backend/internal/office/repository/sqlite/run_sessions.go` —
|
||||
`ListUnfinishedRunSessions`, `ListLiveRunSessionsForWorkspace`, and the
|
||||
reservation path
|
||||
- `apps/backend/internal/office/service/event_subscribers.go` —
|
||||
`handleTasklessAgentCompleted`, the two writes that create the window
|
||||
- `apps/backend/internal/runs/repository/sqlite/runs.go` — `RecoverStale`
|
||||
- `apps/backend/internal/office/service/scheduler_integration.go` —
|
||||
`staleClaimedRunAge` and the sweep that drives it
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 04. Both change `ReconcileRunSessions`; Task 04's fail-closed handling of
|
||||
the discarded requeue results should land first so this work order's new branch
|
||||
is written against the corrected error contract rather than against the discard.
|
||||
Task 02 supplies the test file both use.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Treating every terminal attempt as a completion.** Only `finished` means the
|
||||
turn succeeded. Reading "terminal" as "done" would complete runs whose attempt
|
||||
failed and silently cancel their retry. Drive each terminal state separately.
|
||||
- **Deciding per attempt row instead of per run.** This is the failure mode the
|
||||
ordering rule invites: `created_at, run_id, attempt` ascending is oldest-first,
|
||||
so a per-row settlement reaches attempt 1 `failed` before attempt 2 `finished`
|
||||
and requeues a completed run. Phase 2 exists to prevent exactly that; a test
|
||||
that only ever builds single-attempt runs will not catch it.
|
||||
- **A settlement that is not idempotent.** Two channels can reach the same run.
|
||||
The run's terminal compare-and-set is what makes the second one a no-op; assert
|
||||
that the four side effects it does perform (`"complete"` event, continuation
|
||||
summary, checkout release, finished stamp) do not run twice, not merely that no
|
||||
error is returned. The output summary is out of scope here — it is never
|
||||
written by a settlement, so "does not run twice" is not the question for it.
|
||||
- **Asserting the settlement and forgetting the launch.** The defect is a
|
||||
duplicate execution. A test that checks the run reached a terminal state but
|
||||
never checks the attempt count would pass against the bug.
|
||||
@@ -1,5 +1,5 @@
|
||||
---
|
||||
status: draft
|
||||
status: active
|
||||
system: office
|
||||
created: 2026-09-17
|
||||
owners:
|
||||
@@ -25,7 +25,7 @@ event wake without creating a task merely to host its own execution.
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-TASKLESS-001.1:** An eligible lightweight wake with no task shall start a real agent session, deliver its assembled prompt and permitted tools, and reach a visible terminal outcome without creating any task or task session.
|
||||
- **AC-OFFICE-TASKLESS-001.2:** Every fire and retry shall use a fresh session. The existing routine-scoped or agent-scoped continuation summary shall carry context; a failed attempt shall not replace the last successful summary.
|
||||
- **AC-OFFICE-TASKLESS-001.2:** Every fire and retry shall use a fresh session. The existing routine-scoped or agent-scoped continuation summary shall carry context; a failed attempt shall not replace the last successful summary. A summary write that fails shall leave the last successful summary unchanged, shall leave the completed run complete, and shall be visible on the run.
|
||||
- **AC-OFFICE-TASKLESS-001.3:** Concrete and provider-routed profiles shall both work. Existing budget admission, capabilities, retry/backoff, coalescing and idle-skip rules shall apply. Periodic idle skipping shall not consume a manual or webhook wake.
|
||||
- **AC-OFFICE-TASKLESS-001.4:** Run history shall show the exact session, runtime outcome, actual adapter/model and attributed usage. Duplicate or delayed events from an older attempt shall not complete, charge twice, or clear a newer attempt.
|
||||
- **AC-OFFICE-TASKLESS-001.5:** Workspace pause, explicit run cancellation, agent disable/removal and workspace removal shall prevent new taskless launches and stop live taskless executions. Partial stop failures shall remain visible and retryable, including when another execution stopped successfully.
|
||||
@@ -33,11 +33,86 @@ event wake without creating a task merely to host its own execution.
|
||||
- **AC-OFFICE-TASKLESS-001.7:** A taskless session shall retain its workspace and capability boundaries. Task-specific decision tools shall reject it without a task context. Failed ownership lookup shall reject launch or access, never broaden authority.
|
||||
- **AC-OFFICE-TASKLESS-001.8:** Existing task-bound launches and their task/session lifecycle shall retain their behavior. Historical unsupported taskless failures shall remain history and shall not be automatically replayed.
|
||||
|
||||
The implementation and coverage in this change focus on `.1` to `.4`, `.7` and
|
||||
`.8`. Criteria `.5` and `.6` remain requirements and are outstanding follow-up
|
||||
work; this change does not implement their lifecycle or recovery behavior.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Interactive taskless chat, synthetic tasks, new periodic producers, changing
|
||||
coordinator cadence, and cross-fire ACP session resumption.
|
||||
|
||||
Pruning run-session records when run-history retention deletes their run is also
|
||||
out of scope. Retention removes the run and its events; the run-session record
|
||||
survives until its workspace is deleted. No criterion here reads a run session
|
||||
after its run is gone, and the cost projections join run sessions by session ID.
|
||||
A follow-up that changes this needs a retention pass over the run-session store
|
||||
and a decision on whether attributed usage must outlive the run it belongs to.
|
||||
|
||||
### Outstanding: stop controls and restart recovery
|
||||
|
||||
Two flows were in this requirement as `AC-OFFICE-TASKLESS-001.5` and
|
||||
`.6`. They remain outstanding and are not implemented in this coverage change.
|
||||
The analysis below is the brief a follow-up should start from.
|
||||
|
||||
**Required behavior.** `.5` requires workspace pause, explicit run cancellation,
|
||||
agent disable/removal and workspace removal to prevent new taskless launches and
|
||||
stop live taskless executions, with partial stop failures remaining visible and
|
||||
retryable. `.6` requires unfinished attempts to be reconciled against runtime
|
||||
evidence after a backend restart, a possibly live predecessor to block its
|
||||
replacement until stopped or proven absent, interrupted work to reach an
|
||||
explicit outcome, and no taskless attempt to remain claimed indefinitely solely
|
||||
because it has no task.
|
||||
|
||||
**Why it is not in this change.** This change proves the launch path from an
|
||||
armed trigger to a real session. Stop and recovery need a separate maintainer
|
||||
review because they cross runtime ownership, operator controls and restart
|
||||
ordering. Keeping them as outstanding criteria avoids treating the current
|
||||
gaps as an accepted product decision.
|
||||
|
||||
**What is therefore accepted as a known gap today.** Cancelling a run does not
|
||||
stop a live coordinator: run cancellation is a status write on `runs` with no
|
||||
run-session stop, and agent disable and agent removal cascade through a
|
||||
task-session terminator that never consults `office_run_sessions`, so a
|
||||
run-owned execution outlives all three. Workspace pause and workspace deletion
|
||||
do reach live run sessions. Separately, a crash between an attempt's terminal
|
||||
write and its run's terminal write leaves a terminal attempt whose run is still
|
||||
claimed; because the unfinished-session query selects `preparing` and `running`
|
||||
only, nothing settles the run from that attempt and the generic stale-claim
|
||||
sweep eventually requeues it, duplicating an execution of finished work.
|
||||
|
||||
**What a follow-up must resolve first.** Two problems are known and unsolved,
|
||||
and neither is a drafting gap:
|
||||
|
||||
1. *"Visible and retryable" has no channel for three of the four controls.*
|
||||
Retryability was defined as the next sweep of the workspace re-listing the
|
||||
still-live session. That works for workspace pause, which is swept. Run
|
||||
cancellation, agent disable and agent removal are one-shot operator events,
|
||||
and this design deliberately has no periodic run-session sweep (chosen for
|
||||
parity with task sessions), so a failed stop on those three is visible but
|
||||
retryable only at the next restart. A follow-up either adds a sweep, gives
|
||||
these controls their own retry, or narrows the criterion to say so.
|
||||
2. *Ordering between a control's own mutation and the stop.* The `runs` status
|
||||
write, the agent status write and instance deletion were never ordered
|
||||
against the list-intent-stop sequence, and it was never stated whether a
|
||||
partial stop failure commits or rolls back the control. Agent removal is the
|
||||
sharp case: if the agent row goes first, the guarantee rests on a row whose
|
||||
owner no longer exists.
|
||||
|
||||
Prior analysis worth reusing rather than re-deriving: settlement must be decided
|
||||
per run and not per attempt row, from the run's complete attempt set — skip a
|
||||
run with any live attempt, otherwise let any `finished` attempt complete it
|
||||
settling from the highest-numbered one, otherwise release the claim by
|
||||
requeuing. A settlement writes the run's completion event, refreshes the
|
||||
continuation summary, releases the task checkout and stamps the run finished,
|
||||
and cannot write the run's output summary because that needs a turn and session
|
||||
identity a run-owned execution does not carry. Two channels can notice a
|
||||
settleable run first — startup reconciliation and attempt reservation — and both
|
||||
must decide by the same predicate. `cancel_requested_at` is what distinguishes a
|
||||
deliberate stop from a crash.
|
||||
|
||||
## System design
|
||||
|
||||
[Run-owned sessions](../system-design/taskless-run-sessions.md).
|
||||
[Run-owned sessions](../system-design/taskless-run-sessions.md) covers the
|
||||
launched behavior here. Criteria `.5` and `.6` remain outstanding and need a
|
||||
separate lifecycle and recovery design pass.
|
||||
|
||||
@@ -130,7 +130,7 @@ Each trigger firing creates a routine run record (`office_routine_runs`) with `r
|
||||
|
||||
#### Heavy vs lightweight routines
|
||||
|
||||
- **Lightweight** (`task_template` empty): fire produces a taskless agent run. Continuation summary keyed by `routine:<routine_id>`. Use case: "check upstream PRs" without a trackable artifact.
|
||||
- **Lightweight** (`task_template` empty): fire produces a taskless agent run. Continuation summary keyed by `routine:<routine_id>`. Use case: "check upstream PRs" without a trackable artifact. How that run acquires a session is specified by [run-owned sessions](taskless-run-sessions.md) (`REQ-OFFICE-TASKLESS-001`): the record is an Office-owned `office_run_sessions` row, not a `task_session`. Neither a nullable `task_sessions.task_id` nor a synthetic task is permitted.
|
||||
- **Heavy** (`task_template` set): fire creates a task in the system `routine` workflow (one hidden `in_progress -> done` step). Its `task.created` event evaluates `auto_start_agent` and starts a task-bound run. Use case: "daily review" with trackable output.
|
||||
|
||||
#### Concurrency policy
|
||||
@@ -362,7 +362,7 @@ LIMIT 1
|
||||
|
||||
Each wakeup produces a single agent session that runs to completion and exits. The agent receives a structured prompt describing why it was woken.
|
||||
|
||||
**Taskless runs always start a fresh session.** A defensive `taskID==""` short-circuit in `HasPriorSessionForAgent` ensures we never resume across taskless fires.
|
||||
**Taskless runs always start a fresh session.** A defensive `taskID==""` short-circuit in `HasPriorSessionForAgent` ensures we never resume across taskless fires. The session record itself and its attempt numbering are specified by [run-owned sessions](taskless-run-sessions.md). Cancellation and restart reconciliation remain outstanding requirements and are not implemented by the current coverage change; the follow-up scope is recorded under [Outstanding: stop controls and restart recovery](../requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
|
||||
**Task-bound wakeups use session resume by default**: each subsequent wakeup for a `(task, agent)` pair reloads the prior ACP session via `session/load`, falling back to `session/new` on error. See `office-task-session-lifecycle` for the per-pair model.
|
||||
|
||||
|
||||
@@ -374,7 +374,10 @@ The scheduler reads all `queued` and unexpired-retry wakeup requests on boot and
|
||||
## Taskless execution implementation contract
|
||||
|
||||
The [run-session design](taskless-run-sessions.md) defines durable session
|
||||
ownership, runtime admission, process startup, routing, cancellation and restart
|
||||
reconciliation for the taskless runs described above. It completes the documented
|
||||
ownership, runtime admission, process startup and routing for the taskless runs
|
||||
described above. Stop controls and restart reconciliation remain required by
|
||||
`REQ-OFFICE-TASKLESS-001` and are recorded as outstanding follow-up work under
|
||||
[Outstanding: stop controls and restart recovery](../requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
They are not implemented by the current coverage change. The design completes the documented
|
||||
behavior without creating task records. Its exact run/session/attempt attribution
|
||||
applies to new executions; legacy agent-only attribution is compatibility only.
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
---
|
||||
status: draft
|
||||
status: current
|
||||
system: office
|
||||
requirements:
|
||||
- REQ-OFFICE-TASKLESS-001
|
||||
@@ -12,63 +12,121 @@ requirements:
|
||||
Office owns run scheduling and durable run-session records. The shared agent
|
||||
runtime owns processes and executor resources. Task services retain strict task
|
||||
ownership. Follow [ADR](../../../decisions/2026-09-17-office-taskless-run-sessions.md).
|
||||
This design completes the taskless behavior in scheduler-01/02; it does not
|
||||
replace their queue, continuation or idle-skip rules.
|
||||
This design completes the taskless launch and observation behavior in
|
||||
scheduler-01/02; it does not replace their queue, continuation or idle-skip
|
||||
rules. Operator stop controls and restart reconciliation remain requirements
|
||||
under `REQ-OFFICE-TASKLESS-001` and are not implemented by this coverage
|
||||
change. The outstanding scope is recorded under
|
||||
[Outstanding: stop controls and restart recovery](../requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
This document specifies how a taskless run acquires a session, runs and is
|
||||
observed, and records the current lifecycle gaps for that follow-up.
|
||||
|
||||
The mechanism described here has landed. The imperative voice states a durable
|
||||
contract, not a build order: each "add", "reserve" or "must" is a rule the code
|
||||
has to keep satisfying, not an outstanding task. Where a statement is about the
|
||||
current code rather than the contract, it says so.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Criteria | Sections |
|
||||
| --- | --- |
|
||||
| .1, .2, .8 | Persistence; launch flow |
|
||||
| .1, .2 | Persistence; launch flow |
|
||||
| .3 | Scheduling and routing |
|
||||
| .4 | Events and observation |
|
||||
| .5, .6 | Cancellation and recovery |
|
||||
| .5, .6 | Outstanding lifecycle and recovery work |
|
||||
| .7 | Runtime admission and security |
|
||||
| .8 | Persistence; launch flow; outstanding lifecycle scope (replay clause) |
|
||||
|
||||
All criteria belong to REQ-OFFICE-TASKLESS-001.
|
||||
|
||||
## Evidence and existing seams
|
||||
## Integration seams
|
||||
|
||||
`office/service/scheduler_integration.go:launchAgent` rejects empty task IDs;
|
||||
`office/scheduler/dispatch_routing.go:launchCandidate` independently does so.
|
||||
`agent/runtime.Runtime.Launch` currently prepares an execution; it does not
|
||||
itself start the agent process and deliver the first prompt. Lifecycle
|
||||
`ensureLaunchSessionStillActive` reads task sessions. These are all required
|
||||
integration points, not optional follow-up work.
|
||||
These seams are in place today; the rest of this document is the contract they
|
||||
must keep satisfying.
|
||||
|
||||
`office/service/event_subscribers.go` already has `handleTasklessAgentCompleted`,
|
||||
continuation summaries and run-terminal compare-and-set behavior. Its legacy
|
||||
agent-only run lookup cannot attribute new attempts safely.
|
||||
`office/pause/sweep.go` currently inventories runs but stops processes through a
|
||||
set of task IDs. Existing taskless-failure tests verify the capability gap;
|
||||
replace their unsupported-launch expectation while preserving unrelated
|
||||
failure, race and inbox guarantees.
|
||||
`office/service/scheduler_integration.go:launchAgent` branches on task presence
|
||||
and routes an empty task ID to the routing dispatcher and then to the Office
|
||||
`RunSessionLauncher`; `office/scheduler/dispatch_routing.go:launchCandidate`
|
||||
takes the same launcher for its routed candidates. `failTasklessRun` remains
|
||||
only as the fallback for an unconfigured launcher or a launch that errored, so
|
||||
an unsupported taskless launch is a configuration failure rather than a
|
||||
capability gap. Existing taskless-failure tests assert that narrower meaning and
|
||||
still cover the unrelated failure, race and inbox guarantees.
|
||||
|
||||
`agent/runtime` exposes both preparation and start: `Launch` prepares an
|
||||
execution and keeps its original semantics for task callers, while `Start`
|
||||
additionally registers the execution, starts the process and dispatches the
|
||||
first prompt for run owners. Lifecycle `ensureLaunchSessionStillActive` still
|
||||
reads task sessions and is therefore a task-owner path; run owners are admitted
|
||||
through owner admission instead and never acquire a task-session identity.
|
||||
|
||||
`office/service/event_subscribers.go` carries `handleTasklessAgentCompleted`,
|
||||
continuation summaries and run-terminal compare-and-set behavior. New taskless
|
||||
lifecycle events carry run, run-session, attempt and execution identity, and the
|
||||
legacy agent-only lookup survives only to resolve legacy events.
|
||||
`office/pause/sweep.go` inventories live Office run sessions directly through
|
||||
`ListLiveRunSessionsForWorkspace` and stops them by execution ID, independently
|
||||
of the task-ID set it uses for task-owned work.
|
||||
|
||||
## Persistence
|
||||
|
||||
Add `office_run_sessions` to the Office repository migrations, using portable
|
||||
SQLite/Postgres SQL. Proposed columns: `id`, `workspace_id`, `agent_profile_id`
|
||||
SQLite/Postgres SQL. Columns: `id`, `workspace_id`, `agent_profile_id`
|
||||
(the Office identity), `run_id`, `attempt`, `state`, `execution_id`,
|
||||
`execution_profile_id`, `adapter`, `model`, `acp_session_id`, `created_at`,
|
||||
`started_at`, `finished_at`, `cancel_requested_at`, `error_message` and a version
|
||||
for compare-and-set updates. States are preparing, running, finished, failed,
|
||||
cancelled and interrupted. Unique `(run_id, attempt)` prevents duplicate launch
|
||||
reservation. All provider attempts also have distinct session IDs; record their
|
||||
route-attempt association rather than sharing an ACP session.
|
||||
reservation. Every provider attempt has its own session ID and never shares an
|
||||
ACP session with another attempt. The route ledger `office_run_route_attempts`
|
||||
carries no session column, and no reader may assume its rows map one-to-one onto
|
||||
attempt rows: a candidate rejected before reservation leaves a route attempt
|
||||
with no session behind it. Correlating one route attempt to one session is not
|
||||
supported, and adding a column for it is out of scope here.
|
||||
|
||||
`attempt` starts at 1 and is allocated as one greater than the highest attempt
|
||||
already recorded for that run, so numbering is monotonic per run and a number is
|
||||
never reused. Reservation is an insert that ignores a conflicting
|
||||
`(run_id, attempt)` row: the caller that inserts wins, and the caller that
|
||||
conflicts loses the reservation, leaves the predecessor row untouched, and fails
|
||||
its own launch rather than retrying inline or adopting the predecessor's
|
||||
session. The run then follows the existing failure and retry policy, which
|
||||
allocates the next attempt number on its next pass. Allocation is not refused
|
||||
for a run that already carries a `finished` attempt. The settlement guard that
|
||||
must prevent a duplicate launch remains part of the outstanding `.6` recovery
|
||||
work; this coverage change does not add that guard.
|
||||
|
||||
Reserve the session and bind `runs.session_id` atomically while the run is
|
||||
claimed. Runtime registration persists `execution_id` before process startup.
|
||||
A failed bind must stop/rollback the prepared execution. Existing run history
|
||||
retention deletes associated terminal session records only after confirming no
|
||||
live resources; workspace/agent deletion performs stop-before-delete and retains
|
||||
cleanup evidence when stopping fails. Do not store JWTs or environment secrets.
|
||||
claimed. That column names the run's most recently launched attempt, not its
|
||||
first: reservation binds it only while it is empty, and each successful launch
|
||||
then re-points it at the attempt that launched. It is a pointer to the live
|
||||
attempt, not an admission control, and event resolution is not justified by it —
|
||||
the comparison it would drive never runs for a run-owned event, because such an
|
||||
execution carries no task-session identity and the event supplies no session ID
|
||||
to compare. What decides whether an event may act is the attempt row itself,
|
||||
stated under events and observation. Freezing the column on the first attempt
|
||||
would still be wrong, because run detail would then link a dead session for the
|
||||
rest of the run's life, but that is a surface consequence rather than a guard.
|
||||
`office_run_sessions` stays authoritative for history — the column names one
|
||||
attempt, never the set, so run history, costs and recovery read the session
|
||||
table. Runtime registration persists `execution_id` before process startup. A
|
||||
failed bind must stop/rollback the prepared execution. Workspace deletion
|
||||
performs stop-before-delete and retains cleanup evidence when stopping fails.
|
||||
Workspace pause does the same. The remaining operator controls — explicit run
|
||||
cancellation, agent disable and agent removal — do not reach run sessions, which
|
||||
is an open gap recorded with
|
||||
[the outstanding follow-up](../requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery). Run-history retention deletes runs and run
|
||||
events but not run-session rows, which are removed when their workspace is;
|
||||
pruning them with the run is named out of scope in the requirement rather than
|
||||
claimed as existing behavior. Do not store JWTs or environment secrets.
|
||||
No changes to task-session nullability or synthetic task creation are permitted.
|
||||
|
||||
Shared runtime inventory uses the run-session ID as `executors_running.session_id`
|
||||
and executor correlation identity, with an empty task ID and the typed owner
|
||||
snapshot in inventory metadata. This inventory key has no task-session foreign
|
||||
key. `AgentExecution.SessionID` remains empty for run owners, so task consumers
|
||||
cannot mistake the execution for a task session. Run workspaces live under
|
||||
Shared runtime inventory uses the run-session ID as
|
||||
`executors_running.session_id` and executor correlation identity, with an empty
|
||||
task ID and the typed owner snapshot in inventory metadata. This key has no
|
||||
task-session foreign key. `AgentExecution.SessionID` remains empty for run
|
||||
owners, so task consumers cannot mistake it for a task session. Run workspaces
|
||||
live under
|
||||
`office-runs/<workspace>/<run-session>` and do not carry task ownership markers.
|
||||
Normal stop persists terminal inventory. Recovery validates the owner snapshot
|
||||
and stops the recorded predecessor before allowing a retry; failed stops and
|
||||
@@ -76,23 +134,23 @@ unknown inventory retain the claim.
|
||||
|
||||
## Runtime admission and security
|
||||
|
||||
Proposed runtime APIs (not current symbols): typed `ExecutionOwner` on
|
||||
`LaunchSpec`/execution/events, with `kind=task|run`, workspace/session identity,
|
||||
and run/attempt identity for run owners; an owner admission provider; and
|
||||
`Runtime.Start` to prepare, register, start the process and dispatch the initial
|
||||
prompt. Existing `Launch` callers retain their preparation semantics. Keep all
|
||||
lifecycle imports inside `internal/agent/runtime`; backendapp only composes
|
||||
interfaces. Runtime supplies process mechanics; Office supplies admission and
|
||||
persistent record callbacks through interfaces with no reverse Office import.
|
||||
The runtime carries a typed `ExecutionOwner` on `LaunchSpec`, execution and
|
||||
events, with `kind=task|run`, workspace/session identity, and run/attempt
|
||||
identity for run owners. Admission goes through an owner admission provider, and
|
||||
`Runtime.Start` prepares, registers, starts the process and dispatches the
|
||||
initial prompt. Existing `Launch` callers retain their preparation semantics.
|
||||
Keep all lifecycle imports inside `internal/agent/runtime`; backendapp only
|
||||
composes interfaces. Runtime supplies process mechanics; Office supplies
|
||||
admission and record callbacks through interfaces with no reverse Office import.
|
||||
|
||||
Task owner admission retains current task cleanup and terminal-session checks.
|
||||
Run owner admission checks exact durable session, run claim/attempt, agent
|
||||
eligibility, workspace existence and pause/cancel state before allocation and
|
||||
again around runtime registration/start. Unknown owner, mismatched identity or
|
||||
read error fails closed. A durable cancellation mark participates in registration
|
||||
so a pause racing allocation either observes/stops the new execution or causes
|
||||
registration to roll it back. Do not implement this as an unchecked metadata flag
|
||||
that skips `GetTaskSession`.
|
||||
read error fails closed. A durable cancellation mark participates in
|
||||
registration so a pause racing allocation either observes/stops the new
|
||||
execution or causes registration to roll it back. Do not implement this as an
|
||||
unchecked metadata flag that skips `GetTaskSession`.
|
||||
|
||||
Supply resolved executor profile, workspace, environment, prompt, skills,
|
||||
permissions and Office MCP mode to the runtime. Allocate a session-specific
|
||||
@@ -108,7 +166,7 @@ execution profile but never Office identity or tool authority.
|
||||
|
||||
## Scheduling and routing
|
||||
|
||||
Introduce a proposed Office `RunSessionLauncher` interface alongside TaskStarter,
|
||||
The Office `RunSessionLauncher` interface sits alongside TaskStarter and is
|
||||
implemented through the shared runtime. Branch on task presence after normal
|
||||
queue claim, budget, idle-skip, executor and context preparation. Both the concrete
|
||||
path and `launchCandidate` use the same run-session launcher; candidate model,
|
||||
@@ -116,6 +174,31 @@ provider, flags and environment must reach it without losing the Office prompt
|
||||
or skills. Preserve route ledgers, provider classification, parking and backoff.
|
||||
No new scheduler, background heartbeat producer or default toggle is needed.
|
||||
|
||||
Fallback across provider candidates is not free of session identity. Each
|
||||
candidate enters the launcher separately and therefore reserves its own attempt
|
||||
row, so a run that falls back through three candidates leaves three attempts in
|
||||
`office_run_sessions`, not one row rewritten three times. A candidate that fails
|
||||
must reach a terminal session state and release its runtime resources before the
|
||||
next candidate is launched; no window may exist in which two candidates of the
|
||||
same run hold live executions. That rule binds the failure paths as strictly as
|
||||
the success path: a stop that fails must be observable and must not be followed
|
||||
by a terminal session state, because a terminal row with a live execution behind
|
||||
it is the forbidden window in its least visible form — every later sweep lists
|
||||
only non-terminal rows and so never sees it.
|
||||
|
||||
**The launch-failure path does not satisfy that rule today, and nothing in
|
||||
scope fixes it.** `StartRunSession` discards the error from `runtime.Stop` and
|
||||
writes the attempt terminal regardless, so a stop that fails produces exactly
|
||||
the terminal-row-over-live-execution shape named above. Whether the dispatcher
|
||||
then falls back to the next candidate is decided by `handleLaunchFailure` in
|
||||
`office/scheduler/dispatch_routing.go`, which classifies the launch error by
|
||||
string and continues when fallback is allowed. Making the failure path fail
|
||||
closed remains pending lifecycle work under `.5` and `.6`; no work order in the
|
||||
current coverage change implements it. The rule above is the target
|
||||
the code must eventually meet, stated here so the gap is a named exclusion
|
||||
rather than a silence. `runs.session_id` points at the most recently launched candidate,
|
||||
which is why run history reads the session table rather than that column.
|
||||
|
||||
## Launch flow
|
||||
|
||||
1. Claim the existing run and apply admission/idle gates.
|
||||
@@ -128,16 +211,91 @@ No new scheduler, background heartbeat producer or default toggle is needed.
|
||||
and run, updates the continuation summary, records output and stops resources.
|
||||
A failed attempt follows the existing routing/failure policy after cleanup.
|
||||
|
||||
The continuation scope AC .2 refers to is chosen once, when the run is created,
|
||||
and persisted on the run; it is not recomputed per attempt, so every attempt of
|
||||
a run reads and writes the same summary. A run whose context snapshot carries a
|
||||
routine identity is routine-scoped (`routine:<routine id>`); any other run,
|
||||
including a coordinator wake with no routine, is agent-scoped
|
||||
(`agent:<agent profile id>`). The retired `heartbeat` scope is not reintroduced.
|
||||
Only a successful attempt writes the summary, so a failed or interrupted attempt
|
||||
leaves the last successful summary in place and the next attempt resumes from it.
|
||||
|
||||
A summary write that itself fails is a third case, distinct from an attempt that
|
||||
never earned the write. By then the run is already terminal, so the failure
|
||||
cannot un-complete it and must not try: the run stays complete, the last
|
||||
successful summary stays byte-identical, and the next attempt resumes from it
|
||||
exactly as it would after a failed attempt. What the failure owes is visibility
|
||||
rather than a rollback, so it is recorded on the run's own event stream naming
|
||||
the scope being written, which is what makes a run that completed carrying a
|
||||
stale summary distinguishable from one that refreshed it. Neither a partial
|
||||
summary nor a cleared one is permitted: the write is all-or-nothing against the
|
||||
stored row. The same holds for the load that feeds it — a load that fails writes
|
||||
nothing and is reported the same way.
|
||||
|
||||
## Events and observation
|
||||
|
||||
All new runtime events carry owner kind, workspace, Office agent identity,
|
||||
execution profile identity, run, session and attempt. Office subscribers validate
|
||||
that tuple against the active record before any state transition or side effect.
|
||||
Duplicate event/usage identities are idempotent. Delayed terminal events may
|
||||
finish only their own attempt and cannot clear agent-working state for a newer
|
||||
attempt. Retain legacy resolution only for legacy events; never emit new
|
||||
agent-only events. Runtime task consumers explicitly ignore run-owned events
|
||||
before reading/writing task sessions or workflow state.
|
||||
execution profile identity, run, session and attempt. Before any state
|
||||
transition or side effect, Office subscribers resolve the event to a still
|
||||
claimed run and check the fields that pin identity: run ID, Office agent
|
||||
profile, task ID when the event carries one, the run's bound session ID when the
|
||||
event carries one, and — when the event names a run session — that the session
|
||||
belongs to that run, carries the event's attempt number, belongs to the same
|
||||
agent profile, is not in a superseded terminal state, and does not name a
|
||||
different execution. Owner kind and workspace ride along for observability and
|
||||
are deliberately not part of that check: the run-session ID already pins both,
|
||||
and re-checking a field the event supplies adds no independent evidence.
|
||||
|
||||
Two of those terms are conditional, and neither condition is a loophole. The
|
||||
bound-session comparison applies only to an event carrying a task-session
|
||||
identity; a run-owned execution carries none, so on the taskless path that term
|
||||
never runs and `runs.session_id` neither admits nor rejects an event. The
|
||||
execution comparison applies only once the attempt row has persisted an
|
||||
execution ID: between reservation and runtime registration the row has none to
|
||||
compare. Every new run-owned event carries an execution ID, so the tolerated
|
||||
case is an attempt row not yet registered, never an event that declines to
|
||||
identify itself. For a taskless attempt past registration, the terms that decide
|
||||
admission are the attempt number, the attempt's state and the execution ID.
|
||||
|
||||
Duplicate event/usage identities are idempotent. "Complete" in AC .4 means
|
||||
completing the run: a delayed terminal event from an older attempt may apply a
|
||||
terminal state to its own run session, but must not complete the run, write a
|
||||
second usage ledger row, or clear agent-working state for a newer attempt. An
|
||||
event may complete the run only if the record still admits it, and admission is
|
||||
not a stored column because it does not need to be. Two facts the record already
|
||||
holds decide it: the run must still be claimed, and the attempt row the event
|
||||
names must still be in `preparing`, `running` or `finished`. `finished` belongs
|
||||
in that set because the normal path finishes the session before it finishes the
|
||||
run.
|
||||
|
||||
That filter admits at most one attempt of a run at a time, and what keeps it to
|
||||
one is the rule in scheduling and routing above: a failed candidate must reach a
|
||||
terminal session state before its successor launches. Where that rule is not
|
||||
enforced — the launch-failure gap named there — a superseded attempt can still
|
||||
pass the filter. Closing that is pending lifecycle work, and it does not soften
|
||||
this criterion: AC .4 is unconditional, and nothing in this document permits an
|
||||
event from an older attempt to complete a run that has a live successor. A
|
||||
completion arriving from a superseded attempt is a symptom of that gap, not a
|
||||
tolerated outcome.
|
||||
|
||||
Completing the run is idempotent regardless. The run's own terminal
|
||||
compare-and-set is the duplicate-suppression guard: the first admitted
|
||||
completion to reach it wins, every later one is a no-op with its side effects
|
||||
suppressed, and the run completes exactly once. That guard is not a licence —
|
||||
an event reaches it only after passing the filter above, so it suppresses
|
||||
duplicates of an admitted completion rather than legitimising an inadmissible
|
||||
one.
|
||||
|
||||
Usage is unaffected by an attempt being superseded: each attempt's ledger row is
|
||||
keyed by its own execution ID, so every attempt persists and run totals include
|
||||
all attempts of that run, as provider fallback already relies on.
|
||||
|
||||
Retain legacy resolution only for legacy events; never emit new agent-only
|
||||
events. Runtime task consumers must not act on run-owned events before reading
|
||||
or writing task sessions or workflow state. What enforces that today is
|
||||
structural rather than explicit: a run-owned `AgentExecution` carries an empty
|
||||
`SessionID`, so a task consumer resolves no task session and falls out.
|
||||
Structural tolerance is not a contract, so this rule is pinned by test.
|
||||
|
||||
Workspace cost readers join taskless ledger rows to their durable run session;
|
||||
run totals include every attempt belonging to that run. Serialized event-bus
|
||||
@@ -145,36 +303,59 @@ usage frames have the same decoding and deduplication behavior as typed frames.
|
||||
|
||||
Use the existing Office cost ledger and run events for usage/output projections;
|
||||
do not write run-only events into task-message/session tables with task foreign
|
||||
keys. Add durable usage deduplication keyed by session plus provider event/turn
|
||||
identity. Inspect `office/service/event_subscribers.go` and `office/costs` for the
|
||||
actual write boundary. Keep bounded output/continuation semantics rather than
|
||||
keys. Durable usage deduplication is keyed by a single usage-event identity on
|
||||
the cost ledger, unique across the install wherever that identity is present.
|
||||
It is not a composite of session and turn: specifying it as one would admit the
|
||||
same provider event twice under two sessions. For run-owned usage the identity
|
||||
is synthesized from the execution ID and the prompt generation, and an execution
|
||||
ID belongs to exactly one attempt, so the key is attempt-scoped without naming
|
||||
the session. Run-owned usage has exactly one write path and it always supplies
|
||||
that identity, so the ledger's tolerance for a row without one is unreachable
|
||||
from here. Inspect `office/service/event_subscribers.go` and `office/costs` for
|
||||
the write boundary. Keep bounded output/continuation semantics rather than
|
||||
introducing a new interactive chat UI. Run-detail session links must target a
|
||||
run-owned surface, never a task-session URL for a nonexistent task.
|
||||
|
||||
## Cancellation and recovery
|
||||
## Outstanding: stop controls and restart recovery
|
||||
|
||||
Extend pause inventory with all live Office run sessions, independently of run
|
||||
queue status: a run may already be cancelled while its process is still live.
|
||||
Persist cancellation intent before stopping each execution through Runtime.Stop.
|
||||
Union this inventory with existing task cancellation, count partial failures,
|
||||
and retry only still-live resources. Apply the same seam to run cancel, agent
|
||||
removal/disable and workspace deletion. End-of-turn cleanup is idempotent.
|
||||
Operator stop controls and restart reconciliation remain required by this
|
||||
requirement, but are not implemented by this coverage change. The behavior, the
|
||||
open gaps and the analysis a follow-up should start from are recorded in the
|
||||
requirement under
|
||||
[Outstanding: stop controls and restart recovery](../requirements/taskless-run-sessions.md#outstanding-stop-controls-and-restart-recovery).
|
||||
Two consequences are load-bearing here and are stated so they are not mistaken
|
||||
for oversights: the launch-failure gap named in scheduling and routing above
|
||||
stays open, and a crash between an attempt's terminal write and its run's
|
||||
terminal write leaves a run claimed behind a terminal attempt.
|
||||
|
||||
A service-owned, joined startup reconciliation pass and existing maintenance
|
||||
cadence inspect unfinished sessions. Reattach observation only with proof that
|
||||
the runtime still owns the exact execution/session. Otherwise stop any retained
|
||||
runtime resource and mark the attempt interrupted before applying existing retry
|
||||
policy. Unknown liveness or failed stop parks recovery visibly; never start a
|
||||
replacement beside an uncertain predecessor. No cross-fire ACP resume. A crash
|
||||
between terminal session persistence and run completion is reconciled from the
|
||||
same exact-attempt record, with idempotent cost/summary side effects.
|
||||
**AC .8's replay clause stays in scope and is satisfied structurally.** A
|
||||
historical taskless run already recorded as failed must remain history and must
|
||||
not be automatically replayed. No mechanism in scope can replay one: the
|
||||
unfinished-session query selects `preparing` and `running` only, so a recorded
|
||||
failure is not in any population this design inspects, and nothing here adds a
|
||||
channel that relaunches from run-session state. The clause is therefore a
|
||||
non-regression statement over the code as it stands, verified by the existing
|
||||
task-bound and taskless lifecycle tests rather than by new recovery machinery.
|
||||
It would stop being free when the outstanding lifecycle work lands, which is why
|
||||
the follow-up owes it a case.
|
||||
|
||||
## Verification and observability
|
||||
|
||||
Log sanitized run/session/attempt/execution identities and lifecycle transitions;
|
||||
never credentials or full raw provider output. Test SQLite and the repository's
|
||||
Postgres migration harness, concrete and routed launch, real mock-agent prompt
|
||||
completion, usage dedup, cancellation at allocation/registration/start, mixed
|
||||
successful/failed stop inventories, restart with a live/absent/unknown predecessor,
|
||||
and stale completion during a successor. Keep existing task-bound tests in the
|
||||
same targeted checks. See [delivery plan](../../../plans/office-mode-repairs/plan.md).
|
||||
Log sanitized run/session/attempt/execution identities and lifecycle
|
||||
transitions; never credentials or full raw provider output. Test SQLite and the
|
||||
repository's Postgres migration harness, concrete and routed launch, real
|
||||
mock-agent prompt completion, and usage dedup. Cancellation, stop inventories
|
||||
and restart reconciliation are not listed because they belong to the pending
|
||||
follow-up. Routed launch is named separately because it has no coverage at all: a candidate
|
||||
that falls back must be shown to reserve its own attempt, carry the Office
|
||||
prompt and skills to the launcher, and leave its predecessor terminal. Keep
|
||||
existing task-bound tests in the same targeted checks.
|
||||
|
||||
The same standard governs the plan's traceability table. An entry cites the test
|
||||
that asserts the criterion, not a test in the same area or file: a criterion
|
||||
whose only evidence is a neighboring test is uncovered, and recording that is
|
||||
cheaper than discovering it in Build.
|
||||
|
||||
Delivered under [office-mode-repairs](../../../plans/office-mode-repairs/plan.md);
|
||||
remaining verification is tracked in
|
||||
[office-taskless-run-sessions](../../../plans/office-taskless-run-sessions/plan.md).
|
||||
|
||||
Reference in New Issue
Block a user