WS-27 parity backlog closed · WS-29 multi-tenancy started — see ai-company-brain/HANDOVER.md - #399
Open
vjvarada wants to merge 19 commits into
Open
WS-27 parity backlog closed · WS-29 multi-tenancy started — see ai-company-brain/HANDOVER.md#399vjvarada wants to merge 19 commits into
vjvarada wants to merge 19 commits into
Conversation
…s one
"Every operations cadence is recurring. Without it those live in someone's head
or in ClickUp."
NO SCHEDULER, AND THAT IS FORCED RATHER THAN CHOSEN. §5's non-goals: "A second
automation engine. ADR-028/D6: /workflows is the only engine; WS-27 contributes
events and node types to it." A recurrence worker here would be exactly that.
So the successor is created WHEN A TASK CLOSES — apply_status_transition
already owns that moment, so a task finished from the board, from My work, from
an automation or from a bulk edit all recur identically. A second call site
would be a fifth way to finish a task that forgets to.
What that costs, stated rather than discovered: a series only advances when
somebody finishes the current one. A monthly report nobody closes does not pile
up twelve copies, which is right; a daily stand-up nobody ticks does not appear
tomorrow, which is the honest limitation. Materialising ahead is already
reachable through the engine that owns scheduling — a cron trigger plus the
pm_task node WS-27f added — so nothing here has to be undone to get it.
THE ANCHOR IS PER RULE, because the two answers mean different things. `due`
keeps the schedule: "stock count on the 1st" stays on the 1st however late the
last one was closed, so the series does not drift. `completed` measures the
interval from when the work was actually done: "water the plants every 3 days"
restarts when you water them. A `due` anchor also CATCHES UP — a monthly task
closed six weeks late would otherwise produce a successor already overdue the
moment it appeared — and the missed occurrences are skipped rather than
backfilled, because nobody wants four copies of a stand-up they did not attend.
THE DATE ARITHMETIC is where this is either right or quietly wrong for a year,
so it is pure and each case is one assertion:
- January 31st monthly: the day is clamped at COMPUTATION time and stored as
asked. Storing the clamped value permanently demotes the rule to the 28th
after its first February.
- February 29th yearly: the same shape, once every four years.
- "Every other Monday and Thursday": within a week the rule takes the next
allowed day, and only jumps `interval` weeks when the week runs out. A naive
+14 days alternates between the two days instead of giving both days of every
second week.
- A stand-up at 09:00 stays at 09:00.
Closing a task twice must not spawn twice. A task can cross into `done`
repeatedly — close, reopen to add a note, close again — and every crossing
reaches the same seam. recurrence_spawned_at is the guard, and it is never
cleared: reopening undoes completed_at but does not un-emit a successor that
already exists and may already have been worked on.
Stopping a series keeps the work: the rule is deleted and its tasks detached,
not removed. They are real work, some finished, and a "stop repeating this"
button that swept away three months of completed reports would be the last time
anybody pressed it.
TWO BUGS THE LIVE RUN CAUGHT AND READING COULD NOT:
1. The weekly CHECK passed the very row it existed to reject.
CHECK (freq <> 'weekly' OR array_length(weekdays, 1) >= 1) looks correct and
is not: array_length('{}', 1) returns NULL, NULL >= 1 is NULL, and a CHECK
only FAILS on false. A weekly rule with no weekdays inserted happily.
coalesce(…, 0) fixes it, and a test asserts the coalesce is present because
the hermetic suite has no database to try the expression on.
2. _next_number and _default_status were reimplementations, and one invented a
column (last_number; the real one is last_value). Both replaced by core's own
next_task_number and load_default_status — the same mistake WS-27n had just
been careful to avoid, made two tickets later in the same package.
A third, caught by its own test: int(rule.get("interval") or 1) turns an
explicit 0 into "every 1" — a typo that looks exactly like a save, and one the
database's CHECK would then have refused as a 500 rather than a 422.
In the browser the SENTENCE is the feature. A form of five controls is a shape;
"Every 2 weeks on Mon, Thu, keeping to the schedule" is something somebody can
check before committing to it — shown live rather than on save, because picking
the wrong anchor is invisible until a cadence has drifted for three months. The
occurrence limit reads as what is LEFT rather than the cap, and switching
frequency clears the fields the new one does not use so a stale day_of_month
cannot reappear.
45 hermetic + 27 vitest cases, 31 mutants killed and reverted byte-identical,
39 checks against a real Postgres.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
…e nobody had written down
"pm_task_links and parent_task_id both exist, unreachable from the board. Data
with no surface is a promise the product does not keep."
GET /projects/tasks/{id}/relations, lib/relations.ts, and a relations block in
the task panel. No migration — the tables have been right since 146.
BOTH HALVES WERE UNREACHABLE, AND FOR DIFFERENT REASONS. Links could be created
and deleted since WS-27a but never LISTED: get_task returns a `links` COUNT and
nothing else, so no client could draw one. Subtasks could be created from the
panel but never listed either — ?parent_task_id= has existed on the list
endpoint since WS-27a and nothing called it.
THE RULE NOBODY HAD WRITTEN DOWN: `blocks` may not form a cycle.
assert_no_task_cycle has guarded parent_task_id since WS-27a, and the identical
hazard sat unguarded on links the whole time. A blocks B blocks C blocks A is a
deadlock no human can resolve by finishing something, and every walk over it
runs forever. assert_no_block_cycle closes it, bounded by the same MAX_DEPTH its
sibling uses, and it TRACKS WHAT IT HAS SEEN — data can already contain a loop,
since every link created before the guard existed went in unchecked, and the
walk has to terminate over one rather than spin.
Only `blocks` is guarded. A cycle in relates_to or duplicates is redundant, not
harmful, and refusing one would be a rule with no failure to prevent.
BLOCKED-NESS IS DERIVED AND SHOWN, NEVER ENFORCED. A task is blocked when
something that blocks it is still open, so a blocker reaching `done` makes the
section go quiet — that is how you learn you can start. Refusing to CLOSE a
blocked task is the obvious next step and is deliberately not taken:
dependencies in a real workspace are frequently approximate, and a tool that
will not let somebody finish work they have finished is a tool they route
around — after which the links stop being maintained and the feature is worse
than absent.
VISIBILITY IS APPLIED TO THE CHILDREN, not inherited from the parent. A subtask
can be moved into a project the reader cannot see, and listing it because its
parent is readable would disclose a title from behind a grant. The live run
asserts both that it is absent and that its title does not appear.
One endpoint carries both directions, because `blocks` outgoing means "this
holds those up" and incoming means "this is waiting" — a client given one side
would have to ask twice and would still not know which was which. Blocked by is
shown first, since it is the only section that changes what somebody should do
next, and empty sections are dropped: six empty headings on every task is how a
panel becomes something people scroll past.
Progress counts the status CATEGORY rather than completed_at, for the same
reason everything else in this app does — a project can name its finished lane
"Shipped" or "Signed off", and `cancelled` counts as resolved even though
nothing was completed. It reads as "1 of 3" rather than a percentage, because
33% is a worse answer than "1 of 3" to the question people are asking.
21 hermetic + 16 vitest cases, 11 mutants killed and reverted byte-identical,
19 checks against a real Postgres including a subtask and a link in a project
the reader has no grant on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
vjvarada
changed the title
"the UI, kanban, task cards etc can be taken from the tasks app right?"
Familiar, yes. Taken, no. /tasks' TaskCard is 395 lines bound to
useTaskStore and to GtdItem's own fields — energy, deepWork, disposition —
none of which pm_tasks has or should grow, and D-PM-6 retires gtd_items at
WS-27h, which would take the Projects board with it. What moves is the
VOCABULARY: @/lib/taskCard decides which chips a task earns, what counts as
overdue and what an avatar's letters are; @/components/TaskMeta is the one
file that turns a tone name into a colour. Neither knows about a store.
A card can only show what the list endpoint returns, and it was returning
almost nothing. Links and parent_task_id have been readable since WS-27p —
one task at a time. A board draws them on every card at once, so most of
this is backend: two aggregates over the page's ids fill subtasks
{done,total} and blocked_by_count on every row. Per card that is N+1 across
an imported workspace, and at three-task scale it looks identical.
A finished blocker does not block, and the count says so in SQL rather than
after — a card still marked blocked after its dependency shipped is a card
people learn to ignore. Archived subtasks leave the denominator for the
matching reason: counted, "2/3" could never reach 3/3.
A zero earns no chip; chip order is fixed so the row is scanned, not read.
Overdue changes the icon as well as the tone, so the signal survives a
reader who cannot tell muted from destructive.
One behaviour change outside Projects: /tasks' isOverdue checked only the
date, painting every completed task with a past due date red forever.
Sharing the function fixed that side too.
The fake needed teaching, as it did for WS-27n, and the same lesson applied:
every clause is mirrored only when the statement carries it, and which end
of a `blocks` link is the blocked one is read off the SQL rather than
assumed — a mirror that filters unconditionally agrees with itself no matter
what the route stops emitting.
Verified: 326 backend + 988 frontend tests; ruff and xenon clean; theme
conformance green; next build clean; seven mutants killed (two behavioural
kills for the SQL clauses, not only structural); one equivalent mutant found
and removed along with the test that asserted nothing; live Postgres 16 run
against real endpoint functions all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
vjvarada
added a commit
that referenced
this pull request
Aug 7, 2026
…on number somebody else had claimed Diff-review repair round. Both P1s needed a cursor that had actually been running to see, which is why the first cut passed its own seven mutants. P1-1 — activated_at guarded the wrong thing. It stops a deep resync, and it does nothing at all about a flag that was on, turned off for four weeks, and turned back on: the cursor still carried day-1's anchor, so the first ON cycle minted the entire OFF window in one batch. The reviewer measured 27 of 27, each born zoho_dirty and each pushing unattended into the live tenant. The anchor now means the CURRENT ON epoch: a gap in the step's own runs beyond REANCHOR_GAP_SECONDS (3600, six scheduler periods) re-stamps the cursor, mints nothing from the gap, and says so at WARNING with gap_seconds. Fail-closed both ways — an OFF window and a real outage each skip their backlog, because a missed lead is hand-creatable and visible in the mailbox and 27 unattended tenant pushes are neither.⚠️ DEVIATION, with its reason: dormancy is measured on a THIRD column, last_run_at, not on processed_watermark as ruled. The watermark tracks MAIL, not runs, and the literal predicate has two production failures. A mailbox that is merely quiet over a weekend carries a 60-hour-old watermark while the step has run faithfully every 600s — it would be re-anchored, and Monday's first message, the one this whole feature exists to catch, falls before the new anchor and mints nothing. Every Monday. And a poison head message holds the watermark still ON PURPOSE under P1-2 below; a watermark-based test would re-anchor past it after an hour and quietly undo the stall that was supposed to stay visible. last_run_at is stamped on every cycle including empty and stalled ones, so "quiet" and "not running" stay different facts. The ruling's constant, log key and behaviour on a genuine OFF window are unchanged. Both named tests are in the file, including the control that a quiet-but-running mailbox is never re-anchored. P1-2 — a failure advanced the cursor over the work it lost. This step opens a SECOND session per lead through create_record while holding the batch's own, so pool exhaustion fails many candidates at once; the old code advanced past all of them. Measured: 3 candidates, 3 errors, watermark advanced, three leads gone for good. The watermark now advances over the contiguous PREFIX that actually wrote its leads and stops at the first that raised. Later successes in the same batch are simply re-considered next cycle, which is free — the third unknown-sender step finds the lead they already created. A held cursor logs sync.auto_lead_stalled at WARNING EVERY cycle, because a held cursor and a quiet mailbox both create nothing and only the level can tell them apart; that trades silent loss for a visible stall on a genuinely poison head message, deliberately, fail-closed toward the CRM. The counter-case matters as much: a failed first ACTIVITY does NOT hold the cursor. The lead is already committed, so the message would be skipped on re-consideration and the activity never retried — holding for it would stall forever on work that cannot be redone. It is counted separately as activity_errors, and `created` now increments the moment the lead commits rather than after its activity: the row exists and will push either way, and a cycle logging created=0 beside a queued lead sends whoever reads it looking in the wrong place. P2-4 — the Sent probe inherited @>'s case-exactness. The owner wrote to Asha@AcmeRobotics.com; she replied from asha@acmerobotics.com and was minted as a cold lead. This module now asks with EXISTS over jsonb_array_elements and lower(). _maybe_block_cold is left alone on purpose: it is the email package's predicate with its own blast radius. The fake learned to READ the new shape — and the hard part was stripping it, since its inner lower(recipient->>'email') = :address is exactly what _JSONB_LOWER_CMP matches, which would have filtered the outer rows on a column they do not have. P2-6 — cfo@mail.fracktal.in passed both colleague gates. Gate 2 is now suffix aware, anchored on a leading dot so notfracktal.in is still a prospect. P2-5 — the cap had no tiebreak. ORDER BY rules_processed_at, id; fetch cap+1; and when the cap falls INSIDE a group sharing one timestamp, defer the whole group. Only then — deferring the last group unconditionally would shrink every capped batch by one message for nothing. Latent in production, where the rules runner stamps one transaction per message. Also: lead_name and the activity subject are clipped (120/500, with a marker). Nothing upstream bounds a display name, and it becomes a column every list, board card and Zoho push carries. MIGRATION RENUMBERED 157 → 158. Open PR #399 claims 157, and two migrations sharing a number replay in filename order against the wrong schema. The ladder carries a deliberate reservation gap until #399 lands; the header names it, the test finds the file by content, and the contiguity assertion is replaced by the uniqueness one — which is the property that actually protects the ladder and the only one this branch can hold. The step's import moved inside the gated branch so the OFF state does not load routes/crm on the mail path. The predicate stays above the gate on purpose: auto_lead_enabled is the flag's ONE definition, and reading settings.crm_auto_lead in the hook would make two places responsible for agreeing what the flag means. The import-inside-try divergence from the five sibling steps is now recorded rather than left to be rediscovered. 52 → 73 tests; 7 → 13 mutants red and reverted, one of them precision-checked (reverting the Sent probe must not redden the lower-case already-emailed case, or the mutant broke it rather than narrowing it). _crm_fakes gained fail_on(after=N), because where in a batch a failure lands IS the property under test. Still BUILT, NOT FLIPPED, NOT DEPLOYED. R4 in the same change: the ticket's cursor paragraph rewritten to the epoch-anchor semantics, done-when 8 and 9 added, the as-built block's deviation list grown to seven, plus work_plan's WS-26 row and §6(b) — which now tells an owner that turning the flag off is a stop, not a pause that accumulates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ninth backlog row, and the first view that cannot be paginated. A month
with ninety tasks read at page_size=50 draws forty and leaves the other days
looking EMPTY. A short page announces itself; a short month does not, and
nobody investigates a quiet week. So GET /projects/calendar takes a window,
returns everything in it, and says `truncated` when the cap is reached
rather than handing back a plausible-looking month.
start_date has existed since migration 146 with no surface — the same
complaint §11.14 makes about links, and the reason the calendar was the view
worth building: a task is a bar from its start to its due date, not a dot.
Overlap, not equality. A task starting Monday and due Friday belongs on
Wednesday's cell; `due_at BETWEEN` puts it on Friday alone, which is exactly
the week somebody looks at Wednesday and concludes they are free. A task
with NEITHER date falls out through NULL — correct and invisible — so
`undated` counts them with the same filters and the view says so.
The window is read in UTC and the client asks for a day of slack. A
start_date is a floating date and a due_at is an instant; no single frame
makes both exact, so the server over-selects and the browser places.
start_date is anchored with AT TIME ZONE 'UTC' rather than CAST(… AS
timestamptz), which would silently read the connection's TimeZone — pinned
by a live run with the session set to America/Los_Angeles.
Filters carry across the switch. FastAPI ignores an unknown query parameter,
so a filter the board sends and the calendar does not declare is not an
error — it is a filter that quietly stops applying, which reads as the
filter breaking. A test asserts the calendar's parameter set covers the
list's minus a named, reasoned exclusion list. due_before is excluded
(it bounds the same column as the window); overdue is not, because
"already late" is a fact about the status as much as the date.
No second write path: dragging a card is PATCH /tasks/{id}. Dragging a bar
moves the whole bar and keeps its time of day — writing only the dropped
date leaves the other end behind and inverts the interval the moment you
drag left.
new Date("2026-08-07") is midnight UTC, the 6th west of Greenwich, so the
grid works in YYYY-MM-DD keys throughout. That claim is only behaviourally
testable west of Greenwich, so the suite runs in four timezones AND pins the
rule structurally, because CI runs in one.
Building it found a hole in the fake: overdue's date half (due_at < now())
had never been mirrored, so every overdue test since WS-27k was asserting
only the status half and would have passed with the comparison deleted.
Verified: 605 backend + 1025 frontend tests; calendar grid green in UTC,
Asia/Kolkata, America/Los_Angeles and Pacific/Kiritimati; ruff and xenon
clean; theme conformance green; next build clean; 14 mutants killed across
both halves, one equivalent mutant found and the code simplified rather than
the test kept; live Postgres 16 run all green — and it caught a wrong
assertion of mine about sort order before it reached the suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
…ecisions Owner asked for a timeline view where tasks and subtasks can be made dependent on each other by wiring them together, Jira/ClickUp style, and whether Paca has UI to take. Three findings, written up rather than acted on: 1. We do not have a timeline. Backlog row 9 was "Calendar / timeline view" and WS-27q built the calendar half; closing the whole row was wrong. A month grid answers "what is due when"; bars on a continuous axis answer "what runs alongside what". Row split — 9a closed, 9b open — and §11.16 now says so at the top rather than reading as if both shipped. 2. Gantt was a recorded non-goal. Reversed, struck rather than deleted, because the reversal is the interesting part: the original note treated Gantt as decoration, which is true of the chart and false of what was actually asked for — a surface where a dependency is DRAWN rather than typed. pm_task_links has been cycle-guarded since WS-27p and reachable only through a dropdown and a task number. 3. Paca has the chart and not the wiring. roadmap-view.tsx (438 lines, Apache-2.0) is a real Gantt worth taking geometry from — sticky task column, PX_PER_DAY, month cells, today line, undated tasks listed and unbarred. It draws no dependency arrows (zero matches for arrow/svg/path/depend) and is entirely read-only (zero for drag/resize). So Paca answers the layout; the wiring is ours to design. WS-27t is minted with a table of what is already built (all of it — dates, links, cycle guard, both-direction read, blocked counts, overlap query, the PATCH write path), so the ticket is scoped honestly as rendering and gesture rather than schema. Two decisions, and only one is agent-proposable: - D-PM-11 — what earns a bar. Proposes hierarchy depth over Paca's reserved "Epic" type, because inventing a reserved type name to make a chart legible is a data-model change in service of a rendering problem, and D-PM-2 put types in each project's hands deliberately. - D-PM-12 — does an arrow constrain the schedule or only describe it. OWNER-ANSWER REQUIRED, and registered as WS-27 owner gate (e). It changes what data already in the database means: WS-27p decided in writing that blocked-ness is derived and shown, never enforced, and Jira's auto-push is the opposite — adopting it strikes that paragraph rather than extending it, and turns one drag into an unbounded cascade of real PATCHes each carrying a field_change activity and a notification. Recommendation on the record (warn, do not push); all three options and their costs written out. No code. WS-27t does not start until D-PM-12 is answered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
"a timeline view that can also make tasks and subtasks dependent on each other, with wiring them to each other, similar to how it works on Jira and ClickUp." Two things, and the second is the one that matters — a Gantt with no dependency gesture is decoration, which is why Gantt was a non-goal until it was asked for. Almost none of this is schema work. Dates, links, the cycle guard, the both-direction read, blocked counts and the overlap window all shipped in WS-27a/p/q/s. This is one aggregate, some geometry, and a gesture. The window is the resource, so there is no /projects/timeline: the calendar endpoint grew include_links, and calendar and timeline are two renderings of one question — the rule §11.8 states for list and board, a third time. An edge is returned only when BOTH ends are in the window, because an arrow needs two bars; the edge to an off-window blocker survives as the blocked_by_count badge, which is the honest rendering of "something you cannot see is holding this up". D-PM-11 — hierarchy depth decides what earns a bar. A parent with no dates borrows its children's span, marked derived and drawn dashed, or the default view is blank for the projects that use subtasks properly. A subtask whose parent is off-window is promoted, not hidden — hiding it is how a filtered timeline silently drops work. Paca's reserved "Epic" type was rejected: pm_task_types is per-project data with no reserved names (D-PM-2), so it would need a seeded row or a name-match that breaks on rename. D-PM-12 — an arrow WARNS, never reschedules. The owner delegated the choice; chosen deliberately over Jira's auto-push. The useful half of a dependency is knowing two tasks now disagree. Auto-push delivers that and also silently rewrites other people's dates — a cascade landing in the activity spine as dozens of field_change rows and notifications with no single act to point at, and the first time a negotiated date moves untouched, people stop trusting the dates. WS-27p's "derived and shown, never enforced" therefore stands unamended. Three sub-rules keep the warning worth reading: equal dates are not a conflict (a handover), a missing date is not a conflict (unknowable), and a finished blocker never conflicts. One rule, two surfaces: conflicts() is pure, the timeline colours arrows with it and RelationsBlock writes its sentence with it — which is why /relations grew start_date and due_at. The cycle check is NOT duplicated in the browser; canLink refuses only self-links and duplicates and lets a→b after b→a through so assert_no_block_cycle answers with its own message. Paca gave the layout (sticky column, PX_PER_DAY, month cells, today line, undated listed and unbarred) and nothing else — it draws no arrows and is read-only, so the gesture is ours. Two rounding traps: dayPx rounds because a DST-straddling range is 23 or 25 hours across the transition and an unrounded quotient is permanently a fraction of a day off — only catchable in a DST zone, so pinned structurally too. And a bar covers its last day rather than stopping at that day's left edge, or every span is a day short. Found while building: the fake's mirror of the edge query hard-coded which column was the blocker, so a mutant swapping the SQL's aliases — every arrow backwards — passed the whole suite. It now reads roles and membership tests off the statement; both mutants die behaviourally. Verified: 611 backend + 1074 frontend tests, the frontend green in UTC, America/Los_Angeles, Asia/Kolkata and Pacific/Kiritimati; ruff and xenon clean; theme conformance green; next build clean; 17 mutants killed across both halves, one survivor fixed by strengthening the test rather than accepting it, one over-broad structural assertion narrowed after it tripped on the module's own prose; live Postgres 16 run all green including that an edge from an ungranted task is not drawn while its badge still counts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
The last row of the parity backlog. Both halves of "?q= exists on the list
endpoint; there is no search surface" were true, and the second was worse
than advertised.
`_` and `%` were live wildcards on every search anybody had done.
build_task_filters bound %{q}% raw, so `_` — LIKE's single-character
wildcard — meant `task_id` also matched `taskXid`, and `50%` quietly meant
`50`. In a workspace where people search for identifiers all day that is a
steady drip of hits nobody asked for, and it reads as fuzzy matching rather
than as a bug. like_escape fixes it on the SHARED builder, so the board and
every saved view get the fix — patching only the new endpoint would leave
the bug where people actually meet it.
Why a second endpoint, having twice argued against one: the list answers
"which tasks match these filters, in this order, on this page"; search
answers "what did you mean". It ranks, and TASK_SORTS is a column allowlist
that deliberately cannot express relevance. It is capped, not paged —
nobody pages through search results, and page 2 of a relevance ordering is
where relevance ran out. And it names the project. What decides what a
caller may SEE is still shared, so search can never surface what the list
would hide; that is the part that must not be duplicated.
Ranking is in SQL before the LIMIT: ranked afterwards over a capped set, the
best answer is only present if it was already in the arbitrary fifty rows
returned — a defect that presents as "search is bad at long queries".
The palette is ⌘K, not a page: the question is "where is that task", asked
while doing something else. Four rules that only break under real typing
speed live in lib/search.ts as pure functions — "no results" is never
claimed while a request is in flight, a stale response cannot win, the
arrows belong to the palette unless a modifier is held, and the highlight
needle is escaped before it becomes a regex (the browser twin of the very
defect this ticket fixed on the server).
Found by the live run and invisible to all 43 hermetic tests: `:number IS
NOT NULL` names no column, so Postgres cannot infer the parameter's type and
asyncpg answers AmbiguousParameterError — the query never runs. A Python
fake has no type system to be ambiguous about. Fixed with an explicit
CAST(:number AS bigint) and pinned structurally, the only level at which a
hermetic suite can hold it.
The fake also learned to read LIKE properly: like_to_regex translates %, _
and the backslash escape rather than doing a substring match. A mirror that
treated the pattern as a literal would have agreed with both the escaped and
unescaped implementation, making the defect invisible to the suite that
exists to catch it.
Input gains a `ref` prop (React 19 passes it as an ordinary prop, so no
forwardRef wrapper) — a palette you have to click into is one you stop using.
With this the ClickUp parity backlog in §11.2 is closed: all ten rows plus
the two added since.
Verified: 655 backend + 1106 frontend tests; ruff and xenon clean; theme
conformance green; next build clean; 20 mutants killed across both halves
with none surviving; live Postgres 16 run all green, including that the list
endpoint's own q is now escaped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
vjvarada
changed the title
…chet Owner is planning migrations for a multi-tenant CommandCenter and asked that current work take it into account. Measured the blast radius rather than guessing, and it changes what should happen next. MEASURED, off the migration tree and checked against a live Postgres 16: 143 app tables, SIX carry organization_id (app_user, crm_activities, crm_contacts, crm_deals, org_group, org_role), 137 carry none — including all seventeen pm_*. An `organization` table has existed since migration 130 with one seeded row (slug='default') and app_user.organization_id. Tenancy was started and never carried past access control and the CRM; org_settings says so in its own comment. So this is not a Projects problem. WS-27 is 17 of the 137, alongside every gtd_*, email_*, wa_*, workflow*, app*, chat_* table. The number that decides the cost: app_user.email is globally UNIQUE, so today one person = one organization structurally. D-MT-1 asks whether that stays true. If yes, the tenant is derivable from X-User-Email and the identity seam the whole platform is built on does not change shape. If no, UNIQUE(email) becomes UNIQUE(organization_id, email), every request needs a tenant discriminator, and bare emails stop identifying a person — which reopens D-PM-4's grant and assignee vocabulary. That is a product question about how the SaaS is sold, so it is owner-answer; the recommendation on record is (a) for v1, relaxable later behind an org-switcher. Projects is cheaper to retrofit than its size suggests: 128 FROM/JOIN references to pm_* across 16 modules, but ONE closure query. So it is a column on 17 tables, a predicate in one query, and one line in Visibility — contained, and contained only until real data lands. Which is why this blocks something already queued: the production ClickUp import (§6 gate (a)) is not wrong, it is EARLY. Running it now writes a real workspace into 17 unscoped tables and turns a one-line default on empty tables into a backfill on live rows. Registered on the gate itself, since that is where somebody will read it. Ships one piece of code — tests/unit/test_tenancy_boundary.py — a ratchet on the conformance.test.ts model: the 137 are frozen, a NEW table without organization_id fails, and a baselined table that GAINED one fails until it leaves the baseline so the figure never quietly becomes fiction. It reads ALTER TABLE ... ADD COLUMN too, which is how app_user got its key and which a CREATE-only scan reports as unscoped — it did, in the first version, and the parser was checked against the live schema before this was written. Its job is not to demand the retrofit; it is to stop the number growing while D-MT-1 is answered. Verified: the ratchet bites on all three cases (new unscoped table, fixed baselined table, stale baseline entry) and passes a new correctly-scoped table — which the first version wrongly failed, so that assertion was replaced with one that checks something real. 661 backend tests, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Put to the owner with both options costed; the answer was "go ahead with what you think is right", so the recommendation was taken as the decision. ANSWERED: (a) — app_user.email stays globally UNIQUE, and a request's tenant is derived from X-User-Email via app_user.organization_id. No app's auth seam changes shape. Taken rather than left open because it is the REVERSIBLE direction. (a)→(b) is a migration plus an org-switcher, run once, while accounts are few. (b)→(a) takes a capability away from people already relying on it. Given a delegated choice between a door that stays open and one that closes, the open one wins. The trigger to revisit is named rather than vague: the first customer who needs one human in two organizations — an agency or a consultancy. Deciding it then costs a migration; discovering it after that tenant onboards costs their trust. Unblocks WS-29a, which is what gates the production ClickUp import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
⚠️ I dispatched this against a stale premise and the agent corrected it. D1's extraction was already committed at c1eba71 — `DEPLOY_SCRIPT` appears zero times in deploy.yml, and spec §8.1 says so. I read the WS-25 work-plan row and §3, both of which describe the problem state, and did not read §8. The right response was to verify rather than redo, which is what happened: the pre-extraction env: value was reconstructed by YAML-parsing the old deploy.yml and diffed against scripts/vps_apply.sh — byte-identical, 437 lines, sha256 a779724d089319f6. What was NOT delivered, and is here: The extracted script was not shellcheckable, which is half of what D1 was for. Line 1 was `set -e`, because a YAML env: value fed to `bash -s` has no shell to declare — so shellcheck refused to analyse it at all (SC2148, error, exit 1, nothing checked). It was the only .sh in scripts/ without a shebang, a direct artifact of the extraction. Verified by stashing the fix and watching shellcheck go blind again. With a shebang it runs, and reported exactly one real finding: an unquoted `$(date +%s)` and `$deadline` in the healthcheck wait loop (SC2046). Fixed. `shellcheck scripts/vps_apply.sh scripts/vps_pull.sh` now exits 0 at default severity. `--enable=all` adds ~700 opt-in style notes, declined: rewriting 481 lines of production deploy logic for zero defect is the behaviour change this ticket exists not to make. The file stays non-executable (0644, matching vps_pull.sh). Both callers name the interpreter; +x would advertise a fourth way to start a deploy. And the spec gains a MEASURED table where it had a claim. The self-rewrite hazard was demonstrated, not asserted, against a throwaway repo — and it is worse than §3 described. The naive single-stage script fails three ways, and "executes garbage" is the RAREST: rename (what git reset --hard actually does): the fd keeps the old inode, all 12 steps run — v1's steps against a v2 tree — and it exits 0 in-place, shorter: resumes past EOF, remaining steps silently vanish, exits 0 in-place, shifted: resumes mid-token, `iet: command not found`, exits 127 Two of three exit 0, and git's own method is the quietest: HEAD correct, steps stale, nothing red. No exit-code check and no health probe catches it. The two-stage bootstrap was proven against the REAL scripts/vps_pull.sh, not an imitation: v2's twelve steps, in order, exit 0, markers written. I re-ran the harness myself rather than take the report on trust. Left for the owner, deliberately: all four WS-25 acceptance items remain OWNER-GATE, nothing was switched, and the push path still delivers by `bash -s` from stdin. Reported but not changed: ENV_FILE/WB_ENV are hardcoded to /opt/acb/app while APP_DIR is overridable, so a hand-run with APP_DIR=/sandbox would git-reset one tree and write secrets into the live box's .env — documented in place, since D1's whole point is that this file can now be hand-run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
⚠️ CORRECTION FIRST. This spec, the work plan and a commit message all said SIX tables carry organization_id. It is THREE. `crm_activities`, `crm_contacts` and `crm_deals` carry a column spelled `organization_id` that REFERENCES crm_organizations — a customer company, not the tenant root. Verified against pg_constraint on a live database. The CRM is unscoped like everything else, so the real figure was 140 unscoped, not 137. Two consequences, and the second is worse than the miscount: * the column name is TAKEN. Scoping crm_* needs a rename or a different name, and that has to be decided before WS-29d touches those tables. * test_tenancy_boundary.py — which I wrote and shipped two commits ago — matched on the column NAME. It counted the homonyms as scoped, which means any future table with an `organization_id` pointing anywhere at all would have passed the ratchet silently. A guard satisfiable by a coincidence of naming is not a guard. Fix lands with WS-29a, which currently holds that file. THE AUDIT. Four S1 findings, all of them paths a column-plus-predicate retrofit does not close — because the tenant now lives on `Visibility`, and every one of these is a path that never builds one: 1. The entire admin plane resolves its tenant from a hard-coded slug. get_org_id() does `WHERE slug = 'default'` and ignores the caller; 26 call sites — members, roles, groups, permission overrides, access requests, /auth/me. A tenant-B admin lists, invites into and grants roles in tenant `default`. Cross-tenant WRITE into access control, by a correctly-authorised caller. Confirmed by reading _common.py myself. 2. One credential set for the deployment. provider_keys.provider is the PRIMARY KEY, and migration 11 put Zoho/ClickUp/Gmail tokens in the same table; reads go through a module singleton keyed by provider alone, writes go to os.environ and the on-disk .env, which cannot be tenant-scoped in a shared process. LiteLLM's own organization_id is an UNRELATED namespace — do not connect them. 3. The event bus is global and the receiving workflow can write any task. Tenant A edits a task, tenant B's workflow fires, and it patches tenant A's task. Read and write, closed by nothing in the retrofit. 4. Agent tool identity is a process-global env var that is never cleared, so an agent can act as whoever ran last. Under D-MT-1 that string IS the tenant — and it is already a cross-USER bug today, not merely a future multi-tenant one. SAFE, with reasons, which is worth as much as the leaks: there is no object storage at all (no S3/MinIO/presigned URLs anywhere in first-party code — attachments are local disk, uuid4-named, never served by path), /projects/ search cannot be widened from the query string, pm_task_counters is per-root-project, and notifications resolve each recipient's own authority. Email and WhatsApp scope on user_id and are safe ONLY because D-MT-1 makes email globally unique — a cost that now belongs in D-MT-1's write-up. The audit also proposes a THREE-way baseline split over my two, and the argument is right: NEVER_SCOPED / DEPLOYMENT_GLOBAL / NOT_YET_SCOPED, because "this is deliberately global" is a decision and hiding it among "not done yet" is how it gets made by accident. Also generalises the import warning: INGESTION_CONSUMER=1 and CRM_ZOHO_SYNC=1 write unscoped rows UNATTENDED and are one env var away from doing so. The ClickUp import is merely the one with a button. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
⚠️ INTERMEDIATE COMMIT. The suite is green and the tree is coherent, but this ticket is NOT finished and must not be read as such. Committed because the working tree cannot be left dirty across turns, not because the work is complete. What is verified, by me, against a live Postgres 16 rather than taken on trust: * migration 158 applied — all 17 pm_* tables carry organization_id, all 17 NOT NULL, and every one REFERENCES organization(id), confirmed by reading pg_constraint (not by reading the migration). * core.py carries the tenant: Visibility.organization_id, a _TENANT_PROJECTS_SQL for the unrestricted (`data:org:read`) path, and `g.organization_id = CAST(:vis_org AS uuid)` inside the grant closure — which is the line that makes `subject = 'org'` mean "everybody IN THIS ORGANIZATION" rather than everybody. It is flagged in place as the most dangerous line in the retrofit, which it is. * the ratchet moved the 17 into EXPECTED_SCOPED and dropped BASELINE_UNSCOPED from 137 to 120, which is the ratchet working as designed rather than being edited around. * 661 tests pass, from 45 failed / 611 passed an hour ago. What is NOT done, and why this is `wip`: * 661 is EXACTLY the pre-tenancy count. No two-tenant isolation test has been added yet — and that is the single most important thing this ticket owes. A tenant boundary with no test proving a second tenant cannot see the first is a boundary nobody has checked. * no live-Postgres two-tenant run yet. Every ticket this session has had a bug that only the live run found; there is no reason to think this one is the exception, and this is the ticket where the bug would be a leak. * EXPECTED_SCOPED still contains crm_activities/crm_contacts/crm_deals, which the leak audit proved are NOT tenant-scoped — their organization_id references crm_organizations, a customer company. That is my defect (the ratchet matches column NAME, not FK target) and it is fixed next, once the agent releases the file. The agent is still running. I will not call this done until the isolation proof exists, the live run is clean, and the homonym bug is out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
…efect Supersedes c52c34d's `wip`. That commit was mine, made to satisfy a repo-hygiene hook mid-build; the agent doing the work reasonably flagged it as an unexplained third-party commit. Labelled NOT YET DONE at the time because it wasn't. It is now. WS-29a — organization_id NOT NULL REFERENCES organization(id) on all 17 pm_* tables, idempotent throughout.⚠️ The "empty tables" premise in my brief was WRONG: the live database held 10 pm_tasks and 2 pm_projects of fixture residue from WS-27's own live runs, and SET NOT NULL does not care where a row came from. Backfilled to slug='default' first, and a missing default org fails the deploy loudly rather than guessing. The parent-consistency guard is a TRIGGER, not a CHECK, because a CHECK cannot read another table — Postgres refuses the subquery. One generic function with 21 attachments: it FILLS a NULL from the parent and REFUSES a mismatch. I tested both halves myself against live Postgres rather than taking them on trust: a pm_tasks row claiming org B under org A's project is refused with a message naming both organizations, and a NULL is filled from the parent. That trigger is the load-bearing design decision — it is why 43 INSERT sites across 16 modules did not have to grow a tenant argument. The app decides the tenant in exactly two places; the database derives the rest. WS-29b — Visibility carries organization_id, resolved from X-User-Email BEFORE the data:org:read check, and `vis_org` is always bound so None fails closed by SQL semantics rather than by an `if`. THE LEAK THE LIVE RUN FOUND, which the hermetic suite structurally could not: /projects/assigned-to-me and /projects/my/inbox have no visibility clause AT ALL — deliberately, because assignment IS the claim, so there was no grant clause to notice was missing and every grant-shaped test was already green. Under multi-tenancy that makes them reachable by typing an address: anyone in org B puts a member of org A on their task and its title, description and dates appear in that person's list. Worse than a read — WS-27e's personal mirror SYNCS /assigned-to-me into gtd_items, so the leak would be copied into a second app and outlive the request. Both now carry the tenant; the grant clause stays absent by design. Two more: attachments' `if not vis.unrestricted:` was correct while the unrestricted clause was TRUE and a whole-database leak once it became the tenant; and both importers matched an existing department by lower(name) with no tenant, which is a cross-tenant WRITE — org B's whole workspace poured into org A's department. 18/18 mutants killed, reverts byte-identical. Three survived the first pass and were fixed rather than accepted, two from one instructive class: the test regexes used a single space where the migration column-aligns, so `UPDATE pm_\w+ SET` matched exactly 1 of 17 statements. The tests now assert the match COUNT equals the table count. AND MY OWN DEFECT, found by the leak audit. test_tenancy_boundary.py matched the column NAME, so it counted crm_activities/crm_contacts/crm_deals as tenant-scoped on the strength of an organization_id that REFERENCES crm_organizations — a customer company. A guard satisfiable by a coincidence of naming is not a guard: any future table with an organization_id pointing anywhere would have passed silently. It now matches the foreign key's TARGET, and a probe table referencing crm_organizations is correctly refused. The three move into the baseline where they belong, so the count is 137 → 123 (−17 pm_*, +3 homonyms) and the debt was understated, not over. Verified by me, not reported to me: 1673 backend tests pass (775 in the projects/tenancy slice, from 661), 1106 frontend, ruff and xenon clean, the two-tenant live run re-executed here and green across 40 assertions including 404 — never 403 — for another tenant's task under data:org:read. Recorded, not smuggled in: clickup_id stays globally UNIQUE, so a second org importing the same ClickUp workspace now fails loudly on the constraint instead of writing into the first org's tree. Widening it needs an importer change, so it is written up in migration 158 §6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
A LIVE cross-user bug, not a future multi-tenant one. Both executors did `if _mu: _set_memory_user_id(_mu); os.environ["ACB_AGENT_USER_EMAIL"] = _mu` — set CONDITIONALLY, never unset. The conditional is what made the never-unset lethal: a run whose payload named nobody kept whatever the last run left, and four agents/skills fed that straight into X-User-Email. Measured against the real run_agent before anything was touched: run 1 as alice, run 2 names nobody → run 2 acted as alice same, on the same asyncio task → the ContextVar leaked too alice and bob concurrently → alice's agent observed bob's address alice concurrent with an unattributed run → the unattributed run was alice Under D-MT-1 that email string IS the tenant, so this is also the cleanest cross-tenant path in the system: no database predicate touches it. TWO CORRECTIONS TO THE AUDIT, in both directions, which is why the agent was told to verify before fixing: OVERSTATED — the audit blamed WS-27f's dispatch at agent_dispatch.py:144 for passing a bare string past the guard. It never impersonated anybody: a string payload raises AttributeError at executor.py's list(payload.keys()) before anything runs, and the dispatch is recorded failed. Verified by execution. The live paths are the callers that pass a dict and no user — the workflow agent node and sub-agent dispatch. UNDERSTATED — the audit called the ContextVar correct. It was a bare .set() with no reset, and an awaited coroutine runs in its CALLER's context, so deleting the env var alone would have left the leak intact. The fix keeps the existing seam (acb_skills.memory_tools) and gives it a scope. Binding is UNCONDITIONAL including the empty string, so a run that cannot name its user has nobody to act as and its clients refuse — fail closed rather than inherit. A second ContextVar separates "an open scope" from "a value lying around", which is what still admits the one legitimate inheritance: a sub-agent dispatched with no user from inside a parent that bound one. Release is in the same `finally` as the other per-run teardown, and a reset raising in a foreign context clears rather than leaves standing. os.environ is gone from the identity path entirely. 17 tests driving the REAL executors and the real agent client. 13 of 15 failed on the pre-fix tree — reverted byte-identically to prove it, then restored — with assertions like "a run that named nobody acted as 'alice@fracktal.in'". Concurrency is asserted on what the CLIENT resolves, not on what was .set(), and gated by asyncio.Barrier so both runs are open at once. 7 mutants, 7 killed, reverts byte-identical. Two survived the first pass and were fixed rather than accepted: inheriting any non-empty ContextVar rather than only an open scope, and a reader reinstating the env fallback. Verified here, not taken on report: 1261 passed across the projects/tenancy/ agent/auth slice over two consecutive runs, ruff clean. An earlier run showed five failures that did not reproduce — the other wave agent's mutation harness was live-editing files mid-run, which is a real hazard of parallel agents and the reason for the second confirming run. CONSEQUENCE, deliberately left: workflow agent nodes now fail closed. They build a payload with no user and set actor to "workflow:<name>", not an email, so their gateway-calling tools refuse where they previously acted as the last human. Threading started_by through is the correct close, but it touches the run lifecycle and belongs with "tenant on the event". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
…d slug
get_org_id(db) did `WHERE slug = 'default'` and ignored the caller, across
27 call sites — members, roles, groups, permission overrides, access
requests, /auth/me. A tenant-B admin listed, invited into and granted roles
in tenant `default`: a cross-tenant WRITE into access control, by a
correctly-authorised caller, which is the worst shape of leak in the system
because it grants further access.
Fixed in three layers, because fixing only the first leaves the bug intact:
1. get_org_id(db, user) resolves the CALLER's tenant, reusing WS-29b's
resolve_organization_id rather than deriving a second answer to "which
tenant is this". DEFAULT_ORG_SLUG is DELETED, not demoted to a
fallback — a fallback here re-creates the bug — and the comment in its
place records that the slug now survives only in provisioning paths
unreachable from a request.
2. find_member/get_member take the org and predicate on it. Every
member-targeted route reaches its subject BY ADDRESS, so a
caller-derived org id in front of an unscoped lookup is the same
cross-tenant write with an extra query in front of it.
3. the provisioning upsert grew a tenant fence, and its SET became a
COALESCE. app_user.email is UNIQUE, so inviting another tenant's
address conflicts with THEIR row — and the old arm moved that person
into the inviter's organization, with set_roles (which replaces
assignments wholesale) next in the same transaction.
WHAT THE LIVE RUN FOUND, invisible to every hermetic test and to me:
app_user_email_key is UNIQUE (email) — BYTE-EXACT — while every lookup in
this codebase matches lower(email) (R10). So a row stored as
Casey@Alpha.Example does not conflict with the lower-cased address the
inviter inserts: the fence never fires because there is no conflict, and
Postgres writes a SECOND app_user row for the same human in the other
organization. I reproduced it directly — two rows, two organizations, one
person. Under D-MT-1(a) that makes resolve_organization_id return whichever
row the planner hands back, so a person's tenant becomes non-deterministic.
⚠️ This weakens a claim I wrote into multi_tenancy.md §1.1 and built D-MT-1
on: that one-person-one-organization holds STRUCTURALLY because email is
unique. It holds only for identically-spelled addresses. The decision stands
— it is still the reversible direction — but its enforcement was
application-level, not structural, and the spec said otherwise. Corrected
there, and the migration that makes it structural follows.
Closed in application code meanwhile (the one deliberately cross-tenant
read, answering 404 like every other miss so it is not an existence oracle),
and the fakes now model the index byte-exactly so it is reproducible
hermetically.
15 mutants, no survivors, reverts byte-identical. M9 survived the first
round — the fake reads the clause out of the statement and cannot evaluate
SQL, so `OR TRUE` left the substring it looks for exactly where it was;
killed with a structural assertion that the roster carries no disjunction.
M6 and M7 are killed only structurally, which is defence in depth working:
with the other guards intact they are not behaviourally exploitable.
The frontend needed no change, and the agent checked rather than assumed:
lib/access.ts already types organization as optional and nothing derives an
org client-side. The single-org assumption was entirely server-side.
Verified here: 1508 passed across the projects/tenancy/auth/people/admin/
agent slice, ruff clean on the changed files.
LEFT AS A DECISION, not debt: the sign-in queue is genuinely shared.
access_request has no tenant column and cannot straightforwardly have one —
an address knocking at the door has no organization yet. Admin B can see and
DENY admin A's pending knock: a cross-tenant DoS on onboarding. Approve is
now fenced; deny cannot be without a routing rule (domain? invite token?).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
multi_tenancy.md §1.1 rested the whole tenant model on app_user.email being
unique, and said so with the word "structurally". It was UNIQUE (email) —
byte-exact — while every lookup in this codebase matches lower(email) (R10).
The two disagreed.
WS-29's S1-1 live run found the gap; I reproduced it directly against
Postgres before writing anything:
INSERT app_user ('Yan.Probe@Alpha.Example', org P) -- ok
INSERT app_user ('yan.probe@alpha.example', org Q) -- ALSO ok
-- one human, two rows, two organizations
The consequence is worse than a duplicate row. resolve_organization_id —
which WS-29b made the answer to "which tenant is this caller", and which
S1-1 then made the answer for the entire admin plane — matches on
lower(email) and returns whichever row the planner hands back. A person's
tenant becomes NON-DETERMINISTIC, and so does everything scoped by it.
S1-1 closed it in application code for provision_member, the one write path
that could reach it. That guard is right and it stays, but it guards one
path: any future insert bypasses it, and "remember to lower-case here" is
the class of discipline that produced 137 unscoped tables. Migration 159 is
the version that cannot be forgotten — the functional index is created
BEFORE the byte-exact constraint is dropped, so there is never a window with
neither.
Verified: applied twice against live Postgres (idempotent — the second run
just notices the constraint is already gone), and the exact probe above is
now refused with `duplicate key value violates app_user_email_lower_key`.
Checked the live data for pre-existing case-duplicates first; none.
Deliberately does NOT normalise stored addresses. created_by, assignee,
subject and updated_by are bare address strings across a dozen tables
(D-PM-4) and none are foreign keys, so a normalising UPDATE would silently
orphan them. Stored casing is presentation; matching is already
case-insensitive by R10. The migration says so, and a test asserts no UPDATE
is present.
Five static assertions in the house style. One of them caught me the same
way lib/timeline.ts did earlier: my CONCURRENTLY check tripped on the
migration's own comment EXPLAINING why it is not concurrent. A structural
test that trips on the prose justifying the rule it enforces is a test
somebody deletes — it now strips comments before matching.
§1.1 corrected. The decision stands; the claim about its enforcement did
not, and the spec now says which is which.
Verified: 315 passed across org_access/tenancy/admin, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
Written for a coding agent with database access, which is exactly the gap this branch was built against: a scratch Postgres, no production, no deploy, no ability to apply a migration to the real box. §1 branch state. 16 commits ahead, tree clean, PR #399. 2151 backend and 1106 frontend tests green, tsc/ruff/xenon/theme clean. And the first thing the next agent must do: TWO MIGRATIONS EXIST ON NO REAL DATABASE — 158 (organization_id on all 17 pm_* plus the parent trigger) and 159 (UNIQUE (lower(email))). Both idempotent, both applied twice here. Points at schema_migrations as the ledger rather than letting anyone assume, and warns that schema.generated.sql is stale enough to mislead. §2 the house rules, with the reason each exists. The verification protocol — hermetic tests, never bare pytest on the directory, mutation testing with byte-identical revert, and the live Postgres run that found a bug in EVERY ticket on this branch including several where the whole hermetic suite was green. Plus the mirror discipline: fingerprints must be specific not merely present, and read the SQL's own column choices rather than assuming them — both learned by watching a mutant survive. §3 the ticket queue in dependency order, with the two landmines flagged in place: crm_* cannot use `organization_id` because the name is taken by a homonym pointing at a customer company, and gtd_* is scheduled for retirement so keying it is wasted work. §4 what an agent must refuse, and the one gate this branch LIFTED — the ClickUp import was blocked on the pm_* tenant key, which migration 158 delivered. Replaced with the condition that actually remains: apply 158 to the target database first. §5 the traps, which is the part worth reading twice. asyncpg's CAST type inference and AmbiguousParameterError; array_length('{}',1) being NULL when a CHECK only fails on FALSE; a CHECK being unable to read another table; byte-exact UNIQUE against lower() matching; LIKE metacharacters; midnight UTC and DST rounding — and that both date traps are only behaviourally testable in some timezones, so they need structural pins too; git reset renaming so a self-rewriting script runs its old steps and exits 0; a structural test tripping on the prose that justifies its own rule; and parallel mutation harnesses making verification unreliable. Corrections in these documents are marked⚠️ and kept rather than erased, including two of mine that were wrong in writing. A document that quietly edits its mistakes teaches nobody where the traps are — which is the whole point of §5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
vjvarada
changed the title
⚠️ A gap in my own handover, found by checking it rather than asserting it. HANDOVER.md §2.1 told the next agent that the live-Postgres run is non-negotiable and to copy the patterns from the scratchpad — and all twelve scripts lived in /tmp, in a container that gets reclaimed. They would have been gone before anybody read the sentence pointing at them. These found a bug in every single ticket they were written for, several times with the entire hermetic suite green. That is the whole argument for keeping them, and it is why they belong in the repository. They are not unit tests: they need a real Postgres 16 with the full migration set, they drive the REAL endpoint functions, and they exit non-zero on any failed assertion. Named live_*.py so pytest does not collect them — verified both ways (tests/live/ collects nothing; the unit suite is unaffected). The README tabulates what each one pins, which doubles as a list of the things a fake structurally cannot catch: no type system, so AmbiguousParameterError is invisible; no planner, so an ambiguous ORDER BY is invisible; no constraints, so a CHECK that never fires looks like one that works; no lower(), so a byte-exact UNIQUE agrees with code that assumed case-folding. Every one of those was a real defect on this branch.⚠️ Documented prominently: most of them TRUNCATE pm_projects CASCADE in seed(). Safe against a scratch database, catastrophic against anything you care about — and the next agent has database access, which is precisely why that warning has to be louder than it needed to be here. Also includes prove_bootstrap.sh, which demonstrates rather than asserts that `git reset --hard` renames, so a self-rewriting deploy script keeps its old inode, runs stale steps against a new tree, and exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
A document whose whole value is being accurate about the branch should not be wrong about the size of the branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VmFScimSbeyHcLdut7RT4W
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
👉 Handing this to another agent? Start at
ai-company-brain/HANDOVER.md. Branch state, the two migrations that are on no real database yet, the verification protocol, the ticket queue in dependency order, and every trap that cost real time.Two workstreams. 17 commits.
WS-27 — the ClickUp parity backlog (
project_management_app.md§11.2) is CLOSEDAll ten original rows plus the two added since. Tickets o, p, q, r, s, t landed here, on top of a–n already on
main.blockscycle guard/tasksand/projectsblocksdependenciesTwo architectural decisions are in the diff, and reviewers should read them before the code: D-PM-11 (hierarchy depth decides what earns a timeline bar, not a reserved
Epictype) and D-PM-12 (a dependency arrow warns; it never reschedules — Jira's auto-push was rejected because it contradicts WS-27p's written position and turns one drag into an unbounded cascade ofPATCHes). Gantt was a recorded non-goal and is reversed, struck rather than deleted.WS-29 — multi-tenancy, started deliberately unfinished
Measured, not recalled: 143 app tables, 3 carried a real tenant key, 140 did not. An
organizationtable has existed since migration 130 with one seeded row; tenancy was started and never carried past access control.organization_id NOT NULLon all 17pm_*, plus a parent-consistency trigger (aCHECKcannot read another table). That trigger is why 43INSERTsites across 16 modules needed no tenant argument.UNIQUE (lower(email)), replacing a byte-exact constraint that let one human become two rows in two organizations.ACB_AGENT_USER_EMAILprocess-global, never cleared). A live cross-user bug, not just a future multi-tenant one.Defects found in already-shipped code
_and%were live LIKE wildcards on every search —task_idalso matchedtaskXid. Fixed on the shared builder.deliverableprobed only the project-grant closure — assignees without a grant were judged undeliverable, so assignment notified nobody.overdue's date half was never mirrored in the fake — everyoverduetest since WS-27k was asserting only the status half./assigned-to-meand/my/inboxhad no visibility clause at all — by design, since assignment is the claim. Under multi-tenancy, reachable by typing an address, and WS-27e's mirror syncs it intogtd_items.Corrections to my own work, kept visible
Marked⚠️ in the specs rather than quietly edited: the tenant-scoped table count was 3, not 6 (the CRM's column points at a customer company), and the claim that one-person-one-organization held structurally was false until migration 159.
Verification
2151 backend · 1106 frontend · frontend green in four timezones ·
tsc,ruff,xenon, theme conformance clean · every ticket driven against a live Postgres 16 with the full migration set applied, which found a bug in every one.Mutation testing throughout; where a mutant survived, the test was strengthened rather than the pass accepted. Three mutants turned out to be equivalent and the code was simplified instead.
The deploy path is broken (WS-25). Merging this does not put it on the box, and neither migration is on any real database.