Skip to content

fix(automations,#239): delete reads on fields the desktop rules out; guard the domain - #263

Merged
kevinthelago merged 1 commit into
mainfrom
239-automations-dead-reads
Jul 29, 2026
Merged

fix(automations,#239): delete reads on fields the desktop rules out; guard the domain#263
kevinthelago merged 1 commit into
mainfrom
239-automations-dead-reads

Conversation

@kevinthelago

Copy link
Copy Markdown
Owner

The mobile half of #239 — needs no desktop change. Part of #234.

The finding

Two reads were written as "not yet published — they light up the moment the desktop starts publishing them." Neither is coming. The desktop has ruled both out, with regression tests pinning it:

Read Why it can never resolve
targetTab / targetPaneIdx Deliberately withheld. storeProjections.test.ts asserts the serialized payload does not contain targetTab — the card is schedule + outcome, the dispatch target stays desktop-side.
builtin on a hook Cannot exist. Hook is exclusively user-authored config, and HookCard field-exactness is asserted desktop-side.

The consequences were dead UI, not wrong UI:

  • AutomationVM.targetLabel was permanently null → the target arrow never rendered.
  • HookVM.builtin was permanently falsehasSystemFloor permanently falsethe system-floor note and the built-in tag were unreachable.

Change

  • Removed targetLabel, builtin, and hasSystemFloor along with their reads, the dead note and tag, and the orphaned floorNote style.
  • Rewrote the module doc, which previously promised the fields were on their way — that comment is the reason the dead code survived review.
  • automationsView.test.ts loses the target-pane case (it encoded an expectation the desktop has explicitly ruled out) and gains one asserting the VM does not regrow those fields when a payload happens to carry them.
  • Unchanged, verified correct: run-record parsing and the defensive re-sort. AutomationRun.at is epoch ms on both sides, so the ISO-string trap that broke security does not occur here; the desktop caps runs at 10 and prepends, so the re-sort is redundant but agrees.

Harness

automations moves from PENDING_DOMAINS into DECODERS — Layer A strict re-encode, plus Layer B asserting the schedule card renders from real values: the when union formats (rather than falling back to an em-dash), runs stay newest-first, and no run status falls back to unknown.

Pending domains: 4 → 3.

Still open desktop-side

The substantive gap in #239 is untouched, because no mobile change can reach it: the always-on system floor — SYSTEM_HOOKS (bsc-deny, bsc-confine, bsc-scope) — is a module constant the projector never reads, so it crosses no frame at all.

That means a user seeing zero enabled hooks concludes nothing is enforced, while three PreToolUse blockers are active under every permission posture. It needs AutomationsPayload.systemHooks: {name, event, purpose}[]; the floor note returns with it. #239 stays open for that.

Gate

  • npx tsc --noEmit clean.
  • 530 tests pass, including Layer A: automations payload round-trips through the mobile model.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RAPZC6xEyCrH9qSKUKLQdy

…guard the domain

The mobile half of #239 — no desktop change needed.

Two reads were written as "not yet published, light up when the desktop sends them".
Neither is coming. The desktop has ruled both out, with regression tests pinning it:

- **`targetTab` / `targetPaneIdx`** — deliberately withheld; `storeProjections.test.ts`
  asserts the serialized payload does not contain `targetTab`. The card is schedule +
  outcome; the dispatch target stays desktop-side. So `AutomationVM.targetLabel` was
  permanently null and the target arrow never rendered.
- **`builtin` on a hook** — cannot exist. `Hook` is exclusively user-authored config and
  `HookCard` field-exactness is asserted desktop-side. So `HookVM.builtin` was permanently
  false, which made `hasSystemFloor` permanently false, which made the system-floor note
  and the built-in tag dead code.

Removed all three, plus the now-orphaned `floorNote` style, and rewrote the module doc that
promised the fields were coming. `automationsView.test.ts` loses the target-pane case (it
encoded an expectation the desktop has explicitly ruled out) and gains one asserting the VM
does NOT regrow those fields when a payload happens to carry them.

Run-record parsing and the defensive re-sort are unchanged — verified correct against
`AutomationRun`: `at` is epoch ms on both sides, so the ISO-string trap that broke `security`
does not occur here.

**Harness:** `automations` moves from `PENDING_DOMAINS` into `DECODERS` — Layer A re-encode
plus Layer B asserting the schedule card renders from real values (the `when` union formats
rather than falling back to an em-dash, runs stay newest-first, no run status falls back to
`unknown`).

## Still open desktop-side (tracked on #239)

The substantive gap is untouched and needs a base-studio-code change: the always-on system
floor — `SYSTEM_HOOKS` (bsc-deny / bsc-confine / bsc-scope) — is a module constant the
projector never reads, so it crosses no frame. A user seeing zero enabled hooks concludes
nothing is enforced while three PreToolUse blockers are active under every permission
posture. Needs `AutomationsPayload.systemHooks`; the floor note returns with it.

Gates: `npx tsc --noEmit` clean; 530 tests pass.

Refs #239

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RAPZC6xEyCrH9qSKUKLQdy
@github-actions

Copy link
Copy Markdown

📦 Preview build ready

Artifact: preview-pr-263-e20a7c9e417ca13246c0bb3de167f1378843e27c

⬇️ Download from Actions run

Built from commit e20a7c9

@kevinthelago
kevinthelago merged commit 2cc46da into main Jul 29, 2026
5 checks passed
kevinthelago added a commit that referenced this pull request Jul 29, 2026
The mobile half of #240 — no desktop change needed.

**`version` is deleted, not filed as a projection gap.** `mcpView` read
`readString(r.version, '')`, but no `version` exists in `McpServer` or anywhere on the wire —
and unlike a missing projection, none *can* exist, because there is no version string to
project. So `McpServerVM.version` was permanently null and `installTag`'s version branch was
unreachable: every installed server showed the bare "installed" literal. Removed the field,
the read, and the dead branch. The test that asserted the VM "carries a version through when
present" is replaced by one asserting it does NOT, even when a payload invents one.

**`builtin` is KEPT, and that is a deliberate asymmetry with #239.** The automations
`Hook.builtin` was deleted because it can never exist — `Hook` is exclusively user-authored
config. `McpCard.builtin` is the opposite: it can and should exist, and the payload already
carries the built-in ids in `installedIds` in a form nothing can use. The fix is
`withBuiltins(input.servers)` in the same builder. Until then `builtins` is empty and the
"Built-in tools" section does not render — honest rather than dead, since it is one desktop
field away from live.

**MCP install/update STATUS (`McpStat`) is out of scope for the mirror — a decision, not a
follow-up.** It is Tauri-invoke-derived, name-keyed, ephemeral, and only defined for
downloadable first-party servers. It does not belong in a store projection.

**Harness:** `mcp` moves from `PENDING_DOMAINS` into `DECODERS` — Layer A re-encode plus
Layer B asserting both install states resolve (neither falls back to `unknown`), the stdio
server has no url and the http one does, and that `builtins` is still empty. That last
assertion is written to FAIL the day the desktop starts shipping built-ins, so the mobile
section gets switched on rather than silently staying empty.

Pending domains: 3 → 2 (`components` #241, `blueprints` #236).

## Still open desktop-side (tracked on #240)

`McpCard` needs `builtin?: boolean` and `buildMcpPayload` needs `withBuiltins(input.servers)`,
preserving the case-insensitive name-shadowing rule so a user server named Research does not
render twice. Two always-enabled, always-installed servers are currently invisible to the
phone, so the tool inventory it shows is incomplete.

Stacked on the #239 branch (PR #263) because both edit the same harness file; this diff
reduces to the mcp commit alone once #263 merges.

Gates: `npx tsc --noEmit` clean; 532 tests pass.

Refs #240

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RAPZC6xEyCrH9qSKUKLQdy
@kevinthelago
kevinthelago deleted the 239-automations-dead-reads branch July 29, 2026 06:43
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