Skip to content

fix(server): reserve parent/plan schema field keys (TASK-1912)#786

Merged
xarmian merged 3 commits into
mainfrom
feat/reserved-parent-plan-field-keys
Jul 3, 2026
Merged

fix(server): reserve parent/plan schema field keys (TASK-1912)#786
xarmian merged 3 commits into
mainfrom
feat/reserved-parent-plan-field-keys

Conversation

@xarmian

@xarmian xarmian commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

A collection schema field literally keyed parent or plan makes schemaHasField return true, which silently skips fields-JSON parent-link extraction at all three sites (create handlers_items.go:584, PATCH :851, list filter :2147) — disabling subtask linking for that collection with no error anywhere. Nothing prevented creating such a schema. Stage 1 of the IDEA-1746 parent-edge consolidation plan.

Changes

  • Server: collection create rejects (400) any schema field keyed exactly parent or plan; collection update rejects them only when newly added — keys already present in the stored schema are grandfathered so existing workspaces keep working.
  • Server: malformed schema JSON on create/update now returns 400 Invalid schema JSON (previously stored unvalidated — a small contract tightening). PATCH {"schema": ""} is likewise rejected instead of stored verbatim, which previously poisoned the collection into 500s on later item creates (found in review round 2).
  • Web: parent and plan added to RESERVED_FIELD_KEYS so the field editor steers authors away before the 400.
  • CLI/MCP inherit the server-side rejection (both go through the HTTP handlers).

Intentional asymmetry: server matching is exact + case-sensitive, mirroring schemaHasField's plain f.Key == key comparison; the web layer lowercases (stricter). Client-stricter-than-server is safe; "fixing" the server to case-insensitive would reject keys the extraction guard never skips on. There's a test asserting Parent is NOT rejected, with a comment explaining why.

Known limitations

This guard lives at the HTTP handler layer, not in store.CreateCollection. Two server-side paths bypass it by design: store.SeedCollectionsFromTemplate (template seeding calls the store directly — repo grep confirms no shipped template uses these keys) and store.ImportWorkspace (raw-SQL insert; imported bundles round-trip pre-existing schemas unchanged, matching the grandfathering spirit). Revisit if stage 2 of IDEA-1746 touches this area.

Review

3 codex rounds with rotated framing: R1 (plain) CLEAN → R2 (contract lens) found the PATCH empty-string gap, fixed in e5e384f → R3 (pointer-vs-value lens) CLEAN. Converged.

Test plan

  • go test ./... (SQLite) — all packages, including 10 new subtests (reserved-key create/update, grandfathering, case-sensitivity control, malformed + empty-string schema, nil-schema untouched)
  • make lint — 0 issues
  • make test-pg (Postgres) — full suite green
  • web: npm run check (0 errors), npm test (89/89), npm run build

Refs: TASK-1912, IDEA-1746

https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS

xarmian added 2 commits July 3, 2026 04:33
A collection schema field keyed exactly "parent" or "plan" makes the
parent-link extraction sites in handlers_items.go silently skip
fields-JSON extraction, disabling subtask linking with no error
anywhere. Reject newly-added occurrences of these keys on collection
create/update (grandfathering keys already present in a prior schema),
and add them to the web's reserved-key list so authors are steered
away before hitting the 400.
Codex round 2: handleUpdateCollection's validation guard was skipped
whenever input.Schema was a non-nil pointer to "", so a PATCH with
{"schema": ""} stored the empty string verbatim and every later
item-create against that collection 500'd instead of the mutation
being rejected up front. Drop the empty-string carve-out so "" flows
into json.Unmarshal, fails, and returns the existing 400 "Invalid
schema JSON". Omitting the schema field entirely (nil) is unaffected.
@xarmian

xarmian commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Go (SQLite) job failure is the pre-existing internal/server 30-minute suite-timeout flake, not this diff — identical signature failed main's Postgres job at f235a04 (README-only commit), and 5+ of the last 7 main runs are red from it across both job variants. Filed as BUG-1913 with full evidence. Re-running the failed job.

@xarmian
xarmian merged commit e41ed8a into main Jul 3, 2026
4 checks passed
@xarmian
xarmian deleted the feat/reserved-parent-plan-field-keys branch July 3, 2026 07:27
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