Skip to content

fix: 对抗式审查发现的 10 处缺陷 + 计费适配器补测 - #381

Open
oratis wants to merge 1 commit into
mainfrom
claude/review-fixes-v2
Open

oratis wants to merge 1 commit into
mainfrom
claude/review-fixes-v2

Conversation

@oratis

@oratis oratis commented Sep 9, 2026

Copy link
Copy Markdown
Owner

六个审查代理从跨流、契约、安全、行为回归、资源生命周期、构建六个维度读了已合并的优化工作,每条发现再交给三个独立代理专门反驳,13 条通过验证。其中两条(Mac 向导的 Node 下限、apple-touch-icon 链接)main 上已独立修复,本 PR 采用 main 的版本。

影响最大的三条

非 Anthropic 的首次运行链路整条空转。 保存配置写入 LISA_MODEL,但服务端从启动起就把模型固化在十四个位置。首次运行没有密钥,服务端只能以默认模型启动;用户在 provider 选择器里选了 DeepSeek、贴了密钥,界面显示「已配置」,而每一轮仍发往 Claude——那里根本没有密钥。现在模型是保存成功后重新解析的可变绑定,并有测试断言 /session 报告 deepseek-chat

lisa doctor --probe 把卡顿的服务判成宕机。 可达性把 payload.ok !== false 也算了进去,而 /health 正是在事件循环卡顿时返回 ok:false——于是一个忙但活着的后端打印「✗ unreachable — is the backend running?」并以非零退出,而运维手册把非零探测接到强制重启。现在可达 = 答复了,降级是警告。测试夹具原本写 ok:true,所以从没抓到;现在它发送卡顿实例真正会发的东西。

/health 在 Mac 上泄露指纹。 main 已对云端脱敏,但 Mac 版仍向任何人、在鉴权之前返回完整负载:版本、运行时长、内存、会话数、在途轮次。--host 0.0.0.0 是文档推荐的手机访问方式,等于把这些摆在同一个咖啡馆网络里。现在两个版本统一一条规则:健康状态(ok + 延迟 + edition)公开,其余需要令牌;Mac 上回环仍算主人,本机 --probe 保持完整读取。

其余修复

  • 每次重启都重新推送通知:初始扫描把会话记为「未扫描」,首次解析必然像状态变化,于是 launchd 重启、lisa upgrade 或看门狗都会为几小时前已通知过的会话重发「出错」「需要授权」。启动那一趟改为只记录不广播。
  • 「自定义 OpenAI 兼容」从选择器消失:客户端用服务端列表整体替换了自己的表,而服务端无法枚举一个写 LISA_API_KEY + LISA_BASE_URL 的条目。Ollama、LM Studio 和所有自建端点因此在图形界面里不可达。命名 provider 仍以服务端为准,客户端专有的自定义条目在合并后保留。
  • 服务端返回但客户端无提示的 provider 拿到 needsModel:false:Grok、Mistral、Perplexity、Ark、MiniMax、Hunyuan 全部落进这个坑——密钥存了但没有模型,路由继续走 Claude。改为从服务端行推导,用 modelPrefixes[0] 作默认。
  • 409「已出生」被当作失败:重试按钮只会再次 409,在一个已经存在的 Lisa 面前形成死胡同;SSE 在 done 帧前被切断时就会走到这里。现在按成功处理。
  • macOS CI 只跑 swift build,从不编译测试目标,向导的决策表对 CI 不可见。改为 swift test

两条「只验证声明」的测试改为验证事实

清单测试原本只读 JSON 内容,于是在 maskable 条目指向一个没有任何步骤生成的文件时照样通过了好几天。现在它会真的请求每个 icons[].src

同一个悬空文件名还躺在服务工作线程的预缓存列表里——addAll() 只要一项 404 就整体拒绝,而安装处理器把它 catch 掉了:一个过期文件名会静默停掉整个离线缓存,任何地方都不报错。新增一条测试,把那个列表里的每个路径也抓一遍。

计费适配器补测,以及它带出的第 12 条缺陷

FirestoreOutboxStore——决定托管租户被扣一次、两次还是零次的那段代码——一个测试都没有,模块 47 个函数里 18 个从未执行。现在有 14 个测试用 firestore 客户端自己的内存 REST 假件覆盖它:幂等追加、索引先写、提交排空、悬空 id 清理、分片聚合、粘性注册、清理失败的尽力而为语义。outbox.ts 函数覆盖 61.7% → 93.75%。

写这些测试时带出一条没人报告过的缺陷:干净排空的租户永远留在粘性注册表里update() 会直接把已提交的 id 从索引移除,于是清理路径永远看不到悬空 id,注册表只增不减,15 分钟一轮的对账扫描随「曾经结算过的账户总数」单调增长、永不回落。现在索引文档存在且为空即注销该租户——以「存在」为前提,因为 append() 先注册后建索引,在那个窗口注销会让一笔费用永远得不到对账。

