feat(042): approvable tier changes on active licence assignments

[studert]

Summary

A tier-upgrade request arrived for a licence someone already held, and the Hub had no way to action it. Approving it failed outright:

Requester already has an active assignment for this tool (assignment #N). Revoke the existing assignment first, or cancel this request.

Revoke-then-recreate wasn't a workaround — it orphans the comment thread, resets assigned_at, drops the stored API key, and double-bills the person in the switch month.

The root problem turned out not to be a missing feature but three unreconciled implementations of the same operation. This unifies them and makes "change the tier of an active assignment" approvable. No schema change, no migration.

Spec, plan and full running log: specs/042-assignment-tier-change/ — implementation-plan.html for the design, implementation-notes.html for the challenge pass, deviations, corrections and verification.

Licence-request approval now has three modes

approveRequestSchema becomes a discriminated union, not a flat flag:

mode	Requester already holds the tool	Outcome
create	no	Insert, exactly as before
create	yes	Refused (advice updated: retier instead of revoke)
change_tier	yes, manual seat	New — retier in place, link the request
change_tier	yes, same tier	No-op on the row, but still approves and links
link_existing	yes	New — approve and link, mutate nothing

A union rather than a flag because loadToolAndTier runs before the transaction and unconditionally demanded an API key — so create-only rules had to be unreachable from the other modes, not merely skipped. Claude Console is the only requires_api_key tool and has the deepest tier ladder, i.e. exactly the upgrades a shared shape would have broken.

link_existing is what gives GitHub Copilot an approvable outcome at all: all 63 live seats are sync-managed, so a Business→Enterprise request was refused by the create guard and by the sync guard. Now it matches 032-v2's provision-first philosophy — upgrade the plan in GitHub, approve as linked, the tier arrives via sync.

Why the tier change mutates in place

Per-period expected spend sums cost_at_assignment_cents by date overlap, flat monthly price, no proration. Closing one row and opening another therefore double-counts the switch period — one seat, two prices.

Both options are wrong, in different places:

	Error	Where
Close old + open new	1 period × old price	The switch month only
In place (chosen)	N closed periods × delta	Every closed period the row spans

In-place is the larger total error. It was chosen anyway because it is the convention this codebase already committed to — actions/tools.ts states it in a comment and migration 0024 backfilled production to match — and because the double count corrupts the current month, the figure people act on. A feature whose purpose is to reconcile three rival semantics shouldn't introduce a fourth costing model. Confirmed with the requester after the trade-off was laid out; alternatives (boundary-snapped supersede, proration, freezing expected spend at period close) are on record as deferral D-1.

Bugs fixed along the way

• The assignment detail page crashed for every sync-managed seat — Tooltip used without a TooltipProvider; the assignments table only works because DataTable happens to supply one. Found by the browser pass.
• The tier control rendered empty on every assignment — radix resolves a Select's display value from the selected item's children, which load asynchronously. Pre-existing, but this feature's primary control.
• The detail page's tier <Select> was unguarded against Copilot sync — an admin could change a synced seat's tier, see [SAVED], and have cron silently revert it. The gate is tool-based, not source-based, because sync takes over manual rows too.
• loadToolAndTier never checked access_tiers.is_active — the request path could put a live seat on a deactivated tier, which both other entry points already refuse.
• The tier write had no status guard — a concurrent revoke could be retiered, corrupting the historical snapshot that revoked rows are supposed to preserve. Now a conditional UPDATE on observed status + tier_id (which doubles as an optimistic lock against the price cascade) with a zero-rows sentinel.
• Cost invalidation was both wrong and wasteful — it duplicated updateTier's path list (already drifted) and fired on every edit, including workspace/API-key-only changes and no-ops. Now one shared list, gated on cost actually changing.
• Dead code removed — getExpectedSpendForPeriod had zero callers and carried a status filter the surviving readers don't, inviting a future "reconcile these two" against a function nobody read.

Notable non-changes

Tier semantics live in one pure module (lib/assignments/tier-change.ts) shared by updateAssignment and approveRequest. No new action, no new dialog, and no tier_changed_at column — change_history already stores old/new tierId with actor and timestamp, and now backs a tier timeline on the assignment page.

Verification

Gate	Result
pnpm typecheck	clean
pnpm test	659 passed, 53 files (from 629)
pnpm lint / eslint on changed files	clean
pnpm test:integration (full directory)	8 files, 56 passed, 7 todo, 2 skipped
Playwright browser pass	9/9 checks

Integration and browser runs were against a branch of the real database — 223 users, 5 tools, 309 active assignments, 27 requests, 1933 change-history rows.

The double-count property is pinned by a unit test. The overlap predicate was extracted from getBudgetWithCosts specifically so it runs in CI: the integration-tests job in .github/workflows/ci.yml is commented out, so an integration test could never have gated a merge. fetchPerToolByPeriod now shares that predicate too, with its long-standing > vs >= edge case preserved as an explicit parameter and its own test.

Browser pass, end to end: a real Claude seat moved Standard Seat → Premium Seat → Standard Seat (both directions); header and cost followed ($25.00 ↔ $125.00); feedback as inline [SAVED], not a toast; the timeline rendered both hops attributed and dated. Confirmed directly in the database afterwards: exactly one active row for that user+tool, with tierId 5→6 and costAtAssignmentCents 2500→12500 written under a single timestamp. A real copilot-sync seat showed "Managed by sync", tier control disabled but still legible, and no re-pricing disclosure.

Two integration-test defects the run exposed, both fixed here: this spec's audit rows broke the suite's teardown via change_history.changed_by's onDelete RESTRICT (the suite reported 12/12 tests passing and a failed file); and one of my own assertions tested the wrong layer, expecting approveRequest to render a template it deliberately stores verbatim.

Process: plan → 5-lens adversarial challenge pass (2 blocking + several major findings adopted; scope roughly halved, and it disproved the plan's original costing argument) → implement → /simplify (4 lenses, 10 applied, 4 skipped with reasons) → verify. All logged in implementation-notes.html, including two claims I had to retract.

Known limitations: the approval dialog's three modes are covered server-side but weren't clicked through in a browser. Copilot's sync-managed path was verified as gated, not as an end-to-end GitHub round trip.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
ai-developer-hub	Ready	Preview	Aug 4, 2026 8:31am

[Copilot]

Pull request overview

This PR (spec 042) makes “change the tier of an active license assignment” an approvable, unified operation across existing entry points (admin edit + request approval), without schema changes. It consolidates tier-change semantics into shared pure helpers, introduces an approval-mode discriminated union to make create-only rules unreachable for other flows, and hardens cost attribution + cache invalidation behavior.

Changes:

• Add three approval modes (create, change_tier, link_existing) and implement in-place tier mutation with correct rollback semantics.
• Centralize tier-change and cost-attribution logic (buildTierChange, shared overlapsPeriod/sumExpectedSpendCents) with new unit tests that can gate CI.
• Add sync authority gating (tool-based) and UI updates (tier timeline, disabled tier controls for sync-managed seats, “Change tier” affordance).

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/license-requests/render-template.test.ts	Updates template context test data for tier.previousName.
tests/unit/assignment-tier-change.test.ts	New pure-logic tests for tier-change semantics and period overlap/cost attribution invariants.
tests/unit/actions/assignment-tier-change.test.ts	New mocked-action tests covering updateAssignment retier path + approveRequest mode matrix.
tests/integration/actions/license-requests.test.ts	Integration updates for new approval schema discriminator and new 042 tier-change/link behaviors + teardown fixes.
src/lib/validators.ts	Converts approveRequestSchema to a discriminated union and adds legacy linkExistingAssignmentSchema.
src/lib/license-requests/render-template.ts	Extends template context with always-bound tier.previousName and registers the variable path.
src/lib/budget-utils.ts	Extracts and exports overlapsPeriod + sumExpectedSpendCents for shared cost attribution and CI-unit-test coverage.
src/lib/assignments/tier-change.ts	New pure module for shared tier-change semantics (buildTierChange, delta helper, sync-managed error message).
src/lib/assignments/sync-authority.ts	New tool-based sync authority helpers (isCopilotSyncActive, name-based sync-managed checks).
src/lib/assignments/revalidate.ts	New shared cache revalidation helpers for cost-derived pages.
src/components/assignments/sync-managed-badge.tsx	New reusable “Managed by sync” badge that safely owns a TooltipProvider.
src/app/users/[id]/user-detail-client.tsx	Adds “Change tier” CTA per active assignment with server-provided sync-managed tool gating.
src/app/users/[id]/page.tsx	Computes syncManagedToolNames via isCopilotSyncActive() and passes down to client.
src/app/settings/license-templates/template-editor-dialog.tsx	Updates sample/template-preview context to include tier.previousName.
src/app/requests/[id]/request-detail-client.tsx	Threads activeAssignments into approval + record dialogs.
src/app/requests/[id]/record-assignment-dialog.tsx	Adds legacy “link existing seat” path when a duplicate active seat exists.
src/app/requests/[id]/approval-dialog.tsx	Implements mode-aware approval UX (create/retier/link), template selection, and tier delta copy.
src/app/assignments/assignments-client.tsx	Reuses SyncManagedBadge for table rows instead of duplicating tooltip logic.
src/app/assignments/[id]/page.tsx	Adds sync-managed gating + tier-history fetch and passes data to client.
src/app/assignments/[id]/assignment-detail-client.tsx	Disables tier select when sync-managed, fixes Select value rendering, and renders tier timeline.
src/actions/tools.ts	Reuses shared cost-surface revalidation helper instead of duplicating path list.
src/actions/reports.ts	Uses shared overlapsPeriod predicate (with explicit revoked bound behavior preserved).
src/actions/license-requests.ts	Core 042 logic: new approval modes, rollback discipline, retier/link flows, validation changes, and cache invalidation gating.
src/actions/history.ts	Adds assignment tier timeline reader (getAssignmentTierHistory).
src/actions/budget.ts	Removes dead getExpectedSpendForPeriod and switches to shared expected-spend summation helper.
src/actions/assignments.ts	Unifies tier-change semantics via shared helper and adds race-safe conditional update + transactional audit.
specs/042-assignment-tier-change/implementation-plan.html	Design/plan documentation for 042, including tradeoffs and acceptance criteria.
specs/042-assignment-tier-change/implementation-notes.html	Implementation log, deviations, verification notes, and rationale record.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[studert]

Fixed in f4bb80b — you're right, and this was a real defect, not just a hardening gap.

loadToolAndTier proves the tier belongs to approval.toolId, but nothing tied the assignment to it, so the two were free to disagree. Both consequences you name are real: a stale dialog or crafted payload could write another tool's tier onto the seat, and because the sync-managed refusal also keys off the selected tool rather than the row's own, passing a Copilot assignmentId with a non-Copilot toolId would have walked straight past that gate.

Added the explicit target.toolId === approval.toolId check before buildTierChange, which mirrors the same-tool rule updateAssignment already gets for free by scoping its accessTiers lookup to assignment.toolId — splitting that validation across two loads is exactly what opened the hole. Also added a unit regression test that asserts both the refusal and that nothing is written to the assignment.

Worth noting the adversarial challenge pass missed this: it scrutinised the mode matrix and the sync gate separately, but never their interaction. Recorded in implementation-notes.html.

[studert]

Fixed in f4bb80b — restricted to with: { changedByUser: { columns: { name: true } } }, since name is the only field rendered.

One clarification on severity, so the record is accurate: the mapped return shape is { id, previousTierName, newTierName, changedByName, createdAt }, so passwordHash never reached the client. It was needless loading of a password hash into server memory rather than an exposure. Still worth fixing, and it now matches the columns: {...} convention used elsewhere in this file.

[studert]

Fixed in f4bb80b — you're right, the comment was wrong and I've rewritten it.

Confirmed against updateTier (src/actions/tools.ts): the cascade sets costAtAssignmentCents on rows matched by tierId, leaving tier_id itself untouched — so an eq(tierId, observed) predicate cannot see it. Same for syncBillingData.

The comment now states only what the guard actually covers (a status flip to inactive from any of the four revoke paths, and a concurrent retier), then names the price-cascade race it does not cover and why it is tolerable: our cost value is read before the transaction and would win, but the error is narrow and self-correcting — the next cascade or retier overwrites it. A real fix is row-level compare-and-swap on updatedAt, which the plan deliberately scoped out (the guard was scoped to the tier-snapshot invariant); that trade-off is recorded in implementation-notes.html under the simplify pass's skipped findings.