Skip to Content

← All archived runs

Run: remove-dead-lead-fields

run.md

Run: remove-dead-lead-fields

  • branch: claude/remove-dead-lead-fields-v44l7w
  • pr: #808

02_define/output/spec.md

Spec: Remove the lead model's dead fields

  • slug: remove-dead-lead-fields
  • personas: Admin
  • touches: packages/services/src/db/models/lead.ts, packages/services/src/db/migrations/, apps/web/components/service-leads/lead-header/lead-header.tsx
  • complexity: trivial

Problem

The lead schema carries five fields that nothing in the repo reads or writes: attachments (an unbounded embedded array — the schema shape most likely to bloat documents silently), expiresAt, remarks, isApproved, and awaitingApproval. The last is worse than merely dead: the lead header renders an "Awaiting approval" badge off it (lead-header.tsx:139) that can never appear, because no code path ever sets the flag. Every lead document written since the fields appeared carries them as permanent dead weight, and every engineer reading lead.ts has to work out for themselves which of the model's booleans are load-bearing.

This is the demo-data-quality epic's dead-weight strand (audit finding #10, decision D1) — the leanness half of "Refine the bridge" / Q2-2026 Objective 1, Establish Product-Market Fit with Vendor Partners. A demo tenant a vendor is shown around should not be modelled on a schema where a quarter of the lead's flags mean nothing.

Proposed change

Delete attachments, expiresAt, remarks, isApproved and awaitingApproval from the lead model — the Mongoose schema, the ILeadPopulated interface they flow through, and the now-orphaned ILeadAttachment type. Delete the impossible "Awaiting approval" badge from the lead header. Add a $unset migration that strips the same five keys from existing leads documents so stored data matches the schema.

Deadness is established, not assumed: a full-repo git ls-files | xargs grep finds no reader or writer for any of the five outside lead.ts and the one badge line. The other isApproved, remarks and attachments matches in the repo belong to invoice, proposal and the @sustentus/ui prompt-input component respectively, and are untouched by this run.

The migration follows the sweep shape of 1784400000000-remove-service-fee.ts: operate through connection.collection("leads"), not the app schema, and deliberately not tenant-scoped — these are platform-wide dead keys being retired for every tenant at once, global by design rather than by omission. down cannot be a true inverse: $unset discards the stored values and the fields have no live writer to recreate them, so down is a documented no-op rather than a restore that would invent false defaults on documents that never carried the keys. The migration's header comment says so in those terms.

Acceptance criteria

  • attachments, expiresAt, remarks, isApproved and awaitingApproval no longer appear in packages/services/src/db/models/lead.ts — in the LeadSchema definition or the ILeadPopulated interface — and the orphaned ILeadAttachment interface is gone with them.
  • A new timestamped migration under packages/services/src/db/migrations/ $unsets exactly those five keys from the leads collection, untenant-scoped, with a header comment stating why the change is irreversible, and a down that throws rather than reporting a rollback it cannot perform. (Corrected at Verify: this criterion originally asked for a silent no-op down, which contradicts the non-negotiable rule in packages/services/AGENTS.md / the db-migration skill — "if a change is truly irreversible, the down must say so loudly (throw with an explanation), not silently no-op". The convention wins; the wording was corrected so the ticked box is honest.)
  • The awaitingApproval flag branch is gone from apps/web/components/service-leads/lead-header/lead-header.tsx:139; the "Active project" flag and every metadata chip on that header render exactly as before.
  • No other lead field is touched — in particular isActive, bidPool, isActiveProject, isBrdApproved and isRejected survive this run unchanged.
  • invoice.isApproved, proposal.remarks and the @sustentus/ui attachments component are unchanged — the field names collide, the fields do not.
  • CI is green: typecheck across the monorepo is the proof that nothing read the removed fields, since a surviving reader is a compile error by construction.

Out of scope

  • The parallel stage booleans (bidPool, isActiveProject, isBrdApproved, isRejected) and isActive. Audit finding #10 flags them as desync risk against the doctrine-correct status string, but they are load-bearing today and consolidating them into the workflow engine needs its own scope (deferred whole-scope in the epic breakdown).
  • Any change to leads indexes — including the { status: 1, isActive: 1 } index the audit questions. That belongs to hot-path-tenant-indexes (stub 2 of this epic).
  • The equivalent dead clusters on other models — user (remove-dead-user-fields), and counter / invoice / activity / notification / skill / status-history (remove-dead-misc-schema).
  • Backfilling or replacing the removed fields with a working equivalent. Nothing consumed them; an attachments feature on leads, if ever wanted, is a product decision and a fresh scope.