验证

在本基线上:typecheck、typecheck:client、lint、format:check、check:api-contract、2,065 个单元测试27 个端到端、覆盖率门槛、构建、23 个 Swift 测试,全部通过。

🤖 Generated with Claude Code

…n found

Six review agents read the merged optimization work across cross-stream,
contract, security, behaviour-regression, resource-lifecycle and CI dimensions;
each finding was handed to three independent agents told to refute it. Eleven
survived. Every one lives in the seam between two work streams, which is why no
single stream could have seen it. Two were fixed independently on main before
this landed (the Mac wizard's Node floor, the apple-touch-icon link) and are
dropped here in favour of main's versions.

**The non-Anthropic onboarding path was inert.** `/api/config/save` wrote
LISA_MODEL and the running server never read it: the model was captured from
`opts.model` at boot, in fourteen places. A first run has no key, so the server
boots on DEFAULT_MODEL; someone who chose DeepSeek in the provider picker got
the key saved, the UI said "configured", and every turn still went to Claude —
which fails, because there is no Anthropic key. The model is now a mutable
binding re-resolved after a successful save, with a test that saves a DeepSeek
key and asserts /session reports deepseek-chat.

**`lisa doctor --probe` called a lagging server dead.** Reachability folded in
`payload.ok !== false`, but /health reports ok:false when the event loop is
lagging — so a busy-but-alive backend printed "✗ unreachable — is the backend
running?" and exited 1, which the runbook turns into a forced restart.
Reachable now means answered; degradation is a warning. The test fixture said
ok:true, which is why nothing caught it; it now sends what a lagging instance
really sends.

**/health leaked a fingerprint on the Mac.** main already redacts it for the
hosted edition, but the Mac edition served the full payload — version, uptime,
RSS/heap, session and in-flight-turn counters — to anyone, pre-gate. `--host
0.0.0.0` is the documented way to reach Lisa from a phone, which puts that in
front of everyone on the café Wi-Fi. One rule for both editions now: health
(ok + lag + edition) is public, the rest needs the token. Loopback still counts
as the owner on the Mac, so `--probe` keeps its full read locally.

**Every restart re-notified the phone.** initialScan records each session as
"unknown"/"unscanned", so the first parse always looked like a state change and
emitted one — meaning a launchd restart, `lisa upgrade` or the watchdog
re-pushed "errored" / "needs permission" for sessions the user was told about
hours ago. The startup sweep now records without announcing.

**"Custom (OpenAI-compatible)" vanished from the picker.** The client replaced
its provider table wholesale with the server's list, and the server cannot
enumerate a row that writes LISA_API_KEY + LISA_BASE_URL rather than a named
provider's key. Ollama, LM Studio and every self-hosted endpoint became
unreachable from the GUI. The served list stays authoritative for named
providers (one the server does not know would be refused by the save
whitelist); the client-only custom row survives the merge.

**Providers the client had no hint for reported needsModel:false**, so the gate
saved a key with no LISA_MODEL and routing stayed on Claude — Grok, Mistral,
Perplexity, Ark, MiniMax and Hunyuan all landed in this hole. It is derived
from the served row now, with modelPrefixes[0] as the suggested default.

**409 "already born" was rendered as a failure** with a Try-again button that
could only 409 again — a dead end in front of a Lisa that exists, reachable
whenever the SSE stream is cut before the done frame. Treated as success.

**The macOS CI job ran `swift build`**, which never compiles the test target,
so the wizard's decision table was invisible to CI. It runs `swift test`.

**Two tests that assert declarations now assert reality.** The manifest test
checked only what the JSON said, and passed for days while the maskable entry
named a file no build step produced. It fetches every `icons[].src` now. The
same dangling name also sat in the service worker's precache list, where
`addAll()` rejects wholesale on one missing URL and the install handler
swallows it — one stale filename silently disables the entire offline cache,
with no error anywhere. A new test fetches every path in that list too.

**FirestoreOutboxStore had no test at all** — the adapter that decides whether
a hosted tenant is charged once, twice or never, with 18 of the module's 47
functions unexecuted. Fourteen tests now exercise it against the in-memory REST
fake the firestore client's own tests use: idempotent append, index-first
ordering, commit draining, stale-id pruning, shard aggregation, sticky
registration and best-effort prune failure. Function coverage on outbox.ts goes
61.7% → 93.75%.

Writing those tests surfaced a twelfth defect nobody had reported: a tenant
that drains cleanly never produced a stale id, so it stayed in the sticky
registry forever and the 15-minute reconcile sweep grew with every account that
ever settled anything and never came back down. An index document that exists
and is empty now deregisters the tenant — guarded on existence, because
append() registers before it indexes and deregistering in that window would
strand a charge.

Verified on this baseline: typecheck, typecheck:client, lint, format:check,
check:api-contract, 2,065 unit tests, 27 e2e, coverage floors, build, and 23
Swift tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant