# ADR-133 All Job Creation Flows Through sales.create_job; Tour-Request Approval Is a Single Transaction; job_name Becomes a Free Display Label

## Status

Proposed, 2026-08-06.

## Status History

```yaml
status_history:
  - date: 2026-08-06
    status: Proposed
    changed_by: hkl (via DL laptop Claude Code session, Opus 4.8, chunk C0 of the Job-Creation Unification + Batch Creation plan)
    reason: >
      Authored per hkl's frozen decisions D1-D7 of JOB_CREATION_PLAN.md, settled in an interactive
      Q&A on 2026-08-06 and reviewed against an Opus 4.8 advisory pass. Today sales.jobs has exactly
      TWO divergent writers (the Sales form and RentVeh approveRequest) producing structurally
      different job rows, and the RentVeh approval fires ~6 sequential non-atomic PostgREST POSTs
      that orphan job_tours on interruption (DF-02). This ADR freezes the decision to funnel ALL job
      creation through a single SECURITY DEFINER RPC sales.create_job(...), wrap the RentVeh approval
      in a second transactional RPC sales.approve_job_tour_request(...), drop the global job_name
      UNIQUE in favour of a partial UNIQUE on campaign_id, add a Sales batch-creation screen, remove
      the mandatory-zone UI rule, and clean-slate + reseed the dev job data through the new RPCs. It
      is Proposed now and becomes Accepted once chunks C1-C8 verify the RPCs and both PWAs over
      REST-as-anon and Playwright. Companion build plan:
      C:\Users\Lenovo\Documents\tour-job-architecture\JOB_CREATION_PLAN.md (chunk C0).
    changed_via: adr-kit (360lm) — DL session, dev-first, execution gated on hkl go-ahead
```

## Context

**`sales.jobs` is the canonical job record (ADR-111), but it has two divergent writers today.**
Verified on the live `lm360` dev DB, 2026-08-05/06:

1. **The Sales form** (`sales/index.html` ~L1719) writes a **rich** record: it composes
   `campaign_id` as `<year>-<BRAND>-<CLIENT>-<CAMPAIGN>-P<phase>`, sets `job_name = campaign_id`,
   requires zones through the UI (zone chips are mandatory), and writes `status='operational_draft'`.
2. **RentVeh `approveRequest`** (`rentveh/index.html` ~L1400) writes a **thin** record: a free-text
   `job_name`, **no** `campaign_id`, **no** zones, **no** `type`, and `status='draft'` — then fires
   roughly **six sequential PostgREST POSTs** (job insert → tour → tour_jobs → request-status update
   → `rentveh.expenses` backfill → `tourexp.expenses` backfill) with no transaction wrapping them.

The divergence is not hypothetical. Two live jobs prove it: `JOB-1778820423725`
(`26-Epson-EverythingElse-BTLActivityInd-001`) and `JOB-1778744623251`
(`26-Epson-EverythingElse-BTLActivityEps-001`) both carry `status='draft'` and an **empty
`campaign_id`** — the thin shape only the RentVeh path produces. Two writers, two record shapes,
one table.

This context closes four logged defects/observations (`PHASE3_BACKLOG.md`):

- **DF-02 — approve is non-atomic.** The ~6 sequential POSTs have no cross-statement atomicity
  (PostgREST cannot give it client-side). An interruption between them leaves an orphaned job with
  a half-built `job_tours`/`tour_jobs` set, or a request stuck mid-approval.
- **DF-03 — the inline request flow is single-job only.** The current approve path can create at
  most one job, but real approvals need 2+ jobs at approve time (the KFC + McDonald's shared-tour
  case).
- **DF-04 — zones are a dead-end.** The Sales form makes zones UI-mandatory, but `client.zones` has
  0 rows, so a job for a client with no configured zones cannot be created at all.
- **OBS-01 — writer divergence.** The two writers drift structurally, and nothing forces a new
  writer (e.g. a future `installation` migration) to produce a well-formed row.

**Governance already in force.** `web_anon` currently holds **full DML** on `sales.jobs` (verified),
under the shared-role model of ADR-128 — so an RPC over the same table is *security-neutral* until
the direct grants are revoked. Changes follow expand→migrate→contract (ADR-067) with
`NOTIFY pgrst,'reload schema'` after every function/DDL change. In-house RPC precedents exist
(`sales.convert_lead_to_customer`, `sales.next_numeral`, `installation.my_tours`). All of the
below is **dev-only (`lm360`); no prod cutover.**

## Decision

**Decision Maker:** hkl (frozen decisions D1-D7, JOB_CREATION_PLAN.md §0, 2026-08-06).

### 1. All job creation flows through one RPC — `sales.create_job(...)` (the single door)

A single `SECURITY DEFINER` function, owner `postgres`/`lmadmin`, with **one signature and DEFAULTs
for everything optional** (never overloaded — PostgREST resolves by request-body key-set, so
overloads are ambiguous):

```sql
sales.create_job(
  p_job_name text, p_brand text, p_campaign_name text, p_year int,   -- the 4 true NOT NULLs
  p_actor text,                                                       -- audit/integrity only (see §4)
  p_client text DEFAULT NULL, p_phase int DEFAULT 1,
  p_type text DEFAULT 'btl_branding', p_site text DEFAULT NULL,
  p_description text DEFAULT NULL, p_customer_id text DEFAULT NULL,
  p_zones text[] DEFAULT '{}',                                        -- optional (DF-04)
  p_compose_campaign_id boolean DEFAULT false                         -- true = Sales-form path
) RETURNS TABLE (job_id text, job_name text, campaign_id text)
```

- **Intersection-invariant contract.** The function requires **only the 4 true `NOT NULL` columns**
  — `job_name`, `brand`, `campaign_name`, `year`. Everything else is optional with a DEFAULT. Any
  writer (present or future) that supplies those 4 gets a well-formed job and the same guarantees;
  no caller-specific assumptions live inside the function.
- **Server-side sequence ID.** `job_id = 'JOB-' || nextval('sales.job_id_seq')`. The ID stays
  opaque, monotonic, and business-fact-free (ADR-130 §1). The move off `Date.now()` is deliberate —
  see Related Decisions / ADR-130 §3.
- **`campaign_id` composed only on request.** When `p_compose_campaign_id` is true (the Sales-form
  path) the function builds `<year>-<BRAND>-<CLIENT|BRAND>-<CAMPAIGN>-P<nn>` inside the same
  transaction (calling `sales.next_numeral(...)`, which must be verified concurrency-safe before it
  is relied on); otherwise `campaign_id` is left NULL and enriched later in Sales (the lean
  approval/batch path).
- **Status is always `operational_draft`.** Both writers unify on this; `draft` is reserved for
  genuinely unsubmitted work.
- **Actor is validated as AUDIT/INTEGRITY only.** `p_actor` is rejected if NULL/empty and validated
  to exist in `hub.employee_pwa_access`. This is a data-integrity check, **not authorization** —
  see §4.
- `NOTIFY pgrst,'reload schema'` after the DDL; grant `EXECUTE` to `web_anon`.

### 2. Tour-request approval becomes one transaction — `sales.approve_job_tour_request(...)`

Atomicity is the one thing PostgREST cannot provide client-side, so the RentVeh approval moves
server-side into a second RPC (closes DF-02 + DF-03):

```sql
sales.approve_job_tour_request(
  p_request_id bigint, p_actor text,
  p_jobs jsonb          -- ARRAY; each element is a union:
                        --   {"existing": "<job_id>"}                       -> link as-is
                        --   {"new": {job_name, brand, campaign_name, year, ...}} -> create via create_job()
) RETURNS TABLE (tour_id text, job_ids text[])
```

- **Statement 1 is the idempotency guard.** `UPDATE sales.job_tour_requests SET status='approved'
  WHERE id=p_request_id AND status='pending'`; **0 rows updated ⇒ already handled** (RAISE). This is
  free idempotency — it defeats double-click, retry, and concurrent approvers with no explicit locks.
- **Then, all in the SAME transaction:** resolve each `p_jobs` element (create a new job via
  `create_job`, or verify an existing `job_id`) → create one `sales.job_tours` tour (title/dates
  from the request) → insert one `sales.tour_jobs` row per job (`status='active'`, `end_date NULL`)
  → backfill `rentveh.expenses` **and** `tourexp.expenses` on `pending_request_id`.
- **Full rollback on any invalid element.** A bad/unknown existing `job_id` (or any failure) rolls
  the whole transaction back with a per-element error — **never a partial link**. This is the direct
  DF-02 fix.
- The **union input** delivers multi-job approve (D1) and existing-job linking in one call; the
  reject path stays client-side (a single UPDATE needs no atomicity).

### 3. Constraint change — drop the global name-UNIQUE, add a partial campaign_id-UNIQUE

- **DROP `jobs_job_name_key`** (the global `UNIQUE` on `sales.jobs.job_name`). Under the ADR-130
  identity model, `job_id` is the only identity and display names are renameable labels that may
  duplicate. This exercises the reversibility ADR-130 §4 reserved for itself (see Related Decisions).
- **ADD a partial `UNIQUE` index on `campaign_id WHERE campaign_id IS NOT NULL`.** The semantic
  campaign string stays collision-proof where present — preserving Sales' friendly "already exists —
  change Phase or Campaign name" behaviour, which rides on the `23505` unique-violation. Free-text
  `job_name` is now fully unconstrained; the composed `campaign_id` carries the light identity guard.
  The `23505` is deliberately allowed to propagate **uncaught** from the RPC so the existing Sales UI
  handler keeps working unchanged.

### 4. Authorization — honestly framed: this ADR adds integrity, NOT authorization (D2)

`p_actor` is **self-asserted** under the shared `web_anon` model (ADR-128): any caller can pass any
actor string. The `hub.employee_pwa_access` check is **audit + data-integrity only** — it records
*who claims to have acted* and rejects garbage, but it is **not** an access-control boundary and must
never be sold as one. `web_anon` already holds full DML on `sales.jobs`, so wrapping writes in an RPC
is **security-neutral** until the direct grants are revoked (§5).

**Real enforcement — deriving the actor from ADR-105 signed-JWT claims rather than a request
parameter — is explicitly DEFERRED to a future, separate ADR.** It is out of scope here.

### 5. Contract step — REVOKE web_anon's direct DML (the step that makes the RPC the only door) (D7)

After both PWAs **and** the batch screen are verified on the RPCs, `REVOKE INSERT, UPDATE, DELETE ON
sales.jobs FROM web_anon` (SELECT and RPC `EXECUTE` remain). This is the only step that genuinely
reduces the attack/mistake surface — before it, the RPC is one door among many; after it, it is the
*only* door, and any undiscovered direct writer fails loudly (which is the point). **Gated on
explicit hkl confirmation at execution time**, per the ADR-067 change-safety gate; rollback is a
one-line re-GRANT.

### 6. Batch job creation (D6)

A Sales **"Batch Add" screen** with (a) a multi-row entry grid and (b) Excel/CSV upload with
**AI header-mapping** for messy headers (the pattern proven in the `shiprocket-field-mapping` skill)
→ a preview table → confirm → **N × `create_job`** calls → a per-row report (created / failed +
reason). The **minimal-4** columns are required (`job_name, brand, campaign_name, year`); client,
phase, type, site, description, customer are optional. **Rows fail independently** — one bad row
never blocks the rest.

### 7. Zones become soft-optional (DF-04)

The mandatory-zone UI rule is **removed**. `create_job` accepts `p_zones` optional (default `{}`),
and the Sales form's blocking zone-chip requirement (and its dead-end hint) is deleted. AI-suggested
zones with a user-confirm flow for MIS is a **future, separate** effort — explicitly out of scope.

### 8. Dev data clean slate (D4)

All existing dev jobs — including the Lenovo seed and the `bt-job-*` P&L fixtures — are deleted
**after a dependency sweep first** (enumerate FK + soft references before any DELETE), then
**reseeded VIA the new RPCs** (dogfooding). Test-plan docs (`HUMAN_TEST_PLAN.md`,
`FIELD_TEST_CHECKLIST.md`, `TEST_PLAN.md` baselines) are re-anchored to the new seed. **Dev-only; no
prod.**

## Implementation Notes

- **Sequencing is expand→migrate→contract (ADR-067):** DDL (sequence + constraint swap) and both
  RPCs land while both PWAs still use their old paths (EXPAND, verified over REST-as-anon); then each
  UI migrates independently with its own `CACHE_VER` bump (MIGRATE — **RentVeh has TWO version
  constants, `index.html` AND `sw.js`; bump both**); then the clean-slate reseed and the gated
  `REVOKE` (CONTRACT). Chunk breakdown: JOB_CREATION_PLAN.md §5 (C1-C9).
- `NOTIFY pgrst,'reload schema'` after **every** function/DDL change.
- **Single-signature evolution rule:** new parameters are only ever added with DEFAULTs — never a
  second overload, because PostgREST resolves overloads by body key-set and would become ambiguous.
- **Error-shape note:** `23505` (the `campaign_id` partial-unique violation) propagates **uncaught**
  from `create_job` for UI compatibility — the existing Sales handler already understands that shape.
- **Test RPCs via the REST endpoint as `anon`** (curl with `Accept-Profile: sales`), never as
  `lmadmin` in psql — the anon path is what production callers use, and admin-in-psql bypasses the
  grants under test.
- **Contract for future writers:** any new writer (e.g. an `installation` job-creation migration)
  calls `create_job` with the 4 NOT NULLs and inherits the sequence ID, `operational_draft` status,
  and campaign_id/zones handling for free.
- Dev (`lm360`) only.

## Alternatives Considered

- **Keep the client-side multi-POST orchestration for approve (status quo).** Rejected — this is the
  whole reason the second RPC exists. PostgREST gives no cross-statement atomicity, so an interruption
  mid-sequence orphans `job_tours`/`tour_jobs` (DF-02). Only a server-side transaction can make
  approve atomic + idempotent.
- **Overload `create_job` per caller** (a thin signature for approve, a rich one for the Sales form).
  Rejected — PostgREST resolves function overloads by the request-body key-set, which is fragile and
  ambiguous the moment two signatures share keys. One signature with DEFAULTs is unambiguous and
  evolvable.
