lt.sortable only wired dragstart/dragend/dragover/drop — the native
HTML5 Drag-and-Drop API. iOS Safari doesn't implement HTML5 DnD on
arbitrary elements at all, and mobile Chrome's support is poor, so
kanban card status-drag was effectively unusable via touch, despite
README's "Touch-friendly controls" claim.
Added touchstart/touchmove/touchend/touchcancel handling that mirrors
the existing mouse-based behavior: a small movement threshold (8px)
distinguishes a tap/scroll from drag intent, the dragged card is
repositioned via fixed positioning to follow the finger (reparented to
document.body to avoid clipping by an overflow:hidden ancestor), and
elementFromPoint resolves the hover target for the same
placeholder-insertion logic dragover already uses, including
cross-column moves via the shared group check.
touchmove/touchend/touchcancel are registered on document rather than
the sortable list itself: since touch events keep targeting their
touchstart element for the whole gesture regardless of DOM mutations,
and the dragged item gets reparented to document.body mid-drag, a
listener on the original list would stop receiving bubbled events
once that reparenting happens.
lt.sortable lives in base.js, the shared web_template copy used by
other LotusGuild apps (per its own header comment) — this fix should
be contributed upstream too, not just kept local to this repo.
Verified with a jsdom harness (stubbing getBoundingClientRect and
elementFromPoint against known layouts) covering: sub-threshold
movement not starting a drag, same-column reorder, cross-column move
with correct final DOM parent and order, and touchcancel cleanly
resetting state. This caught a real bug during development — an
earlier version listened on the list element for touchmove/touchend,
which silently stopped receiving events after the drag-start
reparenting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
api/upload_attachment.php enforced a per-file size cap but nothing
bounded the total number of attachments on a single ticket or their
cumulative size over time — an authenticated low-privilege user could
slowly fill the uploads/ disk by attaching many files across tickets,
bounded only by the general rate limiter (which throttles request
rate, not storage volume).
Added MAX_ATTACHMENTS_PER_TICKET (50) and
MAX_TOTAL_ATTACHMENT_SIZE_PER_TICKET (100MB) config defaults, enforced
before move_uploaded_file() using AttachmentModel::getAttachmentCount()
and getTotalSizeForTicket() — both already existed in the model with
zero callers, apparently added for exactly this purpose but never
wired in.
Verified against a local MariaDB instance: with 3 existing 1MB
attachments and a 3-attachment cap, the count check correctly rejects
a 4th; with a 5MB total cap, a 2.5MB upload that would push the ticket
over the limit is correctly rejected while a small one that fits is
not.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Primary fix (#63): added the 5 env vars config.php reads and README
documents but .env.example never listed: MATRIX_DOMAIN,
SYNAPSE_ADMIN_URL, SYNAPSE_ADMIN_TOKEN, MATRIX_NOTIFY_COMMENTS, and
MATRIX_NOTIFY_ASSIGNMENTS. A deployer following only .env.example had
no indication these existed, silently missing watcher Matrix DMs and
comment/assignment notifications.
While verifying the fix by actually running .env.example through
parse_ini_file() (what config.php calls), found this file could not
be parsed at all — a real, currently-live outage for anyone following
its own first-line instruction ("Copy this file to .env and fill in
your values"):
1. PHP's ini parser treats "#" comments as fragile: punctuation like
parentheses or quotes inside a "#" comment can throw a syntax error
even though the line is meant to be inert. The file's header
comment itself (and 15+ other comment lines) tripped this. Switched
every comment to ";", which parse_ini_file treats as a true inert
comment regardless of content — verified with isolated repros of
both prefixes under all three INI_SCANNER_* modes.
2. LDAP_BIND_DN's example value contained unquoted "=" and commas,
violating the file's own documented quoting rule and causing a
second, independent parse failure. Quoted it (and the two other
comma-bearing LDAP DN values) to match the rule.
config.php has zero fallback for a parse failure — it die()s
immediately — so either bug alone would have taken down every fresh
deployment that didn't hand-edit the example file's comments first.
Verified end-to-end: copied .env.example to a real .env file
unmodified and ran it through config.php's exact parse_ini_file +
quote-stripping logic; it now parses cleanly with all 23 keys
(including the 5 new ones) and LDAP_BIND_DN resolves to the correct
unquoted DN string.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Two small findings from the same dashboard audit pass:
1. Charts rendered a bare empty frame with no message when the
filtered dataset was empty (fresh install, or a non-admin's
visibility-filtered ticket set happening to be zero). makeDonut/
makeBar now show a "No data for current filters" message in the
chart's place instead of silently doing nothing.
2. Stat cards had two independent, redundant click-handler
implementations. lt.statsFilter.init() (base.js, shared web_template
code) read each card's data-filter-key/data-filter-val attributes
and called window.lt_onStatFilter(key, val) on click — but that
global is never defined anywhere in this app, so it only toggled a
cosmetic .active class with no functional effect. The actual
navigation logic is the separate handler at ~line 1282 that ignores
those attributes entirely. Both fired on the same click with no
visible symptom, but the markup looked load-bearing and wasn't — a
trap for a future edit that touches one implementation assuming
it's the only one. Removed the dead lt.statsFilter.init() call and
the now-unused data-filter-key/data-filter-val attributes from this
app's DashboardView.php (left the shared lt.statsFilter module in
base.js itself untouched, since other LotusGuild apps consuming the
same shared template file may define their own lt_onStatFilter).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
remember() had no protection against a slow cache-miss recomputation
overwriting a fresher write. If Request A started computing stats just
before a ticket mutation + invalidateCache(), and Request B started
just after (correctly computing fresh, post-mutation data), A could
finish (using stale pre-mutation data) after B and overwrite B's fresh
cache entry — extending staleness by up to another full TTL.
Added a per-prefix invalidation epoch: delete() bumps it, and
remember() snapshots it before running the callback and only writes
if the epoch hasn't changed since — otherwise a newer invalidation
happened mid-computation and the result being written is already
stale, so it's dropped (the caller still gets its own result; only the
cache write is skipped).
Verified with two real concurrent PHP processes racing against the
same cache key (a slow "Request A" callback vs. a fast "Request B"
that invalidates then recomputes): the cache ends up holding B's fresh
value, not A's late stale overwrite.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
addDependency()'s "already exists" check only matched the exact
(ticket_id, depends_on_id, dependency_type) tuple. A user could add
"A blocks B" from ticket A's page, then separately add "B blocked_by
A" from ticket B's page — wouldCreateCycle() correctly found no cycle
(both normalize to the same precedence edge), so the insert was
allowed, creating two DB rows describing one real relationship (shown
twice on ticket B's page: once under Dependencies, once under
Dependents).
Added an inverse-relationship check before the insert: blocks/
blocked_by are inverses of each other, relates_to is its own inverse
(symmetric). duplicates has no defined inverse type in the schema, so
both directions remain independently insertable, which is correct —
"A duplicates B" and "B duplicates A" are distinct claims.
Verified against a local MariaDB instance: the exact repro from the
issue (A blocks B, then B blocked_by A) is now rejected, relates_to's
symmetric case is rejected in both directions, and duplicates in
either direction is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
api/upload_attachment.php did a raw move_uploaded_file() with zero
image processing. A photo attached from a phone retained embedded
EXIF, including GPS coordinates, and download_attachment.php streams
the file byte-for-byte back to any user with ticket visibility — for
an infrastructure company, this could leak a data center or office's
precise physical location through a routine ticket photo, especially
on Confidential-visibility tickets whose whole point is restricting
exactly this kind of detail.
Added stripImageMetadata(): decodes and re-encodes JPEG/PNG/GIF/WebP
uploads via GD, which drops EXIF chunks that aren't part of the pixel
data. Best-effort — leaves the file untouched on any failure (corrupt
image, unsupported format, GD unavailable, or an oversized decoded
pixel count guarding against a decompression-bomb-style crafted image)
rather than blocking the upload.
Verified with a real GPS-tagged JPEG (generated via piexif) and a
GD/PHP harness: GPS EXIF is gone after stripping, the image stays
valid and correctly sized, PNG alpha transparency is preserved, and
corrupt files / non-image MIME types are left byte-for-byte unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Dev Note #24 listed 7 callers; api/ticket_status_api.php (the Bearer
API's status-change endpoint) also correctly calls invalidateCache()
but wasn't in the list. Behavior was already correct — this is a pure
documentation completeness fix so the caller list stays an accurate
reference for future maintainers deciding whether a new mutating path
needs the same call.
Verified via grep -rl "invalidateCache" that these 8 files (plus
StatsModel.php itself, and an unrelated same-named method on
UserModel) are the complete set of real callers.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Zero callers anywhere in the app — confirmed via grep for
"OutputHelper::" across the whole codebase. Every view actually calls
htmlspecialchars() directly instead, which a prior audit confirmed is
done consistently, so escaping was never actually at risk. This was
just a misleading, unused class that README.md's file reference
implied was part of the app's active XSS-prevention story. Removed the
file and its README Project Structure entry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
config/requirements.php only checked PHP version and 6 extensions. A
deployment on a host with a low default memory_limit (e.g. shared-
hosting-style 128M) passed the startup requirements check cleanly and
only surfaced as a mysterious failure under real load — a large CSV
export, an oversized dashboard query on a big install.
Added min_memory_limit_mb (256) and min_max_execution_time (30s)
thresholds to config/requirements.php, checked as warnings (not hard
failures, since a low limit doesn't break every request) in both
scripts/check_requirements.php (CI) and api/health.php (production
monitoring). -1/0 (unlimited) always passes.
Verified the ini-size parsing and warning logic directly with
low/high/unlimited memory_limit and max_execution_time values.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
bulk_operations.performed_by and ticket_templates.created_by had no
ON DELETE clause (defaulting to RESTRICT), unlike every other
user-reference FK in the schema (tickets.*, ticket_attachments,
ticket_dependencies, recurring_tickets, api_keys), which all use
SET NULL. Deleting a user who ever ran a bulk operation or created a
template hard-failed at the DB level instead of nulling the
reference, breaking the pattern used everywhere else.
performed_by was NOT NULL, so it had to become nullable to support
SET NULL, matching how every other SET NULL column is defined.
- Fixed 000_baseline.sql for fresh installs.
- Added 003_fk_on_delete_set_null.sql for existing deployments.
Verified against a local MariaDB instance: reproduced the old RESTRICT
schema, ran the migration (twice, for idempotency), then confirmed
deleting a user with rows in both tables now nulls the references
instead of failing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
deleteOldLogs() ran a single unbounded DELETE. created_at is indexed
so row selection itself is cheap, but on a large qualifying set (first
run after enabling/changing AUDIT_LOG_RETENTION_DAYS, or after the
cron silently missed runs) an unbounded single-statement DELETE holds
row locks for the full duration — risking contention with the frequent
concurrent INSERTs the audit log receives from live traffic. Now
deletes in batches of 1000 (parameterized), looping until nothing
qualifies.
Verified against a local MariaDB instance with a batch size of 10
forcing multiple loop iterations: deleted exactly the stale rows,
left recent rows untouched, correct total count returned.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
api/get_users.php returned every user's user_id/username/display_name
to any authenticated session with no pagination or limit — needed for
mention/assignment typeahead, but a blanket enumeration a compromised
low-privilege session could scrape in one call. Added a LIMIT 500;
every caller already only uses this for typeahead/dropdown filtering,
never a literal full roster, so this doesn't change behavior for any
real deployment size while bounding the response.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
The last_activity subquery had no WHERE clause on the report's
date-range filter, so it always showed true all-time last activity
even when the page was filtered to e.g. "last 7 days" — inconsistent
with every other column on the same report. Added the same
DATE(created_at) BETWEEN ? AND ? clause used by the report's other
subqueries.
Verified against a local MariaDB instance: an out-of-range audit_log
row is correctly excluded from last_activity once the filter is
applied.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
setInterval(loadNotifications, 60000) ran unconditionally regardless
of tab visibility, and failures retried at the same fixed 60s cadence
forever. Now skips polling while document.hidden, resumes immediately
via visibilitychange when the tab regains focus, and backs off
exponentially (capped at 5 min) on repeated fetch failures, resetting
to the normal 60s cadence on the next success.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
toggleActive() flipped is_active without touching next_run_at. If a
schedule was disabled while next_run_at was still in the future, then
re-enabled after that date had passed, the next cron tick saw
next_run_at <= NOW() and fired immediately — surprising for an admin
expecting a re-enabled "daily" schedule to wait until its next natural
occurrence. Now recomputes next_run_at from the current time when
transitioning to active, matching what a fresh schedule creation would
produce; disabling is unchanged.
Verified against a local MariaDB instance: re-enabling a schedule
whose next_run_at was in 2020 recomputed it to tomorrow at the
scheduled time; disabling leaves next_run_at untouched.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
README Dev Note #12 mandates utf8mb4_general_ci for new tables, but
these two tables were created with utf8mb4_unicode_ci in the baseline
schema — inconsistent with every other table, and a future join or
comparison against a general_ci column would need explicit COLLATE
casts or hit "Illegal mix of collations" errors.
- Fixed 000_baseline.sql so a fresh install matches the convention
directly.
- Added 002_fix_collation_consistency.sql for existing deployments.
MariaDB silently drops the inline CHECK (json_valid(...)) constraint
on saved_filters.filter_criteria when that column is MODIFYed (found
by actually running this against a local MariaDB instance), so the
migration explicitly re-adds it after the collation conversion.
Verified against a local MariaDB 10.11: baseline applies cleanly,
migration is idempotent (safe to run twice), and the json_valid CHECK
is still enforced afterward.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
syncUserFromAuthelia() did a plain check-then-insert with no
transaction, so two simultaneous first-visit requests for the same
brand-new user (e.g. two tabs opened right after SSO login) could
race: the second INSERT hits users.username's UNIQUE KEY, which
mysqli throws on (uncaught, PHP 8.1+ default report mode) rather than
returning false. Switched to INSERT ... ON DUPLICATE KEY UPDATE
followed by a re-fetch by username, so the losing request updates the
winner's row instead of throwing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
The JS side claimed to "mirror the PHP crc32 % 4 logic" but actually
implemented a different rolling hash (classic String.hashCode()-style),
so the same display name could get different avatar colors depending
on whether a comment was server-rendered or client-rendered (new
comment, reply, watcher avatars, "Load more" pagination).
Added a real CRC-32 (IEEE 802.3/zlib polynomial, UTF-8 byte sequence)
to ticket.js and switched all three JS call sites (avatarColorClass,
watcher avatars, and buildCommentEl in TicketView.php) to use it,
verified to produce identical output to PHP's crc32() including for
non-ASCII names.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
A user could set an end date before a start date, or priority_min >
priority_max, and the filter would be silently sent as an
unsatisfiable range with zero results and no explanation. Now swaps
min/max (and from/to) before building the query string when they're
in the wrong order.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Two small dead-code cleanups from a dashboard.js state-management
audit:
- sortTable(table, column) had zero callers — actual table sorting is
wired through lt.sortTable.init() via initTableSorting().
- setViewMode() wrote localStorage['ticketViewMode'], but nothing ever
read it back; the real view-mode restoration on page load reads
lt_activeTab_<path>, written separately by lt.tabs in base.js.
Both looked load-bearing but weren't, risking a future edit assuming
otherwise.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
renderAttachments() decided whether to render an image thumbnail by
regex-matching the display filename extension, rather than the
finfo-verified mime_type the API already returns. A file whose real
type differs from its display name (e.g. a PDF a user named
photo.png) rendered a broken <img> instead of falling back to the
file-type icon.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Zero callers anywhere in the codebase — superseded by claimForRun(),
which the cron script actually uses and which additionally guards
against the double-fire race between concurrent cron invocations that
this method lacked. Removing it so a future reuse doesn't silently
reintroduce that race.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
CreateTicketView.php was the one remaining spot using the native
confirm() dialog, violating README Dev Note #21. Split loadTemplate()
into a confirm check + applyTemplate(), routed through the project's
styled showConfirmModal(), matching every other destructive-action
confirmation in the app.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
gotoFilter() built a brand-new URLSearchParams containing only the
clicked chart segment's filter keys, discarding every other active
filter (search text, date range, saved-filter selection, etc.) on
navigation. Now merges the segment's filter into the current
location.search, same fix approach already applied to #22.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
quickAssign() wired lt.combobox.init() with an onSelect callback, but
combobox only supports the multi-select onChange(selected[]) contract
— onSelect is never invoked, so _quickAssignUserId stayed undefined no
matter what the user picked and Quick Assign always showed "Please
select a user from the list." Switched to lt.typeahead.init(), which
does support onSelect, matching the already-working Bulk Assign modal.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
The 'update' notification formatter unconditionally read
details['status']['from']/['to'], so any title/priority/description/
category/type/visibility-only edit fell through to '?' on both sides
and produced a broken "changed status on #123: ? → ?" title regardless
of what actually changed. Now it branches on the delta shape actually
present: the flat {field, from, to} shape used for visibility changes,
then each per-field {from, to} delta in priority order, falling back
to a generic "updated ticket" message only if none match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
clearAllFilters() deleted the nonexistent date_from/date_to query
params. Every actual date filter (sidebar, Advanced Search, saved
filters, stat-card links) uses created_from/to, updated_from/to, and
closed_from/to, so clicking the button silently left any active date
range in place.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
add_comment.php computed a trimmed copy of comment_text only to check
for empty input, then passed the original untrimmed $data through to
CommentModel::addComment(), so any leading/trailing whitespace the
user typed (or pasted) was written to ticket_comments.comment_text as-is.
update_comment.php already trims before saving edits, so a comment
could pass through this endpoint once with untrimmed text (creation)
and be silently corrected the moment it was next edited — inconsistent
storage that, combined with the markdown parser's line-anchored regexes
(headings, tables, lists all match on ^), could make a markdown-enabled
comment mis-render after a reload depending on whether its first line
carried leading whitespace.
Also trims in the "Load more comments" pagination re-render path in
TicketView.php, matching the two on-load renderers in markdown.js so
all three code paths that call parseMarkdown() on stored comment text
treat leading whitespace consistently.
Closes #18
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All three charts (priority donut, status donut, category bar) now navigate to
the same URL filters the stat cards already use, with a pointer cursor on
hover, a title hint, and "click to filter" in the tooltip.
The status each click applies is explicit rather than left to the default. With
no `status` param the controller falls back to the viewer's
default_status_filters preference, which can be anything, so the list would not
necessarily match what the chart counted. StatsModel builds by_priority and
by_category with `status != 'Closed'` while by_status spans every status, so
only the priority and category charts pin the open set; the status chart filters
on the clicked status alone (which is how clicking "Closed" works at all).
Verified two ways:
- 17/17 in headless chromium, driving the real chart script from this view with
the Chart constructor stubbed, asserting the exact query each click produces
and that a click hitting no segment navigates nowhere.
- Against the live database, every segment's count equals the number of tickets
its filter returns — 12/12 across all three charts — so the list you land on
matches the number you clicked.
A modal can be dismissed four ways: the ✕ button, Cancel, a backdrop click, or
Escape. base.js handles the last two globally (a document click handler and
registerKey('escape', closeAllModals)), so the status-change modal — which wired
only the two buttons — never learned it had been dismissed. The status dropdown
kept displaying the new status even though update_ticket.php was never called,
so the ticket looked closed with no comment until a reload showed it still open.
The same gap left every dynamically-inserted modal in the DOM when dismissed
that way, so the next open inserted a duplicate id that shadowed the live one.
- base.js closeModal now dispatches a bubbling lt:modalclose event (synced to
web_template as bbec859), and _statusCommentModal treats it as "no comment".
- ticket.js reverts the dropdown on any dismissal, guarded against the re-entry
its own lt.modal.close() would otherwise cause.
- dashboard.js gains openModalWithDismiss() so all seven dynamic modals plus the
generic prompt modal tear down however they are dismissed.
Verified in headless chromium against all four dismissal routes plus a
confirm-with-comment control: 22/22. Against the pre-fix files the same test
fails 6 assertions — backdrop and Escape leave the dropdown on "Closed *" with
an orphaned overlay — so it reproduces the reported behaviour exactly.
Setting the textarea's .value programmatically does not fire an 'input' event,
so updatePreview() never ran and the preview kept showing the just-posted
comment's rendered markdown underneath an empty composer.
(This change was already present in the working tree at the start of the
session; committing it on its own rather than folding it into an unrelated fix.)
.ticket-preview-popup used var(--lt-surface), which is not defined anywhere, so
the background always fell through to the hardcoded #0a0e14. In light mode that
left a near-black panel — and since the rule set no `color`, the inherited
near-black body text was effectively invisible on it. The border was hardcoded
neon green and the shadow a heavy rgba(0,0,0,0.5).
Now uses --bg-card / --text-primary / --accent-green-border / --shadow-color,
and .preview-id uses --accent-cyan instead of the undefined --lt-cyan.
base.css gains the two tokens the light theme was missing (--accent-green-border
and --shadow-color), synced from web_template 0d633bd.
Verified with computed styles in headless chromium: light body-text contrast on
the panel goes from invisible to 17.7:1, dark stays at 13.2:1, and the ID accent
clears 3:1 in both themes.
Two separate causes, both light-mode-only:
1. base.css `.lt-select` sets `color-scheme: dark` on the element itself, which
outranks the `color-scheme: light` the light theme sets on <html>, so the
native dropdown popup kept dark chrome. The option list is also hardcoded
#0d1117/#c9d1d9 with no light override. Fixed with light overrides for both
(synced from web_template, where the same fix landed as 378a8cd).
2. ticket.css coloured the status select with var(--lt-success), --lt-amber,
--lt-cyan and --lt-danger — none of which are defined anywhere in the
project, so all four always fell through to hardcoded neon fallbacks. Now
uses the --accent-* tokens, which carry the same hues and are redefined for
light mode. The selectors also lead with .lt-select: at two classes they lost
to base.css's `html[data-theme="light"] .lt-select` (0,2,1) and every status
was repainted near-black in light mode.
Verified with computed styles in headless chromium — all four statuses in both
themes (8/8), plus the popup colour-scheme and option colours.
Found while verifying #21 against the live schema: the model writes
'completed_with_errors' (21 chars) when a bulk operation finishes with
per-ticket failures, but bulk_operations.status was varchar(20), so the
write failed with "Data too long for column 'status'".
This was latent — bulk status changes previously forced every transition
through, so failed was always 0. Now that they honour the Workflow
Designer, a partially-skipped batch is a normal outcome and hits it.
- migrations/001 widens the column to varchar(32) (idempotent).
- The baseline is updated to match, for fresh installs.
- The bookkeeping UPDATE is wrapped in a try/catch: it runs after the
ticket changes are committed, so an instance deployed ahead of its
migrations must not turn a completed operation into an error response.
Verified against the live database with a disposable-ticket harness:
comment-required rejection changes nothing, undefined transitions are
refused per ticket with a reason, allowed transitions still work, mixed
batches apply the valid half, and an already-Closed ticket is a no-op.
Bulk status changes previously bypassed the workflow entirely — the model
carried an explicit "admin-only escape hatch" note — so bulk edit could
drive tickets through transitions the designer forbids and skip comments
the designer requires.
BulkOperationsModel now applies the same rules as the single-ticket path:
- Transitions absent from status_transitions are refused per ticket and
reported with a reason, instead of being forced through.
- requires_comment is checked up front across the whole selection, so a
batch is rejected before any ticket is mutated rather than half-applied.
- The reason is persisted as a comment on each ticket changed, matching
what a single-ticket close records.
- Tickets already in the target status are a no-op success, not a failure.
requires_admin needs no extra check: api/bulk_operation.php already gates
the endpoint on admin.
Client: both bulk modals now collect a reason, the close path gets a real
modal instead of a bare confirm, and per-ticket skip reasons surface in
the result toast instead of a bare failure count.
- README: Bearer API table (list/read/comment/status), scope explanation,
and the new endpoints in the API Endpoints table.
- /admin/api-keys API Usage section: scopes note + copy-paste cURL examples
for create, list/triage, read-one, comment, and close (uses APP_DOMAIN).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the Bearer-key API beyond create-only (all rate-limited, scope-
enforced, per-key-label attribution):
- GET /api/tickets_api.php: triage the queue (status/priority/host title
match + pagination) or read one ticket + its comments. read scope.
- POST /api/ticket_comment_api.php: post a comment as the key (user_name =
key name, linked to the key owner). read_write scope.
- POST /api/ticket_status_api.php: change/close status with workflow
validation + requires_comment; posts the close reason in the same call,
fires the Matrix status notification, invalidates stats. read_write scope.
Reuses TicketModel/CommentModel/WorkflowModel/NotificationHelper; a read
key cannot mutate. Reachability requires the reverse-proxy Authelia bypass
(handled separately).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Foundation for extending the Bearer API beyond create-only:
- api_keys gains a scope column (read | read_write); baseline schema updated
and the column applied to the live DB. Existing keys default to
read_write so the hwmon create key keeps working.
- ApiKeyModel: createKey() takes a validated scope; validateKey() always
surfaces scope (defaults read_write); getAllKeys() is paginated
({keys,total,page,perPage}, key_hash stripped).
- ApiKeyAuth: expose getKeyContext() (scope/key_name/created_by/api_key_id)
and requireScope() (403 on insufficient scope); existing return values
unchanged.
- create_ticket_api.php: require read_write scope (a read key can't create).
- Admin /admin/api-keys: scope selector on the create form, a scope column,
and pagination (revoked keys were stacking up).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI has been red since the CSRF-drift changes landed a trailing space on the
'success' => false line in these two endpoints, which blocks the deploy job
(and therefore beta/prod). No logic change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stored markdown comments rendered fine in the live preview (parses the raw
textarea value) but broke after refresh: the server template emitted the
comment text on an indented line, so the on-load renderer parsed
element.textContent with ~20 spaces of leading indentation. Markdown treats
4+ leading spaces as a code block, so the first line (e.g. a heading or
table row) was mis-parsed and blocks got wrapped in <p>, producing invalid
HTML that broke the page layout.
- markdown.js: trim the text before parseMarkdown in both on-load renderers
so template indentation can't be parsed as a leading code block.
- TicketView.php: emit the comment text inline (no surrounding whitespace)
so the element's textContent is exactly the stored markdown.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jaredandClaude Sonnet 5