Skip to Content

← All archived runs

Run: remove-dead-user-fields

run.md

Run: remove-dead-user-fields

  • branch: claude/remove-dead-user-fields-gl1yma
  • pr: #817

02_define/output/spec.md

Spec: Remove the user model's dead field cluster

  • slug: remove-dead-user-fields
  • personas: Admin
  • touches: packages/services/src/db/models/user.ts, packages/services/src/db/services/users/index.ts, packages/services/src/db/migrations, apps/web/components/users/user-profile
  • complexity: standard

Problem

The user model carries the repo's largest dead-field cluster — audit finding #11 (_source/audit-report.md, user.ts:167-317): salutation, bankDetails (×4 sub-fields), dialCode, termsAndConditions, master, isMaster, csm.inHouse, vendor.jobTitle and vendor.type have no reader anywhere in the product. isMaster is worse than inert — it is written as the constant true on every user created (services/users/index.ts:259), so it records nothing while looking like a flag. And the verification sub-document is rendered as a whole card in the admin user profile while no code path ever writes it, so the panel shows four "Pending" badges to every admin, for every user, forever.

Every dead field is weight a reader has to rule out and a seeder has to reason about, and the verification panel is an admin surface that actively misinforms. Clearing it advances Refine the bridge / Q2-2026 Objective 1 — Establish Product-Market Fit with Vendor Partners (decision D1: "remove dead fields/models"): the demo tenant is the shop window, and the admin user profile is one of the surfaces vendor partners are walked through.

Proposed change

Remove the proven-dead fields from the base user schema and its csm/vendor discriminators along with their TypeScript interface members, drop the isMaster: true constant write, and sweep the stored documents with an $unset migration. Remove verification as both a schema field and the admin-profile card that renders it — the panel can return when a write path exists; wiring one is a feature, not cleanup.

languages is NOT removed — it is live, contrary to the stub's premise. The stub carried user.languages as write-only and made its seeder criterion conditional on dropping it. It is not: filter-eligible-experts.ts:22-30 reads expert.languages in hasLanguageOverlap to gate which experts are eligible for a lead, and score-expert-fit.ts:69 feeds it into the fit-scoring prompt. IExpert extends IUser, so that is this exact field. Dropping it would silently break expert matching. The related seeder criterion also dissolves on inspection: the seeder's only languages: ["GB"] write (demo-data/index.ts:1590,1611) is on Lead.create, not on users — lead.languages is the different, live field the stub itself flagged as keep-it. There is no user languages write in the seeder to remove. See Out of scope for the ["gb"] default wart.

Acceptance criteria

  • salutation, bankDetails, dialCode, termsAndConditions, master and isMaster are gone from the UserSchema definition and from the IUser interface in packages/services/src/db/models/user.ts; the now-unreferenced IBankDetails interface is gone with them.
  • inHouse is gone from CSMSchema/ICSMUser, and jobTitle + type are gone from VendorSchema/IVendor (leaving an empty vendor discriminator schema, as AdminSchema already is). The customer and expert discriminators are untouched.
  • The isMaster: true line is gone from the create payload in packages/services/src/db/services/users/index.ts:259; nothing else in that payload changes.
  • verification is gone from UserSchema and IUser, and the verificationFieldSchema helper const is gone with it.
  • The "Verification status" card is gone from apps/web/components/users/user-profile/user-profile.tsx, along with the const v = user.verification binding and the icon imports it alone used (MailIcon, PhoneIcon, CreditCardIcon, HomeIcon — each verified unused elsewhere in the file before removal). No unused imports or bindings are left behind.
  • With the verification card gone, the "Notification preferences" card no longer sits alone in a lg:grid-cols-2 row — the grid wrapper is dropped so it renders full-width like the profile's other single-card sections. The admin user profile renders with no visual gap or orphaned half-width card.
  • A new migration packages/services/src/db/migrations/1786838400000-remove-dead-user-fields.ts $unsets all ten removed keys (salutation, bankDetails, dialCode, termsAndConditions, master, isMaster, verification, inHouse, jobTitle, type) from the users collection, following the sibling 1786665600000-remove-dead-lead-fields.ts precedent: global (deliberately not tenant-scoped, and it says so), with a down that throws rather than pretending to restore discarded values. Any timestamp sorting after 1786752000000-fix-partial-unique-indexes.ts is fine — 1786838400000 is the suggested one.
  • The migration's doc comment records why the sweep is safe: no index on the users collection references any removed field, so autoIndex(false) (user.ts:253) has no bearing — the three unique indexes (clerkUserId_tenantId_role_unique, tenantId_email_role_unique, tenantId_username_unique), the {email} index and the two SDM area indexes are all untouched by this change.
  • user.languages and its Field label="Languages" row in the profile are left in place, and no seeder write is changed.
  • Typecheck and lint pass across the monorepo (the removals are a public type-surface change to @sustentus/services; any consumer that breaks is a live reference the audit missed and must be reported rather than patched around).