- **Keep the `'JOB-' + Date.now()` generator** (ADR-130 §3's retained choice). Rejected **here** for
  a reason ADR-130 §3 never weighed: `Date.now()` collides under concurrent/batch creation (two jobs
  created in the same millisecond get the same ID). The batch screen (§6) makes concurrent creation
  concrete. A DB sequence is collision-proof and stays opaque, so ADR-130 §1 still holds — see
  Related Decisions.
- **Enforce real authorization now** (derive/verify the actor as an access boundary). Rejected —
  impossible under the shared `web_anon` model where every caller is anonymous; real enforcement
  needs the ADR-105 signed JWT and is deferred to its own ADR (§4, D2).
- **Leave the two writers divergent** and just document the difference. Rejected — that is OBS-01,
  the exact drift this ADR removes; a documented-but-unenforced convention is how the divergence
  arose.
- **Keep zones mandatory** and require every client to configure zones first. Rejected — `client.zones`
  is empty and its editor is a separate PWA, so mandatory zones is a hard job-creation dead-end
  (DF-04) for zero present value; zones become soft-optional.

## Consequences

**Positive:**
- One creation path, one record shape — OBS-01's divergence is closed, and every future writer that
  supplies the 4 NOT NULLs inherits the same guarantees.
- Approve is atomic + idempotent + multi-job (DF-02 + DF-03 closed); no more orphaned `job_tours` on
  interruption, and double-click/retry/concurrent-approver are all safe with no explicit locks.
- Zones no longer dead-end job creation (DF-04 closed).
- Collision-proof server-side IDs; the friendly duplicate-campaign UX is preserved via the partial
  `campaign_id` unique + uncaught `23505`.
- After the gated `REVOKE`, the RPC is the *only* door — any stray direct writer fails loudly.

**Negative / Trade-offs:**
- **Creation logic moves into PL/pgSQL.** More behaviour now lives in the database (composition,
  idempotency, rollback) rather than in JS — harder to read at a glance, and every function change
  needs `NOTIFY pgrst`.
- **Single-signature discipline is a standing constraint:** future params must be DEFAULTed, never
  overloaded.
- **`23505` propagates uncaught by design** — a deliberate leak of the DB error shape to the UI for
  compatibility; a reader expecting the RPC to translate all errors will be surprised.
- **Testing must go through REST-as-anon, not psql-as-admin** — admin bypasses the grants under test,
  so a psql-only "it works" is not evidence.
- The `REVOKE` (§5) is security-neutral until it lands and is human-gated; until then the RPC adds
  integrity and atomicity, **not** access control (§4).

**Risks and mitigations:**

| Risk | Mitigation |
|---|---|
| `next_numeral` is not concurrency-safe, so composed `campaign_id`s collide under load | Verify `next_numeral` concurrency behaviour before `create_job` relies on it (C2); it must run INSIDE the same transaction |
| A caller reads `p_actor` as authorization | §4 states it is integrity-only, in the ADR and (per house rule) at the enforcement site; real authz is the deferred ADR-105 JWT ADR |
| The clean-slate DELETE (D4) drops rows other tables still reference | Dependency sweep FIRST (enumerate FK + soft refs), report before deleting, cascade-order the DELETE |
| A stale PWA still POSTs directly after the REVOKE | That is the intended loud failure; the REVOKE is gated behind both PWAs + batch being verified on the RPCs (C4/C5/C6 before C8) |

## Related Decisions

- **ADR-111** — Unified Job Record in `sales.jobs` (canonical). This ADR makes `create_job` the single
  writer of that canonical record; ADR-111 is untouched.
- **ADR-125** / **ADR-126** — Tours↔Jobs and entity↔job effective-dated M:N bridges (`sales.tour_jobs`).
  `approve_job_tour_request` hardens the write path *into* those bridges (atomic tour + `tour_jobs`
  rows). Both remain Accepted and are untouched here; an append-only execution note about
  `approveRequest`'s replacement is added under ADR-125 in a **later** chunk (C9), not this one.
- **ADR-128** — Shared `web_anon`/authenticator role model. The reason the RPC is security-neutral
  until §5's REVOKE, and the reason `p_actor` is self-asserted (§4).
- **ADR-130** — Opaque immutable IDs + renameable display names. This ADR **exercises two of
  ADR-130's numbered, self-declared-reversible calls**, without editing ADR-130:
  - **§4 (KEEP `job_name` UNIQUE)** pre-declared itself reversible "if duplicate live names become a
    genuine need, the constraint can be dropped without touching any ID." §3 above exercises exactly
    that clause — the drop is the reversibility ADR-130 §4 reserved, not a contradiction of it.
  - **§3 (KEEP the `'JOB-' + Date.now()` generator)** argued against a counter, but only weighed
    *cosmetic symmetry* as the reason to switch; it never considered concurrency. The move to
    `'JOB-'||nextval(seq)` is justified by same-millisecond collision under concurrent/batch create,
    a reason §3 did not evaluate. The new ID stays `JOB-`-prefixed, monotonic, and business-fact-free,
    so **ADR-130 §1 (opacity) still holds** — only the collision-prone generator changes.
- **ADR-105** — Signed JWT for proxy + native auth. The basis for the **deferred** real authorization
  (§4); this ADR does not implement it.
- **ADR-067** — Cross-PWA Change Safety Gate. Governs the expand→migrate→contract sequencing and the
  explicit human confirmation required before the §5 REVOKE (a shared-infrastructure grant change).

## References

- `sales.jobs` divergence proof (live `lm360` dev DB, 2026-08-06): `JOB-1778820423725`
  (`26-Epson-EverythingElse-BTLActivityInd-001`) and `JOB-1778744623251`
  (`26-Epson-EverythingElse-BTLActivityEps-001`) — both `status='draft'`, empty `campaign_id` (the
  thin RentVeh-writer shape).
- Writers: Sales form `sales/index.html` ~L1719 (rich, composed `campaign_id`, mandatory zones,
  `operational_draft`); RentVeh `approveRequest` `rentveh/index.html` ~L1400 (thin, no
  `campaign_id`/zones/type, `draft`, ~6 sequential POSTs).
- Defects closed: DF-02 (approve non-atomic), DF-03 (single-job inline flow), DF-04 (zones dead-end),
  OBS-01 (writer divergence) — `PHASE3_BACKLOG.md`.
- RPC precedents: `sales.convert_lead_to_customer`, `sales.next_numeral`, `installation.my_tours`.
- `web_anon` holds full DML on `sales.jobs` (verified 2026-08-05/06); `sales.job_tour_requests`
  currently 0 rows (no in-flight migration risk); `client.zones` 0 rows.
- Frozen decisions D1-D7 + target design: JOB_CREATION_PLAN.md §0 + §2 (hkl, DL session, 2026-08-06,
  `C:\Users\Lenovo\Documents\tour-job-architecture\JOB_CREATION_PLAN.md`).

## Execution Findings (2026-08-06)

Built and verified same day (DL session, Fable 5 + Haiku execution): all chunks C0-C9 of JOB_CREATION_PLAN.md completed. C1 DDL (job_id_seq created; jobs_job_name_key constraint dropped; partial unique index on campaign_id created; schema reload notified). C2 `create_job` RPC built per §2.1 (ONE signature with DEFAULTs for all optional params; id generation via sequence; campaign_id composition conditional on p_compose_campaign_id flag; actor integrity check; status='operational_draft' always); REST-as-anon verified (happy path, missing NOT NULLs rejected, duplicate campaign_id→23505 uncaught for Sales UI handler). C3 `approve_job_tour_request` RPC built per §2.2 (idempotency guard via request-row pending→approved status; all inserts/updates in one transaction; multi-job support via jsonb array; full rollback on any invalid existing job_id); REST-as-anon verified (idempotency proved, 2nd call returns 'already handled', no dup rows; mixed existing+new jobs→1 tour+N links; invalid id→FULL rollback, no orphan; expense backfills present). C4 Sales form migrated (form calls `/rpc/create_job` with p_compose_campaign_id=true; zones optional, no longer blocking; post-save panel shows job_name prominent + job_id beneath per ADR-130; CACHE_VER sales-v22). C5 RentVeh approve migrated (approve dialog per §2.3 multi-job slots; sequential POSTs deleted; calls `/rpc/approve_job_tour_request`; bumped BOTH RentVeh version constants index.html v15→v16 + sw.js v10→v11). C6 batch page live at /sales/batch/ (grid multi-row entry + Excel/CSV upload with AI header-mapping; per-row create_job calls; independent row failures; self-hosted libs per ADR-013). C7 clean-slate reseed (FK-order corrected: challans→invoices→offers; 7 old dev jobs + 17 dependents deleted; reseeded via RPCs: JOB-20 'Acme Diwali Counters', JOB-21 'Acme Diwali Flex', 2 tours T-000001 'Acme Combined Trip' planned with both jobs, T-000002 'Acme Week 1' completed; final baseline 2 jobs / 2 job_tours / 3 tour_jobs / 2 requests / 4 rentveh.expenses). C8 executed with scope refinement (revoked INSERT+DELETE from web_anon on sales.jobs; UPDATE deliberately retained for 3 live PATCH flows: ~L1755 edit, ~L1828 status, ~L4329 enrichment; also revoked TRUNCATE/TRIGGER/REFERENCES found wrongly granted; schema reload notified). C9 documentation/memory close-out (PHASE3_BACKLOG.md DF-02/03/04/OBS-01 moved to Closed; HUMAN_TEST_PLAN.md DF-02 re-marked as FIXED; project_rentveh_tour_job_linkage.md banner added; MEMORY.md updated; JOB_CREATION_PLAN.md execution log added).

**Key technical findings:** (a) client-segment omit in campaign_id composition is AUTHORITATIVE from live UI and now the RPC (omit segment when client is blank); (b) tour-id MAX+1 collision race noted as acceptable (PK collision → clean transaction rollback, no application-level lock needed); (c) C8 verified INSERT 401, RPC 200, PATCH 204 — permissions correctly gated.

**Status:** ADR-133 remains **Proposed** (pending hkl acceptance after Phase 2 field-test completion). All dev. No prod cutover.
