AirCoding/Opus4.7三视角审查.md

# Opus 4.7 三视角审查

> 由 Claude Opus 4.7 并行启动三个独立审查代理生成,基于约 12K 行最新文档集。
> 生成日期:2026-05-28

---

## 一、架构师视角 (Architect Perspective)

# AirCoding V1 Architecture Review

Reviewing 11 architecture documents as a senior software architect. Note: I deliberately did not invoke TaskCreate/TaskUpdate; the review is read-only and the existing task list already tracks the architecture review itself.

---

## 1. Top Critical Architectural Issues

### C1. Vocabulary split: "V1.0.0 Alpha" vs "MVP" undermines scope discipline
The two newest documents (`solution-architecture.md §14`, `runtime-semantics-v1.md §13`) use **"V1.0.0 Alpha cut line"** and explicitly include a *complete* C++ workflow plus plugin foundation. But six older-but-canonical docs still say **"MVP cut line / Post-MVP"** with narrower content:
- `scheduler-state-machine-v1.md §11` "MVP Cut Line"
- `scope-escalation-v1.md §13` "MVP Cut Line"
- `security-model-v1.md §14` "MVP Cut Line"
- `capability-trust-v1.md §14` "MVP Cut Line"
- `cross-platform-matrix-v1.md §13` "MVP Cut Line"
- `c4/module.md §12` "MVP Skeleton Module Cut"

Effect: implementers cannot tell whether "MVP" in those docs is a synonym for V1.0.0 Alpha or a smaller earlier slice. The user's stated goal ("complete C++ workflow + plugin foundation, NOT half-finished MVP") is therefore not unambiguously expressed in the binding documents.

### C2. `main-agent-state-machine.md` is out of step with the rest of V1
- Mixed Chinese/English ASCII art, undated, no V1 status header — clearly an earlier draft.
- Uses event names (`UserMessageReceived`, `PlanProduced`, `ExecutionRequested`, `ImpactAssessmentCompleted`, `UserRequirementChanged`) that do not match the canonical event names used elsewhere (`requirement.changed`, `architecture.impact.completed`, `task.created/started/completed`).
- Introduces a `DIRECT_MODE` state with "Executor-level permission" (line ~22) without specifying: write-area conflict with Scheduler-owned tasks, audit trail beyond a single mention, how it interacts with `permission_profile: "main_direct"` in `AgentRuntimeContext`, or how `/done` reconciles in-flight side effects.
- Violates `solution-architecture.md §2` precedence rule: this is a state machine doc but doesn't carry V1 framing or align to the event registry it implicitly depends on.

### C3. C++ DiagnosticParser ownership is contradicted across documents
- `baselineV1.md §20`: "DiagnosticParser: **LLM-based** compiler/linker parsing and semantic signatures."
- `c4/code-view.md §6`: `DiagnosticParser` lives inside `packages/toolchain-cpp` and is wired into `CMakeConfigurator`, `CppBuilder`, `CppTestRunner`, `CppcheckRunner`.
- `runtime-semantics-v1.md §12`: "**LLM-based interpretation belongs to runtime Debugger/Reviewer context, not hidden inside low-level toolchain code.** … `toolchain-cpp → contracts`, `runtime/debugger → llm/provider facade`."

If runtime-semantics is authoritative (and it claims to be for V1.0.0 Alpha), then baselineV1 §20 wording and code-view's placement together imply an implicit `toolchain-cpp → llm` dependency that violates the package direction in `c4/module.md §3`.

### C4. TUI ↔ Runtime transport boundary is unspecified
- `c4/module.md §2` shows TUI and runtime as separate containers; TUI must **not** query SQLite or EventBus directly.
- `c4/code-view.md §7` says TUI uses a `ProjectionClient` and a "narrow runtime UI command API."
- No document specifies whether TUI is in the same Bun process as runtime, in a child process, or remote-capable. No protocol (function call vs. NDJSON vs. pipe) is defined for `ProjectionClient.subscribe`, permission prompts, or user blocker confirmations.

This is a load-bearing decision: if TUI is in-process, the "do not import runtime private modules" rule is enforced only by convention; if out-of-process, an entire IPC contract is missing from the V1 baselines.

### C5. `PRAGMA foreign_keys = OFF` on the source-of-truth DB
`solution-architecture.md §6` mandates FK off, while:
- The same schema requires transactional `events` insert + domain projection (§6).
- `runtime-semantics-v1.md §6` adds outbox-style cross-DB compensation for `debug-records.db`, `learned-memory.db`, artifact files, and rules.
- Recovery semantics rely on consistency of `tasks`/`task_attempts`/`agents`/`workspaces`/`tool_runs`/`command_runs`.

Disabling FK is defensible for migration flexibility, but no document states the rationale or compensating invariant checks. Combined with cross-DB outbox, the correctness burden on application code is significant and currently unguarded.