Open questions

  • none

03_build/output/notes.md

Build notes: remove-dead-lead-fields

  • commits: feat: remove-dead-lead-fields — drop five dead fields from the lead model, feat: remove-dead-lead-fields — $unset the dead lead fields from stored documents, feat: remove-dead-lead-fields — drop the impossible awaiting-approval badge

What changed

  • packages/services/src/db/models/lead.ts: removed attachments, expiresAt, remarks, isApproved and awaitingApproval from both LeadSchema and ILeadPopulated, and deleted the ILeadAttachment interface that only attachments referenced. The five load-bearing booleans the spec ring-fences (isActive, bidPool, isActiveProject, isBrdApproved, isRejected) are untouched, as is the { status: 1, isActive: 1 } index — that one belongs to hot-path-tenant-indexes.
  • packages/services/src/db/migrations/1786665600000-remove-dead-lead-fields.ts: new migration, raw driver (connection.collection("leads")), one updateMany $unsetting exactly those five keys. Global rather than tenant-scoped, stated in the header as by design.
  • apps/web/components/service-leads/lead-header/lead-header.tsx: removed the awaitingApproval branch that pushed the "Awaiting approval" badge. The "Active project" flag, the chip list and the render below are unchanged.

Deviation from the spec — down throws, it does not no-op

The spec's second acceptance criterion asked for "a header comment stating why down is a no-op and cannot restore the values". I implemented a throwing down instead, because packages/services/AGENTS.md and the db-migration skill make this non-negotiable: "If a change is truly irreversible, the down must say so loudly (throw with an explanation), not silently no-op." A silent no-op reports a successful rollback that restored nothing.

The reasoning the criterion actually wanted is still in the header comment, and it is now stronger: remarks, expiresAt and attachments held free-form data that no default approximates, and isApproved/awaitingApproval defaulted to false, so a restoring down could only $set them onto every lead in the collection — fabricating the field on documents that never carried it. The header points at a collection backup as the real recovery path.

If the literal no-op is preferred over the repo rule, it is a three-line change to the down body.

Acceptance criteria status

  • Five fields gone from LeadSchema and ILeadPopulated, ILeadAttachment deleted with them — git ls-files | xargs grep for awaitingApproval and ILeadAttachment now returns nothing outside .icm/ prose.
  • Timestamped migration $unsets exactly those five keys from leads, untenant-scoped, with the header comment — see the deviation above: down throws rather than no-ops.
  • awaitingApproval badge branch removed from lead-header.tsx; "Active project" and every metadata chip untouched.
  • No other lead field touched — isActive, bidPool, isActiveProject, isBrdApproved, isRejected all still present in schema and interface (grep-confirmed).
  • invoice.isApproved, proposal.remarks and the @sustentus/ui attachments component are not in the diff at all — three files changed, none of them theirs.
  • CI green — pending; the Quality run on this push is the proof.

Notes for Verify

  • The down deviation above is the one judgement call in this diff — worth a look first.
  • No tests written, deliberately. The criteria are "field absent from the schema" (which tsc proves, an excluded class per CONVENTIONS.md → Testing → What not to test), a raw-driver migration (needs Mongo; only the unit tier is configured, and no migration in the package has a test), and a badge removal in apps/web page markup (also excluded). Nothing here is unit-assertable without improvising a tier the config does not have.
  • The migration cannot be run from the agent sandbox (TCP 27017 is blocked); it applies on merge to main via .github/workflows/db-migrate.yaml. It has not been executed anywhere yet.
  • lead-header.tsx's flags array keeps its "secondary" | "outline" variant union although only "secondary" is now constructed. Narrowing it is beyond the criteria and changes nothing observable, so I left it.

04_verify/output/verify.md

