arutsh · arutsh · Sep 19, 2026 · Sep 19, 2026
diff --git a/docs/development/WORKFLOW.md b/docs/development/WORKFLOW.md
@@ -0,0 +1,87 @@
+# OpenSpec → Issue → Branch Workflow
+
+How a piece of work moves from an OpenSpec change to merged code, and how the
+naming stays consistent end to end.
+
+## 1. OpenSpec change naming
+
+Change directories (`openspec/changes/<name>/`) are a flat identifier to the
+`openspec` CLI, so the name stays a single kebab-case string — no slashes:
+
+```
+<service>-<type>-<issue>-<description>
+```
+
+Example: `shared-feat-292-audit-mixin-auto-population`
+
+- **service** — lowercase, one of: `ai`, `backend`, `budget`, `chat`,
+  `feature`, `frontend`, `platform`, `shared`, `users` (same set used in
+  branch names, see below).
+- **type** — `feat` | `fix` | `chore` | `refactor`, matching the commit
+  prefixes already used in this repo (not GitHub's `bug` label).
+  - `feat` — new capability
+  - `fix` — bug fix
+  - `chore` — infra/tooling/deps, no behavior change users would notice
+  - `refactor` — structural change, no behavior change
+- **issue** — the GitHub issue number of the **parent** issue for this
+  whole change. Its only job is to hold the per-group sub-issues (§3) — it
+  doesn't carry its own task list.
+- **description** — short kebab-case summary.
+
+**Ordering with the issue:** a change is often proposed before a ticket
+exists. Flow is:
+
+1. `/opsx:propose` with a temporary kebab-case name (no issue number yet).
+2. Once `tasks.md` exists and the scope is real, create the parent ticket:
+   `scripts/new-issue.sh "<title>" "<body>"`.
+3. Rename the change directory to fold in the issue number:
+   `mv openspec/changes/{<temp-name>,<service>-<type>-<issue>-<description>}`.
+
+## 2. Branch naming
+
+```
+<Service>/<type>/Issue-<sub-issue>/<description>[-group<N>]
+```
+
+`Service` is title-cased (`Shared`, `Budget`, `Frontend`, `AI`, ...); `type`
+stays lowercase, taken straight from the OpenSpec change name. `<sub-issue>`
+is that **group's own** sub-issue number (§3), not the parent's — e.g.
+`Shared/feat/Issue-301/audit-mixin-auto-population-group2`.
+
+## 3. Task groups get their own sub-issue
+
+Each `tasks.md` group is tracked as a real GitHub sub-issue of the parent
+(GitHub's native sub-issue relationship, not just a text mention) — created
+automatically the first time a group is started via `scripts/start-group.sh`:
+
+- Title: `<description>: <group title> (group N)`.
+- Body links back to the parent issue and the group's `tasks.md` section.
+- Linked to the parent through GitHub's sub-issues API, so it shows up as a
+  checklist/progress bar on the parent issue.
+- Added to project board 8.
+- The sub-issue number is written back onto the group's header line in
+  `tasks.md` (`## N. Title — Issue #<sub-issue>`), so re-running the script
+  for the same group reuses it instead of creating a duplicate.
+
+## 4. Triggering a task group's branch
+
+Don't rely on remembering to create the sub-issue and branch correctly at
+each group boundary — run:
+
+```
+scripts/start-group.sh <change-name> <group-number>
+```
+
+First run for a group: creates its sub-issue (as in §3), links it under the
+parent, and derives the branch from it. Later runs for the same group: reads
+the sub-issue back out of `tasks.md` instead of recreating it. Either way it
+creates and checks out the branch and flips the sub-issue's project item to
+**In Progress**. Refuses to run if the change name doesn't parse or the
+group doesn't exist in `tasks.md`.
+
+## 5. PR / issue-closing convention
+
+Each group's PR closes its own sub-issue: `Closes #<sub-issue>` (fires the
+board's Done automation for that item). Once every sub-issue under the
+parent is closed, close the parent too — it's just a tracking issue at that
+point.
diff --git a/openspec/changes/audit-mixin-auto-population/tasks.md b/openspec/changes/audit-mixin-auto-population/tasks.md
@@ -21,11 +21,11 @@ One task group = one GitHub ticket = one PR, merged before the next group starts
 
 - [x] 3.1 Resolved at design stage (design.md Decision 5): manual assignments confirmed redundant/in-sync with the automatic listener across all 4 services, no on-behalf-of divergence found. No per-call-site removal needed.
 - [ ] 3.2 (dropped — manual assignments stay in place per Decision 5; not removed)
-- [ ] 3.3 Add/update a test proving the previously-stale-`updated_by` bug is fixed: create a row as user A, update it as user B, assert `updated_by` now equals B (not still A).
-- [ ] 3.4 Run `services/budget`'s test suite clean; PR merged.
+- [x] 3.3 Add/update a test proving the previously-stale-`updated_by` bug is fixed: create a row as user A, update it as user B, assert `updated_by` now equals B (not still A). (`services/budget/tests/test_budget_crud.py::TestUpdateBudgetAuditTrail` — exercises `update_budget`, which never sets `updated_by` itself, proving the automatic listener is what fixes it.)
+- [x] 3.4 Run `services/budget`'s test suite clean; PR merged. (370 passed [+1 net new]. PR not yet opened.)
 
 ## 4. Enable in users service — depends on 1, 2
 
-- [ ] 4.1 Verify `DonorGranteeModel` and `BugReportModel` (both already use `AuditMixin` but currently leave the columns `NULL`) now get `created_by`/`updated_by` populated automatically with no CRUD changes needed.
-- [ ] 4.2 Add regression tests for both models asserting `created_by` is populated on creation via their existing routes.
-- [ ] 4.3 Run `services/users`'s test suite clean; PR merged.
+- [x] 4.1 Verify `DonorGranteeModel` and `BugReportModel` (both already use `AuditMixin` but currently leave the columns `NULL`) now get `created_by`/`updated_by` populated automatically with no CRUD changes needed. (Confirmed: `create_bug_report`/`create_donor_grantee` never set `created_by`/`updated_by`; the automatic listener now fills both with no crud.py changes.)
+- [x] 4.2 Add regression tests for both models asserting `created_by` is populated on creation via their existing routes. (`services/users/tests/test_audit_trail_routes.py` — real JWT through the actual routes, not `make_client`'s `get_validated_user` override, which would bypass the contextvar-setting code being tested.)
+- [x] 4.3 Run `services/users`'s test suite clean; PR merged. (196 passed [+2 net new]; `shared` re-run clean too — 117 passed, 1 pre-existing unrelated worktree-scanning failure. PR not yet opened.)
diff --git a/openspec/changes/budget-fix-295-category-cascade-delete-fix/.openspec.yaml b/openspec/changes/budget-fix-295-category-cascade-delete-fix/.openspec.yaml
@@ -0,0 +1,5 @@
+schema: spec-driven
+created: 2026-09-19
+author: Norair Arutshyan
+priority: high
+skip_specs: true
diff --git a/openspec/changes/budget-fix-295-category-cascade-delete-fix/design.md b/openspec/changes/budget-fix-295-category-cascade-delete-fix/design.md
@@ -0,0 +1,33 @@
+# Design
+
+## Context
+
+See proposal.md - Why for the failure mode. Relevant existing state:
+
+- `budget_categories.budget_id` has a DB-level FK with `ondelete="CASCADE"` (`services/budget/migrations/versions/000014_scope_budget_categories_to_budget.py`), added deliberately when categories were scoped one-per-budget — the DB already expects categories to disappear when their budget does.
+- `BudgetModel.categories` (`services/budget/app/models/budget.py`) is a plain `relationship(..., back_populates="budget")` with no `cascade` and no `passive_deletes` setting.
+- `delete_budget_service` (`services/budget/app/services/budget_services.py`) calls `get_budget_service`, which does **not** eager-load `categories` in this path, then calls `session.delete(budget)` + `commit()`.
+- Without `passive_deletes=True`, SQLAlchemy's unit-of-work treats "this budget is being deleted" as "I must disassociate its `categories` collection" — since there's no delete cascade configured, it lazy-loads the (unloaded) collection during flush and issues an `UPDATE budget_categories SET budget_id = NULL ...` for each row, which the DB rejects (`budget_id` is `nullable=False`), surfacing as `IntegrityError` → the generic 400.
+- Because the collection load happens inside `flush()`, which the SQLAlchemy asyncio extension runs inside a greenlet, this doesn't crash as a `MissingGreenlet` — it produces a normal `IntegrityError`, so it slipped past the `async-persistence` capability's existing "no implicit lazy-load" guard without tripping any greenlet-specific detection.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Make `DELETE /api/v1/budgets/{id}` succeed for a budget whose only remaining "problem" is a category row (no lines, reports, receipts, or currency conversions).
+- Keep the DB as the single source of truth for the cascade, rather than teaching the ORM to duplicate it.
+
+**Non-Goals:**
+- Cleaning up orphaned `budget_categories` rows when a line is deleted (separate, pre-existing bookkeeping gap; not required for this fix — see proposal.md).
+- Changing cascade behavior for `budgets.lines` or `budgets.reports` — those intentionally block budget deletion via `IntegrityError` today (a budget with real lines/reports/receipts should not be silently hard-deletable), and that behavior is unaffected by this change.
+
+## Decisions
+
+**Use `passive_deletes=True` on `BudgetModel.categories`, not ORM-level `cascade="all, delete-orphan"`.**
+- `passive_deletes=True` tells SQLAlchemy "don't manage this relationship's deletes yourself, the DB's `ON DELETE CASCADE` already does it" — it skips the lazy-load-and-null-out step entirely and just issues `DELETE FROM budgets WHERE id = ...`, letting Postgres cascade.
+- Alternative considered: `cascade="all, delete-orphan"`. This would make SQLAlchemy load the collection and emit an explicit `DELETE FROM budget_categories WHERE id IN (...)` before deleting the budget — functionally similar end state, but it still requires the lazy-load (same eager-load discipline the `async-persistence` capability calls for) and does redundant work the DB already does via its own `ON DELETE CASCADE`. `passive_deletes=True` is the smaller, more precise fix that matches the existing DB-level intent from migration `000014`.
+- `passive_deletes=True` requires the DB constraint to actually be `ON DELETE CASCADE` (true here, confirmed in the migration) — it would be the wrong choice if the constraint were `RESTRICT`/`NO ACTION`, since then rows would just silently fail to delete.
+
+## Risks / Trade-offs
+
+- [`passive_deletes=True` silently relies on the DB constraint matching model expectations; if a future migration changes `budget_categories_budget_id_fkey` back to `RESTRICT` without updating the model, budget deletion would start failing again with a real (unmasked) `IntegrityError`] → Acceptable: that failure mode is the same generic `IntegrityError` → 400 path already in place for lines/reports, so it fails safe (blocks deletion, doesn't corrupt data) rather than silently.
+- [Orphaned categories with zero lines still accumulate until their budget is deleted] → Out of scope per Non-Goals; tracked separately as a known bookkeeping gap, not a correctness or security issue.
diff --git a/openspec/changes/budget-fix-295-category-cascade-delete-fix/proposal.md b/openspec/changes/budget-fix-295-category-cascade-delete-fix/proposal.md
@@ -0,0 +1,26 @@
+# Proposal
+
+## Why
+
+`DELETE /api/v1/budgets/{id}` returns a 400 ("Budget cannot be deleted while it has existing reports, funding receipts, or currency conversions") for budgets that have none of those, whenever a budget line was ever created and later deleted on that budget. This blocks legitimate budget deletion and was caught by the `auth-budget-chain` e2e spec's DELETE-budget step in CI, but it is a real product bug, not a test bug — any grantee who adds then removes a line before deleting a draft budget hits it.
+
+## What Changes
+
+- `BudgetModel.categories` gets `passive_deletes=True` so SQLAlchemy's ORM defers to the database's existing `ON DELETE CASCADE` on `budget_categories.budget_id` instead of trying to lazy-load the collection and null out each child's non-nullable `budget_id` during flush (which raises the spurious `IntegrityError`).
+- Out of scope: cleaning up an orphaned category when its last line is deleted is a separate, pre-existing bookkeeping gap (also related to the already-known global-category-namespace issue) and is not needed to fix this bug — `passive_deletes=True` makes budget deletion correct regardless of whether an orphaned category row exists.
+
+## Capabilities
+
+### New Capabilities
+
+None.
+
+### Modified Capabilities
+
+None — this restores compliance with the already-declared `async-persistence` requirement ("No lazy-load MissingGreenlet risk": every relationship a route/service reads SHALL be explicitly eager-loaded or never accessed outside an awaited context, and no request path SHALL trigger an implicit lazy-load) and the `budget-categories` requirement that every category belongs to exactly one budget without ever becoming a dangling reference; it does not change either requirement's text.
+
+## Impact
+
+- `services/budget/app/models/budget.py` (`BudgetModel.categories` relationship)
+- Fixes `DELETE /api/v1/budgets/{id}` for any budget that ever had a line added and removed
+- Unblocks `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` in the `E2E Tests` CI workflow
diff --git a/openspec/changes/budget-fix-295-category-cascade-delete-fix/tasks.md b/openspec/changes/budget-fix-295-category-cascade-delete-fix/tasks.md
@@ -0,0 +1,10 @@
+# Tasks
+
+Workflow rule: one task group = one GitHub sub-issue (of this change's parent issue) = one PR, merged before the next group starts.
+
+## 1. Fix budget deletion cascade for orphaned categories
+
+- [ ] 1.1 Add `passive_deletes=True` to `BudgetModel.categories` in `services/budget/app/models/budget.py`
+- [ ] 1.2 Add/extend a `services/budget` test that creates a budget, adds a line (auto-creating a category), deletes the line, then deletes the budget, and verify `DELETE /budgets/{id}` returns 200 with `success: true` instead of a 400
+- [ ] 1.3 Run `frontend-typescript/e2e/specs/api/auth-budget-chain.spec.ts` against the local docker-compose e2e stack (or the `E2E Tests` CI workflow) and verify the DELETE-budget assertion passes
+- [ ] 1.4 Run `services/budget`'s test suite and lint clean; PR merged
diff --git a/openspec/changes/frontend-fix-294-budget-delete-confirm-click/.openspec.yaml b/openspec/changes/frontend-fix-294-budget-delete-confirm-click/.openspec.yaml
@@ -0,0 +1,5 @@
+schema: spec-driven
+created: 2026-09-19
+author: Norair Arutshyan
+priority: low
+skip_specs: true
diff --git a/openspec/changes/frontend-fix-294-budget-delete-confirm-click/design.md b/openspec/changes/frontend-fix-294-budget-delete-confirm-click/design.md
@@ -0,0 +1,23 @@
+# Design
+
+## Context
+
+See proposal.md - Why. This is a single-file, single-method fix in a Playwright page object; none of the triggers for a fuller design doc (cross-cutting change, new dependency, data model change, security/perf/migration complexity) apply here.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Make `BudgetListPage.deleteBudget()` actually complete the delete flow the UI requires (click delete, then confirm).
+
+**Non-Goals:**
+- Changing the underlying `ConfirmDeleteButton` UX or the archive-vs-hard-delete product behavior.
+- Adding a dedicated `confirmDelete()` page-object method — one extra click inline is proportionate to the size of this fix.
+
+## Decisions
+
+- Add the "Yes" click directly inside `deleteBudget()` rather than exposing a separate confirmation step, since every current caller of `deleteBudget()` wants the delete to fully complete and there's no case where a caller wants to stop at "pending confirmation".
+- Locate the "Yes" button scoped to the same row (`this.row(name).getByRole("button", { name: "Yes" })`) rather than a bare page-level `getByRole`, so the click can't accidentally hit a Yes/No pair open on a different row.
+
+## Risks / Trade-offs
+
+- [Confirmation button text changes later] → Low risk; it's a static UI string in `ConfirmDeleteButton`'s own render, easy to keep in sync since a text change would need a page-object update anyway for the "Delete budget" title too.
diff --git a/openspec/changes/frontend-fix-294-budget-delete-confirm-click/proposal.md b/openspec/changes/frontend-fix-294-budget-delete-confirm-click/proposal.md
@@ -0,0 +1,25 @@
+# Proposal
+
+## Why
+
+The browser e2e spec (`auth-budget-crud.spec.ts`) fails in CI on its final assertion: after calling `BudgetListPage.deleteBudget()`, the budget row is expected to disappear but never does. The helper only clicks the delete icon once, which merely reveals a Yes/No confirmation — it never confirms, so no delete/archive ever happens and the test fails on a false negative unrelated to any product bug.
+
+## What Changes
+
+- `BudgetListPage.deleteBudget()` clicks the "Delete budget" icon button, then clicks the resulting "Yes" confirmation button, so the archive mutation actually fires before the test asserts the row is gone.
+
+## Capabilities
+
+### New Capabilities
+
+None.
+
+### Modified Capabilities
+
+None — this fixes the `e2e-testing` capability's existing browser CRUD journey requirement (delete step) to actually execute as already specified; it does not change what that requirement says.
+
+## Impact
+
+- `frontend-typescript/e2e/pages/BudgetListPage.ts` (test helper only)
+- No production/application code changes
+- Unblocks the `browser` project's CRUD journey test in the `E2E Tests` CI workflow
diff --git a/openspec/changes/frontend-fix-294-budget-delete-confirm-click/tasks.md b/openspec/changes/frontend-fix-294-budget-delete-confirm-click/tasks.md
@@ -0,0 +1,9 @@
+# Tasks
+
+Workflow rule: one task group = one GitHub ticket = one PR, merged before the next group starts.
+
+## 1. Fix delete confirmation click in BudgetListPage
+
+- [ ] 1.1 Update `BudgetListPage.deleteBudget()` in `frontend-typescript/e2e/pages/BudgetListPage.ts` to click the "Yes" confirmation button (scoped to the same row) after clicking "Delete budget", and verify by reading the updated method against `ConfirmDeleteButton`'s rendered Yes/No markup
+- [ ] 1.2 Run `frontend-typescript/e2e/specs/browser/auth-budget-crud.spec.ts` against the local docker-compose e2e stack and verify the delete-budget assertion (`toHaveCount(0)`) passes
+- [ ] 1.3 Run the full e2e suite (`api` + `browser` projects) locally or via the `E2E Tests` CI workflow, confirm no regressions; PR merged