From ecd3efc01b819615e1eb3219d774dfb0e3a44cfd Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 9 Sep 2026 14:52:18 +0800 Subject: [PATCH] fix(office): stop multi-agent reviews from deadlocking by default (#3515) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .../internal/backendapp/canvas_edit_test.go | 52 +-- apps/backend/internal/common/config/config.go | 9 +- apps/backend/internal/profiles/profiles.yaml | 22 +- .../internal/profiles/profiles_test.go | 47 +++ ...office_session_identity_graduation_test.go | 71 ++++ .../backend/internal/runtimeflags/registry.go | 3 +- docs/public/configuration.md | 15 +- docs/public/operations.md | 2 +- docs/specs/office/README.md | 2 + .../session-identity-graduation.md | 338 ++++++++++++++++++ ...-quorum-decision-recording-reevaluation.md | 24 +- 11 files changed, 527 insertions(+), 58 deletions(-) create mode 100644 apps/backend/internal/runtimeflags/office_session_identity_graduation_test.go create mode 100644 docs/specs/office/requirements/session-identity-graduation.md diff --git a/apps/backend/internal/backendapp/canvas_edit_test.go b/apps/backend/internal/backendapp/canvas_edit_test.go index 8fd9604371..8a6ebb7c0a 100644 --- a/apps/backend/internal/backendapp/canvas_edit_test.go +++ b/apps/backend/internal/backendapp/canvas_edit_test.go @@ -220,32 +220,32 @@ func stringContains(value, want string) bool { func newCanvasEditTestDependencies() (*fakeCanvasEditTaskStore, *fakeCanvasEditCanvasStore, *fakeCanvasEditReleaseStore, *fakeCanvasEditArtifactStore) { return &fakeCanvasEditTaskStore{ - workspace: &models.Workspace{ - ID: "workspace-a", - DefaultAgentProfileID: stringPointer("profile-default"), - DefaultExecutorID: stringPointer("executor-default"), - }, - }, &fakeCanvasEditCanvasStore{canvas: &canvas.Canvas{ - ID: "canvas-a", - PluginInstanceID: "instance-a", - WorkspaceID: "workspace-a", - ScopeKind: canvas.ScopeWorkspace, - Status: instances.StatusActive, - ActiveReleaseID: "release-a", - ActiveReleaseStatus: instances.ValidationValid, - EffectiveGrants: []canvas.GrantProjection{{ - PermissionKind: "api_read", Resource: "tasks", ScopeCeiling: instances.ScopeWorkspace, - }}, - }}, &fakeCanvasEditReleaseStore{release: instances.Release{ - ID: "release-a", - InstanceID: "instance-a", - PackageDigest: "digest-a", - ArtifactPath: "releases/digest-a", - ValidationStatus: instances.ValidationValid, - }}, &fakeCanvasEditArtifactStore{files: map[string][]byte{ - "manifest.yaml": []byte("manifest"), - "index.html": []byte("index"), - }} + workspace: &models.Workspace{ + ID: "workspace-a", + DefaultAgentProfileID: stringPointer("profile-default"), + DefaultExecutorID: stringPointer("executor-default"), + }, + }, &fakeCanvasEditCanvasStore{canvas: &canvas.Canvas{ + ID: "canvas-a", + PluginInstanceID: "instance-a", + WorkspaceID: "workspace-a", + ScopeKind: canvas.ScopeWorkspace, + Status: instances.StatusActive, + ActiveReleaseID: "release-a", + ActiveReleaseStatus: instances.ValidationValid, + EffectiveGrants: []canvas.GrantProjection{{ + PermissionKind: "api_read", Resource: "tasks", ScopeCeiling: instances.ScopeWorkspace, + }}, + }}, &fakeCanvasEditReleaseStore{release: instances.Release{ + ID: "release-a", + InstanceID: "instance-a", + PackageDigest: "digest-a", + ArtifactPath: "releases/digest-a", + ValidationStatus: instances.ValidationValid, + }}, &fakeCanvasEditArtifactStore{files: map[string][]byte{ + "manifest.yaml": []byte("manifest"), + "index.html": []byte("index"), + }} } func stringPointer(value string) *string { return &value } diff --git a/apps/backend/internal/common/config/config.go b/apps/backend/internal/common/config/config.go index e3b352d75d..22efc38837 100644 --- a/apps/backend/internal/common/config/config.go +++ b/apps/backend/internal/common/config/config.go @@ -478,12 +478,13 @@ type FeaturesConfig struct { // OfficeSessionIdentity keys an Office task's session identity on the run's // own agent instead of the task's runner seat, and binds an agent's // decision re-evaluation to its own calling session instead of the task's - // most-recently-started session. Off in every embedded profile because it - // is an experimental, high-risk, path-scoped change to durable session - // identity and existing session rows are not migrated. A live + // most-recently-started session. On in every embedded profile; it remains a + // high-risk, path-scoped change to durable session identity. A live // (task_id, agent_profile_id) pair is guarded in-transaction on the office // session creation path, not by a table-level constraint, and pre-existing - // duplicate rows are deliberately retained and resolved by selection. + // duplicate rows stay safe by selection, not migration. The toggle remains a + // kill switch that restores runner-seat binding and task-active-session + // decision re-evaluation when disabled. OfficeSessionIdentity bool `mapstructure:"office_session_identity" json:"officeSessionIdentity"` } diff --git a/apps/backend/internal/profiles/profiles.yaml b/apps/backend/internal/profiles/profiles.yaml index 8d9df8a502..f3873f3e28 100644 --- a/apps/backend/internal/profiles/profiles.yaml +++ b/apps/backend/internal/profiles/profiles.yaml @@ -92,17 +92,19 @@ features: dev: "false" e2e: "false" - # High-risk Office per-agent session identity experiment: keys an Office - # task's session on the run's own agent instead of the runner seat. Off in - # every profile because it is an experimental, high-risk, path-scoped change - # to durable session identity and existing session rows are not migrated. A - # live (task_id, agent_profile_id) pair is guarded in-transaction on the - # office session creation path instead of by a table-level constraint. - # Pre-existing duplicate rows are retained and resolved by selection. + # High-risk Office per-agent session identity: keys an Office task's session + # on the run's own agent instead of the runner seat, and binds an agent's + # decision re-evaluation to its own calling session. Default-on in every + # profile; it remains a high-risk, path-scoped change to durable session + # identity. A live (task_id, agent_profile_id) pair is guarded in-transaction + # on the office session creation path instead of by a table-level constraint. + # Pre-existing duplicate rows stay safe by selection, not migration. Disabling + # this toggle and restarting reverts to runner-seat binding and task-active- + # session decision re-evaluation. KANDEV_FEATURES_OFFICE_SESSION_IDENTITY: - prod: "false" - dev: "false" - e2e: "false" + prod: "true" + dev: "true" + e2e: "true" # ── Mock providers ──────────────────────────────────────────────── # Swap real third-party services for in-memory fakes. Production diff --git a/apps/backend/internal/profiles/profiles_test.go b/apps/backend/internal/profiles/profiles_test.go index d10ca46277..33ff7471cf 100644 --- a/apps/backend/internal/profiles/profiles_test.go +++ b/apps/backend/internal/profiles/profiles_test.go @@ -92,6 +92,53 @@ func TestCanvasFeatureFlagIsDisabledInEveryProfile(t *testing.T) { } } +// TestOfficeSessionIdentityFeatureFlagIsEnabledInEveryProfile pins +// AC-OFFICE-IDENTITY-GRADUATION-004.1 and -004.2: the default-on release +// ships "true" for KANDEV_FEATURES_OFFICE_SESSION_IDENTITY in prod, dev and +// e2e alike, flipping together rather than only in prod. +func TestOfficeSessionIdentityFeatureFlagIsEnabledInEveryProfile(t *testing.T) { + for _, profile := range []struct { + name string + selector map[string]string + }{{ + name: "prod", + }, { + name: "dev", + selector: map[string]string{ + "KANDEV_DEBUG_DEV_MODE": "true", + }, + }, { + name: "e2e", + selector: map[string]string{ + "KANDEV_E2E_MOCK": "true", + }, + }} { + t.Run(profile.name, func(t *testing.T) { + clearProfileSelectors(t) + clearProfilesYAMLVars(t) + for key, value := range profile.selector { + t.Setenv(key, value) + } + + defaults, err := EnvironmentDefaults() + if err != nil { + t.Fatalf("EnvironmentDefaults: %v", err) + } + if got := defaults["KANDEV_FEATURES_OFFICE_SESSION_IDENTITY"]; got != "true" { + t.Fatalf("KANDEV_FEATURES_OFFICE_SESSION_IDENTITY = %q in %s, want true", got, profile.name) + } + + featureDefaults, err := FeatureFlagDefaults() + if err != nil { + t.Fatalf("FeatureFlagDefaults: %v", err) + } + if got := featureDefaults["office_session_identity"]; got != "true" { + t.Fatalf("office_session_identity = %q in %s, want true", got, profile.name) + } + }) + } +} + func TestEnvironmentDefaultsSelectsProfileWithoutMutatingEnvironment(t *testing.T) { clearProfileSelectors(t) clearProfilesYAMLVars(t) diff --git a/apps/backend/internal/runtimeflags/office_session_identity_graduation_test.go b/apps/backend/internal/runtimeflags/office_session_identity_graduation_test.go new file mode 100644 index 0000000000..a8fe7c07eb --- /dev/null +++ b/apps/backend/internal/runtimeflags/office_session_identity_graduation_test.go @@ -0,0 +1,71 @@ +package runtimeflags + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// TestOfficeSessionIdentityDoesNotClaimUniqueIndexPrecondition pins +// AC-OFFICE-IDENTITY-GRADUATION-003.7: no operator-visible description of +// features.officeSessionIdentity may state that an unshipped +// (task_id, agent_profile_id) unique index is a precondition for enabling +// it. That precondition was dropped in favor of selection-only safety +// (REQ-OFFICE-IDENTITY-GRADUATION-003); a reintroduced claim would tell +// operators to wait on work that will never ship. +func TestOfficeSessionIdentityDoesNotClaimUniqueIndexPrecondition(t *testing.T) { + def, ok := DefinitionByKey("features.officeSessionIdentity") + if !ok { + t.Fatal("features.officeSessionIdentity definition missing") + } + assertNoUniqueIndexClaim(t, "runtime flag registry RiskDescription", def.RiskDescription) + + repoRoot := officeSessionIdentityRepoRoot(t) + for _, relPath := range []struct { + path string + locator string + }{ + {path: "apps/backend/internal/profiles/profiles.yaml", locator: "KANDEV_FEATURES_OFFICE_SESSION_IDENTITY"}, + {path: "docs/public/configuration.md", locator: "features.officeSessionIdentity"}, + {path: "docs/public/operations.md", locator: "Office session identity"}, + } { + content, err := os.ReadFile(filepath.Join(repoRoot, relPath.path)) + if err != nil { + t.Fatalf("read %s: %v", relPath.path, err) + } + assertNoUniqueIndexClaimInSection(t, relPath.path, string(content), relPath.locator) + } +} + +func assertNoUniqueIndexClaim(t *testing.T, surface, content string) { + t.Helper() + if strings.Contains(strings.ToLower(content), "unique index") || + strings.Contains(strings.ToLower(content), "unique-index") { + t.Fatalf("%s still claims a unique-index precondition for features.officeSessionIdentity", surface) + } +} + +func assertNoUniqueIndexClaimInSection(t *testing.T, surface, content, locator string) { + t.Helper() + lowerContent := strings.ToLower(content) + index := strings.Index(lowerContent, strings.ToLower(locator)) + if index < 0 { + return + } + start := max(0, index-100) + end := min(len(content), index+600) + assertNoUniqueIndexClaim(t, surface, content[start:end]) +} + +func officeSessionIdentityRepoRoot(t *testing.T) string { + t.Helper() + _, sourceFile, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + // sourceFile: apps/backend/internal/runtimeflags/ -> repo root + // is four directories up. + return filepath.Clean(filepath.Join(filepath.Dir(sourceFile), "../../../..")) +} diff --git a/apps/backend/internal/runtimeflags/registry.go b/apps/backend/internal/runtimeflags/registry.go index d07188e697..987ed24b78 100644 --- a/apps/backend/internal/runtimeflags/registry.go +++ b/apps/backend/internal/runtimeflags/registry.go @@ -169,7 +169,8 @@ var registrations = []runtimeFlagRegistration{ RiskLevel: RiskHigh, RiskDescription: "Changes durable Office session identity: each participant agent gets its own session per task instead of sharing the runner's, and existing session rows are not migrated. " + "A live (task_id, agent_profile_id) pair is guarded in-transaction on the office session creation path, not by a table-level constraint; pre-existing duplicate rows are deliberately retained and resolved by selection rather than repaired. " + - "The guard relies on SQLite's process-local single-writer pool or PostgreSQL's database task-row lock. Two Kandev processes must not write the same SQLite file.", + "The guard relies on SQLite's process-local single-writer pool or PostgreSQL's database task-row lock. Two Kandev processes must not write the same SQLite file. " + + "Disabling this toggle and restarting reverts to runner-seat binding and task-active-session decision re-evaluation.", RestartRequired: true, Mutable: true, }, diff --git a/docs/public/configuration.md b/docs/public/configuration.md index 7cbf8e6d65..85e8111ffc 100644 --- a/docs/public/configuration.md +++ b/docs/public/configuration.md @@ -498,7 +498,7 @@ Copying this entire file is unnecessary and can freeze old defaults in a deploym | `features.multiTenancy` | `KANDEV_FEATURES_MULTI_TENANCY` | off | Experimental organizations above authenticated users. Requires `features.auth`; startup is refused otherwise. | | `features.dynamicAgentRouting` | `KANDEV_FEATURES_DYNAMIC_AGENT_ROUTING` | off | Experimental dynamic profiles with ordered provider-error fallback. | | `features.canvases` | `KANDEV_FEATURES_CANVASES` | off | Experimental agent-authored isolated web-app canvases for tasks and workspaces. High risk. | -| `features.officeSessionIdentity` | `KANDEV_FEATURES_OFFICE_SESSION_IDENTITY` | off | Experimental Office participant sessions. On the Office session creation path, the live `(task_id, agent_profile_id)` pair is guarded in-transaction, not by a table-level index. Pre-existing duplicate rows are retained and resolved by selection. Two Kandev processes must not write the same SQLite file. | +| `features.officeSessionIdentity` | `KANDEV_FEATURES_OFFICE_SESSION_IDENTITY` | on | Office participant sessions: each participant agent gets its own session per task. The live `(task_id, agent_profile_id)` pair is guarded in-transaction, not by a table-level index. Pre-existing duplicate rows are retained and resolved by selection. Two Kandev processes must not write the same SQLite file. | | `debug.devMode` | `KANDEV_DEBUG_DEV_MODE` | off | High-risk diagnostic endpoints and ACP frame logging. | The `KANDEV_FEATURES_*` values have no canonical YAML keys. They are selected @@ -516,12 +516,13 @@ The database can contain canvas migrations, but Kandev does not read or change canvas data while the flag is off. See [Agent-authored Canvases](canvases.md) for the experimental user workflow. -For a risky release feature, keep the flag off in the shipped profiles, enable it -only on a selected install through an admin override or explicit environment, -restart, and test it there. Promote the `prod` profile default only after the -feature is ready for everyone; keep the registry entry as a kill-switch until -the rollout is complete, then remove the live flag and move its key and -environment variable to the runtime registry's append-only retired identities. +For a risky release feature that has not graduated, keep the flag off in the +shipped profiles, enable it only on a selected install through an admin override +or explicit environment, restart, and test it there. Promote the `prod` profile +default only after the feature is ready for everyone. Keep the registry entry as +a kill-switch until the planned retirement release, then remove the live flag and +move its key and environment variable to the runtime registry's append-only +retired identities. Plugins are part of the base product and are not a runtime toggle. The source checkout's `make dev` activates the embedded development profile, which enables Office, debug surfaces, ACP logging, and a mock agent; authentication, organizations, and Claude background prompt handoff remain opt-in. Installed `run`/desktop builds select the safe production profile unless the environment explicitly opts in. E2E mock variables and routes are test-only and must never be enabled on a public deployment. diff --git a/docs/public/operations.md b/docs/public/operations.md index 6a223caac7..61bd9fc28d 100644 --- a/docs/public/operations.md +++ b/docs/public/operations.md @@ -555,7 +555,7 @@ Kandev warns when its live WebSocket connection has not recovered for three seco **Settings > System > Feature Toggles** currently exposes: - **Office mode**: experimental, medium risk, and off in the production profile by default. -- **Office session identity**: experimental, high risk, and off in every profile by default. The live `(task_id, agent_profile_id)` pair is guarded in-transaction on the office session creation path, not by a table-level index; pre-existing duplicate rows are retained and resolved by selection. Two Kandev processes must not write the same SQLite file. It gives each Office participant a separate task conversation and requires a restart. +- **Office session identity**: experimental, high risk, and on in every profile by default. The live `(task_id, agent_profile_id)` pair is guarded in-transaction on the Office session creation path, not by a table-level index; pre-existing duplicate rows are retained and resolved by selection. Two Kandev processes must not write the same SQLite file. It gives each Office participant a separate task conversation and requires a restart. Disabling the toggle restores the pre-graduation runner-seat binding and task-active-session decision re-evaluation. - **App status bar**: stable, low risk, and off in the production profile by default. Enabling it adds the desktop/tablet bar and phone Status entry after restart; disabling it again does not stop connections, metrics collection requested by other clients, or plugins. Urgent WebSocket connectivity warnings still remain visible while the feature is off. - **Claude background prompt handoff**: experimental, high risk, and off in every profile by default. Enabling it lets Claude Code accept another prompt after its foreground yields while recognized async subagent, `run_in_background` shell, or Monitor work remains active. ACP lifecycle gaps can misclassify activity or overlap prompts; use it only for controlled testing. - **Unread divider**: a per-user setting at **Settings > General > Task Actions**. It defaults off, takes effect immediately, and controls both the Slack-style **New** divider and read-cursor updates while that user's transcript view is visible. diff --git a/docs/specs/office/README.md b/docs/specs/office/README.md index 878bee967b..318d2ac4be 100644 --- a/docs/specs/office/README.md +++ b/docs/specs/office/README.md @@ -59,7 +59,9 @@ dashboard projections, and Office testing contracts. - [Office Agent Runtime — Error Handling Contract](requirements/runtime.md) - [Office Scheduler](requirements/scheduler.md) - [Office Stall Visibility](requirements/stall-visibility.md) +- [Office Session Identity Graduation](requirements/session-identity-graduation.md) - [Step Entry Sequence Execution](requirements/step-entry-sequence-execution.md) +- [Office Task Session Identity](requirements/task-session-identity.md) - [Office Tasks](requirements/tasks.md) - [Office: E2E Mock Harness for Task Sessions and Messages](requirements/testing.md) - [Office: Slack-Style Unread Divider](requirements/unread-divider.md) diff --git a/docs/specs/office/requirements/session-identity-graduation.md b/docs/specs/office/requirements/session-identity-graduation.md new file mode 100644 index 0000000000..c061d6ca83 --- /dev/null +++ b/docs/specs/office/requirements/session-identity-graduation.md @@ -0,0 +1,338 @@ +--- +status: draft +system: office +created: 2026-09-07 +owners: + - kandev +--- + +# Office Session Identity Graduation Requirements + +## Overview + +`features.officeSessionIdentity` decides whose session an Office run binds to. +With it off, a reviewer's or approver's run binds to the task's *runner seat*, so +its verdict is recorded under an identity that does not own the seats the step is +guarded on, the guard counts zero decisions, and Review reports **"Stuck — +Blocked: no active session to re-evaluate this step."** With it on, each +participant agent gets its own durable session per task and multi-agent Office +completes autonomously. The flag is therefore the difference between multi-agent +review working and deadlocking. This capability makes the on-behaviour +unconditional, then retires the identity. + +Office owns this contract because it owns the notion that a `(task, agent)` pair +names one durable conversation. The platform owns the toggle machinery +(`REQ-PLATFORM-FEATURE-TOGGLES-001`); the task system owns `task_sessions` and +every non-Office creation path. This document changes neither, and it references +the Tasks-owned re-evaluation rule below. + +## Terminology + +- **Runner seat** — the agent profile on the task as its assignee + (`AssigneeAgentProfileID`): the agent that does the task's work. +- **Participant identity** — the agent profile of the run being started, captured + by the scheduler before step and routing overrides can mutate it. A reviewer or + approver run's differs from the runner seat. +- **Pair** — a `(task_id, agent_profile_id)` tuple with non-empty + `agent_profile_id`. +- **Terminal state** — `COMPLETED`, `FAILED`, `CANCELLED`. **Live** is its + complement, not an enumerated list. +- **Active state** — `CREATED`, `STARTING`, `RUNNING`, `WAITING_FOR_INPUT`: the + set `GetActiveTaskSessionByTaskID` and decider-session resolution both use. + `IDLE` is in neither this set nor *Terminal*, so an Office session between turns + is unresolvable for re-evaluation though not terminal. +- **Default-on release** — the first release whose shipped `prod` profile value + for this flag is `"true"`. +- **Retirement release** — the release in which the live flag is removed and its + identity appended to the append-only retired-identity set. + +## Prior art + +**Both outside legs could not run**, recorded rather than faked: no `wiki-query` +vault on this runner, no `search_fsm_docs` tool. The predecessor spec +[`task-session-identity.md`](task-session-identity.md) ran the saas-kb leg on +2026-08-31 and found no vendor documenting per-agent session identity; receipts +are in the task plan. + +**In-repo prior decisions, load-bearing:** + +- [`task-session-identity.md`](task-session-identity.md) + (`REQ-OFFICE-SESSION-IDENTITY-001` … `-004`) **explicitly rejects** a unique + index on `task_sessions(task_id, agent_profile_id)`: + `AC-OFFICE-SESSION-IDENTITY-004.1` forbids enforcing the invariant "through any + table-level database constraint", `-004.5` forbids any new index, column, + backfill or repair of `task_sessions`, and it records that a previous attempt at + that index broke three shipped flows. +- `AC-OFFICE-TASKS-001.2` asserted one session per `(task, agent)` pair before + either document existed; where they could drift, `tasks.md` wins. +- **ADR 0005** made `agent_profile_id` shared between kanban and Office, so the + pair cannot be constrained at schema level: no discriminator lets an index apply + to Office rows only. + +**Divergence from the card.** The card names a `UNIQUE(task_id, +agent_profile_id)` index plus a dedup migration as the blocking precondition. It +is obsolete: both halves of the hazard were closed on 2026-09-01 by a different +mechanism. Duplicate live rows are prevented by an in-transaction guard on the +Office creation path only (`REQ-OFFICE-SESSION-IDENTITY-001`), not a constraint, +so kanban relaunch and workflow replacement keep working; stranding is closed by +the live-preferring lookup (`REQ-OFFICE-SESSION-IDENTITY-002`). The precondition +is **already satisfied, by selection rather than repair**; adding the index would +regress `AC-OFFICE-SESSION-IDENTITY-004`. + +**Session-selection contract — Tasks owns the shared rule.** +[`workflow-quorum-decision-recording-reevaluation.md`](../../tasks/requirements/workflow-quorum-decision-recording-reevaluation.md) +`AC-TASKS-QUORUM-REEVALUATION-001.1`/`-001.4` own re-evaluation session +selection for every decision surface. A decision with a validated calling +session uses that session. A decision without a calling session uses +`GetActiveTaskSessionByTaskID`: the task's most recently started session in an +active state, ordered by `started_at` descending, limit one. This document adds +Office-specific acceptance criteria and does not supersede the shared Tasks +contract. + +## Requirements + +**Release staging — how to read REQ-001 and REQ-002.** REQ-004 stages this +capability across two releases. REQ-001 and REQ-002 state the **end state**: they +bind **as of the retirement release**. In the default-on release the same outcomes +hold while the toggle is on, with the off path governed by +`AC-OFFICE-IDENTITY-GRADUATION-001.7` and `-002.7`; nothing in REQ-001 or REQ-002 +requires removing the conditional early. REQ-003 binds in both. + +**This capability's implementation delivers the default-on release only**: +`AC-OFFICE-IDENTITY-GRADUATION-004.1`, `-004.2`, `-004.3` and all of REQ-003. The +retirement criteria (`-004.5` … `-004.10`) define the successor release and are +not built here; `-004.4` forbids delivering both at once. + +### REQ-OFFICE-IDENTITY-GRADUATION-001: Per-agent session binding is unconditional + +**Intent:** An Office run binds to its own agent's session, for every +installation, with no configuration that can restore runner-seat binding. + +**User story:** As an operator running multi-agent Office review, I want each +participant's run to use that agent's own session, so a reviewer's verdict counts +against the seats it occupies instead of deadlocking the step. + +#### Acceptance criteria + +- **AC-OFFICE-IDENTITY-GRADUATION-001.1:** When a run is started for an Office + task and the run carries a participant identity, the system shall bind that + run's session to the participant identity, whatever the task's runner seat is. + As of the retirement release it shall reach that outcome with no dependence on + any profile default, environment variable, or stored installation override. +- **AC-OFFICE-IDENTITY-GRADUATION-001.2:** While a run on an Office task carries + no participant identity and the task has a runner seat, the system shall bind + the run's session to the runner seat. Removing the toggle shall not change this + case: both positions already produce it. +- **AC-OFFICE-IDENTITY-GRADUATION-001.3:** While an Office task has no runner + seat yet, the system shall bind the run's session to the participant identity + when the run carries one, and otherwise to the run's resolved execution + profile identity. While at least one of those two identities is non-empty, the + system shall not fall back to a generic workspace session. While both are + empty, the system shall prepare a session by the pre-existing per-launch path, + adding no new error for it. +- **AC-OFFICE-IDENTITY-GRADUATION-001.4:** When the task is not Office-owned, + the system shall prepare a session by the pre-existing per-launch path, + unchanged. +- **AC-OFFICE-IDENTITY-GRADUATION-001.5:** The system shall leave the Office + find-or-create contract — `REQ-OFFICE-SESSION-IDENTITY-001` through `-003`, + covering the creation guard, the live-preferring lookup and its `started_at` / + `id` tiebreak, and convergence with bounded retry under concurrency — + unchanged. This capability shall add no session-creation path, + no lookup ordering, no reuse rule, and no coordination point. +- **AC-OFFICE-IDENTITY-GRADUATION-001.6:** When two runs for the same pair are + started concurrently, the system shall behave exactly as + `AC-OFFICE-SESSION-IDENTITY-003.1` already requires. It shall not change what + happens when two callers contend for the same pair. +- **AC-OFFICE-IDENTITY-GRADUATION-001.7:** During the default-on release, while + the toggle is off and a run on an Office task carries a participant identity + that differs from the task's runner seat, the system shall bind the run's + session to the runner seat. This is the pre-graduation behaviour the kill + switch restores and the observable difference between the toggle's two + positions. It shall become unreachable at the retirement release. + +### REQ-OFFICE-IDENTITY-GRADUATION-002: Decisions re-evaluate against the decider's own session + +**Intent:** An agent's recorded decision is checked against the seats its own +calling session occupies, so a reviewer whose agent is not the runner can move +a step. + +#### Acceptance criteria + +- **AC-OFFICE-IDENTITY-GRADUATION-002.1:** When an agent records a decision + through the agent-decision tool, the system shall re-evaluate the step against + the decider's own calling session. As of the retirement release this holds with + no dependence on the toggle; during the default-on release it holds while the + toggle is on, with `AC-OFFICE-IDENTITY-GRADUATION-002.7` governing the off + path. +- **AC-OFFICE-IDENTITY-GRADUATION-002.2:** As of the retirement release, the + system shall not resolve an agent decision against the task's + most-recently-started session. That fallback shall be unreachable for agent + decisions, enforced **at the agent-decision tool boundary**: the system shall + reject a call supplying no calling session, or one whose session does not exist + or belongs to another task, and write no decision row for it, rather than + silently selecting a session. +- **AC-OFFICE-IDENTITY-GRADUATION-002.3:** **At the re-evaluation resolution + step**, once a decision has been accepted, while the supplied calling session + is unresolvable — absent, belonging to a different task, or not in an + *active state* — the system shall record the decision, skip re-evaluation, and + not reject it or error for that reason. It shall keep all three cases + defensively: `AC-OFFICE-IDENTITY-GRADUATION-002.2`'s boundary already rejects + the first two for agent decisions, and a session bound and active at that + boundary can reach this test unresolvable only by leaving an active state + first. An agent recording through its own live session is active by + construction, so this criterion never contradicts + `AC-OFFICE-IDENTITY-GRADUATION-002.5`. A repository or transport error raised + while resolving is **not** an unresolvable session: the system shall surface it + as an error rather than recording and skipping. +- **AC-OFFICE-IDENTITY-GRADUATION-002.4:** When a decision is recorded by a human + decider rather than an agent, the system shall resolve the task's active session + per `AC-TASKS-QUORUM-REEVALUATION-001.4`, unchanged. +- **AC-OFFICE-IDENTITY-GRADUATION-002.5:** Given an Office task whose reviewer + participant is a different agent from its runner seat, when that reviewer + records a decision satisfying the step's quorum, the system shall transition the + step and not report "no active session to re-evaluate this step". +- **AC-OFFICE-IDENTITY-GRADUATION-002.6:** When the step write succeeds but the + post-write re-evaluation fails, the system shall report success and retain the + recorded decision's identity, and shall not convert a re-evaluation failure into + a write failure. +- **AC-OFFICE-IDENTITY-GRADUATION-002.7:** During the default-on release, while + the toggle is off, the system shall resolve an agent decision against the + task's active session, as it did before graduation. The kill switch shall + revert session binding and decision re-evaluation together; reverting one + without the other is not conformant. +- **AC-OFFICE-IDENTITY-GRADUATION-002.8:** When two participants record decisions + concurrently on one task, the system shall behave as the workflow engine's + existing decision-write and quorum-evaluation contract already requires. It + shall not change the engine's duplicate-write handling, decision ordering, or + which caller owns a resulting transition, and adds no decision write and no new + ordering. + +### REQ-OFFICE-IDENTITY-GRADUATION-003: Pre-existing duplicate rows stay safe by default + +**Intent:** Turning identity separation on everywhere must not expose the +duplicate `(task_id, agent_profile_id)` rows already in shipped databases, nor fix +them by a mechanism that breaks non-Office flows that deliberately create +duplicates. + +#### Acceptance criteria + +- **AC-OFFICE-IDENTITY-GRADUATION-003.1:** Given a pair holding one live row and + one or more terminal rows, when an Office session is requested for that pair, + the system shall return the live row and shall not fail. +- **AC-OFFICE-IDENTITY-GRADUATION-003.2:** Given a pair holding more than one + live row written before the guard shipped, when an Office session is requested + for that pair, the system shall return exactly one of those rows and shall not + fail. The row returned shall be determined by the live-preferring order + incorporated at `AC-OFFICE-IDENTITY-GRADUATION-001.5` — `started_at` descending, + then `id` descending as a total tiebreak — so repeated requests against + unchanged data return the same row. +- **AC-OFFICE-IDENTITY-GRADUATION-003.3:** The system shall satisfy + `AC-OFFICE-IDENTITY-GRADUATION-003.1` and `.2` by selection alone: it shall run + no dedup or repair migration, and shall not insert, merge, cancel or delete any + `task_sessions` row or move a `metadata.acp_session_id` between rows. The reuse + writes `AC-OFFICE-SESSION-IDENTITY-001.9` permits on the single row a lookup + returns — `IDLE` to `RUNNING`, and execution-profile rebinding including + clearing that row's own `metadata.acp_session_id` — are preserved by + `AC-OFFICE-IDENTITY-GRADUATION-001.5` and are not forbidden here. +- **AC-OFFICE-IDENTITY-GRADUATION-003.4:** The system shall introduce no unique + index and no other table-level constraint on `task_sessions`, and no new + column on `task_sessions`, in service of this capability. This restates the + binding obligation of `AC-OFFICE-SESSION-IDENTITY-004.1` and `-004.5` where it + is most likely to be violated. +- **AC-OFFICE-IDENTITY-GRADUATION-003.5:** When a kanban task is relaunched for + an agent profile whose session for that task is still live, the system shall + create a second, distinct live session and succeed, with participant binding on + by default. +- **AC-OFFICE-IDENTITY-GRADUATION-003.6:** When a workflow replacement session is + prepared for an existing environment while the current session for the same + pair is live, the system shall create the replacement and succeed, with + participant binding on by default. +- **AC-OFFICE-IDENTITY-GRADUATION-003.7:** No operator-visible description of + this flag shall state that an unshipped `(task_id, agent_profile_id)` unique + index is a precondition for enabling it, and none shall state a default that + contradicts the shipped profile values. This obligation is to the whole set of + operator-visible surfaces describing the flag, not to an enumeration; at the + time of writing: shipped profile metadata, the runtime flag registry's risk + description, the public configuration reference, and the public operations + reference. It applies for as long as the flag remains present. + +### REQ-OFFICE-IDENTITY-GRADUATION-004: Staged graduation, then identity retirement + +**Intent:** The default flips first and keeps a working kill switch for one +release; only after that does the flag identity disappear, permanently. + +#### Acceptance criteria + +- **AC-OFFICE-IDENTITY-GRADUATION-004.1:** In the default-on release, the shipped + `prod` profile value for `KANDEV_FEATURES_OFFICE_SESSION_IDENTITY` shall be + `"true"`. +- **AC-OFFICE-IDENTITY-GRADUATION-004.2:** In the default-on release, the + shipped `dev` and `e2e` profile values shall also be `"true"`, flipping with + `prod` in the same release. +- **AC-OFFICE-IDENTITY-GRADUATION-004.3:** In the default-on release, the toggle + shall remain listed at **Settings > System > Feature Toggles**, and an + administrator who sets it off and restarts shall observe runner-seat binding + restored per `AC-OFFICE-IDENTITY-GRADUATION-001.7` **and** decision + re-evaluation restored to the task's active session per + `AC-OFFICE-IDENTITY-GRADUATION-002.7`. The kill switch shall be operable, not + merely present. +- **AC-OFFICE-IDENTITY-GRADUATION-004.4:** The system shall not remove the live + flag in the same release that first makes it default-on. The default-on + release and the retirement release shall be distinct releases. +- **AC-OFFICE-IDENTITY-GRADUATION-004.5:** In the retirement release, the exact + key `features.officeSessionIdentity` and the exact environment variable + `KANDEV_FEATURES_OFFICE_SESSION_IDENTITY` shall be present in the append-only + retired-identity set, and shall fail the generic active-reuse check. Neither + identity shall ever be bound to a different capability. +- **AC-OFFICE-IDENTITY-GRADUATION-004.6:** In the retirement release, a stored + installation override row for the retired key shall be inert: it shall produce + no active runtime state, shall not change behaviour, and shall not be deleted. +- **AC-OFFICE-IDENTITY-GRADUATION-004.7:** In the retirement release, setting the + retired environment variable shall produce no active runtime state, shall not + change behaviour, and shall not prevent startup. +- **AC-OFFICE-IDENTITY-GRADUATION-004.8:** In the retirement release, the retired + key shall be absent from the `/api/v1/features` response body, from the + runtime flag definitions the Feature Toggles page reads, and from every + shipped profile. Frontend consumers observe absence as `false` by the + fail-closed rule in `AC-PLATFORM-FEATURE-TOGGLES-001.8`; this adds no new + frontend behaviour. +- **AC-OFFICE-IDENTITY-GRADUATION-004.9:** In the retirement release, every + behaviour required by `REQ-OFFICE-IDENTITY-GRADUATION-001`, `-002` and `-003` + shall retain at least one test that does not reference the toggle, so removing + the flag's own enabled/disabled tests does not reduce behavioural coverage. +- **AC-OFFICE-IDENTITY-GRADUATION-004.10:** In the retirement release, no + startup-configuration surface shall continue to describe the retired + environment variable as an active runtime feature flag, and any audited + inventory fixture mirroring that surface shall agree with it. Retiring the + identity in the flag registry alone, while a configuration catalog still + classifies the variable as live, is not conformant. + +## Out of scope + +- **A unique index or dedup migration on `task_sessions`.** Requested by the card, + rejected per *Prior art*. The blocker is **discharged, not deferred**: nothing + left to schedule. `AC-OFFICE-IDENTITY-GRADUATION-003.4` makes the exclusion + binding, not advisory. +- **Repairing the existing duplicate pairs.** No count is load-bearing: the census + drifts while the system runs, and `-003.3` forbids repair at any count. The + card's 2026-09-06 measurements motivate this document; no criterion depends on + them. +- **Coupling `features.officeSessionIdentity` to `features.office`.** Rejected: it + keeps two identities alive forever. The end state is one flag fewer. +- **When the retirement release happens.** Only that it be later than the + default-on one (`-004.4`); bake duration and cadence belong to the release + process, not this contract. +- **Cross-process concurrency on one database file**, and **convergence of + concurrent recoveries carrying different execution profiles** (last-writer-wins). + Both inherited unchanged from `task-session-identity.md`. +- **Any new user-visible surface.** The default-on release updates existing + Feature Toggles and public configuration entries. The later retirement release + removes those entries. No new copy, no i18n work, no route gate. +- **The stale DDL in `../system-design/tasks-01.md`.** Editorial, separate + change. + +## System design + +No paired system design is required: this capability removes a conditional and +retires an identity, adding no new component, model, contract or control flow. It +preserves [`task-session-identity-01.md`](../system-design/task-session-identity-01.md). diff --git a/docs/specs/tasks/requirements/workflow-quorum-decision-recording-reevaluation.md b/docs/specs/tasks/requirements/workflow-quorum-decision-recording-reevaluation.md index 637e1aca20..a0c2c23337 100644 --- a/docs/specs/tasks/requirements/workflow-quorum-decision-recording-reevaluation.md +++ b/docs/specs/tasks/requirements/workflow-quorum-decision-recording-reevaluation.md @@ -32,19 +32,25 @@ Recording a verdict causes the current step's guarded transitions to be evaluate - **AC-TASKS-QUORUM-REEVALUATION-001.1:** WHEN a decision is recorded through any surface, THE SYSTEM SHALL re-evaluate the current step's `on_turn_complete` guarded transitions before - returning to the caller, using the task's active session as defined in AC-TASKS-QUORUM-REEVALUATION-001.4 - — including on the human HTTP path, which has no calling session of its own. + returning to the caller, using the re-evaluation session selected by + AC-TASKS-QUORUM-REEVALUATION-001.4. - **AC-TASKS-QUORUM-REEVALUATION-001.2:** WHEN that re-evaluation finds a satisfied guard, THE SYSTEM SHALL apply the transition, and the recording call SHALL still report success. - **AC-TASKS-QUORUM-REEVALUATION-001.3:** WHEN that re-evaluation fails or errors, THE SYSTEM SHALL keep the recorded decision, report the recording as successful, and surface the re-evaluation failure through the observability required by AC-TASKS-QUORUM-DIAGNOSTICS-001.2. -- **AC-TASKS-QUORUM-REEVALUATION-001.4:** THE SYSTEM SHALL resolve the re-evaluation session as - `GetActiveTaskSessionByTaskID` already defines it - (`task/repository/sqlite/session.go`): the task's most recently started - session whose state is one of `CREATED`, `STARTING`, `RUNNING`, or - `WAITING_FOR_INPUT`, ordered by `started_at` descending, limit one. A task is - "unresolvable" for this purpose exactly when that query returns no row. +- **AC-TASKS-QUORUM-REEVALUATION-001.4:** THE SYSTEM SHALL resolve the + re-evaluation session as follows: + - When the decision supplies a validated calling session, THE SYSTEM SHALL use + that calling session. + - When the decision supplies no calling session, THE SYSTEM SHALL use + `GetActiveTaskSessionByTaskID` as defined in + `task/repository/sqlite/session.go`: the task's most recently started session + whose state is one of `CREATED`, `STARTING`, `RUNNING`, or `WAITING_FOR_INPUT`, + ordered by `started_at` descending, limit one. + + A task is "unresolvable" for this purpose exactly when the selected lookup + returns no row. - **AC-TASKS-QUORUM-REEVALUATION-001.5:** WHEN no session is resolvable under AC-TASKS-QUORUM-REEVALUATION-001.4, THE SYSTEM SHALL record the decision and skip re-evaluation without erroring, matching the existing blank-session behavior in `Engine.RecordParticipantDecision`, and SHALL report @@ -235,4 +241,4 @@ Recording a verdict causes the current step's guarded transitions to be evaluate -Behavior outside this acceptance-criteria group is defined by the core quorum requirement and the other quorum capability requirements. \ No newline at end of file +Behavior outside this acceptance-criteria group is defined by the core quorum requirement and the other quorum capability requirements.