Skip to content

bug: plan/apply robustness — unenforced version gate, 500s rendered as recreate, stale permission ids after recreate, raw JSON parse errors #33

Description

@2000game

Four verified plan/apply robustness gaps from the 2026-07-08 whole-codebase review — grouped because each is a small, self-contained hardening fix. Roughly severity-ordered.

1. The minimum-CT-version gate is never enforced (CONFIRMED)

src/api/version.ts:3-4 claims "the CLI asserts a minimum before it will plan/apply" and ct auth login tells the user "plan/apply will refuse" (commands/auth.ts:43) — but meetsMinVersion is only ever called inside auth login as a non-fatal warn(). plan.ts / apply.ts / destroy.ts never fetch /info or check anything.

Failure: against a CT < 3.96, ct apply proceeds; tier-0/1 writes succeed, then hierarchy/dynamic endpoints fail — the exact half-applied structure the gate exists to prevent.

Fix: assert the version in authedSession (one /info GET, cached) or at the top of plan/apply/destroy; hard-fail below minimum as documented.

2. Transient fetch errors render as "[recreate — missing in ChurchTools]" (CONFIRMED)

A non-404 error fetching an actual records a fetchError but leaves the key out of actual (src/engine/build.ts:49-57); computePlan then emits a create noted recreate (plan.ts:127-137) or a stale — prune state no-op (:166-177) — indistinguishable from a real 404. ct plan renders the factually wrong label with only a generic INCOMPLETE warning appended (commands/plan.ts:40-52); only apply aborts.

Failure: a transient 500 makes plan advise pruning/recreating a healthy resource; a future buildPlan caller that forgets the fetchErrors guard would POST a duplicate.

Fix: thread fetch-failed keys into computePlan (e.g. an unresolved set) so those resources render as ? <key> — fetch failed (500) and are excluded from create/stale classification.

3. Permission plan not re-resolved after recreate — grants written with stale dataIds (CONFIRMED)

buildPermissionPlan resolves scope keys → dataIds from pre-apply state (commands/apply.ts:100), but applyPermissionPlan replays those tuples after executePlan has upserted new ids (:149 → :159). A scope-target group recreated during the same apply gets its grant PUT with the old, dangling id; self-heals only on the next full apply.

Fix: re-resolve scope dataIds (and domainIds, once #25 makes them reference-based) against the post-execute state just before applyPermissionPlan — the same re-resolution point the scope-bootstrap-deadlock fix needs.

4. Unguarded res.json() on 2xx responses (PLAUSIBLE)

204 is handled (src/api/ctClient.ts:103-104), but any other 2xx with an empty/non-JSON body hits the bare await res.json() (:106; safeBody only covers !res.ok). All DELETE call sites (permission grants, hierarchy edges, rulesets) route through request().

Failure: a 200-with-empty-body surfaces as a raw SyntaxError: Unexpected end of JSON input naming no method/path/resource, aborting apply mid-run with no clue which request succeeded.

Fix: guard on content-length/content-type (or try/catch the parse) and return undefined for empty 2xx bodies; wrap parse failures in CtApiError with method+path.

Acceptance

  • Version below minimum → plan/apply/destroy refuse before any write (test with mocked /info).
  • Plan output distinguishes fetch-failed from vanished; no recreate/stale advice for failed fetches (test).
  • Recreate + scoped grant in one apply writes the grant with the new id (test).
  • Empty 2xx body → clean undefined/CtApiError, never a raw SyntaxError (test).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions