From 06f3161e1d3fb343b1b0d972f2882bc9f1a67528 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Wed, 4 Nov 2026 20:05:10 +0800 Subject: [PATCH] =?UTF-8?q?docs(en):=20=E5=BB=BA=E7=AB=8B=E8=8B=B1?= =?UTF-8?q?=E6=96=87=E6=96=87=E6=A1=A3=E7=9B=AE=E5=BD=95=20=E2=80=94?= =?UTF-8?q?=E2=80=94=20=E7=B4=A2=E5=BC=95=20+=20=E6=95=99=E8=AE=AD?= =?UTF-8?q?=E5=85=A8=E6=96=87=20+=20=E8=AE=BE=E8=AE=A1=E6=91=98=E8=A6=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 问题 仓库是公开的，README / README_EN 已经双语，但 `docs/` 只有 `docs/zh/`： 英文读者读完首页就撞墙。 ## 做法：摘要而非全译，并写明为什么 `docs/zh/` 有 27 份文档、约 7000 行设计。**全译会烂得比审得还快**， 而一份陈旧的译文比没有更糟 —— 读者不知道它落后于原文多少。 所以按"内容变化速度"分级： | 类型 | 译法 | 理由 | | --- | --- | --- | | 教训 | ✅ 全文 | 复用价值最高，失败形态会反复出现 | | 索引 | ✅ 全文 | 没有它其余都不可发现 | | 设计文档 | ⚠️ 一页摘要 + 状态行 | 7000 行全译跟不上演进 | | 历史快照/证据 | ❌ 不译 | 时点产物，翻译无增益 | | 未实施计划 | ❌ 不译 | 会变；且按 §4.3 本就不该进仓 | 判据写在 `docs/en/README.md` 里： > 如果一份文档的内容变化速度快过译文能被审阅的速度，就译它的**摘要**而不是正文。 ## 三份文件 **docs/en/README.md** —— 索引。含分类表（哪些已译、哪些只有中文摘要、 哪些不译）、状态行约定、以及"本索引必须在同一提交里更新"这条硬规则 （滞后的索引比没有索引更糟：它教读者信任错的地图）。 **docs/en/engineering-lessons.md** —— §1（判据会骗人）+ §5（判据形态） 全文翻译，§2–§4 摘要。§5 是本轮新增的核心内容： 5.1 判据必须把被测模块当黑箱（含三个正当例外） 5.2 禁止读源码做断言（附"想保证什么 → 错误做法 → 正确做法"表） 5.2.1 判据不得以文档为依据 5.3 测试与业务代码分离 5.3.1 三种测试位置的实测边界 5.3.2 覆盖率必须用 -coverpkg=./... 5.4 CI 必须覆盖每个独立 module 5.5 CI 与本地必须跑同一套门 每个小节都带实测数据与变异验证结果，不是原则罗列。 **docs/en/design-summaries.md** —— 6 份核心设计的一页摘要： kernel-modularization 6 层 20 模块；依赖图在构建期算好并嵌进二进制； depgraph.bin 必须与二进制同目录，放数据目录会崩溃循环 secret-firewall security 是外围到核心之间的防火墙，自持数据结构、 只接受事实不接受策略；出入向都要洗 input-scheduler L1–L4 + 现场保存；工具执行是临界区不可抢占 resident-subagent 与 rtchild 语义相反；单数/复数共用 Cleanup toolcall-contract assistant(tool_calls) 后必须紧跟每个 id 的 tool 消息；批内不得插入任何东西 context-overflow-l4 下沉到冷上下文而非截断；InactivePages 是本次渲染 的临时标记（json:"-"） 另有 13 份（插件相关 / 数据报告型）列为 "Chinese only" 并说明理由。 ## 中文索引加了英文入口 `docs/zh/README.md` 顶部增加指向 `../en/README.md` 的一行，并明确： > 本目录是**中文原文，也是唯一权威**；英文侧只做摘要，与本文冲突时以本文为准。 这一条很重要 —— 否则两份文档会逐渐分叉，而读者不知道该信哪个。 ## 验证 go build ./... PASS go vet ./... PASS gofmt -l 干净 go test ./... 65 包全过，0 FAIL --- docs/en/README.md | 87 ++++++++ docs/en/design-summaries.md | 199 +++++++++++++++++ docs/en/engineering-lessons.md | 383 +++++++++++++++++++++++++++++++++ docs/zh/README.md | 3 + 4 files changed, 672 insertions(+) create mode 100644 docs/en/README.md create mode 100644 docs/en/design-summaries.md create mode 100644 docs/en/engineering-lessons.md diff --git a/docs/en/README.md b/docs/en/README.md new file mode 100644 index 00000000..f415396f --- /dev/null +++ b/docs/en/README.md @@ -0,0 +1,87 @@ +# docs/en — English documentation index + +> **Status: index (current).** Established 2026-11-04. +> +> **中文索引**: [`../zh/README.md`](../zh/README.md) + +--- + +## Why this directory exists + +The repository's documentation grew Chinese-first, but this is a **public** repository +and `README.md` / `README_EN.md` are already bilingual. That asymmetry meant an +English-speaking reader could read the front page and then hit a wall at `docs/`. + +This directory is the English side. It does **not** mirror `docs/zh/` file-for-file — +see "Translation policy" below. + +--- + +## Translation policy + +Not everything is worth translating, and pretending otherwise produces stale +translations that are worse than none. + +| Document class | Translated? | Why | +| --- | --- | --- | +| **Lessons** (`engineering-lessons`) | ✅ Full | Highest reuse value; the failure shapes recur across the codebase | +| **Runbooks** (`deploy-`, `ci-cd-`, `site-infra-`) | ✅ Full | Operators who cannot read Chinese cannot deploy | +| **Index** (this file, `../zh/README.md`) | ✅ Full | Entry point; without it nothing else is discoverable | +| **Design docs** (`*-design.md`) | ⚠️ Summary only | Full translation of ~7000 lines would rot faster than it could be maintained. Each gets an English **summary + status line**, with a pointer to the Chinese original as the authority | +| **Historical snapshots / evidence** | ❌ No | Time-stamped by nature; translation adds no value | +| **Unimplemented plans** | ❌ No | Will change; see `docs/zh/README.md` §一 for why plans are not kept anyway | + +**Rule of thumb**: if a document's content changes faster than a translation can be +reviewed, translate its *summary* instead of its body. + +--- + +## Document classes (same taxonomy as `docs/zh/`) + +| Class | Answers | Naming | +| --- | --- | --- | +| **Design** | Why is it built this way? | `*-design.md` | +| **Runbook** | How do I do it? | `*-runbook.md` | +| **Lessons** | What went wrong before? | `engineering-lessons.md` | +| **Open work** | What is not done yet? | `future-plan.md` | + +Plans (`*-plan.md`, `*-checklist.md`) are **not** kept: once executed they read as +"what we intended" while being read as "what is". See `docs/zh/README.md` for the +full rationale. + +--- + +## English documents + +| Document | Class | Status | +| --- | --- | --- | +| [`engineering-lessons.md`](./engineering-lessons.md) | Lessons | ✅ Translation of the criteria-shape sections (§1, §5) | +| [`design-summaries.md`](./design-summaries.md) | Design | ✅ One-page summary per design doc | + +## Chinese-only documents (with English summary available) + +| Document | Class | English summary | +| --- | --- | --- | +| `docs/zh/kernel-modularization-design.md` | Design | [`design-summaries.md`](./design-summaries.md#kernel-modularization) | +| `docs/zh/kernel-secret-firewall-design.md` | Design | [`design-summaries.md`](./design-summaries.md#secret-firewall) | +| `docs/zh/input-scheduler-design.md` | Design | [`design-summaries.md`](./design-summaries.md#input-scheduler) | +| `docs/zh/resident-subagent-design.md` | Design | [`design-summaries.md`](./design-summaries.md#resident-subagent) | +| `docs/zh/toolcall-contract-and-sequence-design.md` | Design | [`design-summaries.md`](./design-summaries.md#toolcall-contract) | +| `docs/zh/context-overflow-l4-design.md` | Design | [`design-summaries.md`](./design-summaries.md#context-overflow-l4) | +| `docs/zh/commondev-design.md` | Design | plugin-side, Chinese only | +| `docs/zh/commondev-agent-runtime-design.md` | Design | plugin-side, Chinese only | +| `docs/zh/deploy-runbook.md` | Runbook | Chinese only — deploy targets are local hosts | +| `docs/zh/ci-cd-runbook.md` | Runbook | Chinese only — see `.github/workflows/` for the authoritative steps | + +--- + +## How to keep this honest + +Two rules, both learned from `docs/zh/README.md`'s own history: + +1. **Every document carries a status line.** Allowed values: `current`, + `implementation record`, `archive (historical)`, `plan (not implemented)`. + A document without one is assumed stale. +2. **This index must be updated in the same commit** that adds, removes, or + reclassifies a document. An index that lags is worse than no index — it teaches + readers to trust the wrong map. diff --git a/docs/en/design-summaries.md b/docs/en/design-summaries.md new file mode 100644 index 00000000..f5ea2084 --- /dev/null +++ b/docs/en/design-summaries.md @@ -0,0 +1,199 @@ +# Design summaries (English) + +> **Status: summaries only.** Each design document exists in full only in Chinese: +> `docs/zh/*-design.md`. Those are the authority. This file gives an English reader +> enough to know *what problem each design solves* and *whether it is in effect*, +> plus a pointer for the details. +> +> **Why summaries instead of translations**: the design docs total ~7,000 lines. A +> full translation would rot faster than it could be reviewed, and a stale translation +> is worse than none. For content that changes quickly, translate the summary. + +--- + +## kernel-modularization + +**Source**: `docs/zh/kernel-modularization-design.md` (1,502 lines) +**Status**: implemented and deployed (designed 2026-10-16, landed 2026-10-24, fully +revised 2026-10-31). This document is the single authority on assembly architecture. + +**Problem it solves**: the kernel had 17 serial startup steps. Any module that needed +another module's result had to be started later than it, which produced a hand-maintained +ordering that silently broke whenever a dependency was added. + +**Shape**: every kernel module declares `Requires()` (facilities) and `Provides()` +(facilities). A dependency graph is computed **at build time** and embedded in the binary +as `depgraph.bin`. At startup the kernel assembles in layers — independent modules in the +same layer start concurrently. Current production shape: **6 layers / 20 modules, peak +concurrency 4**. + +**Two invariants worth knowing**: + +1. The dependency graph and the binary must match. The kernel compares the embedded + graph digest against `depgraph.bin` on disk and **refuses to start** on mismatch + (`★ 依赖图与本二进制不配套`). This is deliberate: a mismatched graph means the + assembly order is wrong, and a wrong order produces silent `nil` reads. +2. **`depgraph.bin` lives next to the binary, not in the data directory.** Deploying it + into the data dir produces a crash loop. + +**Key concepts** for reading the code: + +- *facility* — a named capability one module provides and others require +- *layer* — computed from the graph; modules in a layer have no dependencies between them +- *`kstate`* — the single mount point for shared data. Other packages mount onto it; + it never knows about them (leaf package). + +--- + +## secret-firewall + +**Source**: `docs/zh/kernel-secret-firewall-design.md` (952 lines) +**Status**: implemented and deployed; architecture converged 2026-10-31. + +**Problem it solves**: secrets (API keys, platform credentials) must not reach the model's +context, logs, or disk in plaintext — but plugins legitimately need the real values to make +API calls. So there must be a translation layer at the boundary. + +**Shape**: `security` is a **firewall between the peripheral kernel and the core kernel**. +It owns its own data structures and makes its own decisions. Callers supply **facts** only +(`CheckTool(name)`, `ScanContent(text)`) — never a policy, never a "trust me" flag. + +**Three properties that matter**: + +1. **Aliases, not values.** The model sees `sec_xxx`; the firewall substitutes the real + value inside the executor, after the argument has left the model's context. +2. **Direction matters.** Inbound (model → plugin) fills aliases. Outbound (plugin → model) + scrubs any secret accidentally echoed back — e.g. `enc_shell` running `curl` whose + response body contains the key. Both directions have separate tests because only + inbound was implemented at first, and the outbound leak was found in production logs. +3. **`Store` delegates encryption to `Codec`, and domains must differ.** Store ciphertext + uses domain `security/variables`; config-at-rest uses `config-at-rest`. Sharing a domain + means one leak informs the other. + +**Judgement criterion** the design insists on: `CheckPath` / `CheckNetwork` / `CheckTool` +are only real if something in the *execution chain* calls them. A policy that exists but +has no caller is indistinguishable from no policy — and this happened here twice. + +--- + +## input-scheduler + +**Source**: `docs/zh/input-scheduler-design.md` (642 lines) +**Status**: implemented and deployed (`internal/agent/core/scheduler.go`). + +**Problem it solves**: a single agent receives messages from many channels (chat, CLI, +device bridge) with wildly different urgency. A naive FIFO queue either delays urgent +interrupts behind long tasks, or lets interrupts destroy work in progress. + +**Shape**: four interrupt levels (L1–L4) plus a queue for non-interrupting work; +preemption with **state snapshots**, so a suspended task resumes exactly where it stopped +rather than restarting. Interrupts form a **strict LIFO stack** bounded by the number of +levels. + +**Invariants** (each has a test): + +- L4 does not preempt L4 (same-level interrupts queue rather than nest) +- Stack depth is bounded; exceeding it demotes rather than unbounded recursion +- A suspended task and its resumed counterpart see the same state (`SuspendedEqualsResumed`) +- Tool execution is a **critical section**: interrupts do not cancel mid-tool (side effects + are not rollback-able). The interrupt is recorded and applied after the tool completes. +- The notify hook fires on push and pop, **while holding the lock** (a hook that takes the + lock itself would self-deadlock) + +--- + +## resident-subagent + +**Source**: `docs/zh/resident-subagent-design.md` (712 lines) +**Status**: implemented and deployed. + +**Problem it solves**: some work spans many messages over hours (a project conversation, a +phone call). Stateless sub-agents lose everything between turns; giving every sub-agent a +full agent loop is too heavy. + +**Shape**: a resident child has its own input queue, its own two-level memory, and a +parent-child interrupt relationship. Residual tasks left at teardown have four states +(keep / drop / park / return) rather than being silently lost. + +**Why it is separate from `rtchild`**: the rt (realtime) child has the *opposite* lifetime +semantics — no task queue, no residuals, a fresh context window per call. They share the +registry skeleton but not the semantics. + +**Known trap** (this is why `child.Registry` exists): the singular stop path used to +unregister the inbound channel while the plural path forgot it, so residue accumulated +monotonically with each create/destroy until restart. Both paths now call the same +`Cleanup`. + +--- + +## toolcall-contract + +**Source**: `docs/zh/toolcall-contract-and-sequence-design.md` (839 lines) +**Status**: implemented and deployed. + +**Problem it solves**: with parallel tool execution, the wire format has hard constraints +that are *not* optional — an `assistant` message declaring `tool_calls` must be followed +**immediately** by one `tool` message per `tool_call_id`. Anything inserted in between +(multimodal attachment, memory recall, a system note) makes providers reject the whole +request with HTTP 400. + +**Shape**: the batch executor collects all tool results first, then writes messages +sequentially, then flushes deferred attachments (`flushBatchAttachments`). Nothing may be +inserted inside the batch. + +**Measured failure modes** (all three happened): + +1. Parallel path appended `ToolResults` but never `ToolCalls` ⇒ results outnumbered calls +2. Budget truncation in L1 rendering cut between `assistant(tool_calls)` and its `tool` + messages ⇒ dangling calls +3. A rejected/skipped tool recorded a call but not a result ⇒ same mismatch, persisted into + `context.json` and replayed every turn + +The third is the instructive one: **"skipped" ≠ "no result"**. The rejection reason *is* the +result, and the model already saw it. The context copy must match what the model saw. + +--- + +## context-overflow-l4 + +**Source**: `docs/zh/context-overflow-l4-design.md` (163 lines) +**Status**: implementation record; **terminology in the document is outdated** (it still +says `doc_query` / `doc_commit`, renamed to `coldcontext_*`). + +**Problem it solves**: when the context window is exhausted, the two conventional options +are both bad — truncate (lose information) or ask the model to summarize (costly, lossy, +and the model may summarize wrongly). + +**Shape**: instead of discarding, low-relevance events are **sunk** into a cold-context +layer, and from there distilled into a graph. Nothing is deleted; it changes storage tier. +Overflow handling recurses to L4 when trimming alone cannot help. + +**Why it matters for reading the code**: `InactivePages` in `ContextEvent` is the "this page +is hidden for *this* render" marker. It is deliberately `json:"-"` — hidden pages are a +per-render decision, not persisted state. The constraint is that a *whole* tool call and its +results are hidden or shown together; hiding one half produces the 400 in +`toolcall-contract` above. + +--- + +## Other design docs (Chinese only, not summarized here) + +| Document | Scope | +| --- | --- | +| `commondev-design.md` | The `commondev` plugin (multi-repo supervisor) | +| `commondev-agent-runtime-design.md` | Runtime wiring for that plugin | +| `triple-distill-llm-design.md` | Triple extraction/distillation pipeline | +| `triple-extract-attention.md` | Attention weighting for extraction | +| `recall-as-association.md` | Recall as spreading activation rather than lookup | +| `recall-capability-tiers.md` | What each recall tier can and cannot do | +| `multimodal-space.md` | Unified multimodal embedding space | +| `vector-provider-decision.md` | Choice of embedding provider and why | +| `abstention-criterion.md` | When the agent should decline to answer | +| `legacy-issues.md` | Known legacy problems | +| `legacy-table-retirement.md` | Old-table retirement plan (cited by production code) | +| `kernel-performance.md` | Performance measurements and budgets | +| `production-recall-validation.md` | Recall validation on production data | + +These are not translated because they are either plugin-specific or data-report shaped — +their value is in the numbers and the local context, both of which the Chinese original +carries adequately. diff --git a/docs/en/engineering-lessons.md b/docs/en/engineering-lessons.md new file mode 100644 index 00000000..9cb998b9 --- /dev/null +++ b/docs/en/engineering-lessons.md @@ -0,0 +1,383 @@ +# Engineering lessons — criteria shape + +> **Status: translation of the criteria-shape sections.** Source of truth: +> [`docs/zh/engineering-lessons.md`](../zh/engineering-lessons.md). +> Sections §1 (criteria that lie) and §5 (criteria shape) are translated in full; +> §2–§4 are summarized at the end. If the two disagree, the Chinese version wins — +> and this file should be corrected. + +Why this document is worth the effort: the failure shapes below recur. They are +more durable than any particular implementation, and each one cost a production +incident to learn. + +--- + +## §1 Criteria that lie + +### 1.1 Structural assertions are not quality assertions + +**Symptom**: 16 criteria, 6 mutations, **all green** — and the feature was completely +unusable. + +**The case**: a memory-retrieval rewrite. The criteria checked "field is non-empty", +"row count is right", "does not panic". Actual retrieval results: + +```text +query "how is doc-layer recall designed" → score=0.7834 +query "hmap plugin install verification" → score=0.7922 +query "QQ avatar" → score=0.7601 +``` + +Every score landed in 0.75–0.79. **Discrimination ≈ 0**; the returned content had +nothing to do with the query. + +⇒ "All green" proved only that *I implemented what I thought I implemented*. + +**Ask**: would this criterion go green if the feature were completely dead? If yes, +it is not a quality criterion. + +### 1.2 Testing "after wiring" ≠ testing "is it wired" + +The most frequent defect shape in this repo (5 instances so far). + +| Criterion | What it does | Why it is always green | +| --- | --- | --- | +| `security_toolgate_test.go` | calls `a.SetSecurity(rt)` manually, then asserts interception | **the criterion supplies the missing wiring itself** | +| `retention_test.go` | writes `month_2026_01.tar.gz` by hand, then asserts deletion | never ran `compressMonthly`'s **real output** (which produces `month_2026-27` — parsing necessarily fails) | + +Both green. Both features dead in production. + +**Ask**: would this criterion still pass under the *production* assembly? + +### 1.3 "Verification passed" ≠ "architecture is right" + +16 end-to-end triple-extraction checks passed. But they verified *extraction quality* +(can a sentence be split into subject-predicate-object), not *the correct storage +shape*. The whole approach was rejected after the verification. + +**Ask**: am I verifying the thing I actually want to guarantee? + +### 1.4 Fixtures invented from imagined fields + +The fixture used hand-written field names that did not match the real struct, so the +criterion tested a contract that never existed. + +⇒ Criterion inputs must come from **real artifacts**, not from memory. + +### 1.5 Writing tests after changing the code + +Changed the code first, added tests second. The tests then only proved "I changed +what I thought I changed". + +--- + +## §5 Criteria shape + +§1–§4 describe how criteria *lie*. §5 describes why a criterion's **shape** can be +wrong — which is more expensive, because a mis-shaped criterion freezes an +implementation detail into a contract. Refactoring the internals then necessarily +breaks the criterion, and the author must choose: don't change the code, or delete +the criterion. + +### 5.1 A criterion must treat the module as a black box + +**The point of modularization is that internal changes do not break external +interfaces.** A criterion that does not respect that layer cancels the benefit. + +White-box criteria turn internal state into contract. Measured costs: + +| White-box criterion | What it froze | Consequence | +| --- | --- | --- | +| `if sf.Step != StepLLM` | which state-machine step it stops at | renaming or splitting `StepLLM` turns it red while behavior is correct | +| `if f.ToolIdx == 3` | how the loop advances | switching to batch-commit turns it red; the result is identical | +| `strings.Contains(src, "kstate.Mount(ks)")` | a source literal | rewriting as `kstate.Mount(state)` turns it red (that rewrite is correct) | +| `os.ReadFile("../../events/bus.go")` + regex | which file a constant lives in | moving the constant to `types.go` turns it red | + +**Ask**: does this criterion constrain an *external promise*, or *how it happens to be +implemented today*? If the latter, it should not exist as a criterion — it belongs in +a code comment explaining why the code is shaped that way. + +#### Three legitimate exceptions + +1. **Pure algorithms** — input/output *is* the whole semantics (trimming, version + comparison, pagination). White-box here does not freeze "flow". +2. **Concurrency / lifetime invariants** — timing constraints invisible from outside, + e.g. "emit must not read a released pointer during Dispose". These must be pinned + with `-race` plus concurrent pressure, **not** by asserting internal fields. +3. **Circuit-breaker / retry state machines** — the internal state is itself required + by the spec ("3 crashes ⇒ open"). Assert the **observable effect** of the + transition (is it still healthy? did it enter the reload queue?), not the field value. + +#### Operable test + +> **"If I rewrote this module's internals completely, keeping the public interface +> identical, would this criterion still be green?"** +> +> Green ⇒ shape is right. Red ⇒ it pins the implementation, not the contract. + +### 5.2 Do not assert on source text + +`os.ReadFile("*.go")` + `strings.Contains` / `regexp` is an **anti-pattern**, even when +the motive is sound (e.g. "prevent someone from deleting the wiring line"). + +**Why it looks attractive**: reading source is cheap compared to building a heavy +fixture. + +**Why it must be banned**: + +- It tests **text**, not **semantics**. Adding a space, renaming a variable, or an + equivalent refactor breaks the check (false red or false green). +- It makes file layout part of the contract. Moving code to a new file turns it red. +- It cannot distinguish "the code exists" from "the code runs". + +**Correct alternatives**, in priority order: + +| What you want to guarantee | Wrong | Right | +| --- | --- | --- | +| production assembly wires something | grep for `kstate.Mount(ks)` | start the real assembly from zero, assert an **observable result** (`kstate.Get() != nil`) | +| both protocol ends agree on fields | regex-extract string literals on both sides | start the handler, send a real event, assert the **received JSON** | +| a guard was not deleted | grep the source | construct the trigger condition, assert the **guard fires** (request rejected) | +| nobody deletes a criterion | grep the test file for a function name | CI-level guard: `git diff --name-status` detects the deletion | + +**Only exception**: CI configuration self-checks (e.g. "the exclusion table must not +silently degrade"). There the *object under test is the config file itself* and it has +no other observable behavior. Even then, the comment must say so explicitly. + +### 5.2.1 A criterion must not cite documentation + +**"Tests citing docs" is wrong on its face** — docs can be edited, deleted, rewritten, +while criteria must be anchored to **current code behavior**. + +Real counter-example (`internal/agent/core/interrupt_invariant_test.go:336`): + +```go +// Fact list (from probing, see docs/zh/interrupt-subsystem-split-plan.md §1.5). +// This only asserts "these fields exist on scheduler", as an anchor for the rewrite; +``` + +Three layers of wrong: + +1. **The cited document is an unimplemented plan** — it is itself labeled + "status: plan (not implemented)". The criterion is anchored to a fact that does + not exist. +2. **It tests no contract.** The actual assertion is "after enqueueing, the queue has + 1 item" — the most basic correctness of interrupt handling, unrelated to the + claimed "reverse-dependency snapshot". +3. **It turns undone work into a passing criterion.** Nothing can turn it red except + a total failure of the enqueue function — at which point other criteria already fail. + +⇒ The right move is to **delete it** (or rewrite it into a real criterion) and record +the TODO in `future-plan.md`. **There is no legitimate shape called "a permanently +green criterion holding a TODO".** + +**Ask**: is this criterion's basis code behavior, or a document that may change? If +the latter, it protects prose, not the system. + +### 5.3 Separate test files from business code + +**Measured state (2026-11-04)**: + +```text +test files repo-wide 521 +test lines of code 112,068 +internal/agent/core/ 196 files, 144 of them tests (73%) +internal/plugins/webui/ 29 test files, all `package webui` (white-box) +repo-wide `package X_test` only 3 +``` + +The problem is not "a lot of tests". It is: + +1. `ls internal/agent/core/` does not show what the module consists of. +2. 100% white-box ⇒ 100% of criteria can touch internals. +3. Converting to black-box requires moving files — and the source-scanning criteria + block the move. + +**Convention** (mandatory for new code, migrate existing incrementally): + +- Black-box criteria use `package _test`, in `*_contract_test.go`. +- White-box criteria (only the three exceptions in §5.1) go in `*_internal_test.go` + with a header explaining *why white-box is unavoidable*. +- New criteria default to black-box. Writing white-box requires answering in the file + header: can this be expressed black-box? Why not? + +**This is not a style preference**: if 90% of a package's criteria are white-box, that +package has no module boundary in practice — nobody dares change its internals. + +### 5.3.1 Three real limits on test placement (measured) + +After writing §5.3 I acted on it and hit three cases that cannot be migrated. Recording +them so nobody (including me) assumes "all tests can move to test/". + +#### Works: a `/test/` subdirectory + +Go allows `pkg` and `pkg_test` in one directory, and allows `/test/` as a separate +package. **Key fact**: writing `package pkg` inside `pkg/test/` still cannot reach +unexported symbols — + +```go +// pkg/test/a_test.go +package pkg +func TestX(t *testing.T) { _ = internalVar } // undefined: internalVar +``` + +Go partitions packages **by directory**. So a subdirectory and an external package are +equivalent in terms of visibility: both force black-box. + +#### Cannot migrate 1: `package main` (CLI) + +```text +import "x/cmd" is a program, not an importable package +``` + +`tools/hmapdev` (19 test files), `cmd/homed`, etc. **must stay in-package**. + +#### Cannot migrate 2: tests that construct private fields + +SDK tests wrote `&PluginSDK{regStage: regStage, name: "test"}` — after migration they +cannot even **construct the object under test** +(`cannot refer to unexported field regStage`). + +**But this does not mean white-box is required**: `New(name, sett, regTool, regStage, +regAPI, regOutput)` already takes these as parameters. It is equivalent *and* it is the +production path. All 19 instances were convertible, and the criteria still caught +mutations afterwards. + +⇒ When you "need to set a private field", first ask: **is there an exported constructor +that already accepts it?** Most of the time there is; the test was just taking a shortcut. + +#### Cannot migrate 3: criteria that scan "the current directory's source" + +```go +files, _ := filepath.Glob("*.go") // childkind_test.go +``` + +After moving to a subdirectory this globs an **empty directory**, and the criterion +silently loses its subject — no compile error, just green. Convert these to a fixed +relative path, or rewrite per §5.2. + +#### Migration order: one file at a time, let the compiler talk + +My first attempt migrated 11 files in bulk and then ran a script to prepend `core.` +to identifiers. The script also rewrote `import "…/internal/agent/core"` itself +(producing `core.core.core`) and clobbered `strconv.Itoa`. + +Text substitution is unreliable for "prepend a package qualifier" — it hits import +paths, comments, and standard-library symbols. **Migrate one file, read the compiler +error, fix by hand.** Slow, but it leaves no half-migrated state (`agent/core` had to +be fully rolled back once because of this). + +### 5.3.2 Coverage requires `-coverpkg=./...` + +After moving criteria to `/test/`, the **default** coverage accounting loses them: + +```bash +go test ./... -coverprofile=cover.out +``` + +It only counts coverage measured *from tests inside each package*. But the tests +covering `child` live in `child/test/` — a different package. So the main report shows: + +```text +internal/agent/core/child 0.0% ← wrong +internal/agent/core/child/test 87.6% ← actually covering child +``` + +With `-coverpkg=./...`: + +```text +internal/agent/core/child 85.7% +global 56.9% → 63.0% +``` + +**The earlier number was a systematic underestimate.** Under-reporting is more dangerous +than over-reporting: it pushes people to "write empty criteria to raise the number", +which is exactly the trap in §1.1. + +⇒ Separating criteria (§5.3) and collecting coverage **must change together**. Change +only the former and the metric degrades into a lie. + +### 5.4 CI must cover every independent module + +**Measured gap (2026-11-04)**: the repo contains `third_party/hologramagentsdk/`, which +has its own `go.mod` **and its own `.git`** (remote: +`github.com/JianFeeeee/hologramagentsdk`). + +Consequence: + +```text +go list ./... | grep third_party → 0 +``` + +The main repo's `go test ./...` **never includes** any SDK test. + +- The SDK repo's own CI does run them (its `.github/workflows/ci.yml` has + `go test ./...` and separately handles the nested module) +- But the **main repo's CI only verifies "it compiles"**, not "the SDK's criteria + still pass" + +⇒ A main-repo change may depend on SDK internals, and the main repo will not notice +when SDK criteria go red. + +**Rules**: + +1. A `go.mod` in a subdirectory ⇒ it is an independent module, invisible to `go test ./...` +2. Every independent module needs its **own** test step in CI +3. A directory with its own `.git` is **not managed by the main repo**; the main repo + only vendors what it needs. When `go.mod` `replace`s it, CI must ensure those files + exist and match upstream HEAD + +### 5.5 CI and local must run the same gates + +**Measured gap**: the commit discipline requires `gofmt -l` to be clean, but `gofmt` +appeared **0 times** across three workflows. The rule was enforced only by human +diligence. Same for `staticcheck` / `govulncheck` / coverage. + +**Rule**: every item written into the commit discipline must have a corresponding CI +step. Otherwise it is a **suggestion**, not a discipline. + +| Discipline item | CI backstop | +| --- | --- | +| `go build ./...` | ✅ | +| `go vet ./...` | ✅ | +| `gofmt -l` clean | ✅ (added 2026-11-04) | +| `go test -race` green | ✅ | +| clean worktree compiles | ❌ human only | + +--- + +## §2–§4 (summarized) + +**§2 Wiring.** Mechanism existing ≠ being wired (`codegraph_callers` returning 0 is +the test). The dependency graph is the only thing that guarantees init order — reading +another module's field without declaring the facility dependency works only by layer +coincidence. When you change the singular path, do not forget the plural one. + +**§3 Permissions & lifetime.** "Wired up" has three levels: mechanism exists → called +from the execution chain → **model is correct**. Tables keyed by a caller-supplied +name must be cleaned up when the lifetime ends, or same-name re-creation inherits the +previous generation's state. Privilege escalation must be derived by the security +module from *facts*, never declared by the caller. + +**§4 Process.** Never run memory-heavy exports concurrently with tests. Numeric claims +in docs must be produced by a command, not recalled from memory. Plans must not enter +the repository — after completion they read as "what we intended" while being read as +"what is". + +--- + +## Appendix: the common shape + +> **There is always a gap between a signal that looks right and actually being right.** + +- criteria all green ≠ feature usable +- mechanism exists ≠ wired +- layer index happens to be right ≠ dependency declared correctly +- singular path right ≠ plural path right +- same name ≠ same thing +- plan written ≠ work done +- **criterion passes ≠ contract protected** (it may pin the implementation, §5.1) +- **`go test ./...` green ≠ all tests ran** (independent modules are excluded, §5.4) +- **discipline written down ≠ anyone enforces it** (without a CI backstop it is advice, §5.5) + +⇒ When writing a criterion, a document, or code, ask: **under what circumstances would +this signal be false?** diff --git a/docs/zh/README.md b/docs/zh/README.md index 6896d424..549eb9a2 100644 --- a/docs/zh/README.md +++ b/docs/zh/README.md @@ -1,5 +1,8 @@ # docs/zh 文档索引 +> **English**: [`../en/README.md`](../en/README.md) —— 英文索引 + 教训与设计摘要。 +> 本目录是**中文原文，也是唯一权威**；英文侧只做摘要，与本文冲突时以本文为准。 + > 建立于 2026-10-31。目的：**让读者知道该信哪一篇**。 > > 此前本目录混着三种时态 —— 当时实测、历史推演、执行计划 —— 外观完全相同。