Skip to content

fix: first-run FDA assist survives a jq-less machine (#394) - #399

Open
kiki830621 wants to merge 3 commits into
mainfrom
idd/394-fda-assist-before-dep-gates
Open

fix: first-run FDA assist survives a jq-less machine (#394)#399
kiki830621 wants to merge 3 commits into
mainfrom
idd/394-fda-assist-before-dep-gates

Conversation

@kiki830621

Copy link
Copy Markdown
Member

Refs #394

What

  1. plugin/hooks/session-start.shjq/ps dependency gates 從檔案頂端下移first_run_fda_assist 呼叫之後、staleness 區塊之前。FDA assist(mail#355)不需要這兩個工具,卻被擋在 gate 後面 —— 沒裝 jq 的機器(恰是 fresh-install 的目標受眾)整個 assist 靜默消失。staleness 區塊的 graceful-skip 語意逐 byte 不變。
  2. plugin/tests/test-session-start-hook.sh — 兩個測試缺陷修正 + FDA 分支補洞(22 → 42 asserts):
    • Case 5 隔離修正:舊 PATH(/sbin:/usr/sbin)同時移除 jq ps,「jq missing」case 在只有 ps gate 正常時也會過。改用 symlink-farm shim(make_shim_without jq)真正只抽掉一個工具。
    • FDA 分支從零覆蓋到五個 case(Case 9–12 + 核心回歸鎖 Case 13/14):assist 觸發(marker + detached --setup 探針)、once-only、FDA 已授權不擾、舊 binary 版本閘跳過、jq 缺席時 assist 仍要跑(bug: session-start hook 的 jq/ps gate 擋在 first-run FDA assist 之前 — 沒 jq 的機器 FDA 協助永遠 no-op (#335 verify) #394 的存在理由)、ps 缺席對偶 case。舊 suite 的 22/22 之所以蓋不到這些:HOME override 讓 [ -x $HOME/bin/CheAppleMailMCP ] 恆假、assist 每個 case 都自我跳過。

Verification

  • bash plugin/tests/test-session-start-hook.sh42 pass / 0 fail(既有 8 case 全數保持綠 = staleness 行為回歸鎖)
  • bash -n hook + test 皆過

過程中抓到的實作細節

gate 下移讓 PLUGIN_ROOTdirname 先於 gate 執行 → shim 工具清單必須含 dirname(第一輪 shim 漏掉,Case 5 的 stderr 潔淨斷言當場抓到 —— 隔離測試自己證明了自己的價值)。


🤖 /idd-all batch lane 4/6。Do NOT add a GitHub close trailer — issue lifecycle 走手動 close skill。

…394)

The gates guarded the staleness block but sat above the FDA assist, which needs neither tool - a jq-less machine (exactly the fresh-install audience) silently lost the assist. Tests: Case 5 now isolates jq via a symlink-farm shim (the old PATH removed ps too), and the FDA branch gains six cases including the jq-less regression lock; 22 -> 42 asserts, all green.

Refs #394
XDG_STATE_HOME is now pinned inside the sandbox for every hook invocation - without it, three cases went red under a developer's real XDG_STATE_HOME and the once-only marker escaped into their actual state dir, silently suppressing their genuine FDA assist (reproduced by verify). The dependency gates gain CHE_MAIL_HOOK_DEBUG-gated skip messages so Cases 13/14 can assert the gates THEMSELVES fired - previously deleting either gate left the suite green because downstream defenses reproduce the gate's observable behavior. Also: mock binary logs argv pinning the --check-fda --quiet contract; symlink-farm shim fails loudly on unresolvable tools; detached --setup waits by polling, negative asserts get a settle window; Case 14 gains the kill-warning negative assert; hook header documents the per-feature gate principle; plugin CHANGELOG entry. 46/46 asserts green.

Refs #394
…every claim

Round-2 fixes from the #394 verify (PR #399). The cross-model round proved the
round-1 locks were assertion theater, with reproductions:

- replacing both gate `exit 0` with `:` left the suite 46/0 GREEN. The lock
  asserted the diagnostic message, not the gate's effect. The staleness block
  now lives in run_staleness_detection() with `return`-scoped gates, plus a
  debug-gated "staleness gates passed" seam below them; Cases 13/14 assert that
  seam is NOT reached, and new Case 15 is its positive control (without one,
  deleting the seam would make the negative asserts vacuous).
- making the `--setup` launch jq-dependent -- #394's exact regression -- also
  left the suite green: the cases asserted the assist's three stderr lines but
  never its actual product. They now assert the launched setup window and the
  once-only marker.
- the function scoping replaces a comment ("a future tool-independent feature
  belongs ABOVE the gates") with structure. Ordering was #394's root cause;
  leaving bare `exit 0` left the next feature to re-inherit it.

Also from that round:
- CHE_MAIL_HOOK_DEBUG is read as exactly "1". Under `-n`, the value 0 -- which
  every reader takes as off -- turned debug output ON, including in real
  sessions. run_hook neutralizes an inherited value; new Case 16 locks it.
  Before this, CHE_MAIL_HOOK_DEBUG=0 in the environment made the suite 45/1.
- the mock validates full probe argv, so --quietly no longer satisfies the
  --check-fda --quiet contract that the version gate exists to protect.
- Case 12 asserts an old binary is never probed at all, which is the harm the
  version gate prevents -- not merely that no message appeared.
- make_shim_without post-checks that exactly one tool is missing; its silent
  `continue` for builtins could otherwise thin the shim back toward the
  both-missing state Case 5 was fixed for.
- unset HOME under `set -u` would abort with a non-zero exit, the one thing
  this hook promises never to do.

Suite 22 -> 60 asserts across 16 cases. Five mutations run, five caught.

Known gap, deliberately out of scope and tracked in #403: the hook collapses
the four-value `--check-fda --quiet` contract to a boolean, so noMailData
reports "not granted" and burns the once-only marker.

Refs #394
@kiki830621

Copy link
Copy Markdown
Member Author

6-AI verify — round 2 + fixes (#394)

Backend: pai-ensemble 2.20.0 · 4 lenses + devil's-advocate + Codex gpt-5.6-sol (xhigh) · dispatch model opus · 46 findings (2 HIGH / 16 MEDIUM / 18 LOW / 10 INFO).

The two HIGHs were both demonstrated by mutation, not argued — and both showed round 1's regression locks were assertion theater.

The two that mattered

1. The gate-existence lock pinned the diagnostic string, not the gate.
Replacing both gate exit 0 with : — keeping command -v and the debug echo — left the suite 46 pass / 0 fail. The message still printed; execution simply fell through and failed harmlessly a few lines later.

2. Cases 13/14 never asserted that --setup actually launched.
wait_for_file … || true discarded its own result. Making the setup launch jq-dependent — #394's exact regression — also left the suite fully green, because the cases asserted the assist's three stderr lines and never its actual product.

The second is the sharper one: #394's value proposition is "a jq-less machine still gets the FDA assist", and the assist is the setup window, not the text.

What changed

  • The staleness block now lives in run_staleness_detection() with return-scoped gates. The scoping is structural, not positional — a feature added below it still runs without jq/ps. Ordering was bug: session-start hook 的 jq/ps gate 擋在 first-run FDA assist 之前 — 沒 jq 的機器 FDA 協助永遠 no-op (#335 verify) #394's root cause; leaving bare exit 0 left the next feature to re-inherit it, enforced only by a comment.
  • A debug-gated staleness gates passed seam below the gates locks their effect. Cases 13/14 assert it is absent; new Case 15 is its positive control, without which the absent-assertions would pass vacuously if the seam were deleted.
  • Cases 13/14 now assert the launched setup window and the once-only marker.
  • CHE_MAIL_HOOK_DEBUG is read as exactly "1". Under -n, the value 0 — which every reader takes as off — turned debug output ON, in real sessions too. run_hook neutralizes an inherited value; new Case 16 locks it. Before this, CHE_MAIL_HOOK_DEBUG=0 bash … made the suite 45/1.
  • The probe mock validates full argv, so --quietly no longer satisfies the --check-fda --quiet contract the version gate exists to protect.
  • Case 12 asserts an old binary is never probed at all — the harm the gate prevents — rather than merely that no message appeared.
  • make_shim_without post-checks that exactly one tool is missing; its silent continue for builtins could otherwise thin the shim back toward the both-missing state Case 5 was fixed for.
  • Unset HOME under set -u would abort with a non-zero exit, the one thing this hook promises never to do.

Evidence

bash plugin/tests/test-session-start-hook.sh          → 60 pass / 0 fail  (16 cases; was 22)
CHE_MAIL_HOOK_DEBUG=0 bash …                          → 60 pass / 0 fail  (was 45/1)
bash -n  (hook + suite)                               → clean

Five mutations run, five caught — including the two that previously survived (gates that print but do not return; a jq-dependent --setup launch).

Filed, not silently folded in

--check-fda --quiet is a four-value contract (SetupCLI.ExitStatus: 0 granted / 1 denied / 2 noMailData / 3 undetermined) that the hook collapses to a boolean. noMailData — the normal state of a machine where Mail has never been opened, i.e. exactly this assist's audience — makes it report "Full Disk Access is not granted" (unfounded) and burn the once-only marker, so the assist self-disables precisely where it is most needed. Pre-existing since mail#355 and outside #394's gate-ordering scope → #403, with a pointer in the hook.

Honest status

Not independently re-verified. MAX_ROUND=2 was reached; the mutation battery is self-verification, not an adversarial round.

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