Skip to content

Phase 1A: stable task IDs, runtime schemas, and JSDoc type checking - #64

Merged
hypnoticdata777 merged 7 commits into
masterfrom
claude/phase-1-foundations
Oct 7, 2026
Merged

hypnoticdata777 merged 7 commits into
masterfrom
claude/phase-1-foundations

Conversation

@hypnoticdata777

Copy link
Copy Markdown
Owner

Summary

First of three PRs for Phase 1 (foundations). It covers steps 1-3 and keeps behavior identical except where it fixes a bug. Groups B (pure domain layer, one commit() path) and C (event delegation, E2E expansion) will follow once this is merged.

1. Stable task IDs

  • All 262 starter tasks in config.json now have a permanent id like leasing-001, assigned once by npm run ids:assign. The script only adds missing ids, never changes one, and checks its own output before writing.
  • Saved data, imports, backups, undo and dependencies match by id first, then by _configName, then by the task's aliases (former names). The shared matcher lives in js/taskIdentity.js.
  • Rewording a starter task in config.json no longer orphans saved progress (covered by a real-browser test that rewrites config.json and reloads).
  • A task name the user never edited follows config's new wording. An edited name is kept.
  • blockedBy gains taskId (and still writes configName). Export schema is now v4. v2 and v3 files still import.
  • The import review used to match by display name too, while the import itself did not. Both now use the same matcher.

2. Runtime schemas

  • New dependency-free js/schema.js (works from file://) returns { value, errors[] } with field paths. normalize.js is built on it, and its public API is unchanged.
  • The import review now has a "What will be repaired" list naming the record and field, for example: Work order 3 ("Fix sink") — priority: "urgent!!" is not allowed; reset to "medium". It also reports task rows that match nothing and unknown departments. The list is capped at 100 with a count of the rest. Hostile values inside those messages are escaped, with an e2e test.
  • Sync: js/syncEnvelope.js checks the server's reply (state, version) so a malformed reply gives a clear message instead of a TypeError.
  • Storage load: repairs are recorded (getLoadIssues()) and logged to the console once. There is no UI change.
  • Server (server/src/validate.js, CommonJS): pushes are rejected with a 400 naming the problem when the state is not an object, a section has the wrong type, a list is oversized, there is a __proto__ key, nesting is deeper than 20 levels, expectedVersion is not an integer, or the passphrase is over 200 characters. Unknown keys are still accepted.

3. Types without a migration

  • typescript is a devDependency. tsconfig.json uses allowJs, checkJs, strict and noEmit. npm run typecheck now runs in CI before the tests.
  • Shared typedefs live in js/types.js. state, storage, stateSchema, normalize, schema, taskIdentity, syncEnvelope, privacy, showcase, demo, demoMode, templates and utils are fully checked, with one commented any.
  • The other files start with // @ts-nocheck — TODO(types). A guard test keeps the checked set from shrinking.

Also in this PR

  • Copy fixes from the live demo: "Compliance needs clearer ownership" (it said "need" for a single lane), and the demo's team card now says "fictional people".
  • Bug found by the type checker: the demo's tenants and vendors lacked documentUrl. It is fixed, with a test.

Review guide

The commits are meant to be read in order:

  1. feat(config) is the 262 ids, with a mechanical config diff of one inserted "id" per line.
  2. feat(identity) is the matcher and where it is wired in.
  3. fix(copy) is the small demo fixes.
  4. feat(schema) (2 commits plus an e2e commit) is the schema layer, import review, sync and server.
  5. feat(types) is mostly JSDoc.

Most of the line count is tests and the config.json ids. The runtime changes outside annotations in the types commit are three behavior-identical tweaks, listed in its message.

Verification

Every fix was written test-first. I also broke each one on purpose (reintroduced the bug) to confirm the tests fail.

Gate Before Phase 1 Now
npm run typecheck (new) n/a clean
Vitest 346 451
Playwright (dev server) 67 73
Playwright (prod build, /pm-ops-map/ and file://) 27 27
Server Jest 19 33
npm audit --omit=dev (root and server) 0 0
npm run build OK OK

Known and deliberately left alone

  • A work order's unusable assignee becomes '', not 'UNASSIGNED'. The sanitizer's fallback is dead code, and it is outside this step.
  • Phase 5 sync hardening is untouched (passphrase minimum, rate limiter, scryptSync, Docker user, CORS default).
  • The 14 open Dependabot PRs are untouched.

Things to look at closely

  • js/taskIdentity.js and applySavedTasks in js/normalize.js, which is the single place saved data meets starter tasks.
  • The "unedited names follow config" rule.
  • The server limits, which are about twice the client's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr


Generated by Claude Code

claude added 7 commits October 6, 2026 19:34
Every task in config.json now carries an id of the form <dept>-<NNN>
(e.g. maintenance-014), assigned once by scripts/assign-task-ids.mjs.

The script only ever ADDS missing ids: existing ids are never changed,
numbering continues after the highest number used in each department, it
edits config.json as text so the diff is one inserted "id" per task line,
and it re-parses its own output and refuses to write if anything other than
ids would change. `npm run ids:assign` / `npm run ids:check`.

Tests: all 262 present, well-formed, unique, not derived from names; script
is idempotent, never reuses numbers, rejects duplicate ids and layouts it
cannot edit safely. Mutation-checked (reassigning existing ids fails 3 tests).

Runtime matching by id comes in the next commits; behavior is unchanged here.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr
…y permanent task id

Step 1 of Phase 1. Renaming a starter task in config.json no longer orphans
anyone's progress.

- js/taskIdentity.js (pure): stampTaskIdentity() records _configName and
  guarantees a unique, safe id at boot (deterministic fallback for custom
  configs without ids); findConfigTask() matches saved rows by id first, then
  by _configName or the task's `aliases` (former names) inside the saved
  department; findBlockerTask() does the same for dependencies.
- applySavedTasks (the one merge point: storage load, import, backup restore,
  undo, sync) and validateImportedState now share that matcher, so the import
  review can no longer promise a match the import won't make (it previously
  also matched by display name).
- A name the user never edited follows config.json's current wording; an
  edited name is kept. Restoring an unedited row puts the starter name back.
- Dependencies: blockedBy gains taskId (configName still written and read);
  moved/renamed blockers keep blocking.
- Export schema v4 adds id per task. v2 and v3 files (no ids) still import.
- localStorage, undo snapshots, showcase and backups carry the id.
- e2e canary payloads include hostile id/taskId values.

Verified: failing tests first; mutation-checked the matcher (id, alias and
department scoping), storage writer, export payload and dependency writer.
A real-browser test rewrites config.json mid-test and reloads.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr
… team card

Spotted on the live demo:
- Risk Queue said "Compliance need clearer ownership" for a single weak lane.
  coverageRiskDetail() now picks needs/need by count (still names at most 3).
- The team card called the demo's fictional staff "real people"; in demo mode
  it now says "fictional people".

Tests first: singular/plural/cap, and real-workspace vs demo label.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr
…ield and record it repairs

Step 2 of Phase 1 (part 1). js/schema.js is a small, dependency-free schema layer
(file://-friendly): field helpers, validateRecord and validateCollection, and
error objects { path, field, code, message } such as
  Work order 3 ("Fix sink") — priority: "urgent!!" is not allowed; reset to "medium".
at path workOrders[2].priority.

- normalize.js keeps its public API and now builds on the layer; team members,
  work orders, properties, tenants, vendors and saved task rows all report
  what they repaired, dropped (missing required field, duplicate, reserved
  name, over the limit) or unlinked (unknown property / department).
  Result objects gain `errors`; kept/dropped/repaired stats are unchanged.
- validateImportedState adds report.issues (+ issueCount, capped at 100) and
  also reports task rows with no matching task and departments this version
  doesn't have. formatImportReport prints them.
- The import review modal shows a "What will be repaired" list (escaped; hostile
  values are shortened to one quoted line).

All 346 pre-existing tests passed unchanged through the refactor. Failing tests
first; mutation-checked (field errors, sanitizer errors in the report, modal
rendering, unmatched-row reporting).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr
…, and pushes on the server

Step 2 of Phase 1 (part 2).

Client
- js/syncEnvelope.js: pull / version / push / 409-conflict replies are checked
  (object, state is an object, version is a safe non-negative integer) before
  use; a malformed reply now gives "The sync server sent an unexpected
  response: …" instead of a TypeError inside the sync loop (a 200 with a
  non-JSON body parsed to null and crashed on remote.state).
- storage.js: every loader records what it had to repair (source, field path,
  sentence) via getLoadIssues() and warns once in the console. No UI change.
- applySavedTasks labels rows by path when asked for issues.

Server (CommonJS, minimal)
- server/src/validate.js: state must be an object with the right section
  types, bounded list sizes, no __proto__ keys, depth <= 20, optional schema
  name/version checks; expectedVersion must be a whole number; passphrase <= 200
  chars. 400 with a message naming the problem; nothing is stored. Unknown keys
  stay allowed. Minimum passphrase length etc. are untouched (Phase 5).

Tests first for each; mutation-checked (envelope parser, storage recorder,
server state validator and expectedVersion check). Server suite 19 -> 33.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr
…ata/boundary modules)

