diff --git a/CLAUDE.md b/CLAUDE.md index c861e699c..19a8fd3ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,8 +89,19 @@ Rules that make it work: numbers). Specs go stale; the code is the fact. - **Respect the seams.** Extend the shared seam, never add a parallel one: one DB engine (`acb_common.db`), one entitlement intersect, one subject-grammar - validator, one task store, one Center registry (`lib/centers.ts`). A second - implementation of an existing seam is a defect, not a feature. + validator, one task store, one Center registry (`lib/centers.ts`), one status + colour vocabulary (`src/lib/statusAccent.ts`). A second implementation of an + existing seam is a defect, not a feature. +- **The UI is one product, themed centrally.** Every app is a projection, never + a surface with its own look: no app-local palette, no second colour + vocabulary, no hand-rolled control. `workbench/control_plane/DESIGN_SYSTEM.md` + is the contract and `AGENTS.md` beside it carries the seven rules and their + fences — both are auto-loaded when you touch UI code. Owner directive + 2026-08-10. Categorical hues (contexts, tags, labels) go through the + `--cat-1…8` ramp via `src/lib/categorical.ts`, never a raw Tailwind palette + class. The conformance suite checks five regexes and **nothing tests layout + or cross-app continuity**, so the theme-switch check (Fluent → Material → + Graphite, on your surface *and* its neighbour) is the real gate. - **Keep branches short and integrate often.** Long branches are the root cause behind the migration-renumber collisions, the green-alone/red-together PRs and a duplicated tenancy design. Three or four in flight is the ceiling. diff --git a/apps/services/gateway/AGENTS.md b/apps/services/gateway/AGENTS.md index 18d44cff6..27b5b5af8 100644 --- a/apps/services/gateway/AGENTS.md +++ b/apps/services/gateway/AGENTS.md @@ -175,6 +175,22 @@ breaking it — **plus R11, which will bite here hardest when WS-29 lands:** > directory runs against a tenant-bound session, and the binding happens **once** > in `acb_common.db` — so a route that reaches data any other way is the bug. > Build shapes: `saas_multitenancy_implementation.md` §2. +> +> ⚠️ **A background unit in this directory is NOT a route and must not behave +> like one.** It has no request to inherit from, so `tenant_session()` with no +> argument is the bug there — it would act for whoever happened to be in +> context. The rule (H4) is **resolve on an unbound session, then bind +> explicitly, and refuse if the resolution finds nothing**: two sessions in that +> order, the first deciding the tenant and writing nothing. Three built +> examples, three stored facts: `routes/crm/auto_lead` (the mailbox owner's +> org), `routes/workflows/service._pm_lifecycle_sweeper` (the workflow owner's +> org — `workflows` has no `organization_id` column until H3 phase 1) and +> `routes/projects/agent_dispatch` (the task's own org, carried **on the event +> payload**, because an event consumer has nowhere legitimate to look one up — +> the emitter stamps it inside its bound session). A consumer that refuses +> cannot record its refusal in tenant data either: it logs and returns. +> `tests/unit/test_db_engine_seam.py` is the fence — `H2_EXEMPT_FILES`, +> `H2_BASELINE_ELSEWHERE` and the exempt→converted call-edge table. 1. **The app is default-deny; do not opt out to make something reachable.** `require_authenticated` is attached at the app level in `main.py`, so a new diff --git a/apps/services/gateway/gateway/routes/projects/agent_dispatch.py b/apps/services/gateway/gateway/routes/projects/agent_dispatch.py index e443d0d0c..d7899bbd2 100644 --- a/apps/services/gateway/gateway/routes/projects/agent_dispatch.py +++ b/apps/services/gateway/gateway/routes/projects/agent_dispatch.py @@ -21,6 +21,20 @@ **Only NEW assignees dispatch.** `set_assignees` emits the *added* set, not the whole set, so re-asserting an existing assignee cannot start a second run. That property lives in the emitter and is relied on here; both sides say so. + +**The tenant rides the event, and nothing else will do (WS-27aa / H4).** This +module was the H2 ratchet's one Projects exemption, on the grounds that an +event consumer must not inherit the ambient request tenant. That was right, and +the way out was never "convert it" — it was to give it an EXPLICIT one. +`set_assignees` now reads the task's own `organization_id` inside the request's +bound session and puts it on the payload; every session opened here is +`_tenant_session(org)` with that value. A payload without one is **refused**: +no run is dispatched and nothing is written, because writing the refusal would +itself need the unbound session this rule exists to forbid — the refusal is +therefore a WARNING log line (`projects.agent_dispatch_refused`), not a +timeline row. Fenced by `tests/unit/test_projects_automation.py`'s +`test_an_event_without_a_tenant_refuses_*` and by +`tests/unit/test_db_engine_seam.py`, which no longer exempts this file. """ from __future__ import annotations @@ -30,18 +44,15 @@ from acb_common import get_logger -# ⚠️ H4, DELIBERATELY NOT H2 (`saas_multitenancy_handover.md`): this module is -# an EVENT CONSUMER, not a request handler — `on_event` fires from -# `emit_event`'s sink fan-out and `_run_and_record` outlives the request that -# scheduled it. The runbook's rule for that category is "do not let a job -# inherit an ambient tenant", so converting these sites to the ambient -# `tenant_session()` would be exactly the inheritance it forbids. They stay on -# the unbound `get_db()` until H4 threads an EXPLICIT tenant through the event -# payload (`tenant_session(org_id)`), and the H2 ratchet in -# `test_db_engine_seam.py` carries this module as its one named exemption. -# Sequencing is safe: RLS phase 4 (which would starve these reads) is gated on -# H2+H4 both being complete. -from gateway.db import get_db as _get_db +# ⚠️ H4 DONE (WS-27aa), which is why this is `tenant_session` and NOT the +# ambient no-argument form. This module is an EVENT CONSUMER, not a request +# handler — `on_event` fires from `emit_event`'s sink fan-out and +# `_run_and_record` outlives the request that scheduled it — so +# `tenant_session()` with no argument would inherit whatever tenant happened to +# be in context, which is precisely what the runbook forbids. Every call here +# passes the organization the EVENT carried, and there is no path through this +# module that opens a session without one. +from gateway.db import tenant_session as _tenant_session from gateway.routes.projects.core import record_activity from sqlalchemy import text @@ -94,6 +105,17 @@ def build_message(task: Any) -> str: return "\n\n".join(parts) +def event_tenant(payload: dict[str, Any]) -> str: + """The organization the event says it belongs to, or ``""``. + + Exported and tiny so the refusal has ONE definition that both `on_event` + and its tests read. The value is a stored fact about the task, stamped by + the emitter inside the request's bound session — never a field a caller + chose, which is R11 restated for an event payload. + """ + return str(payload.get("organization_id") or "").strip() + + async def on_event(source: str, event_type: str, payload: dict[str, Any]) -> None: """Event sink: start a run for every agent newly assigned to a task. @@ -101,6 +123,17 @@ async def on_event(source: str, event_type: str, payload: dict[str, Any]) -> Non out to both. Best-effort like every sink — `emit_event` swallows sink errors by default and that default is load-bearing (a webhook must never 5xx because a sink failed), so this returns rather than raises. + + ⚠️ **Refuses without a tenant on the payload** (WS-27aa / H4). Not + "falls back to the ambient one", not "looks it up unbound" — both are the + unbounded-leak shape the runbook names. The refusal is a WARNING log line + rather than a timeline row, and that asymmetry is deliberate: the timeline + lives in `pm_activities`, which is tenant data, so recording the refusal + there would need exactly the unbound session being refused. Under RLS + phase 4 such a write lands nowhere anyway. `set_assignees` always stamps + the field (`pm_tasks.organization_id` is NOT NULL since migration 161), so + in practice this fires only for a foreign or replayed emitter — which is + the case worth being loud about. """ if source != "projects" or event_type != "pm.task.assigned": return @@ -112,8 +145,17 @@ async def on_event(source: str, event_type: str, payload: dict[str, Any]) -> Non if not task_id: return - db = await _get_db() - try: + organization_id = event_tenant(payload) + if not organization_id: + _log.warning( + "projects.agent_dispatch_refused", task_id=task_id, + agents=agents, + reason="event carries no organization_id — a sink must not " + "inherit an ambient tenant or resolve one unbound (H4)", + ) + return + + async with _tenant_session(organization_id) as db: task = (await db.execute( text("SELECT * FROM pm_tasks WHERE id = CAST(:tid AS uuid)"), {"tid": task_id}, @@ -123,32 +165,40 @@ async def on_event(source: str, event_type: str, payload: dict[str, Any]) -> Non message = build_message(task) for name in agents: # The timeline entry is written and COMMITTED before the run - # starts, so the handoff is visible immediately rather than when - # the agent finishes. + # starts — the `_tenant_session` block commits on exit, which is + # BEFORE the dispatch loop below — so the handoff is visible + # immediately rather than when the agent finishes. await record_activity( db, activity_type="agent_run", created_by=f"{AGENT_PREFIX}{name}", task_id=task_id, body=f"Assigned to {name}; starting a run.", meta={"agent": name, "state": "started"}, ) - await db.commit() - finally: - await db.close() for name in agents: - await _run_and_record(name, message, task_id) + await _run_and_record(name, message, task_id, organization_id) -async def _run_and_record(agent: str, message: str, task_id: str) -> None: +async def _run_and_record( + agent: str, message: str, task_id: str, organization_id: str, +) -> None: """Run one agent and close its timeline entry either way. A dispatch that fails silently is worse than one that never started: the task shows a session that appears to still be running and nobody knows to pick the work back up. So the failure path writes too. + + ``organization_id`` is threaded down rather than re-read: this coroutine + outlives the transaction that started it, so there is nothing left to read + it from, and re-resolving it would be a second answer to a question the + event already settled. """ try: from orchestrator.executor import run_agent except Exception as exc: # pragma: no cover — orchestrator is a hard dep - await _record_outcome(task_id, agent, ok=False, detail="orchestrator unavailable") + await _record_outcome( + task_id, agent, organization_id, + ok=False, detail="orchestrator unavailable", + ) _log.warning("projects.agent_dispatch_unavailable", error=str(exc)) return @@ -157,25 +207,32 @@ async def _run_and_record(agent: str, message: str, task_id: str) -> None: run_agent(agent, message), timeout=AGENT_RUN_TIMEOUT_SECONDS, ) except TimeoutError: - await _record_outcome(task_id, agent, ok=False, detail="timed out") + await _record_outcome( + task_id, agent, organization_id, ok=False, detail="timed out", + ) return except Exception as exc: - await _record_outcome(task_id, agent, ok=False, detail=str(exc)[:300]) + await _record_outcome( + task_id, agent, organization_id, ok=False, detail=str(exc)[:300], + ) return - await _record_outcome(task_id, agent, ok=True, detail=str(result or "")[:2000]) + await _record_outcome( + task_id, agent, organization_id, + ok=True, detail=str(result or "")[:2000], + ) -async def _record_outcome(task_id: str, agent: str, *, ok: bool, detail: str) -> None: - db = await _get_db() +async def _record_outcome( + task_id: str, agent: str, organization_id: str, *, ok: bool, detail: str, +) -> None: try: - await record_activity( - db, activity_type="agent_run", created_by=f"{AGENT_PREFIX}{agent}", - task_id=task_id, - body=detail if ok else f"Agent run failed: {detail}", - meta={"agent": agent, "state": "finished" if ok else "failed"}, - ) - await db.commit() + async with _tenant_session(organization_id) as db: + await record_activity( + db, activity_type="agent_run", + created_by=f"{AGENT_PREFIX}{agent}", + task_id=task_id, + body=detail if ok else f"Agent run failed: {detail}", + meta={"agent": agent, "state": "finished" if ok else "failed"}, + ) except Exception as exc: # pragma: no cover — the outcome write is best-effort _log.warning("projects.agent_dispatch_record_failed", error=str(exc)) - finally: - await db.close() diff --git a/apps/services/gateway/gateway/routes/projects/automation.py b/apps/services/gateway/gateway/routes/projects/automation.py index 3ffd7bf2c..dba4ccbd6 100644 --- a/apps/services/gateway/gateway/routes/projects/automation.py +++ b/apps/services/gateway/gateway/routes/projects/automation.py @@ -40,6 +40,11 @@ from typing import Any from zoneinfo import ZoneInfo +# WS-27aa / H4: the sweep refuses to run without an explicit tenant, and it +# refuses with the seam's OWN exception rather than a bespoke one — `db.py` +# already defines "no tenant, never defaulted", and a second exception type +# would be a second vocabulary for the same refusal. +from gateway.db import TenantUnbound from gateway.routes.projects.core import ( CLOSING_CATEGORIES, TRIAGE_CATEGORY, @@ -291,12 +296,25 @@ async def _sweep_candidates( async def run_lifecycle_sweep( - db: Any, *, actor: str, now: datetime | None = None, + db: Any, *, organization_id: str, actor: str, now: datetime | None = None, ) -> dict[str, Any]: - """Apply every root project's lifecycle policy once. Idempotent. - - For each ROOT project with a policy enabled (both columns NULL — the - default — means the project is never touched): + """Apply ONE tenant's root-project lifecycle policies once. Idempotent. + + ⚠️ **``organization_id`` is required and is never defaulted** (WS-27aa, + H4). This function is reached only from a scheduled ``/workflows`` node, so + it has no request to inherit a tenant from — and H4's rule for that + category is the whole point: *a job that forgets doesn't leak one row, it + leaks unbounded*. Before this argument existed the roots query was + ``SELECT * FROM pm_projects WHERE parent_project_id IS NULL`` with no + predicate at all, which is one schedule archiving and closing **every + customer's** work. A blank tenant raises :class:`TenantUnbound` rather than + sweeping wider; the caller + (``routes/workflows/service._pm_lifecycle_sweeper``) resolves it from the + workflow owner's ``app_user`` row — a stored fact, never request input + (R11) — and binds it on the session it hands in. + + For each ROOT project **in that organization** with a policy enabled (both + columns NULL — the default — means the project is never touched): * **archive** — tasks whose status category is done/cancelled and whose ``updated_at`` is older than ``archive_after_months`` calendar months @@ -320,9 +338,27 @@ async def run_lifecycle_sweep( ``system:workflow:`` identity), so every change is an ordinary timeline row wearing the automation flag. """ + org = str(organization_id or "").strip() + if not org: + raise TenantUnbound( + "run_lifecycle_sweep needs an explicit organization_id — a " + "scheduled sweep must never inherit an ambient tenant or default " + "to every tenant (saas_multitenancy_handover.md H4)" + ) moment = now or _clock() + # The tenant predicate is on the ROOTS query and nowhere else, on purpose: + # everything below reaches its rows through `project.id`, so one fence at + # the top of the walk bounds the whole sweep. Under RLS phase 4 the bound + # session narrows it a second time — belt and braces, in the order H3's + # cliff needs (the predicate has to be right BEFORE the policy exists, + # because today there is no policy to catch a missing one). roots = (await db.execute( - text("SELECT * FROM pm_projects WHERE parent_project_id IS NULL"), + text( + "SELECT * FROM pm_projects " + "WHERE parent_project_id IS NULL " + "AND organization_id = CAST(:org AS uuid)" + ), + {"org": org}, )).fetchall() swept = archived = closed = 0 diff --git a/apps/services/gateway/gateway/routes/projects/tasks.py b/apps/services/gateway/gateway/routes/projects/tasks.py index f0236e919..3df4c8269 100644 --- a/apps/services/gateway/gateway/routes/projects/tasks.py +++ b/apps/services/gateway/gateway/routes/projects/tasks.py @@ -622,11 +622,23 @@ async def set_assignees( The emitted ``pm.task.assigned`` event is what WS-27f keys agent dispatch off, so it carries the added assignees rather than the whole set: a - re-assert of an existing assignee must not re-dispatch a run. + re-assert of an existing assignee must not re-dispatch a run. It also + carries the task's ``organization_id`` (WS-27aa): the sink runs with no + request behind it, so this is the one place its tenant can come from a + stored fact instead of an ambient binding. """ async with _tenant_session() as db: vis = await resolve_visibility(db, user) - await load_visible_task(db, vis, task_id) + task = await load_visible_task(db, vis, task_id) + # WS-27aa / H4 — the tenant the dispatch sink will bind, read HERE: + # inside the request's already-bound session, off the task's own + # `organization_id` (NOT NULL since migration 161). It is a stored + # fact about the row, never anything the caller sent (R11), and it is + # read here because this is the last place a tenant legitimately + # exists: `agent_dispatch.on_event` fires with no request behind it, + # and a sink that looked the tenant up itself would have to do so on + # an unbound session — the exact thing H4 forbids. + task_org = str(getattr(task, "organization_id", "") or "") wanted = { a.strip().lower() for a in payload.assignees if (a or "").strip() @@ -690,6 +702,9 @@ async def set_assignees( if added: await emit("pm.task.assigned", { "task_id": task_id, "assignees": sorted(added), + # The sink's ONLY tenant source. `agent_dispatch.on_event` refuses + # a payload without it rather than running unbound. + "organization_id": task_org, }) return { "task_id": task_id, diff --git a/apps/services/gateway/gateway/routes/projects/tree.py b/apps/services/gateway/gateway/routes/projects/tree.py index 1b11868f8..43c9cb628 100644 --- a/apps/services/gateway/gateway/routes/projects/tree.py +++ b/apps/services/gateway/gateway/routes/projects/tree.py @@ -91,10 +91,23 @@ def _refuse_lifecycle_on_child(values: dict, parent_project_id: object) -> None: #: Seeded on every ROOT project. The owner reshapes these in the app; they exist #: so a new project has a working board on its first render rather than an empty #: status picker. +#: +#: ⚠️ **The colours here must equal what `CATEGORY_HUES` derives from the +#: category** (`workbench/control_plane/src/lib/statusAccent.ts`), because a +#: stored colour OUTRANKS the category — that is what lets an owner choose. So a +#: seed that disagrees is a seed that silently overrides the shared vocabulary on +#: every project nobody has customised, and /projects goes back to looking +#: different from /tasks. It did: this tuple used to seed `To do` blue and +#: `In progress` amber against a category map of gray and blue, and two of the +#: four default lanes rendered differently in the two apps. +#: +#: These are defaults, not decisions. If the shared vocabulary changes, change +#: them here too; `test_seed_status_colours_match_the_shared_vocabulary` fails +#: until you do. _SEED_STATUSES: tuple[tuple[str, str, int, str, bool], ...] = ( ("Backlog", "gray", 10, "backlog", True), - ("To do", "blue", 20, "todo", False), - ("In progress", "amber", 30, "in_progress", False), + ("To do", "gray", 20, "todo", False), + ("In progress", "blue", 30, "in_progress", False), ("Done", "green", 40, "done", False), ) diff --git a/apps/services/gateway/gateway/routes/workflows/service.py b/apps/services/gateway/gateway/routes/workflows/service.py index 60d5d99d3..adfbb042b 100644 --- a/apps/services/gateway/gateway/routes/workflows/service.py +++ b/apps/services/gateway/gateway/routes/workflows/service.py @@ -43,12 +43,18 @@ # who happened to trigger it, and inheriting the ambient request tenant is # exactly what H4 forbids. They stay on the unbound `get_db()` until H4 # threads an explicit tenant (`tenant_session(org_id)` from the workflow row) -# through the run lifecycle. `_pm_task_updater` / `_pm_lifecycle_sweeper` are -# the Projects automation seam and are likewise H4 — do not change their -# session acquisition here. +# through the run lifecycle. `_pm_task_updater` is the Projects automation +# seam and is likewise H4 — do not change its session acquisition here. +# +# ⚠️ ONE EXCEPTION, and it is done: `_pm_lifecycle_sweeper` (WS-27aa). It +# resolves the workflow owner's organization on an unbound session and then +# opens `_tenant_session(org)` for the sweep — an EXPLICIT tenant from a stored +# fact, which is what H4 asks for, not the ambient inheritance it forbids. It +# is the shape the rest of this module's sites will take. from gateway.routes.workflows.core import ( _get_db, _log, + _tenant_session, parse_jsonb, publish_workflow_activity, ) @@ -196,15 +202,68 @@ async def _update(task_id: str, fields: dict[str, Any]) -> dict[str, Any]: return _update +async def _workflow_organization(db: Any, workflow_id: str) -> str: + """The tenant a workflow's unattended writes belong to. **A stored fact.** + + ⚠️ **The `workflows` table has no `organization_id` column today** — + checked against `infra/postgres/132_workflows.sql` and against a live + catalog, not inferred: that column exists only in the unapplied + `generated/01_add_columns.sql` (H3 phase 1). So the tenant is resolved the + way `routes/crm/auto_lead._owner_organization` resolves the mailbox + owner's: the workflow's ``owner_email`` through ``app_user``. One shape for + "which tenant does this background unit act for", not two. When H3 phase 1 + lands, this becomes a one-column read on the workflow row and this + function is where that change goes. + + Reads on the CALLER's (unbound) session because it is what DECIDES the + tenant — the same ordering identity resolution has on the request path. + **Raises rather than returning None**: a workflow whose owner has no + organization must fail its run loudly, never sweep "the usual" tenant + (`saas_multitenancy_handover.md` §5 rule 3 — fail closed, everywhere). + """ + row = ( + await db.execute( + text( + "SELECT au.organization_id FROM workflows w " + "JOIN app_user au ON lower(au.email) = lower(w.owner_email) " + "WHERE w.id = CAST(:wid AS uuid)" + ), + {"wid": workflow_id}, + ) + ).fetchone() + org = getattr(row, "organization_id", None) if row is not None else None + if not org: + raise NodeExecutionError( + f"workflow {workflow_id} has no resolvable organization " + f"(owner_email -> app_user.organization_id) — an unattended sweep " + f"cannot choose a tenant" + ) + return str(org) + + def _pm_lifecycle_sweeper(workflow_id: str) -> Any: - """The Projects lifecycle sweep (WS-27z), identity bound in. - - Mirrors ``_pm_task_updater`` exactly — closure import so the workflows - package gains no import-time dependency on an app package, the workflow's - ``system:workflow:`` actor bound in, one commit around the whole - sweep. The seam takes no arguments because the policy lives in the - Projects app's own columns; the workflow supplies nothing but the - schedule and the node. + """The Projects lifecycle sweep (WS-27z), identity AND tenant bound in. + + Mirrors ``_pm_task_updater`` for the identity half — closure import so the + workflows package gains no import-time dependency on an app package, the + workflow's ``system:workflow:`` actor bound in, one transaction around + the whole sweep. + + ⚠️ **WS-27aa / H4 — this one is no longer an unbound background site.** + ``run_lifecycle_sweep`` used to walk `pm_projects` with no tenant + predicate, so one workflow's schedule archived and closed every customer's + work. It now takes a required ``organization_id`` and refuses without one, + and the tenant is resolved here from a stored fact + (:func:`_workflow_organization`) and bound **explicitly** on the session + the sweep writes through — never inherited from whoever happened to + trigger the run, which is the inheritance H4 forbids. Two sessions, in + this order and for the reason auto_lead has them: + + 1. an unbound one to RESOLVE the tenant (it is the decision, so it cannot + already be inside it), closed before anything is written; + 2. ``tenant_session(org)`` for the sweep itself, which issues the + ``SET LOCAL app.tenant_id`` the RLS policies will read the moment H3's + phase 4 lands, and commits on clean exit. """ async def _sweep() -> dict[str, Any]: @@ -215,15 +274,19 @@ async def _sweep() -> dict[str, Any]: ) except Exception as exc: # pragma: no cover — Projects ships with the gateway raise NodeExecutionError("the Projects app is not available") from exc - db = await _get_db() + # H4: unbound ON PURPOSE — this session's whole job is to decide the + # tenant, and it writes nothing. + resolver = await _get_db() try: - result = await run_lifecycle_sweep( - db, actor=workflow_actor(workflow_id), - ) - await db.commit() - return result + organization_id = await _workflow_organization(resolver, workflow_id) finally: - await db.close() + await resolver.close() + async with _tenant_session(organization_id) as db: + return await run_lifecycle_sweep( + db, + organization_id=organization_id, + actor=workflow_actor(workflow_id), + ) return _sweep diff --git a/infra/postgres/167_projects_seed_status_colours.sql b/infra/postgres/167_projects_seed_status_colours.sql new file mode 100644 index 000000000..77bfff4d9 --- /dev/null +++ b/infra/postgres/167_projects_seed_status_colours.sql @@ -0,0 +1,88 @@ +-- ============================================================================ +-- 167_projects_seed_status_colours.sql — WS-27ad · repaint the seeded status +-- colours that nobody ever chose. +-- +-- Spec: project-docs/specs/project_management_app.md §9.2 (WS-27ad). +-- +-- WHY THIS EXISTS +-- +-- `pm_task_statuses.color` has been stored since migration 146 and, until +-- WS-27ad, was rendered NOWHERE — every Projects board column drew the same +-- `bg-muted` grey while the /tasks board next door was colour-coded per stage. +-- So these values were never seen by a human, and never chosen by one: they are +-- whatever `routes/projects/tree.py::_SEED_STATUSES` inserted when the project +-- was created. +-- +-- WS-27ad made the column render, through one shared vocabulary +-- (`workbench/control_plane/src/lib/statusAccent.ts`) that both apps consume. +-- In that vocabulary a STORED colour outranks the one derived from the status's +-- category — which is correct, because it is what lets an owner choose. The +-- consequence is the problem this file fixes: a seed that disagrees with the +-- shared vocabulary silently overrides it on every project nobody customised. +-- +-- The old seed disagreed on two of its four lanes: +-- +-- lane seeded shared vocabulary /tasks draws +-- Backlog gray gray gray agree +-- To do blue gray gray DISAGREE +-- In progress amber blue blue DISAGREE +-- Done green green green agree +-- +-- The seed has moved to agree (same commit as this file, and +-- `test_seed_status_colours_match_the_shared_vocabulary` now fails if the two +-- drift apart again). But the seed only governs projects created AFTER the +-- deploy — so without this migration, every project that already exists keeps +-- rendering two of its four lanes in colours that differ from /tasks, which is +-- the exact divergence the whole change set was written to end. +-- +-- WHAT IT WILL NOT TOUCH +-- +-- Each UPDATE matches the FULL pre-change seed tuple — name, category AND +-- colour together. If any one of the three differs, the row is left alone: +-- +-- * renamed the lane ("To do" → "Up next") → skipped +-- * moved it to another category → skipped +-- * already picked a colour (any colour but the old +-- seed value, including one that happens to equal +-- the new one) → skipped +-- +-- A human's decision therefore survives even when it coincidentally matches the +-- value we are replacing, because in that case there is nothing to replace. The +-- only rows that move are rows still carrying, unmodified, a default that was +-- invisible for the whole time it existed. +-- +-- This is data, not schema: no column is added, altered or dropped, so R6's +-- expand/contract rule has nothing to bind here and old code meets no new +-- shape. A gateway still running the previous release reads these rows exactly +-- as before — it simply drew them grey anyway. +-- +-- Idempotent BY CONSTRUCTION rather than by a guard: after the first run no row +-- matches `color = ` any more, so a second run updates zero rows. +-- +-- Tenant note (D15/R5): deliberately unqualified by `organization_id`. This is +-- a migration, run as the schema owner, and the defect it repairs is identical +-- in every tenant — a per-tenant loop here would be a job acting for tenants it +-- was not asked about, which is the shape H4 exists to forbid. Migrations are +-- the one place a cross-tenant write is the correct thing. +-- +-- Depends on: 146_projects.sql (pm_task_statuses), 164_projects_intake.sql +-- (the `triage` category — untouched here; it has no seeded row). +-- ============================================================================ + +-- "To do" was seeded blue; the shared vocabulary says a lane where nothing has +-- started is muted, which is what /tasks has always drawn for it. +UPDATE pm_task_statuses + SET color = 'gray', + updated_at = now() + WHERE name = 'To do' + AND category = 'todo' + AND color = 'blue'; + +-- "In progress" was seeded amber; `bg-primary` is this UI's "active" tone +-- everywhere else, and amber means blocked/waiting throughout the product. +UPDATE pm_task_statuses + SET color = 'blue', + updated_at = now() + WHERE name = 'In progress' + AND category = 'in_progress' + AND color = 'amber'; diff --git a/project-docs/specs/project_management_app.md b/project-docs/specs/project_management_app.md index 57064396a..182177179 100644 --- a/project-docs/specs/project_management_app.md +++ b/project-docs/specs/project_management_app.md @@ -2,7 +2,8 @@ > **Product:** CommandCenter · **Feature:** Projects (the People Center's primary work-management > module, sliced into every other Center) · **Created:** 2026-08-05 · **Updated: 2026-08-10** -> (status truth pass + tenancy alignment — R4) · +> (status truth pass + tenancy alignment — R4; **WS-27ag shell/mobile slice built the same +> day**) · > **Status:** ✅ **WS-27 a–t MERGED AND DEPLOYED** (a b d e f i j k l m n via #390/#393/#394/#398; > o–t via **#399**; **u–z via #408**, 2026-08-10) — migrations **146, 147, 150, 152, 155, 156, > 160, 161, 164, 165, 166 are applied on prod** (164/165/166 log-verified on the 2026-08-10 @@ -11,6 +12,13 @@ > 🟡 **c** two-way sync (waits on WS-1 BO-1a+BO-1b) · 🔴 **g** cutover/retirement · 🟡 **h** > `gtd_items` retirement (data move 🔴) · 🟢 **u–z shipped**, their owner activation steps in > HANDOVER §1 · +> 🟢 **ag BUILT 2026-08-10, on branch, NOT merged and NOT deployed** (§11.20) — the app joins +> the house shell and gets a mobile layout at all: `AppShell` learns `isProjectsPage` +> (Projects · Views · Search), the tree and the mode picker become drawer sheets, an opened +> task is full-screen on a phone, the desktop rail collapses at Tasks' `w-60`, and the +> six-purpose header splits into a title row and an action row. Frontend only — no migration, +> no API change. ⚠️ **The phone-viewport and four-theme visual pass is still owed**: no +> browser was runnable in the build environment (§11.20's closing note). · > **Owner:** vjvarada · **Board row: WS-27** > > **Tenancy (audited 2026-08-10 — this spec previously cited no tenancy decision at all).** @@ -1208,15 +1216,233 @@ beyond the window and closes stale open ones to the project's default closing st distinctly in the timeline; (4) tasks in `triage` (WS-27u) are exempt; (5) the manual archive guard (WS-27w item 1) ships first — this ticket depends on it. -**Deferred small basket** *(no ticket yet — pull individually when adjacent code is -touched)*: peek size escalation + Esc-returns-focus (P-14), Save/**Update view** dirty -affordances (P-15), palette action registry + go-sequences (P-16), calendar week layout + -per-day quick-add/overflow (P-19), filtered-list CSV export (P-26), delta-sync feed + -satellite `updated_at` bump (P-27), `is_epic` flag + per-user view state + session -`user_id` denorm (P-28 rest). Banked for their trigger events: sprints (P-23, when -sprints are wanted), webhook-out checklist (P-24, when `/workflows` grows the node), -email digest outbox (P-25, when PM emails). Owner-decided: docs = knowledge base -(D-PM-13); public boards deferred (D-PM-14). +**Deferred small basket** *(pull individually when adjacent code is touched)*: peek size +escalation + Esc-returns-focus (P-14), Save/**Update view** dirty affordances (P-15), +palette action registry + go-sequences (P-16), calendar week layout + per-day +quick-add/overflow (P-19), filtered-list CSV export (P-26), delta-sync feed + satellite +`updated_at` bump (P-27), `is_epic` flag + per-user view state + session `user_id` denorm +(P-28 rest). **§9.2 promotes P-14/15/16 (WS-27ab), P-19 (WS-27ac) and banks the rest as +WS-27ae.** Banked for their trigger events: sprints (P-23, when sprints are wanted), +webhook-out checklist (P-24, when `/workflows` grows the node), email digest outbox (P-25, +when PM emails). Owner-decided: docs = knowledge base (D-PM-13); public boards deferred +(D-PM-14). + +--- + +### 9.2 The post-tenancy queue (minted 2026-08-10, after H2 landed on `main`) + +Four tickets. The first exists because the **Projects app owns two of the residues that +gate WS-29's phase-4 promotion** (D27 findings 2 and 3) — closing them here means Projects +is not the reason RLS cannot be switched on. The other three drain the deferred basket and +the continuity audit. None needs a migration, so R1 costs this wave nothing; all four +inherit the standing protocol (hermetic tests against the fake, a live Postgres run, and +for anything tenant-shaped, **R8** — verified against a real database, never a fake alone). + +**WS-27aa — the two tenancy residues Projects owns.** ✅ **BUILT 2026-08-10** *(D27 (2); +MT-1d's named site; the H2 ratchet's one Projects exemption — both now struck)*. +Two scheduled/background paths in this app still touch the database with no tenant. +Done when: (1) **`run_lifecycle_sweep` takes an explicit tenant and refuses without one** — +the signature gains a required `organization_id`, the roots query gains +`AND organization_id = :org`, and a sweep constructed without a tenant raises rather than +sweeping every customer's projects (H4's rule: *a job that forgets doesn't leak one row, it +leaks unbounded*); (2) the tenant comes from a **stored fact, never request input** (R11) — +the workflow's owner resolved through `app_user`, the shape +`routes/crm/auto_lead._owner_organization` already uses, and a resolution that finds +nothing is an error, not a fallback; `_pm_lifecycle_sweeper` binds it with +`bind_tenant`/`release_tenant` around the sweep so the writes carry the right GUC the +moment phase 4 lands; (3) **`agent_dispatch` carries its tenant on the event payload** — +`pm.task.assigned` emits the task's own `organization_id`, read inside the request's bound +session at emit time, and `on_event`/`_run_and_record` bind that explicitly instead of +inheriting an ambient one; a payload without an org refuses rather than running unbound; +(4) **two-org proof against a real database**: a +sweep bound to org A leaves org B's stale tasks untouched, and a dispatch bound to A writes +A's activity row — plus a refusal test for each path; (5) the `projects/agent_dispatch` +entry leaves `H2_EXEMPT_FILES` (the file no longer needs it), every seam ratchet stays +green, and the handover's MT-1d site + D27 finding 2 are struck with the measurement that +replaced them. **Not in scope:** the other H4 consumers (ingestion, reconciler, broker) — +they belong to WS-29's own H4 slice; this ticket closes only what Projects owns. + +**As built (2026-08-10), and the three places the ticket above was wrong.** + +*The sweep.* `run_lifecycle_sweep(db, *, organization_id, actor, now)` — required, +never defaulted; a blank one raises `TenantUnbound` (the seam's own exception, not a +second vocabulary) **before any statement is issued**, and the roots query is now +`WHERE parent_project_id IS NULL AND organization_id = CAST(:org AS uuid)`. The fence is +on the roots query alone because everything below reaches its rows through `project.id`. +`workflows/service._pm_lifecycle_sweeper` resolves the tenant on an unbound session +(`SELECT au.organization_id FROM workflows w JOIN app_user au ON lower(au.email) = +lower(w.owner_email) WHERE w.id = CAST(:wid AS uuid)`) and then opens +`tenant_session(org)` for the sweep — auto_lead's shape, re-derived, not a second one. + +⚠️ **The tenant source is the workflow OWNER, not the workflow row.** Verified against +`infra/postgres/132_workflows.sql` *and* a live catalog: `workflows` has **no +`organization_id` column** — it exists only in the unapplied `generated/01_add_columns.sql` +(H3 phase 1). `live_ws27aa.py` asserts that absence, so the day phase 1 lands this script +goes red and `_workflow_organization` becomes a one-column read. + +⚠️ **MT-1d's "it needs a per-tenant loop" is wrong and is struck.** A loop inside the +sweep would be one tenant's scheduled workflow acting for every other tenant — the +unbounded-job shape H4 exists to forbid. The loop is over **workflows**: each tenant +schedules its own, and each one sweeps exactly its own. + +*The dispatch.* `set_assignees` reads the task's `organization_id` inside its already-bound +session (NOT NULL since migration 161) and puts it on `pm.task.assigned`; `on_event`, +`_run_and_record` and `_record_outcome` all open `tenant_session(that_org)` — the argument +form, never the ambient one. + +⚠️ **Done-when 3's "records the refusal on the task timeline" cannot be built and is +struck.** The timeline is `pm_activities`, which is tenant data: writing the refusal there +needs precisely the unbound session being refused, and under phase-4 policies it would +write nothing anyway. The refusal is a WARNING log line +(`projects.agent_dispatch_refused`, carrying task and agents) and **no** write. Since the +emitter always stamps the field, it fires only for a foreign or replayed emitter. + +*Evidence.* `tests/live/live_ws27aa.py` — 23 checks against Postgres 16, two organizations +with identical policies and identically stale tasks: alpha's sweep archives alpha's task +and leaves beta's, beta's sweep then archives its own, both activity rows are stamped with +the sweeping tenant, both refusal paths refuse, and no ambient tenant leaks out. +Mutation-measured: deleting the roots predicate turns 4 live checks red (including +`BETA's stale task is untouched: got True, want False`) and 3 hermetic ones. +`H2_BASELINE_ELSEWHERE` is unchanged at **111** — the sweeper trades its unbound session +for the resolver, which must stay unbound — while `routes/projects` goes from **2** unbound +sites to **0** and `H2_EXEMPT_FILES` loses its Projects entry. + +**WS-27ab — view ergonomics: peek, dirty views, one palette registry.** 🟢 AGENT-SAFE +*(P-14, P-15, P-16)*. +Done when: (1) **peek escalation** — `TaskPanel` offers peek → side → full, the choice +persists per user, and Esc returns focus to whatever opened the panel so the card/row keeps +the cursor (WS-27y's cursor is the thing being returned to); (2) **dirty-view affordances** +— `FilterBar` shows when live filter/sort/group/shown-field state diverges from the saved +view, offering *Update view*, *Save as new* and *Reset*; divergence is **one exported pure +function** over the config with its own tests, never scattered comparisons — the config +round-trip (`toConfig`/`fromConfig`) is the single fact it reads; (3) **palette action +registry** — `SearchPalette`'s commands become a declared registry (`id`, `label`, +`section`, `keywords`, `run`, `when`) instead of inline branches, `g`-sequences navigate +(`g p`, `g m`, `g t`…), and `?` renders a shortcuts sheet **generated from that same +registry** so the help cannot drift from the behaviour; (4) tests over the registry: every +action carries a label and section, every go-sequence resolves to a route that exists, and +no two actions share a key sequence; (5) DESIGN_SYSTEM throughout — no raw colours, the +`Icon`/`Button`/`Input` primitives, theme suite green. + +**WS-27ac — calendar: week layout, per-day quick-add, honest overflow.** 🟢 AGENT-SAFE +*(P-19)*. +Done when: (1) `CalendarView` gains a **week** layout beside month, both driven by the +existing `lib/calendar.ts` date math — one implementation, extended, never a second; +(2) each day cell carries the shared group-context quick-add (`components/QuickAdd.tsx`) +pre-filled with that day's date; (3) **overflow is exact** — a day with more tasks than fit +shows `+N more` with the true count and expands rather than clipping silently; (4) dragging +between days reschedules through the existing `PATCH` path wearing WS-27y's drop-refusal +reason and post-drop flash; (5) the §11.16 parameter-coverage test extends to the week +range, so the `triage` exclusion (WS-27u) cannot be dropped by the new surface. + +**WS-27ad — Tasks ↔ Projects continuity, round 2.** ✅ **BUILT 2026-08-10** *(the backport +agent's recorded gap list, HANDOVER §1; scope extended mid-ticket by the owner to put the +VISUAL layer — board/list/card/colour — first)*. +The first backport promoted chips, cursor, quick-add and flash to shared code. These are +the divergences it recorded and deliberately left. +Done when: (1) **one selection grammar** — the shift-range anchor moves into shared code +beside the cursor, both apps consume it, and Tasks' modal select-mode either becomes the +shared range behaviour or is kept with the reason written next to it (a divergence with a +recorded reason is a decision; an undocumented one is drift); (2) **board chrome +converges** — Tasks' accent caps + drop-gap reorder and Projects' swimlanes + +append-on-drop are reconciled, the winning behaviour implemented once and consumed twice; +(3) Tasks' flat lists (Done/Waiting/Someday/Archive), `WaitingForView` and the Inbox gain +the shared cursor and group-context quick-add, retiring the Inbox's local `j`/`k` idiom; +(4) a test asserts both apps import the shared modules rather than re-declaring them — the +re-export shims stay, a third copy is a failure; (5) calendar asymmetry stays **out of +scope** and stays recorded (Tasks has a ten-file module, Projects one view). + +**As built.** The seam is `src/lib/{statusAccent,selection,cursor,boardDrop,taskCard}.ts` +and `src/components/{StatusChip,TaskCardShell,DropGap,QuickAdd,useFlash,TaskMeta}.tsx`, +fenced by `src/lib/sharedTaskUi.test.ts` (each thing declared once, both apps importing it, +shims staying shims, no second name→class palette). + +- **Colour (owner-directed).** Three vocabularies existed and a fourth fact was stored and + never drawn: `pm_task_statuses.color` (migration 146, on the API since) rendered + *nowhere*, so every Projects column was one `bg-muted` while Tasks' board was + colour-coded. `lib/statusAccent.ts` is now the one palette, resolved **stored colour → + status category (Projects' six) → name keyword (Tasks' user-named stages) → positional**, + with `lastIsDone` as Tasks' own rule. Projects' board caps/headers, swimlane headers, + list group headers and list/table status pills consume it; `projects/lib/tags.chipClass` + and `tasks/lib/stageColors` delegate. Tasks renders byte-identically (pinned). +- **Card.** `components/TaskCardShell.tsx` — Tasks' `rounded-lg / bg-card / p-3 / shadow` + box wins over Projects' `bg-background / p-2` (which was the page colour, i.e. a + card-shaped hole in the column). `shown_fields` gating unchanged. +- **Selection.** `lib/selection.ts` holds `clickSelect` / `range` / `toggle` / `prune` / + `allSelected`; `stepCursor`'s duplicated sweep now reads it. Projects' page and the Tasks + store both drive it, so shift-click and Shift+Arrow behave identically. **Tasks' modal + select-mode is KEPT**, with the reason in `tasks/components/ItemList.tsx`: `selectMode` + changes what a *click means*, and a permanent checkbox on a `TaskCard` would take the + drag-grip gutter or make one gesture mean two things. The mode is the entry; the grammar + inside it converged. +- **Board chrome.** Drop-gap reorder beats append-on-drop and is now + `components/DropGap.tsx` + `lib/boardDrop.ts` (`gapKey`, `dropIndexFor` — the downward + intra-group off-by-one, previously buried in `taskStore.reorderItem`), consumed by both + boards; Projects' unconditional append is gone (a body drop still appends). Accent caps + went the other way. **Swimlanes stay Projects-only**, reason in `tasks/TaskBoard.tsx`: + Tasks' second axes are *computed* (priority/mode from flags × due date), so a lane grid + would be a grid whose cells refuse every drop — the same reason `lib/quickAdd` refuses + those axes. +- **Flat surfaces.** `tasks/components/FlatList.tsx` (Done/Someday/Archive/Engage/Priority) + and `WaitingForView` now run the shared cursor, flash and selection. Per-view quick-add + is `lib/quickAdd.viewQuickAdd`: Someday incubates, Done logs, **Waiting and Archive + refuse with the reason in code** (a create can set the WAITING bucket but not the + delegation, so the box would file under "Unassigned" — a sibling group). +- **Inbox.** The local `j`/`k` walk is retired; arrows and Enter are `lib/cursor`, the + triage keys (`e x t s r 2`) stay local, and the shortcuts sheet says `↑ / ↓`. + +**Recorded, not done:** the calendar asymmetry (out of scope, per done-when 5); +`app/crm/lib/board.ts` holds a *third* name→class palette for pipeline stages, exempted in +the seam test with its reason; `tasks/lib/contextColors.ts` uses raw Tailwind palette +classes (`sky-500`…) rather than semantic tokens — legal under the conformance suite, off +the token system, and a Tasks-only axis with no Projects counterpart. + +**WS-27ae — export, delta-sync, small columns.** 🟢 AGENT-SAFE, **not this wave** *(P-26, +P-27, P-28 rest)*. Filtered-list CSV export on the export-job pattern; a delta-sync list +variant plus satellite `updated_at` bumps for agents/mobile; `is_epic`, per-user view state +and the session `user_id` denorm. Minted so the basket has an owner; dispatch after aa–ad. + +**WS-27af — the themed categorical ramp.** 🟢 AGENT-SAFE. ✅ **BUILT 2026-08-10.** +*(Owner-ruled the same day, choosing the ramp over tokenising to the semantic set or +widening the DESIGN_SYSTEM exception.)* `--cat-1 … --cat-8` in all four theme manifests in +both modes (64 values), bridged to Tailwind, with `src/lib/categorical.ts` as the shared +vocabulary — slot chosen by hashing the item's **name**, never an array index, so nothing +silently repaints when a list is reordered. `tasks/lib/contextColors.ts` became the worked +adapter, the same shape `stageColors.ts` has over `statusAccent.ts`. Also retired: the two +raw-palette sites no exception covered, `SourceBadge`'s hand-rolled chrome (now ``, +so it finally picks up Graphite's uppercase and Material's tracking), and the off-grid type +scale (`text-[12px]`/`text-[13px]` → `text-xs`/`text-sm`, 167 sites — which also restores +the user's density preference, since `--ui-scale` reaches rem and not px). +**Its lasting deliverable is the fence, not the ramp:** conformance gained a fifth rule for +raw Tailwind palette classes, per-file baselines that only go down. `bg-sky-500/10` passed +every previous regex — it is a named class, not a bracket class — which is how ~950 of them +accumulated tree-wide. ⚠️ Measured, and the ticket was wrong: `/tasks` held **142** across +13 files, not 52; the tree holds **952** across 77. + +**WS-27ag — the house shell, and a mobile UI at all.** 🟢 AGENT-SAFE. ✅ **BUILT +2026-08-10** — see §11.20 for the as-built record. + +> ### ⚠️ Owed at review — the check no test in this tree performs +> +> Neither af nor ag could run a browser (Playwright's download fails in the build +> environment), so **the phone-viewport pass and the Fluent → Material → Graphite sweep did +> not happen** for either slice. Both compensated honestly — a production `next build`, +> icon names verified against the theme registry, a hand-traced z-order, and for the ramp, +> the shipped values rendered to PNGs as real composites and inspected, including under a +> simulated deuteranopia transform. That last produced a finding worth keeping: **eight +> qualitative hues collapse to about four under dichromacy** (1/4, 2/8, 6/7 merge), which no +> eight-hue palette survives — so every shipped use pairs the hue with the label it colours, +> and the limit is written into `themes.ts`, `categorical.ts` and `DESIGN_SYSTEM.md` rather +> than left implicit. None of that substitutes for looking at the running app on a phone in +> four themes. That gate is still open. + +**Open, and needing an owner ruling** *(surfaced by af, 2026-08-10)*: WS-27ad standardised +the **shared** card title on `text-[13px]` — deliberately choosing `/tasks`' size as the +common one — while af established the house scale as `text-sm`/`text-xs`/`text-[11px]`/ +`text-[10px]` and removed every other off-grid size. So the one remaining off-grid size now +lives in `src/components/TaskCardShell.tsx:120`, which **both** apps render. The two +decisions contradict; changing it repaints Projects as well as Tasks, so it was left alone +rather than settled by whichever slice touched it last. --- @@ -2297,6 +2523,68 @@ the research doc's license wall is binding on every ticket below. snapshot-on-close, carry-forward — research doc §3.7) so the eventual build starts from a settled shape rather than a blank page. +### 11.20 WS-27ag — the house shell, and a mobile UI at all (built 2026-08-10) + +**The measured problem.** `/projects` shipped twenty-plus letters of function with **no +mobile layout of any kind**. `page.tsx` imported neither `useViewMode` nor +`useMobileDrawer` — the only cross-cutting app in the tree that did not — so a phone got +the desktop tree: a fixed 256px `