### C6. ProjectionStore dependency direction conflicts with module rules
- `c4/module.md §3` Rule 6: "`ProjectionStore → EventBus` … one-way."
- `c4/code-view.md §4` (Runtime Service UML): `ProjectionStore --> SessionStore : hydrate via repositories` and `ProjectionStore --> EventBus : subscribes`.
- `c4/code-view.md §11` State Ownership: "session DB | SessionStore/EventStore | runtime services only" — but `ProjectionStore` reads through SessionStore, which leaks domain repository surface to a UI-projection component.

Not a hard cycle, but the projection layer is doing both event-sourced and DB-replay reads with no documented conflict-resolution rule when DB and live-event ordering disagree under coalescing.

---

## 2. Should-Fix Before Detailed Design

1. **Unify cut-line vocabulary.** Replace every "MVP cut line" with "V1.0.0 Alpha cut line" (or define both, with one being a strict subset of the other) in the six docs listed in C1. Without this, the "complete C++ workflow + plugin foundation" goal is not auditable.
2. **Rewrite or retire `main-agent-state-machine.md`.** Bring its event names into the registry, add a V1 header, define DIRECT_MODE write-scope rules, and align state names with `scheduler-state-machine-v1.md` (`requirement.changed` flow already covers `INTERRUPTING`).
3. **Resolve DiagnosticParser ownership.** Either:
   - keep deterministic extraction in `toolchain-cpp` and explicitly delete LLM language from `baselineV1.md §20`, or
   - introduce a `DiagnosticInterpretationService` in runtime that toolchain-cpp depends on through a contracts-only interface.
4. **Specify the TUI ↔ Runtime transport** (in-process vs. NDJSON child) and add a `ProjectionTransport` contract to `packages/contracts`. Without it, `interface-contracts-v1.md` cannot be considered frozen.
5. **Justify or rescind `foreign_keys = OFF`.** At minimum, document the integrity invariants the runtime promises to enforce in code, and add them to the release-gate test list in `cross-platform-matrix-v1.md §11`.
6. **Define the V1.0.0 Alpha status of Compactor and ExperienceMiner explicitly.** Both appear in `solution-architecture.md §5` and `c4/module.md §5` as runtime agents, but neither `solution-architecture.md §14` nor `runtime-semantics-v1.md §13` lists them as in-scope. The user's "not half-finished" criterion requires a yes/no decision.
7. **Specify ExperienceMiner trigger ownership.** `baselineV1.md §16` lists triggers; `runtime-semantics-v1.md §11` requires an "explicit promotion task." Who creates that task — Main Agent, Scheduler on `task.completed`, or a periodic service — is undefined.
8. **Doctor bootstrap loop.** `core-doctor` is itself a capability (`capability-trust-v1.md §12`). Define how Doctor's own dependencies (Bun, SQLite, basic shell) are validated before the capability lifecycle (`discovered → validated → doctor_checked → enabled`) can run.
9. **Workspace GC policy.** `<project>/.air/local/workspaces/` and the `workspaces` table both grow; only "preserve until merge/cleanup decision recorded" is stated (`scheduler-state-machine-v1.md §9`). Add an explicit cleanup transition and retention bound.
10. **Add `packages/contracts` to `baselineV1.md §4`** package list (currently only listed in §14 and `solution-architecture.md §4`). Trivial but the precedence rule makes baselineV1 a normative source for this list.

---

## 3. Acceptable Tradeoffs

- **Outbox/compensation across session.db ↔ debug-records.db ↔ learned-memory.db ↔ files** (`runtime-semantics-v1.md §6`). Full distributed transactions are out of scope; the session-DB-first-intent pattern with restart reconciliation is a reasonable V1 choice as long as recovery scanners actually exist.
- **Coalesced heartbeats (5s) and derived `command_runs.status`** (`runtime-semantics-v1.md §4–5`). Saves write amplification and a schema column; acceptable while ProjectionStore exposes the derived status uniformly.
- **No process sandbox / no plugin signing in V1** (`security-model-v1.md §14`, `capability-trust-v1.md §14`). Acceptable given Linux-tier-1 + local-only positioning, since `PermissionEngine` is on the unavoidable path.
- **Linux-first, Windows experimental** (`cross-platform-matrix-v1.md §2`). Aligns with stated product positioning; the "PathClassifier must not rely on string prefix before realpath" rule is the right minimum portability guard.
- **Workers as independent Bun processes with NDJSON IPC, exit-code mapped error classes** (`baselineV1.md §8`). Simple, debuggable, and matches the stated "Main Agent must remain idle" principle.
- **Anthropic-canonical internal format with adapter conversion at boundary** (`solution-architecture.md §9`). Correct — minimizes per-call lossy translation when staying on Anthropic.

---

## 4. Strengths