Step 3 of Phase 1. TypeScript is a devDependency only; the client stays plain
JavaScript with no build step and no migration to .ts.

- tsconfig.json: allowJs, checkJs, strict, noEmit over js/ (tests excluded).
- js/types.js: shared typedefs - Task, Department, Employee, Team, WorkOrder,
  Property, Tenant, Vendor, Portfolio, AuditEntry, BlockedBy, WorkspacePayload,
  Issue, ImportReport (no runtime code).
- Strict and clean: state, storage, stateSchema, normalize, schema, taskIdentity,
  syncEnvelope, privacy, showcase, demo, demoMode, templates, utils, types.
  Untrusted input is `unknown` and narrowed; the only explicit `any` (the
  imported-file view in sanitizeWorkspace) says why.
- Everything else starts with `// @ts-nocheck - TODO(types)`; a test pins the
  list so the converted set cannot shrink and new files are checked by default.
- CI runs `npm run typecheck` before the tests.

Found by the checker: the demo's tenants/vendors lacked `documentUrl` (every
sanitized record has it). A failing test (showcase equals its sanitized self)
came first.

Only runtime edits besides that: utils getLeaseStatus uses end.getTime() -
today.getTime(), formatWODate uses `isoStr ?? NaN`, getOpsProfile parses
`?? 'null'` - each identical in behavior. Mutation-checked: wrong argument
types and a typo'd property are both rejected by tsc.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXAgt2SAabUwyuTA9TbMsr

@hypnoticdata777 hypnoticdata777 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed.

@hypnoticdata777 hypnoticdata777 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed.

@hypnoticdata777
hypnoticdata777 merged commit 4cb3f59 into master Oct 7, 2026
3 checks passed
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.

2 participants