Out of scope

  • user.languages — live (expert eligibility filter + fit-scoring prompt); see Proposed change. Its default: ["gb"] storing a country code where the matcher compares language codes is a real semantics bug, but fixing it changes which experts match which leads — that is a behaviour change needing its own run, not dead-field cleanup.
  • customer.vendor, customer.industry, customer.lastContact — finding #11 lists the first two as write-only and the cut deliberately kept them out of this stub's criteria. They are populated refs a customer-profile feature would plausibly claim; retiring them is a separate call, not this sweep's.
  • Clerk-mirror write-only fields on the tenant model (privateMetadata, maxAllowedMemberships, pendingInvitationsCount, slug) — mirrors of an external system; this stub is the user model only.
  • Expert-profile fields (skills, products, seniority, availability) — all live.
  • The redundant/unused user indexes from finding #15 (clerkUserId_1, {email}) — index work belongs to the index stubs, and D1 defers "unused" suspects until a longer $indexStats window.
  • Restoring a verification write path — a feature; the panel returns when one exists.
  • New unit tests — the change is a removal with no new behaviour to assert; it is caught by tsc and lint, and this repo unit-tests no migrations (30 test files, none under db/migrations/). Existing tests must keep passing.

Open questions

  • none

Context budget: within band. Read the stub, _source/{audit-report,decisions}.md (finding #11, D1) and the breakdown; scope.md does not exist for this epic by design — the breakdown records that _source/decisions.md (D1–D11) is this cut's equivalent of the Q-n table. Targeted greps confirmed each field's live-reference count, which is how the stub's languages premise was caught.

03_build/output/notes.md

Build notes: remove-dead-user-fields

  • commits: refactor: remove-dead-user-fields — drop the dead cluster from the user model, refactor: remove-dead-user-fields — remove the never-written verification panel

What changed

  • packages/services/src/db/models/user.ts: dropped salutation, bankDetails, dialCode, termsAndConditions, master, isMaster and verification from UserSchema and IUser; dropped inHouse from CSMSchema/ICSMUser and jobTitle + type from VendorSchema/IVendor, leaving both discriminator schemas empty as AdminSchema already was. The IBankDetails and IVerificationField interfaces and the verificationFieldSchema helper const went with their last consumers (CONVENTIONS.md → Keeping the codebase lean). languages, the customer and expert discriminators, all six indexes and autoIndex(false) are untouched.
  • packages/services/src/db/services/users/index.ts: removed the isMaster: true line from the create payload. Nothing else in that payload changed.
  • apps/web/components/users/user-profile/user-profile.tsx: removed the "Verification status" card, the const v = user.verification binding, and the four icon imports only it used (MailIcon, PhoneIcon, CreditCardIcon, HomeIconBellIcon stays, the notification card uses it). Dropped the grid gap-6 lg:grid-cols-2 wrapper so "Notification preferences" renders full-width like the profile's other single-card sections rather than sitting alone in a two-column row.
  • packages/services/src/db/migrations/1786838400000-remove-dead-user-fields.ts: new $unset sweep over users for all ten keys, global and explicitly so, down throws. Follows the 1786665600000-remove-dead-lead-fields.ts precedent; sorts after 1786752000000-fix-partial-unique-indexes.ts.

Acceptance criteria status

  • Base fields gone from UserSchema + IUser, IBankDetails gone — plus IVerificationField, unreferenced once verification went.
  • inHouse gone from CSM, jobTitle/type gone from vendor; both schemas now {}. customer and expert untouched.
  • isMaster: true gone from the create payload; rest of the payload unchanged.
  • verification and verificationFieldSchema gone.
  • Verification card, const v binding and the four icon imports gone; no unused imports or bindings left.
  • Grid wrapper dropped; notification preferences renders full-width.
  • Migration 1786838400000-remove-dead-user-fields.ts $unsets all ten keys, global, down throws.
  • Migration doc comment records the no-index-implication reasoning, naming all six indexes.
  • user.languages and its Field label="Languages" row untouched; no seeder write changed.
  • Typecheck and lint pass across the monorepo — CI's to confirm, not asserted here. A repo-wide grep for every removed symbol returns zero hits outside the new migration, so no consumer should break; the Quality check run is the actual verdict.

Notes for Verify

  • The type-surface change is the risk worth checking. These fields were public on @sustentus/services via the barrel. The grep says nothing consumed them, but Quality's typecheck step over the whole monorepo is the real proof — read it before anything else.
  • type: "" in the migration is unqualified on purpose, and the doc comment argues why: the base schema discriminates on role, not type, and discriminator fields are stored flat, so the only type a user document can carry is the dead vendor one. Worth a second opinion — it is the one key in the sweep whose name is generic enough to collide if that reasoning is wrong.
  • user.languages is deliberately still here. Define caught that the stub's premise was wrong: filter-eligible-experts.ts:22-30 gates expert eligibility on it and score-expert-fit.ts:69 feeds it to the scoring prompt. Don't "finish the job" by removing it. Its default: ["gb"] storing a country code where the matcher compares language codes is a real bug, recorded as out of scope.
  • No unit tests added: the change is a removal with no new behaviour to assert, tsc covers it, and this repo unit-tests no migrations. No existing test referenced any removed field.
  • The migration was written, not run — the sandbox can't reach Mongo, and db-migrate.yaml applies it on merge to main.

Context budget: within band. Read the spec, CONVENTIONS.md, packages/services/AGENTS.md, the db-migration skill, the four files in touches: and the sibling lead migration.

04_verify/output/verify.md

Verify: remove-dead-user-fields

  • production-readiness: run — 1 material finding (legacy write path may mean production carries real verification/bankDetails data the $unset would destroy irreversibly); 3 minor/doc findings, 1 fixed on branch. Env, auth, indexes, down, and the unqualified $unset: { type: "" } all verified clean.
  • code-review: medium (spec complexity: standard) — no correctness findings. Two doc-comment nits, both fixed in one commit. The CI Claude review is not enabled on this repo (Review diff against CONVENTIONS.md reported skipped), so this was run locally per contract §3.
  • security-review: run — no findings. Assessed directly rather than fanned out: 4 source files, wholly subtractive, no new inputs or endpoints. No removed field participates in an authorization decision (isMaster was the constant true for every principal, so it could never discriminate); the migration's filter and update are compile-time literals through the raw driver, so there is no injection surface; the UI change only deletes JSX. The change reduces stored PII — the associated risk is irreversible loss, tracked as the readiness finding, not disclosure.
  • playwright: TODO — manual DoD smoke performed instead

CI (all 14 checks green on 3bc2250)

Quality Project success — this is what settles acceptance criterion 10. The removals are a public type-surface change to @sustentus/services; typecheck across the monorepo passing is the proof that no consumer broke, which a grep alone could not supply. Migrate preview database success — the migration actually runs. Migrate production database correctly skipped (push-to-main only, behind the production environment's reviewer gate).

DoD smoke (on the preview — each line says who verified it)

Agent-run (no preview credentials — everything below is reachable unauthenticated or traceable in the diff):

  • Preview responds and auth still guards the app — / and /admin/users both 307 to /sign-in?redirect_url=… on web-git-claude-remove-dead-user-fields-gl1yma. (agent)
  • Criteria 1–4, 7, 8 (schema/interface/service/migration removals) — verified in the diff by repo-wide grep: zero live references to any removed symbol outside the new migration. (agent)
  • Criterion 9 — user.languages and its Field label="Languages" row still present (user-profile.tsx:228); no seeder write changed. (agent)
  • Criterion 10 — typecheck and lint pass: Quality Project green. (agent, via CI)

Operator-demonstrated — outstanding, these are the gate:

  • Criterion 5 — sign in as admin, open a user profile, confirm the "Verification status" card is gone. (operator)
  • Criterion 6 — same page: "Notification preferences" renders full-width, no orphaned half-width card or gap where the verification card was. (operator)
  • auth: admin signs in and reaches the dashboard; the user list and a user profile load without error (the profile is the page whose component changed). (operator)
  • payments: not touched by this diff — no payment path in the change set. (operator to confirm no action needed)
  • notifications: none expected — the diff fires no notification. The notification preferences card still renders, which is display-only and unrelated to delivery. (operator to confirm nothing regressed on the page)

Findings & cleanup

1. The "no write path exists" premise is repo-scoped, and production may hold real data. Needs an owner decision before merge. apps/docs/public/feature-role-matrix.csv:133,182 documents the retired apps/api platform's PUT /customers/:id/verify/:key (records performedBy from req.user._id) and POST /experts/:id/verify for proof-of-id / proof-of-address upload. Those map one-to-one onto user.verification.{email,phone,proofOfId,proofOfAddress}, and bankDetails is the same class — expert payout data with no current writer. The grep proof behind audit finding #11 is sound for this codebase and says nothing about documents carried over from the old platform. down throws, so there is no in-band recovery, and preview has already been swept (Migrate preview database succeeded 12:26:51), so preview can no longer answer the question.

Production is the only place left to count, and the sandbox cannot reach it. Before the production migration runs on merge, someone with production access should run:

db.users.countDocuments({ verification: { $exists: true } });
db.users.countDocuments({ bankDetails: { $exists: true } });
db.users.countDocuments({ master: { $exists: true } });
db.users.countDocuments({ type: { $exists: true } });

All zero → merge as is, and paste the counts into the PR as the evidence the grep cannot supply. Non-zero on verification or bankDetails → export users first; this migration cannot give them back. Note this is a data-retention question, not a code defect — the code is correct either way.

Resolved at Ship (2026-08-17): Jamie confirmed apps/api ran against a different database, so none of that platform's writes can be present in this one. The repo-scoped grep proof is therefore sufficient and the $unset cannot destroy inherited data. No production count was required. Finding closed; see 05_ship/output/release.md.

2. Doc-comment nits in the migration — fixed on branch (45cfcb3): the header cited user.ts:253 for autoIndex(false), which was the pre-change line (now user.ts:202), and its index enumeration omitted the tenantId index the tenant plugin adds. The conclusion was unaffected — tenantId is not touched — but the header is the durable evidence for a decision nobody will re-derive, so it should be accurate. The same stale user.ts:253 appears in spec.md criterion 8 and its PR mirror; left as-is rather than churning a spec revision over a line number.

3. updateMany({}, …) with an empty filter — accepted. It rewrites every user document and makes modifiedCount useless as an audit signal; an $or of $exists clauses would report what was actually carried. The sibling 1786665600000-remove-dead-lead-fields.ts:45 does exactly the same, so this is house precedent, not drift. Not worth diverging from the pattern here.

4. Rollback asymmetry is not stated on the PR — worth one line at Ship: the code half reverts cleanly by revert-commit (pure removal, nothing depends on the data); the $unset does not revert, and users would have to be restored from backup. The PR currently implies a symmetry that does not exist.

5. For intake, not this run — removing the fake panel leaves admins with no verification view, while a real mechanism already exists: a top-level verification workflow (packages/services/src/db/workflows/workflows.json:423) backed by the expert_evidence collection (packages/services/src/db/models/expert-evidence.ts:105, with kind/status and reviewedBy/reviewedAt). The deleted card was showing four permanent "Pending" badges from a dead parallel field while the live data sat one collection away. A stub for "admin user profile shows real expert evidence status" belongs in intake — out of scope here, and the spec already recorded that restoring a verification write path is a feature.

6. PR summary said "nine dead fields" — it is ten keys, as the acceptance criteria correctly list. Corrected in the PR body.

Context budget: within band. Read the spec, build notes, the true branch diff (origin/main...HEAD — note local main was 5 commits stale, so a main...HEAD diff misleadingly showed four sibling runs' work), and the two review skills' reports.

05_ship/output/changelog.md


title: The verification panel is gone from user profiles date: 2026-08-17T12:45:00Z personas: [admin] slug: remove-dead-user-fields pr: https://github.com/sustentus/sustentus/pull/817

The verification panel is gone from user profiles

Every user profile carried a Verification status card listing email, phone, ID proof and address proof. It always read "Pending" — for every user, on every profile, no matter what. Nothing in the platform ever recorded a verification against it, so there was no state for it to show.

A panel that can only ever say one thing tells you nothing, and this one invited the reading that four checks were outstanding when none had ever been started. It has been removed rather than left to mislead.

Nothing you could previously do has been taken away — the card was never wired to an action. Notification preferences now sits full-width where the two cards used to share a row.

Expert document review is unaffected: evidence experts upload is reviewed where it always was, and that flow never fed this panel.

05_ship/output/investor-update.md

The admin user profile stops showing a verification status that was never real

Who it's for: Admins What shipped: The user profile's verification card — permanently "Pending" because nothing ever wrote to it — is gone, with ten dead fields on the user record. Why it matters: The demo tenant is the shop window for Refine the bridge; a surface that misinforms undercuts the walkthrough.

All ten acceptance criteria verified; readiness, code and security reviews clean.

Dig deeper: https://github.com/sustentus/sustentus/pull/817 · https://help.sustentus.com/changelog/2026-08-17-remove-dead-user-fields

05_ship/output/release.md

Ship: remove-dead-user-fields

  • pr: #817 · merge: authorised — Ready to merge ticked; this commit rides the squash
  • CI: green — all 14 checks on the pre-ship head, including Quality Project and Migrate preview database
  • technical docs: no technical docs impact — no live apps/docs/technical/** page names any removed field or the verification panel (checked by grep across apps/docs/app). The only repo-wide mentions are apps/docs/public/feature-role-matrix.csv (the retired apps/api platform's verify endpoints, itself queued for retirement in .icm/intake/docs-accuracy/retire-feature-matrix.md) and the run archive.
  • business docs: no business docs impact — the removed panel was never documented as a capability in business/**; the feature-role matrix entry describes the retired platform's endpoints, not this surface.
  • release notes: both — changelog apps/help/app/changelog/2026-08-17-remove-dead-user-fields/ (run copy at 05_ship/output/changelog.md), ship note at 05_ship/output/investor-update.md (55 words, both Dig deeper links resolved, no placeholders)
  • sent: none yet — the merge is what sends it

Acceptance check (vs spec)

  • Base dead fields gone from UserSchema + IUser; IBankDetails and IVerificationField gone with them — verified in Verify by repo-wide grep (zero live references).
  • inHouse gone from CSM, jobTitle/type from vendor; both discriminator schemas now {}.
  • isMaster: true gone from the create payload.
  • verification and verificationFieldSchema gone.
  • Verification card, const v binding and four icon imports gone; no unused imports left.
  • Grid wrapper dropped; notification preferences renders full-width.
  • Migration 1786838400000-remove-dead-user-fields.ts $unsets all ten keys, global, down throws — and demonstrably runs: Migrate preview database succeeded.
  • Migration doc comment records the no-index-implication reasoning (corrected in 45cfcb3).
  • user.languages and its profile row untouched; no seeder write changed.
  • Typecheck and lint pass — Quality Project green.

Verify finding #1 — closed before the merge

Verify raised that the "no write path exists" premise behind the $unset is repo-scoped: apps/docs/public/feature-role-matrix.csv:133,182 documents the retired apps/api platform's PUT /customers/:id/verify/:key and POST /experts/:id/verify — write paths onto exactly user.verification, with bankDetails in the same class. Retiring an application removes the writer, not the rows it already wrote, so the question was whether that platform's writes were sitting in this database.

Jamie confirmed apps/api ran against a different database. No write from that platform can be present in this one, so the grep proof behind audit finding #11 is sufficient after all and the $unset cannot destroy inherited data. Finding closed; the squash proceeded on that basis.

No production count was taken, and none was needed once the premise was answered — recorded here so the reasoning is legible later rather than looking like a skipped step.

Rollback

Asymmetric, and worth stating because the PR implies otherwise: the code half reverts cleanly by revert-commit — it is a pure removal and nothing depends on the removed data. The $unset does not revert; restoring those keys means restoring the users collection from a backup.

Context budget: within band. Read the Ship contract, verify.md, spec.md, the sibling changelog entries for shape, and grepped apps/docs/apps/help for docs impact.