#4806 gave a site admin one write on a help desk group nobody is left to
run, and #4926 made the Person field of that form a people search. Neither
closed the other half: `storeAgent()` was gated on
requireManagerAccessOrOrphanRecovery(), `destroyAgent()` on plain
requireManagerAccess(). So the rescue could appoint and never un-appoint —
the row it had just written sat in the rescuer's own Agents list marked
"Not yours to manage", with no Remove control and a 403 behind the DELETE.
A `manager` press was worse: the write ends the orphan state, so the group
left `orphanedGroupIds` and the admin could not remove the row, appoint
anybody else, or appoint themselves. The only person who could undo it was
the person wrongly appointed — #4806's frozen group, reached through
#4806's cure.
destroyAgent() now asks requireRemoveAccess(): a manager of the group, or a
site admin while the group is STILL orphaned (symmetric with the write and
strictly narrower — on such a group they could appoint themselves and then
remove anything anyway), or a site admin undoing a row their own rescue
wrote. That last arm needs provenance, hence `recovered_by`: "the group
would be orphaned without this row" cannot tell a mistaken appointment from
the sole working manager of a healthy group, and admitting the second would
let any admin strip a manager and seize the desk (#4710). The stamp is
cleared the moment a manager of the group writes the row themselves.
`removableAgentIds` is that gate in prop form, so the Remove button and the
DELETE cannot disagree, and the confirmation says when a removal hands the
group back to having nobody.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#920 made a merged ticket read-only on the server and shipped NO data change,
deliberately: its guard reads `status === 'merged'` and not
`merged_into_id !== null`, so the rows the bug had already produced — the
pointer set, the status back to `open` — stayed EDITABLE and could be corrected
in the product instead of in SQL. What it left behind was rows that are
simultaneously live in the All Open queue and merged into another ticket, with
nothing on any screen saying so and nothing offering to fix them.
Both halves of the card ship, because option A is a BEHAVIOUR RULE and a
behaviour rule is a setting:
B — the office fixes them one at a time. `merged_inconsistent` rides on the
model (Ticket::$appends), so the list row, the index detail pane and the
Show header all draw the same amber badge — "merged_into set but status
open — fix?" — off one predicate rather than three copies of an
expression. The badge IS the button: it opens a modal that says what will
change and what will not, and posts to a new `mark-merged` verb that
writes `status` and nothing else. A new "Merged" view lists every ticket
folded into another one BY THE POINTER, so the consistent and the broken
appear side by side; a status filter would show one or the other and
never the pair worth comparing.
A — the migration re-marks all of them at once, and it is INERT unless
`ticketing.auto_repair_merged_on_deploy` is on. It defaults OFF, which is
also what a box that has never heard of the setting does, so this can
ship ahead of the operator's decision and change nothing unasked. The
toggle is on the Automation tab with the live count beside it, because
nobody should arm a bulk data change without seeing how many rows it
touches.
`MergedTicketRepair` is one implementation with three callers for one reason:
"which rows are broken" and "what fixing one means" must not drift, or the
badge offers a fix on a row the migration skips.
THE PROD COUNT THE CARD ASKS FOR IS NOT IN HERE, and it could not be: this
worker box holds no ssh private key, so central-prod is unreachable from it and
there is no prod container locally. `php artisan ticketing:repair-merged
--dry-run` is the read-only answer for whoever does have access — it prints the
card's own SELECT COUNT and lists the rows, and `--force` (not the bare form)
is the only spelling that writes.
Repairing NEVER reverts. Messages, participants, assignee and title edits made
to a row while it was wrongly editable are somebody's work and the only copy of
it; there is no record of what the fields held before the bad save, so
"reverting" would invent a previous state rather than restore one.
Proved by tests/Feature/Ticketing/MergedTicketInconsistentRowsTest.php (22
tests) in dashboard-shell, over the fixture the card names — one inconsistent
row beside a genuinely merged one, both produced by the real merge endpoint.
The NEGCTRL half is what keeps it honest: the consistent row and an ordinary
open ticket must come out of every path untouched, and a repaired row must keep
the reply and the rename it was given after the merge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merge and Split both 500ed. The audit note each writes uses `source = 'system'`
and the column is `enum('web','email')`, so the insert threw inside the
transaction and nothing committed — which is why the operator saw no state change
at all rather than a half-done merge.
Widened to an enum rather than a varchar, and the reasoning is the same test that
made `tickets.status` a varchar in `2026_04_17_000002`: statuses became
operator-definable rows, so their domain is open. `source` is the opposite — all
three values are set by code, an operator can never add a fourth, and the enum is
what keeps a typo'd source loud instead of a silently unfilterable row.
`TicketMessage::system()` is now the only place a system note is written. Three
scattered literals is how this reopens.
A SECOND 500 ON THE SAME ACTION, unreported and one longer title away. `split()`
appended ' (split)' to a title the product validates at max:255, so splitting a
full-length title produced 263 characters and the identical hard failure. Bounded
at the write rather than widened, and the distinction is written down: widen when
the data is right and the column is wrong; bound when the column expresses a real
product rule. 255 is the product's own limit, so a 263-character title would be
un-editable forever — the edit form would bounce it every time.
And `split()` now honours its own comment. Line 682 has always said "exclude
system notes" while the code copied them; it never mattered because no system row
could exist. The moment the enum accepts them, splitting a merged ticket would
carry "Merged from ticket QAX-0005." onto a ticket that was never merged.
#224 — the next number came from the live rows, so deleting a ticket freed its
reference and two tickets could both be QAX-0004. An old email then points at a
different ticket than it did. The sequence is now held on the group and only ever
moves forward.
#228 — ticketing notifications carried no user, so the Notification Log showed
the message type where the recipient belongs and the per-person opt-out was
ignored: somebody who had opted out still received them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ec3REU4LQmhuchRRn6dZHk
There was no destroy route at all — only settings.groups.store and .update — so
the e2e CRUD spec, which creates an ephemeral group as its documented setup step,
could never clean up (a DELETE-spoofed POST returned 405). Every sweep left litter
behind, and an operator who mistyped a group was stuck with it forever.
Deleting a group cannot be unconditional: tickets are the audit record of real
correspondence, and tickets.group_id is a RESTRICT FK. So destroyGroup counts the
tickets and picks:
0 tickets -> real delete (agent access / projects / email connections are
cascadeOnDelete, so nothing dangles). This is the teardown and
undo-a-typo path.
>=1 ticket -> archived_at = now(). Tickets untouched and still readable.
The caller is TOLD which happened rather than inferring it from a 302: an
expectsJson caller gets {"outcome":"deleted"|"archived","ticket_count":N}, the UI
gets distinct flash wording, and the confirm modal says which it will be BEFORE
you click ("This group has 1 ticket and will be archived, not deleted").
Archiving is idempotent; restore is a separate route.
Archived groups leave the PICKERS but nothing else: index() and create() use
scopeActive, store() validation adds whereNull('archived_at') so a stale id cannot
bypass the picker, and update() rejects transfers INTO an archived group while
always allowing the ticket's own group — otherwise a ticket stranded in an
archived group would become uneditable. Index.vue re-adds that own group to the
transfer select labelled "(archived)" so the select is never blank. Deliberately
NOT filtered: the Settings groups list (archived rows show a badge + Restore), and
agentGroupIds()/isAgent()/isManager(), which read ticketing_agent_access — so
existing agents keep working the tickets that remain. No global scope; exclusion
is per call site.
Gating is byte-identical to updateGroup: inside the existing
permission:ticketing.settings group plus requireManagerAccess(), so a super admin
with no manager row on that group is still refused (asserted).
Migration is additive — one nullable timestamp. down() is a deliberate NO-OP,
matching the myedpuller-sync warnings-column precedent: DashboardDoctor greps the
whole file for drop/rename patterns, so a dropColumn even in down() would raise a
"pending destructive migration" advisory on every host where it is still pending,
which is a confusing warning for a purely additive change. Verified against
doctor's exact regex, not a looser grep.
Tests: 9 (69 assertions). 10 mutations, all 10 detected — including forcing a hard
delete on a group with tickets, which throws a QueryException from the RESTRICT FK,
i.e. the database backs up the application check.
Browser-verified on dev: the confirm modal wording for both cases, the ARCHIVED
badge and Restore, the archived group vanishing from both the create picker and the
All-Tickets switcher while its ticket stays listed, and ZERO native
confirm/alert dialogs. doctor exit 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
