From 453df09c217d46794eea3f329004727415fb043f Mon Sep 17 00:00:00 2001 From: AirCoding Date: Mon, 1 Jun 2026 09:38:16 +0800 Subject: [PATCH] Detailed design: resolve four P1 cross-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply repairs identified by the four-model cross-review (DeepSeek, MIMO 2.5 Pro, GPT-5.5 Pro, Opus 4.8) and verify by regression: - P1-01 Worker exit codes: align overview §11 and detailed-design §8.1 with baselineV1 §8 (0 protocol-level completion / 1 uncaught exception / 2 startup or protocol error / 3 permission error / 4 parent cancelled / 5 hard timeout killed). Record that task outcomes are reported via WorkerResult.status, not exit codes. - P1-02 PromptLayerLevel enum: add "safety" to interface-contracts §16 so the enum fully covers prompt-layering-v1 §2 L0-L9 (plus system_debug applied within L9). - P1-03 EventStore.project error handling: document in detailed-design §5.3 that a project() exception rolls back the full transaction, suppresses EventBus.publish(), returns AirError{kind:"system_error"}, and triggers referential_check() on FK-off inconsistencies. - P1-04 PromptLayerLoader completeness: record in detailed-design §10.2 that PromptLayerLoader only owns L0/L1/L3/L5 while ContextAssembler composes L2/L4/L6/L7/L8/L9 from PermissionEngine, TaskSpec, SessionStore, and ToolRegistry sources; clarify runtime-role prompts. Regression confirms baselineV1, overview, and detailed-design now share identical exit code semantics, the PromptLayerLevel enum covers all ten layers, EventStore error semantics are explicit, and the PromptLayer loading responsibility split is fully documented. Co-Authored-By: Claude Opus 4.7 --- .../architecture/interface-contracts-v1.md | 21 ++--- .../architecture/system-detailed-design.md | 86 +++++++++++++++---- .../architecture/system-overview-design.md | 14 +-- 3 files changed, 86 insertions(+), 35 deletions(-) diff --git a/AirPlan/docs/architecture/interface-contracts-v1.md b/AirPlan/docs/architecture/interface-contracts-v1.md index a985963..d51176d 100644 --- a/AirPlan/docs/architecture/interface-contracts-v1.md +++ b/AirPlan/docs/architecture/interface-contracts-v1.md @@ -1019,16 +1019,17 @@ export interface ContextAssembler { } export type PromptLayerLevel = - | "runtime_invariant" - | "role" - | "project_rules" - | "task_spec" - | "architecture" - | "evidence" - | "tool_output" - | "conversation" - | "user_override" - | "system_debug" + | "runtime_invariant" // L0 + | "role" // L1 + | "safety" // L2 (added to align with prompt-layering-v1 §2) + | "project_rules" // L3 + | "architecture" // L4 + | "task_spec" // L5 + | "evidence" // L6 + | "conversation" // L7 + | "tool_output" // L8 + | "user_override" // L9 + | "system_debug" // (applied within L9 when present) export interface PromptLayer { level: PromptLayerLevel diff --git a/AirPlan/docs/architecture/system-detailed-design.md b/AirPlan/docs/architecture/system-detailed-design.md index 0d7f4c5..9a8e0cf 100644 --- a/AirPlan/docs/architecture/system-detailed-design.md +++ b/AirPlan/docs/architecture/system-detailed-design.md @@ -312,6 +312,13 @@ DatabaseManager.transaction(tx => { EventBus.publish(event) // AFTER commit (contracts §7 rule 3) ``` +**Error handling for `project()`**: If `project(event, tx)` throws an exception (e.g., due to FK-off +referential inconsistency, constraint violation, or programmer error), the entire transaction rolls +back. `EventBus.publish()` is never called. The exception propagates to the caller as an `AirError` +with `kind: "system_error"`. If the failure is due to FK-off inconsistency (e.g., referencing a +non-existent `tasks.id`), the error is logged to developer log and `SessionStore.referential_check()` +is triggered asynchronously to diagnose and repair orphaned references. + `route_text` is always derived `route.join("/")` (event-registry §2 rule 6); `route` itself is append-only (rule 5) — `EventStore` never rewrites prior route entries. @@ -564,9 +571,24 @@ class WorkerProcess ``` `spawn` starts a Bun child process, then performs the handshake (§8.2). `WorkerProcess` owns the -NDJSON pipe; stdout carries protocol only, stderr is fatal/logging (overview §11). Worker exit codes -0-5 map to outcomes (overview §11): 0 success, 1 task failed, 2 crashed, 3 protocol error, -4 cancelled, 5 permission/policy blocked. +NDJSON pipe; stdout carries protocol only, stderr is fatal/logging (baselineV1 §8). + +Worker exit codes (baselineV1 §8, authoritative): + +| Code | Meaning | +|---:|---| +| 0 | protocol-level completion (including task failed/blocked via WorkerResult) | +| 1 | uncaught exception | +| 2 | startup/protocol error | +| 3 | permission error | +| 4 | parent cancelled | +| 5 | hard timeout killed | + +**Design decision**: Task success/failure is communicated through `WorkerResult.status`, not exit +codes. Exit code 0 means the worker completed the IPC protocol correctly and returned a valid +`WorkerResult`; the actual task outcome (`completed`/`failed`/`blocked`/`cancelled`) is in the +result payload. Non-zero exit codes indicate process-level or protocol-level failures that prevent +normal result delivery. ### 8.2 WorkerProtocol and handshake @@ -776,23 +798,49 @@ class PromptLayerLoader implements PromptLayerLoader `PromptLayer.level` is the frozen `PromptLayerLevel` union (contracts §16), ordered L0-L9 per prompt-layering-v1 §2 and overview §13: -| Level enum (contracts) | L# (prompt-layering) | -|---|---| -| `runtime_invariant` | L0 Runtime invariant | -| `role` | L1 Role / agent mode | -| (safety/permission policy) | L2 Safety and permission policy | -| `project_rules` | L3 Project rules and user preferences | -| `architecture` | L4 Architecture baseline and current plan | -| `task_spec` | L5 Task specification and acceptance criteria | -| `evidence` | L6 Relevant code / artifacts / evidence | -| `conversation` | L7 Recent conversation and decision context | -| `tool_output` | L8 Tool result history / diagnostics | -| `user_override` | L9 Immediate instruction | -| `system_debug` | (system debug directive, applied within L9 when present) | +| Level enum (contracts) | L# (prompt-layering) | Loader method / source | +|---|---|---| +| `runtime_invariant` | L0 Runtime invariant | `load_runtime_invariant()` | +| `role` | L1 Role / agent mode | `load_role(role)` | +| `safety` | L2 Safety and permission policy | `ContextAssembler` internal (see below) | +| `project_rules` | L3 Project rules and user preferences | `load_project_rules(project)` | +| `architecture` | L4 Architecture baseline and current plan | `ContextAssembler` internal (see below) | +| `task_spec` | L5 Task specification and acceptance criteria | `load_task_context(spec, refs)` | +| `evidence` | L6 Relevant code / artifacts / evidence | `ContextAssembler` internal (see below) | +| `conversation` | L7 Recent conversation and decision context | `ContextAssembler` internal (see below) | +| `tool_output` | L8 Tool result history / diagnostics | `ContextAssembler` internal (see below) | +| `user_override` | L9 Immediate instruction | `ContextAssembler` internal (see below) | +| `system_debug` | (system debug directive, applied within L9 when present) | `ContextAssembler` internal | -`IMPL` note: contracts §16 enumerates 10 `PromptLayerLevel` symbols; prompt-layering L2 (safety) is -carried by the `runtime_invariant`/`role` immutable layers and a dedicated safety layer is loaded with -`immutable=true`. Higher-priority layers win on budget pressure; `immutable` layers are never dropped +**Design decision — Layer loading responsibility**: + +`PromptLayerLoader` interface (contracts §16) provides 4 methods for layers that require external +configuration or resource loading (L0, L1, L3, L5). The remaining layers are assembled by +`ContextAssembler.load_layers()` internally: + +| Layer | Source | Assembled by | +|---|---|---| +| L2 Safety | `PermissionEngine.current_profile()` + `~/.air/permissions.yaml` + project permissions | `ContextAssembler` | +| L4 Architecture | `TaskSpec.context_refs.arc_ref` → load from plan/ADR/C4 docs | `ContextAssembler` | +| L6 Evidence | `TaskSpec.context_refs.artifacts` + `EvidenceStore.list_for_task()` | `ContextAssembler` | +| L7 Conversation | `SessionStore.messages.list_by_session()` (recent N messages) | `ContextAssembler` | +| L8 Tool output | `SessionStore.tool_runs` + `command_runs` for current task | `ContextAssembler` | +| L9 Immediate | `TaskSpec.description` + `acceptance_criteria` + immediate user instruction | `ContextAssembler` | + +This design keeps `PromptLayerLoader` focused on resource-backed layers while `ContextAssembler` +owns the assembly logic for session/task-specific layers. + +**Design decision — Runtime roles (main/architecture/scheduler)**: + +`AgentType` (contracts §5) only covers worker roles (`executor`/`reviewer`/`debugger`/`compactor`/ +`experience_miner`). Runtime roles (`main`/`architecture`/`scheduler`) are not child processes and +do not use `PromptLayerLoader.load_role()`. Instead: + +- `MainAgent` loads its L1 role prompt from built-in resources directly +- `ArchitectureDesigner` loads its L1 role prompt from built-in resources directly +- `Scheduler` does not use LLM prompts (it is a pure orchestration service) + +Higher-priority layers win on budget pressure; `immutable=true` layers (L0, L1, L2) are never dropped (prompt-layering L0 `Mutable: no`). ### 10.3 CompactionPolicy diff --git a/AirPlan/docs/architecture/system-overview-design.md b/AirPlan/docs/architecture/system-overview-design.md index b726278..86adc6e 100644 --- a/AirPlan/docs/architecture/system-overview-design.md +++ b/AirPlan/docs/architecture/system-overview-design.md @@ -857,12 +857,14 @@ Worker exit codes: | Code | Meaning | |---:|---| -| 0 | success | -| 1 | task failed | -| 2 | worker crashed | -| 3 | protocol error | -| 4 | cancelled | -| 5 | permission/policy blocked | +| 0 | protocol-level completion (including task failed/blocked via WorkerResult) | +| 1 | uncaught exception | +| 2 | startup/protocol error | +| 3 | permission error | +| 4 | parent cancelled | +| 5 | hard timeout killed | + +Note: Task success/failure is communicated through `WorkerResult.status`, not exit codes. Exit code 0 means the worker completed the IPC protocol correctly; the actual task outcome is in the result payload. Workers never write SQLite directly and never perform side effects outside parent-mediated tools.