Verify: remove-dead-lead-fields

  • production-readiness: run (required — the diff touches the database). One real finding (docs drift, fixed on branch); three record/wording fixes, all applied. No env, auth, tenant, notification or index surface touched.
  • code-review: low (spec complexity: trivial). The CI Claude review job (Review diff against CONVENTIONS.md) is skipped on this PR, so it was not available to triage — /code-review run instead, per the contract. One finding (the same docs drift), fixed.
  • security-review: not required — the diff touches no auth, payments, PII or route policies. It removes five never-read fields from one schema, adds one $unset migration, and deletes one badge branch. No ROUTE_POLICIES entry, no server action, no API/cron/webhook surface, no new user-facing string, no HTML render path (proposal.remarks is the sanitized one — different model, untouched).
  • playwright: TODO — manual DoD smoke performed instead.

A note on the first code-review run. It initially reviewed against the stale local main ref rather than the run's true base, and returned two findings in demo-data/index.ts and demo-reset/index.ts — files not in this diff. Those were discarded and the review was re-run scoped to PR #808. The same stale-base trap hit the initial git diff main...HEAD (309KB across the whole .icm migration); the real diff is origin/main...HEAD, 3 code files. Recording it because the next run on a branch cut straight from main will hit it too.

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

  • Five fields gone from LeadSchema + ILeadPopulated, ILeadAttachment deleted — demonstrated: Quality Project typecheck green across the monorepo, which is what makes a surviving reader a compile error; plus repo-wide grep returning nothing outside .icm/ prose. (agent)
  • Migration $unsets exactly those five keys from leads, untenant-scoped, down throws — demonstrated: the Migrate preview database check ran the migration for real against the preview DB and passed. This is execution, not compilation. (agent)
  • awaitingApproval badge branch gone from lead-header.tsx — demonstrated: the branch is absent from the diff's post-image, and the web Vercel preview deployed Ready. (agent)
  • No other lead field touched — demonstrated: isActive, bidPool, isActiveProject, isBrdApproved, isRejected all still present in both schema and interface; the { status: 1, isActive: 1 } index is untouched. (agent)
  • invoice.isApproved, proposal.remarks, @sustentus/ui attachments unchanged — demonstrated: none of the three appears in the diff; all 3 changed code files are the intended ones. (agent)
  • CI green — all 14 check runs pass, including Quality Project (format/lint/typecheck/test) and Audit database. Two are skipped by design: Migrate production database (main only) and the Claude review job. (agent)
  • auth: not applicable — no auth surface in the diff; no sign-in path affected. Not requested of the operator, because there is nothing here for a sign-in to exercise.
  • payments: not touched.
  • notifications: none expected — the diff adds and removes no notification.
  • Lead header renders correctly for a signed-in persona — NOT VERIFIED ON THE PREVIEW. See below; this is the one gap in this pass.

The one honest gap in this pass

Every acceptance criterion is demonstrated above, but no human has loaded a lead detail page on the preview. The agent has no preview credentials, and the lead header is behind sign-in.

I judge this low-risk rather than zero-risk, and want the distinction on the record: the removed branch was guarded by lead.awaitingApproval, a flag with no writer anywhere in the product, so the badge provably never rendered and its removal cannot change what anyone sees. The rest of the header — every metadata chip and the "Active project" flag — is untouched code, and apps/web compiled and deployed green. So the failure mode this check would catch (a broken header render) would have to come from the two removed lines alone, which are a self-contained if block.

If you want it closed properly, the operator check is: open the web preview, sign in as Admin, open any lead detail page, confirm the header chips render and no "Awaiting approval" badge appears. One minute. I have not marked it verified.