1. **Clear precedence rule** (`solution-architecture.md §2`) for resolving conflicts among baselines. Rare in young architectures and immediately useful to implementers.
2. **State ownership is mostly enumerated.** `c4/code-view.md §11` lists 9 ownership rows; `c4/module.md §4` adds component-level data ownership. This is the strongest part of the architecture.
3. **Event-driven core with explicit durable vs. ephemeral split** (`runtime-semantics-v1.md §2–3`, `baselineV1.md §7`). The "EventStore must not create scheduler tasks / permission decisions / memory promotions / doctor fixes by policy" rule (runtime-semantics §2) correctly prevents the most common event-sourced anti-pattern (policy migrating into the event log).
4. **Project-local `.air/{shared,local}` split with `project_id` decoupled from path.** Clean separation between git-shareable rules/plans and machine-local sessions/artifacts/backups.
5. **Read-before-edit token + exact `old_string` semantics** (`runtime-semantics-v1.md §9`). Operationalizes Claude Code-quality discipline as a tool contract, not a prompt instruction — this is the right level for enforcement.
6. **Scope escalation taxonomy** (`scope-escalation-v1.md §2`, 7 levels) is well-scoped and integrates cleanly with `PermissionEngine` actions and Scheduler decisions.
7. **Capability manifest model** (`capability-trust-v1.md`) leaves a credible path to signed third-party plugins post-V1 without restructuring V1 — namespacing rule + permission declaration + Doctor-owned dependency install is the right shape.
8. **Recovery semantics are concrete** (`scheduler-state-machine-v1.md §9`, `runtime-semantics-v1.md §6`) rather than handwaved — process liveness, attempt rows, agent.lost emission, workspace preservation are all explicit.

---

## 5. Overall Score: **7.4 / 10**

**Reasoning.** The architecture is materially better than typical at this stage:
- precedence rule established,
- containers/components/data stores/state ownership documented at three C4 levels,
- event-driven core with explicit durability boundary,
- security and capability models are coherent and not over-engineered,
- recovery semantics are operational, not aspirational.

It is held back from 8+ by:
- **Document drift** between the V1 layer (`solution-architecture`, `runtime-semantics`) and the older layer (six "MVP" docs + `main-agent-state-machine`). For a project whose explicit goal is "not half-finished," the wording must catch up.
- **One genuine contradiction** (DiagnosticParser ownership) that crosses package boundaries.
- **One missing decision** (TUI ↔ Runtime transport) that blocks freezing `interface-contracts-v1.md`.
- **One risk-unjustified rule** (`foreign_keys = OFF` over outbox-coordinated multi-DB writes).
- **Underspecified surface** for DIRECT_MODE, ExperienceMiner triggers, workspace GC, and Doctor self-bootstrap.

None of the issues are structural; all are tractable in a single editing pass before detailed design begins. After fixing items in §2 (especially C1–C4), I would expect this architecture to move into the 8.5+ band.

---

### Files referenced
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/solution-architecture.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/baselineV1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/c4/module.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/c4/code-view.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/runtime-semantics-v1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/scheduler-state-machine-v1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/scope-escalation-v1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/main-agent-state-machine.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/security-model-v1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/capability-trust-v1.md
- /run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/cross-platform-matrix-v1.mdagentId: ae082ba701e248209 (use SendMessage with to: 'ae082ba701e248209' to continue this agent)
<usage>total_tokens: 80627
tool_uses: 11
duration_ms: 213238</usage>
---

## 二、工程师视角 (Engineer Perspective)

# AirCoding Architecture Review — Senior Engineer Pass

## 1. Blocking Implementation Issues (must fix before compiling contracts)

### 1.1 `AirError.cause_ref` shape conflict
- `interface-contracts-v1.md` §3: `cause_ref?: EntityRef` (a `{type, id}` 2-tuple).
- `error-taxonomy-v1.md` §2: `cause_ref?: { event_id?, task_id?, agent_id?, tool_run_id?, command_run_id?, artifact_id?, diagnostic_id? }` (multi-key object).
- These are mutually incompatible. An `AirError` produced under one definition will fail validation under the other. Pick one (the multi-key form is more useful for diagnostics; the `EntityRef` form is too narrow — a single error can reference both a tool_run and a diagnostic).

### 1.2 `EntityRef.type` enum drift
- `interface-contracts-v1.md` §4: 12 entries including `"capability"` and `"provider"`.
- `event-registry-v1.md` §1: 10 entries, omits `capability` and `provider`.
- Will cause `ts(2322)` mismatches anywhere `EntityRef` is shared between event payloads and contract callers (e.g., `RequirementChangedPayload.affected_refs?: EntityRef[]`, `PermissionPromptRequestedPayload.request_ref?: EntityRef`). Single-source `EntityType` from `contracts/ids.ts`.