Findings & cleanup

  • apps/docs documented two fields this PR deletesapps/docs/app/business/service-journey/lead-intake/page.mdx:75-76 listed attachments and expiresAt in the lead Key Fields table. apps/docs is the product source of truth, so after merge it would have described two fields existing in neither the schema nor any document. Both rows deleted on this branch. Found independently by both the readiness pass and the code review.
  • The ticked acceptance criterion asserted something false — the spec and PR body required a down that is "a no-op"; the implementation deliberately throws, per the non-negotiable rule in packages/services/AGENTS.md and the db-migration skill. Both reviews independently confirmed the implementation is right and the spec was wrong. Rather than leave a ticked box asserting a no-op, I corrected the criterion's wording in spec.md (with the reason inline) and reconciled the PR body. This edits a spec that was already approved — the behaviour is unchanged and only the wording moved, but it is Jamie's call at the merge gate whether that correction stands or the run goes back through /pipeline define.
  • Rollback path was documented for the migration but not for a code revert — added to the PR body: forward-only migration, recovery is a leads collection backup, and a code revert is safe only while no reader exists (reverting restores boolean declarations that pre-existing documents would read as undefined, harmless precisely because nothing reads them).
  • PR "Steps to test" was template boilerplate — replaced with what was actually exercised (CI typecheck, the preview migration run) and the one operator check still outstanding.
  • Accepted, not fixed: up uses updateMany({}, …) where the sibling 1784400000000-remove-service-fee.ts filters { $exists: true }. $unset on an absent key writes nothing and neither form can use an index, so the filter buys nothing — and the migration has already applied to preview, after which AGENTS.md forbids editing it. Fix-forward only, and there is nothing to fix.
  • Accepted, not fixed: flags in lead-header.tsx keeps its "secondary" | "outline" union though only "secondary" is now constructed. Narrowing it is beyond the criteria and changes nothing observable.

05_ship/output/investor-update.md

Lead records now carry only fields the product actually uses

Who it's for: Admin What shipped: Five never-used fields — including an unbounded attachments array — and an impossible status badge are gone from the lead model and every stored lead. Why it matters: A leaner data layer under the demo tenant, supporting Q2-2026 Objective 1 — Establish Product-Market Fit with Vendor Partners.

Evidence: the migration ran green on preview.

Dig deeper: https://github.com/sustentus/sustentus/pull/808

05_ship/output/release.md

Ship: remove-dead-lead-fields

  • pr: #808 · merge: authorised — Ready to merge ticked by Jamie; this commit rides the squash
  • CI: green — all 10 check runs pass, including Quality Project (format/lint/typecheck/test), Migrate preview database and Audit database. Two skipped by design: Migrate production database (main only) and the Claude review job.
  • technical docs: no technical docs impact — the change alters no app, package, route, env var, build step or architecture. (turbo.json globalEnv untouched: the diff reads no process.env.)
  • business docs: apps/docs/app/business/service-journey/lead-intake/page.mdx — the lead Key Fields table listed attachments and expiresAt; both rows removed. Done during Verify, when the readiness pass and the code review each found it independently, rather than left to this stage. No other page documents the removed fields — the remaining isApproved reference at business/service-journey/invoicing/page.mdx:66 is the invoice field and is correctly untouched.
  • release notes: ship-note only — no end-user note. Nothing here is observable to a user: the five fields had no reader or writer, and the removed badge was guarded by awaitingApproval, a flag nothing ever wrote, so it could never render. A changelog entry would announce a change no one can see. Recorded per the contract's infra/internal branch.
  • sent: ship note queued to #product-update on merge (2026-08-14) — ship-note.yaml fires on the squash and sends investor-update.md verbatim.

Acceptance check (vs spec)

  • attachments, expiresAt, remarks, isApproved, awaitingApproval and the orphaned ILeadAttachment gone from lead.ts — verified in Verify: monorepo typecheck green (a surviving reader would be a compile error) plus repo-wide grep clean outside .icm/ prose.
  • Timestamped migration $unsets exactly those five keys from leads, untenant-scoped, with the header comment; down throws rather than reporting a rollback it cannot perform — verified in Verify: Migrate preview database executed it green against the preview DB.
  • awaitingApproval badge branch gone from lead-header.tsx; chips and the "Active project" flag unchanged — verified in Verify against the diff; web preview deployed Ready.
  • No other lead field touched — isActive, bidPool, isActiveProject, isBrdApproved, isRejected all intact, as is the { status: 1, isActive: 1 } index.
  • invoice.isApproved, proposal.remarks and the @sustentus/ui attachments component unchanged — none appears in the diff.
  • CI green — all check runs pass.

Carried forward, not resolved here

  • One DoD line was never demonstrated on the preview: no human loaded a lead detail page while signed in. verify.md records it unticked with the reasoning (the removed branch was guarded by a flag with no writer, so the badge provably never rendered). Ship does not close it, and the merge does not make it verified.
  • The spec's second acceptance criterion was reworded at Verify — it originally demanded a silent no-op down, contradicting the non-negotiable rule in packages/services/AGENTS.md. The wording was corrected so the ticked box is honest; the behaviour never changed. Recorded so the merged spec's edit history is not silent.