### 1.3 Provider adapter method-name divergence
- `interface-contracts-v1.md` §15: `list_models`, `validate_model`, `count_tokens` (snake_case).
- `provider-capability-matrix-v1.md` §7: `listModels`, `validateModel`, `countTokens` (camelCase).
- The §1 "snake_case for exported contracts" rule makes interface-contracts authoritative; capability-matrix doc must be updated or contracts will compile but adapters written from the matrix doc will silently miss the interface.

### 1.4 `JsonValue` recursive type compileability
```ts
export type JsonValue = null | boolean | number | string | JsonValue[] | { [key: string]: JsonValue }
```
This compiles in modern TS, but only when used directly. Some older `tsc`/IDE setups will hit "type is referenced before declaration" if emitted into a `.d.ts` without `noImplicitAny` discipline. Confirm `tsconfig` has `"strict": true` and verify with `tsc --emitDeclarationOnly`.

### 1.5 `JsonSchema<T = unknown> = JsonObject` is a phantom generic
`T` is never used inside the alias. `ToolDefinition<I, O>.input_schema: JsonSchema<I>` therefore provides zero compile-time linkage between schema and the inferred I/O types of `ToolExecutor<I, O>`. This is technically compileable but defeats the type contract. Either (a) accept the phantom and document it, or (b) introduce a runtime validator type pair (e.g., `JsonSchema<T> = JsonObject & { __t?: T }` or use a real schema lib like Zod with branded types). At minimum, mark `JsonSchema` as nominal-only in §2.

### 1.6 Missing `Diagnostic` interface
- C4 `code-view.md` §6 declares `DiagnosticParser.parse_compiler_output(output): Diagnostic[]` and `semantic_signature(diagnostic): string`.
- `interface-contracts-v1.md` defines no `Diagnostic` type; only `DiagnosticCreatedPayload` (event payload) and a `diagnostics` DB row exist. Workers/toolchain need a typed `Diagnostic` to flow between parser → registrar → event emit. Add `Diagnostic` (mirror of `DiagnosticRecord`) to `contracts/error.ts` or a new `diagnostic.ts`.

### 1.7 IPC envelope cannot enforce direction by type
`IpcMessage = IpcEnvelope<ControlMessage> | IpcEnvelope<WorkerResult> | …` — but `ControlMessage` is parent→worker only and `WorkerResult` is worker→parent only. There is no compile-time link between `kind`/payload-type and `direction`. A handler accepting `IpcMessage` will not narrow on direction. For V1 this risks hard-to-debug protocol bugs; consider tagged union by both `kind` AND `direction`, or document runtime guard requirements.

### 1.8 IPC handshake/lifecycle gap
`ControlMessage` covers `agent.start | cancel | pause | resume | extend_timeout`. There is no:
- worker-side `worker.ready` / `worker.hello` (version negotiation),
- explicit `worker.exiting` / `worker.exited`,
- protocol-version field at envelope level (only `agent.start.version: 1` is versioned; `cancel/pause/resume/extend_timeout` have no version).
Without a hello, mismatched parent/worker binaries will deadlock on first message. Add `IpcEnvelope.protocol_version` or a mandatory hello round-trip before `agent.start`.

---

## 2. Should-Fix During Contracts Implementation

### 2.1 Permission subject enum drift
- `permission.decision.recorded` payload: `subject: "tool" | "command" | "path" | "network" | "dependency" | "migration"`.
- `PermissionRequestContext` (contracts §13): no `subject` field at all; conveys via free-form `requested_action: string`.
- `PermissionPromptProjection.subject: string`: free-form.
Promote `PermissionSubject` to `contracts/permission.ts` and use it on all three.

### 2.2 `PermissionDecision` missing `decision_id`
The event `permission.decision.recorded.decision_id` cannot be produced unless `PermissionEngine.evaluate()` or `record()` returns it. Add `decision_id: UUID` to `PermissionDecision`.

### 2.3 `MessageRecord.role: string` too loose
DB and contracts both use `string`. Anthropic canonical roles are bounded (`user | assistant | system | tool`). Tighten to a union in contracts; keep DB column TEXT.

### 2.4 Missing enums in DB schema doc
- `task_attempts.status` — no enum given.
- `summaries.type` — no enum given.
- `evidence_refs.kind` — no enum (the contract `EvidenceRef.kind: string`).
- `artifacts.type` — listed informally in `artifact-naming-v1.md` §6 but neither db-schema nor contracts encode the canonical set.
- `diagnostics.severity` — no enum.
At minimum, document closed enums in `db-schema-v1.md` so projections/repositories can validate.

### 2.5 Internal types referenced but undefined
Used by C4 but absent from contracts:
- `RetryDecision` (RetryPlanner)
- `WorkspaceRef`, `MergeResult` (WorkspaceManager)
- `PathRiskClassification`, `CommandRiskAnalysis` (security)
- `PromptLayer`, `BudgetFitResult` (context)
- `ArchitectureImpact`, `DocumentUpdate` (architecture)
- `AgentLost` (AgentMonitor.detectLostAgents)
- `FileReadObservation` (runtime-semantics §9.1) — needed for read-before-edit token plumbing
These are runtime-internal but should still live in `packages/contracts` to avoid circular `runtime → runtime` type imports between sub-modules.

### 2.6 `EventBus.publish` synchronous vs `WorkerRuntime.emit` async
`EventBus.publish: void`, but `WorkerRuntime.emit: Promise<void>`. Workers emit through IPC, so async is correct — but document explicitly that `WorkerRuntime.emit` resolves on send-ack, not on parent-side commit, to prevent subtle ordering assumptions.

### 2.7 `TaskInsert = TaskRecord` is too strict
Forces callers to construct `retry_count: 0`, `created_at`, etc., at insert time even though they're scheduler-owned. Recommend `TaskInsert = Omit<TaskRecord, "retry_count" | "started_at" | "completed_at" | "heartbeat_at" | "worker_result_json"> & { retry_count?: number }`.

### 2.8 EventStore reject-unknown-types policy
`event-registry-v1.md` §5: "Reject unknown durable event types unless explicitly allowed by development-mode config." But `EventSchemaRegistry` is shown only in the C4 directory listing, with no contract. Add `EventSchemaRegistry` interface (`register/validate/list/getVersion`) to `contracts/event.ts`.

### 2.9 Route prefix querying performance
`events.route_text` is indexed for equality, but `EventFilter.route_prefix` requires `LIKE 'a/b/%'`. SQLite can use the index for left-anchored LIKE only with `COLLATE NOCASE` or `BINARY` plus the right query form. Add an explicit note + small migration covering this; or store `route_text` as fixed depth segments.

### 2.10 `ToolExecutor.execute()` return-type union
```ts
execute(...): AsyncIterable<ToolEvent> | Promise<ToolResultEnvelope<O>>
```
Forces every caller (and `ToolRegistry.call`) to runtime-discriminate. Simpler: split `ToolExecutor` and `StreamingToolExecutor` interfaces, with `streaming: boolean` on the definition selecting which is required. This also makes registry `register<I,O>` overloads cleaner and prevents accidentally implementing the wrong shape.

### 2.11 `command_runs` has no `status` column
Runtime derives it (`runtime-semantics-v1.md` §5). Acceptable, but `CommandRunProjection.status: "running"|"ok"|"error"|"cancelled"|"unknown"` is computed each snapshot. Add `idx_command_runs_session_completed` to make "running" filtering cheap, or accept the cost — but call this out as known.

### 2.12 `EventBus.drain?()` optional
Tests need deterministic drainage. Drop the `?` (make required) for V1, or formalize a `TestableEventBus` extension. Currently the absence of `drain` would cause flaky tests for "after publish, subscriber sees X".

### 2.13 `ProviderManager.complete` lacks task/agent linkage
`ProviderCompletionInput.metadata?: JsonObject` is the only carrier for task/agent IDs — too loose. Provider rate limiting and cost tracking per-task need first-class fields.

---

## 3. Acceptable Gaps

- **Foreign keys OFF**: documented rationale; orphan handling deferred. Acceptable for Alpha.
- **`JsonObject = Record<string, unknown>`** vs strict types in events: V1 uses TS-side validators, plan to migrate to Zod/JSON-schema generation post-MVP.
- **Phantom `JsonSchema<T>`**: only blocks if you intend type safety; mark as nominal.
- **`schema_meta` migration story** is conceptual only; deferring an actual `MigrationRunner` migration ledger table is fine for V1 if the runner refuses to open DBs at unknown `schema_version`.
- **GUI/network tools** (`gui.screenshot`, `network.capture`) requiring host tooling (xdotool, tcpdump+sudo) — Doctor checks listed; fallbacks to "blocked" are acceptable.
- **Outbox for cross-DB writes** (debug-records.db, learned-memory.db) is described in semantics §6 but no dedicated table; acceptable since each external store can carry its own pending state, but document recovery scan ownership.
- **No `worker_id` separate from `agent_id`** — using `agent_id` as the worker identity is fine.
- **`message_drafts` not source of truth**: documented; OK.

---

## 4. Strengths

1. **Clear single-writer DB architecture** with explicit "no worker → SQLite direct write" rule (§21 boundary list).
2. **Disciplined event vs domain transactionality** (`runtime-semantics-v1.md` §3) — event-sourcing avoids the typical drift between events table and domain projections.
3. **Layered prompt model L0–L9** is unusually rigorous; explicitly forbids project override of L0 invariants.
4. **`semantic_signature` from `error-taxonomy-v1.md` §6** — the volatile-data exclusion list is the right discipline for retry-loop detection.
5. **Read-before-edit token (`FileReadObservation`)** as a tool-level enforcement, not a prompt rule (`runtime-semantics-v1.md` §9.1). Best practice for execution-discipline integrity.
6. **Compaction ownership rule** ("only `summary.created` inserts a `summaries` row", §7) eliminates duplicate-row class of bug.
7. **Capability manifest v1** allows plugin foundation without runtime coupling: `CapabilityRegistry.register_tools(tool_registry)` is the only seam.
8. **Provider conversion report** (`ProviderConversionReport.omissions/warnings/required_confirmation`) makes lossy translation auditable rather than silent.
9. **TUI → ProjectionClient only** is properly enforced at the package boundary level (`code-view.md` §2).
10. **Permission decision space** (`allow|deny|ask_user|block|refuse|announce_then_run` × `none|once|session|project|global`) is well-thought-out and matches Claude-Code-style flows.
11. **Artifact naming with ULID + temp-then-rename + sha256** (`artifact-naming-v1.md` §8) — production-grade.

---

## 5. Engineering Quality Score: **7.5 / 10**

Reasoning:

- **+** Coverage breadth is high: events, errors, tools, schemas, prompts, permissions, capabilities, IPC, providers all addressed at architecture-only level.
- **+** Persistence/transaction story is consistent and recovery-aware.
- **+** Boundary rules are enforced architecturally (TUI/runtime/contracts/llm/cpp/tui dependency direction).
- **−** Several cross-document type drifts (§1.1, §1.2, §1.3) will cause `tsc` errors on first compile of `packages/contracts`. Must be reconciled before code starts.
- **−** Phantom generics (`JsonSchema<T>`) and loose `unknown`/`string` typing on hot paths reduce the usefulness of the strict TypeScript stance.
- **−** IPC missing handshake/protocol-version & direction-typing — high risk for first multi-process integration milestone.
- **−** ~10 internal types referenced in C4 are not in contracts; contracts package will be rewritten mid-implementation when Scheduler/Workspace/Permission classifiers need them.
- **−** Several enum sets (artifact.type, diagnostic.severity, evidence.kind, permission.subject, summary.type, task_attempt.status, message.role) are open `string` in DB/contracts despite being closed in practice.

Once the §1 blocking items are resolved, this jumps to ~**8.5/10**: the architecture is well-disciplined and demonstrably production-aware — the remaining gaps are typical of an alpha contract pass and are tractable during implementation.

Files referenced (all absolute):
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/interface-contracts-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/db-schema-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/event-registry-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/tool-registry-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/error-taxonomy-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/artifact-naming-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/provider-capability-matrix-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/prompt-layering-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/runtime-semantics-v1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/c4/code-view.md`agentId: a26080831ed2e7713 (use SendMessage with to: 'a26080831ed2e7713' to continue this agent)
<usage>total_tokens: 94458
tool_uses: 10
duration_ms: 224290</usage>
---

## 三、用户视角 (User Perspective)

# AirCoding Architecture Alignment Review (User Perspective)

I'm operating in pure read-only review mode; no files edited, no task tools used since this is a one-shot analysis.

## 1. Original requirements FULLY satisfied

- **Self-owned runtime, not a Claude Code plugin wrapper** — explicit in `baselineV1.md` §1, `solution-architecture.md` §1, `requirements.md` §1.
- **Claude Code-quality execution layer** — D-059, FR-009, principle 1 in solution-architecture; read-before-edit, exact small edits, verification-before-completion enshrined.
- **OpenTUI/Solid TUI reuse, not OpenCode business state** — D-002, D-048, baseline §18, solution §4 (TUI container constraints).
- **Hermes-style learning / Curator / SKILL.md** — D-024, baseline §16, runtime-semantics integration; ExperienceMiner is a first-class worker.
- **Project-local portable state** — D-049, FR-002, `.air/shared` + `.air/local` with project-id UUID, gitignore guidance.
- **Independent worker subprocesses** — D-007, D-039, FR-008, baseline §6/§8 (NDJSON over stdio, exit codes, per-worker independent loops).
- **C++ as first deep profile** — FR-017, baseline §20, plan Phase 5; runtime kept language-agnostic via `toolchain-*` (D-036, NFR-003).
- **Multi-agent set (Main, Architect, Scheduler, Executor, Reviewer, Debugger, Compactor, ExperienceMiner)** — present in §5 of solution-architecture and baseline §6. Note user's "Debugger absorbed Fixer" is honored.
- **High-permission announce-then-run, credentials/system-sensitive explicit confirmation** — D-045, baseline §11/§12, security model.
- **Anthropic canonical message storage + provider boundary conversion** — D-009, D-051, baseline §17.
- **Binary tarball distribution; defer public channels** — D-034, baseline §25.
- **Reference projects taxonomy (OpenCode/CC/Hermes/Codex/Skills/asciinema/atuin/claude-hud)** — preserved in baseline §2.

## 2. Original requirements PARTIALLY satisfied

- **VibeBox downstream branch (ARM Linux Electron appliance)** — `vibeboxbaseline.md` exists at top level but is marked historical; the canonical version is at `branchvibebox/vibeboxbaseline.md` (not read here). The main `plan.md` / `todo.md` / `requirements.md` make **no mention of VibeBox** at all. V1.0.0 Alpha plan and release gate ignore the VibeBox branch entirely. Risk: VibeBox derivation is decoupled but has no scheduling visibility in the Alpha workstream.
- **CLI/TUI experience (OpenCode-style UX)** — TUI container, ProjectionStore, HUD presets defined; but actual interaction surfaces (`/direct`/`/done` modes from idea.md §5.2, command palette, history/rewind/resume/compact UX from idea.md §3.2) are **not mentioned** in plan.md or todo.md. T-027/T-028 cover startup and permission prompts only.
- **Error handling and evidence display to user** — Evidence refs and structured WorkerResult exist; error-taxonomy-v1.md is referenced but not directly visible. The user-facing error escalation/blocker reporting flow exists structurally (T-028) but no UX detail.
- **"Goal upgraded to V1.0.0 Alpha (complete C++ + plugin foundation)"** — captured in requirements §2 and plan, but plan Phase 7 (agent prompt integration) and Phase 5 (C++ workflow) carry the entire weight; the Alpha is realistic only if Phases 4–7 ship cleanly. No contingency for partial delivery.
- **Hermes Nudge Engine N-turn mid-session triggers** — D-024 lists "N turns/tool calls interval" but plan.md/todo.md only deliver T-033 "candidate flow" — mid-session Nudge cadence is not separately scheduled.
- **Memory/skill self-patch ("agent finds rule wrong → patch")** — D-024 design exists, but no todo item explicitly schedules the self-patch path.
- **Doctor `--fix` mode with announce-then-run** — D-045/FR-018 designed; T-034 only delivers read-only Doctor; permissioned fix mode is mentioned in requirements but not scheduled as a distinct todo.

## 3. Original requirements NOT satisfied (or missing in Alpha scope)

- **Direct mode (`/direct` enter, `/done` exit) as a first-class Main Agent lane** — referenced in baseline §6 ("Direct mode is a foreground execution lane") but no contract, no IPC, no todo. User explicitly wanted this.
- **`compact` / `resume` / `history` / `rewind` user-facing commands** (idea.md §3.2) — not surfaced in CLI/TUI todo.
- **`air restore` three-granularity (file / time / session)** — D-032 designs it; plan.md and todo.md never schedule a restore command.
- **Local DebugRecord network with provider interface for sharing** — baseline §13 mentions it, but no T-* item creates `debug-records.db` schema or sharing interface in Alpha. Only T-031 (debugger loop) gets touched.
- **Curator Daemon periodic dedup/archive** — D-024 designs, todo doesn't schedule.
- **`air doctor --bundle` diagnostic export** — D-050 designs; not in todo.
- **Crash detection on next startup with prompt** — D-035 designs; not scheduled.
- **PTY/asciinema-style command capture + Atuin-style structured history search** — referenced in idea.md §3.6, dropped from Alpha entirely. Not necessarily wrong, but unacknowledged scope reduction.
- **VibeBox Alpha integration / branch tracking** — no plan-level treatment; user wanted it as a downstream branch baseline.

## 4. Architecture decisions BEYOND what user asked (over-engineering risks)

- **C4 model + 14 frozen prerequisite baseline docs** (interface-contracts-v1, runtime-semantics-v1, scheduler-state-machine-v1, prompt-layering-v1, provider-capability-matrix-v1, error-taxonomy-v1, artifact-naming-v1, scope-escalation-v1, security-model-v1, capability-trust-v1, cross-platform-matrix-v1, etc.) — user asked for a working agent, not 14 frozen v1 spec docs before any code. This is heavy formalism for an Alpha.
- **`packages/contracts` typed package as Phase 0 hard gate** — sensible engineering, but creates a serialization bottleneck (plan §"Must serialize") that can stall the whole project on contract churn.
- **Provider capability matrix + conversion report mechanism** — user only said "Anthropic + OpenAI + compatibles". Capability matrix is more elaborate than was requested.
- **L0–L9 prompt layering** — formalized to ten layers; user did not specify layering depth.
- **Five worker types as independent Bun processes including Compactor and ExperienceMiner** — user explicitly listed Compactor and ExperienceMiner as separate agents, but spawning them as full child processes (vs in-process background tasks) adds IPC/heartbeat/timeout cost for what are largely background async jobs. Justifiable but heavy.
- **Route-chain (`route: string[]`) instead of correlation/causation IDs** (D-038) — clever but a non-standard choice with no requested benefit.
- **`message_drafts` separate table for streaming intermediate state** — a fine touch, but not requested.
- **Static path whitelist plus LLM escape hatch** for high-risk detection — user said "static whitelist for system paths"; LLM escape hatch is an addition.
- **Scope-escalation v1 model as a separate frozen doc** — user already said "implementation silent / architecture confirm"; turning it into a frozen ADR-class document is over-formalized.

## 5. Architecture decisions that CONFLICT with user intent

- **None outright conflicting**, but borderline:
  - User wanted "Hermes-style learning" with Nudge Engine mid-session triggers; D-024 includes them but **plan/todo do not schedule the mid-session cadence implementation**. If shipped without it, "Hermes-style" reduces to "session-end miner".
  - User wanted Compactor as an independent subagent (idea.md §12.3 "Copy-on-Write … 独立 Compactor 子代理"). Architecture honors this — but `runtime-semantics-v1` (referenced, not read) reportedly governs "compaction ownership", which could centralize control again. Worth verifying.
  - User wanted "高权限模式 announce_then_run … 凭据/系统操作显式确认". D-045 honors this for capabilities/dependencies, but it's unclear whether the same model applies to *general* shell command execution (the security-model-v1 doc, not read here, is the source of truth). Visible parts of the security model show only path classification and `.git/` protection — the broader announce-then-run for arbitrary shell isn't visible at this level.
  - User wanted Codex-style "tool breadth" as a reference. Architecture explicitly subordinates breadth to Claude Code edit discipline (D-059 last paragraph). This is a legitimate choice the user implied, but worth flagging that breadth (image gen, vision, browser/computer use) is mostly deferred.

## 6. User-experience gaps

- **No CLI command surface design.** plan.md/todo.md show `air doctor`, `air init`, `air -- e2e ...`, `bun run air -- tui-smoke`, but there's no consolidated CLI catalog (no `air resume`, `air compact`, `air restore`, `air history`, `air session list`, `air provider list`, etc.).
- **No spec for `/direct` and `/done` modes** in TUI or Main Agent state machine artifacts referenced from todo.
- **Permission prompt UX** is a single todo (T-028) — no design for what the user sees, what defaults exist, how high-permission mode visualizes "announce_then_run", how confirmations are batched.
- **Evidence display in TUI** — ProjectionStore exposes evidence_refs, but there's no defined "user clicks/expands evidence" UX (artifact preview, log tail, screenshot inline).
- **HUD presets (Full/Essential/Minimal)** — referenced in idea.md §16 but not in todo.
- **Error messaging policy** — `error-taxonomy-v1.md` referenced for routing, but not for user-facing copy.
- **Internationalization / Chinese labels** — idea.md notes "中文 label"; nothing in plan.md.
- **Onboarding / first-run experience** — Doctor read-only is scheduled, but there is no "welcome / models.yaml setup / API key prompt" UX.

## 7. Overall alignment score

**7.5 / 10**

Reasoning:

- **Strengths (why ≥7):** The architecture genuinely captures the user's hardest asks — self-owned runtime, project-local state, independent worker processes, Claude Code execution discipline, Hermes-style miner, Anthropic-canonical storage, C++-first multi-language toolchain plan, OpenTUI without OpenCode business state. The core technical bones are well-aligned and internally consistent.
- **Why not ≥8.5:** The user-facing surface (`/direct`/`/done`, restore/resume/compact/history commands, HUD presets, Chinese labels, permission prompt UX, doctor bundle export, mid-session Nudge cadence, Curator Daemon scheduling, VibeBox branch integration) is largely undeferred-but-undelivered: the documents describe these intentions but plan/todo don't schedule them. The Alpha as currently scheduled would ship a strong runtime with a thin user-experience skin.
- **Why not <7:** The over-engineering (14 frozen v1 baseline docs, full C4, route-chain, capability matrix) is real but does not contradict user intent — it's gold-plating that risks burning time, not direction error. The requirements doc and solution-architecture are tight and traceable.

## Key files referenced

- `/run/media/airlongdian/EasyU/AirCoding/idea.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/analysis/requirements.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/solution-architecture.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/baselineV1.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/decisions-round-1.md` / `decisions-round-2.md` / `decisions-round-3.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/docs/architecture/vibeboxbaseline.md` (and the canonical `branchvibebox/vibeboxbaseline.md` not read here)
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/plan.md`
- `/run/media/airlongdian/EasyU/AirCoding/AirPlan/todo.md`

## Recommended follow-ups (non-blocking)

1. Add todo entries for the user-facing CLI catalog and TUI commands (`/direct`, `/done`, `air resume`, `air restore`, `air compact`, `air history`).
2. Schedule mid-session Nudge cadence and Curator Daemon as concrete T-* tasks.
3. Schedule `air doctor --bundle` and `doctor --fix` paths.
4. Decide explicit Alpha posture toward VibeBox (parallel branch in plan, or explicit "post-Alpha").
5. Consider trimming the 14 frozen v1 docs to what Alpha actually consumes; convert the rest to "design-as-needed".agentId: a86a8f5ed442d26e9 (use SendMessage with to: 'a86a8f5ed442d26e9' to continue this agent)
<usage>total_tokens: 76830
tool_uses: 10
duration_ms: 176409</usage>