Without any configuration getAppUrl() invents http://localhost as the relying party, and the UI advertised passkeys that every click answered with not configured. A localhost RP only counts as configured now when the operator declared one. The origin check also binds to the expected origins again instead of accepting every subdomain of the RP id; that widening never shipped in a release.
Deleting a bag over MCP moves every figure, the items fall into the unassigned pile, yet it sent no totals ping; it pings the room now like REST and the plugin RPC do. The client refetches totals after a room re-join and when coming back online, through the same coalescing window. The plugin SDK types learn the create fields the routes accept since #2154, on both hand-kept declarations.
The markers replaced the booking's block but carried none of what it said: no DESCRIPTION (type, confirmation, notes) and no LOCATION fallback. The carrying booking is resolved once now, and the markers inherit its description and location exactly when the block is dropped. A check-in window ending before it starts reads as a late arrival past midnight instead of swallowing the event.
journey_photos.sort_order is nothing but the insert counter, so photos added late landed behind days they belong before. Both gallery readers (the app and the shared journey) order by capture time now, falling back to the timestamp of the stop the photo belongs to and then to upload time, through one shared SQL fragment; ties keep the old order, so existing galleries only move where chronology demands it. The order inside an entry stays the user's manual arrangement.
testSmtp logged nothing and the transport ran on nodemailer's two-minute defaults while the client gave up after eight seconds, so every failure was a generic toast over an empty log. The transport gets real timeouts, every outcome is logged without credentials, and a classified, scrubbed cause (auth, DNS, refused, filtered port, TLS mismatch, certificate) goes into the response the admin sees; the channel test calls wait long enough to receive it. The password field also rendered its mask as value, so replacing a stored password silently saved the bullets as part of the new one; it is a placeholder now, on desktop and the phone admin.
* feat(plugin-sdk): settings-field default + unknown-attribute warning
* feat(shared): default on the plugin settings-field contract
* feat(server): persist and serve plugin settings-field defaults
* feat(client): pre-fill plugin settings defaults in the admin and user forms
* feat(server): enforce required plugin settings on both save paths
* feat(client): block plugin-settings save on an empty required field
* feat(server): resolve plugin settings defaults at runtime
A settings-field `default` was stored and served to the form but never
reached the plugin: the child's init envelope, `ctx.settings.get()`, the
channel dispatch's readAll and the OAuth provider config all read the
stored blob only. Fold `stored ?? default` in on every path that hands
config to a plugin (settings-defaults.ts), and let a default satisfy
`required` on both save gates — refusing a save the runtime would have
accepted was a contradiction.
Also: one `isFilled` rule shared by the save gate and `hasRequired`
(checkbox exempt, whitespace empty); the install parser drops a
non-boolean checkbox default or a select default outside its options,
matching what the SDK already refuses; export SETTING_FIELD_KEYS so the
SDK's mirror can be parity-tested.
* feat(client): share the plugin settings form rules across all three forms
The mobile user settings screen was a third consumer of the fields/config
contract that never got `default` pre-fill or the required check, and
the seed/patch loops lived in five hand copies. Extract
seedSettingsValues / findMissingRequired / settingsPatch into one module
and use it from the admin instance form, the desktop user tab and the
mobile screen. The user forms now surface a 4xx `{ error }` from the
server (the required-field refusal) instead of the generic toast.
* feat(plugin-sdk): seed dev fixtures from manifest defaults
`trek-plugin dev` handed the plugin only the fixture values, so a plugin
relying on its manifest defaults read undefined in dev and only in dev.
Seed ctx.config / ctx.settings from the declared defaults (a fixture
value still wins) via a new exported settingDefaults(manifest, scope).
SETTING_FIELD_KEYS is now parity-tested against the host's list.
* docs(plugins): document settings defaults as runtime fallbacks
A `default` is the field's effective value wherever nobody set one, not
just a form pre-fill; `required` is enforced on both ends; unknown
attributes are dropped at install and warned on by validate. Add a
Settings section with the attribute table to the SDK README and update
the ctx type comments, the shared schema, and the wiki guide.
* add missing exports
An adversarial review of the diff found sixteen defects that survived
verification. The substantive ones:
- The new `packing:bag-totals` event was never classified in wsEventPolicy,
so the registry-parity ratchet was red. It belongs in
HANDLED_OUTSIDE_TRIP_STORE: it carries no state to apply, which is the
point of it.
- The Journey Settings tracks switch called onSaved(), which closes the
dialog. That destroyed the switch the moment it was flipped AND skipped
the unsaved-changes guard, so a title typed but not saved was silently
lost. It refreshes instead.
- Deleting a bag sends no totals ping, yet packing_items.bag_id is
ON DELETE SET NULL: everything in the bag lands in the unassigned pile and
both figures move. The comment claiming otherwise was wrong.
- Only the REST handlers pinged. The MCP tools and the plugin RPC are two
more live write paths, and since the displayed number now comes from the
server rather than from the item events, weights simply stopped updating
for anything written through them.
- Every item write pinged, including checking an item off — the most
frequent packing write there is — and clearing twelve checked items sent
twelve. The ping now fires only when a weight-relevant field changed, and
the clients coalesce bursts and never overlap two refetches.
- Bags have no repo and no Dexie table, so offline the server totals froze
at the last online read and went blind to the mutation queue. A stale
absolute number measured against an airline limit is worse than an honest
partial one, so offline the surfaces sum what they can see.
- SIGTERM arriving mid-bootstrap called server.close() on undefined, and the
fire-and-forget promise turned that into an unhandled rejection: on Node 22
a crash, which is the opposite of the fix.
- Promise.allSettled swallowed a rejected nestApp.close(), reporting a clean
exit 0 while the crons and the plugin children were never torn down. This
bug class is diagnosed from container logs.
Also: the track fetch gate read a stale `current` during journey-to-journey
navigation and fetched the previous journey's tracks; listBags ran the weight
aggregate twice per request, on a route the ping now fires constantly; the
grand total counted unassigned weight whose row was not rendered; the client
declared show_trip_tracks boolean while the wire carries SQLite 0|1; and the
six broadcast calls had no test, so removing all of them left the suite green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5VwmiBQ9sPCJwfmpv64r3
A bag's weight was summed in the browser over the item list the viewer is
allowed to see. Personal items belonging to other members are filtered out of
that list by design (#858), so their weight vanished with them: two people
looking at the same shared bag saw two different totals, and neither saw the
real one.
That number is then measured against `weight_limit_grams`, an absolute airline
limit. A per-viewer subtotal there is not merely incomplete but wrong — a
shared bag could sit over its limit and warn nobody.
The sum now happens in SQL over every row and ships as `total_weight_grams` on
the bag, with `unassigned_weight_grams` alongside so the "no bag" pile and the
grand total follow the same rule; a screen mixing true totals with per-viewer
ones would be worse than either. Only integers cross the wire: a member learns
that a bag is heavier than the items they can see, never a name, category,
quantity or owner. That disclosure is deliberate and bounded, and it is what
the issue asks for.
The item lists stay viewer-scoped, so #1767 ("someone shared it with you, but
they are the one bringing it") is unchanged — it just no longer decides what a
bag weighs.
Totals also had to stay live, because the reported repro is two people looking
at the same screen. A private item's event reaches only its owner, which is the
very rule that made the totals wrong, so item writes now also ping the room
with a content-free `packing:bag-totals` and everyone re-reads the numbers.
All three surfaces — the bag sidebar, the bag modal and the mobile sheet — read
the server figure and fall back to the local sum only for a bag cached offline
before this change.
Closes#2191
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5VwmiBQ9sPCJwfmpv64r3
#1260 drew the GPX tracks of every trip behind a journey onto its map, with
nothing to switch off — the reasoning being that a route imported into a
linked trip is part of the journey's story. For a trip carrying a season of
recorded drives it is instead a map nobody asked for, drawn from places that
never became an entry.
So it becomes a journey-level setting, owner-only like the other Journey
Settings, and it starts off: a journal should show what its author put in it,
and #1260's set is everything the linked trips happen to contain. The switch
gates the request, not just the rendering — the endpoint ships full unthinned
geometry, which is megabytes fetched on every journey open for a map nobody
wanted.
`journeyTracks()` itself is deliberately untouched, so TREK Studio keeps the
set its saved books were laid out with.
Also fixes what the reporter's screenshot actually shows: JourneyMap fed track
coordinates into `fitBounds`, so one drive across a country zoomed the journey
map out until the entries were specks. Tracks now only frame the map when
there is nothing else to frame, which is what JourneyMapGL already did — the
same journey used to open at two different zooms depending on the renderer.
Closes#2194
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5VwmiBQ9sPCJwfmpv64r3
* chore: bump version to 4.1.0 [skip ci]
* fix(collections): stop findDuplicateCollectionPlace merging places by coordinates alone
Three places answered "is this a place the trip already has?", not two:
PlacesService.findDuplicatePlace and isPlaceDuplicate got the shared strategy
list in #2130; CollectionsService.findDuplicateCollectionPlace was missed and
kept its own hand-rolled copy of the pre-#2130 order.
Unlike findDuplicatePlace, this one had no isPlaceDuplicate() guard in front of
it — savePlace calls it directly — so the coordinate fallback for a NAMED
candidate was live, not latent: two distinct places sharing an address (a
restaurant and the bar upstairs) would report each other as duplicates the
moment their coordinates round to the same ~11 m box, regardless of name.
It also never read google_place_id/google_ftid/osm_id at all, even though
every collection_places row stores them, so a place renamed since it was
saved could be saved again instead of being recognised by its provider id.
Now walks placeMatchStrategies() from @trek/shared, same as
findMatchingPlaceId: provider id, then name, then coordinates only when there
is no name. savePlace passes the provider id fields through since they are
already on the request body; the two bulk call sites (importablePlaces,
saveFromTripPlaces) are unchanged, since their source rows don't carry
provider ids to pass.
Closes#2137
* fix(collections): ask the same question at all three call sites
savePlace was not the only caller of findDuplicateCollectionPlace. The bulk copy
already carries google_place_id/google_ftid/osm_id into the row it writes but
asked without them, and the import picker never selected them at all, so a
renamed place was recognised on one path and not on the other two. The picker
answering differently from the import is the drift the method exists to prevent:
a row shown as new would come back refused.
COLLECTIONS-SVC-102 and -103 cover the bulk copy and the picker.
* fix(map): give the Leaflet map its own zoom ceiling, so a vector basemap cannot take the planner down
Opening a trip on 4.1.0 could land on the error boundary with "Map has no maxZoom
specified", and so could the map settings tab, which left no screen from which to
pick a different basemap.
Leaflet answers getMaxZoom() from the map options first and otherwise from a
layer that brought one, and only a GridLayer ever contributes via its beforeAdd
hook. A vector basemap is a GL canvas on a plain L.Layer, so it contributes
nothing, and MapContainer set no ceiling of its own. MarkerClusterGroup.onAdd
refuses an infinite one by throwing, before any clustering, so an empty trip hit
it too.
That path opened when the default basemap became a MapLibre style. It reaches
further than an operator choosing OpenFreeMap: resolveTileUrl sends a keyless
CARTO template to the app default, which is now a vector style, so every
instance still carrying the old CARTO basemap fell into it on upgrade.
The ceiling belongs on the map rather than on the base layer, because it has to
hold whichever of the three branches renders. 19 matches what the raster and
satellite layers already carried, so nothing changes for the maps that worked.
SharedTripPage gets the same treatment: it has no cluster to throw, but it draws
the same vector basemap and its fitBounds asks for a ceiling.
mapZoomCeiling.test.ts pins Leaflet's own semantics against the real library
rather than a mock, since a mocked react-leaflet would have kept passing through
all of this.
* fix(map): put the CARTO key back on the template the settings preview draws
The map settings fields hold what the user is editing rather than what
useTileUrl already resolved, so the preview received a bare template with no key
on it. resolveTileUrl then read it as a keyless CARTO url and did what it is
meant to do with one: fall back to the app default. The preview drew
OpenFreeMap while the fields said CARTO, whether or not a key was saved.
Same reason the tab crashed for anyone on a CARTO basemap even after entering a
key, which is the loop that made this hard to escape: the key is entered on the
page the fallback had taken down.
FE-COMP-MAP-033b pins the template the preview is handed, with and without a key.
* fix(admin): put the CARTO key back on the default-settings map preview too
Same shape as the user-facing map tab, and reported alongside it: the field holds
what the admin is editing, so the preview resolved a keyless CARTO template and
drew the app default instead of the basemap being set.
* fix(mobile): put the CARTO key back on the phone map settings preview too
The third preview with the same shape, and the one that matters most while the
crash is live: the phone layout is where someone would go to change the basemap
when the desktop tab will not open.
* fix(calendar): a fully timed stay is its two hand-overs, not a week-long bar
Closes#2136
A stay was always exported as an all-day block across its whole range, and the
check-in and check-out markers came on top of it. When the stay records both
ends of its clock the markers already say everything the block does, and the
block is the half nobody can act on: it takes a week of calendar to repeat what
two one-hour events state precisely. Same shape as the parking and rental
windows in #2068.
Only where the markers actually stand in for the booking, though. They are
emitted once per stay and titled from its lowest-id reservation, so a second
room on the same stay would otherwise be left with nothing at all. A stay that
knows only one end keeps its block too, since nothing else carries the other
end's date.
CAL-025b covers the drop, -025c the one-sided stay, -025d the second room.
* fix(mobile): the phone admin map preview needs the CARTO key too
The fourth preview with the same shape, missed when the other three were fixed.
Sonar pointed at it: the desktop and phone admin pages are near-identical, so the
duplication report named the file the change had not reached.
* chore: bump version to 4.1.1 [skip ci]
* feat(plugins): admin editor for instance-wide plugin settings
The scope:'instance' settings a plugin declares had API endpoints but no UI.
- shared: plugins.schema.ts — the settings-field descriptor and the admin
instance-config GET/PUT wire contracts, imported by both sides
- server: GET /api/admin/plugins/:id/config now returns the declared fields
alongside the (masked) values; PUT re-spawns an ACTIVE plugin (its child
reads config once, at init) and reports it via 'restarted'; the admin list
carries instanceSettingsCount so the UI gates its menu item without a fetch
- client: 'Instance settings' row action opening a form modal (desktop) /
MSheet (phone) in both admin shells — checkbox/select/secret rendering as
in the user settings form; an untouched secret mask is never sent back
- i18n: three new admin.plugins.* keys in every locale
* refactor(plugins): move the admin body contracts to @trek/shared
The remaining plugin DTO schemas (install, link, activate, uninstall,
retrust, update, egress-hosts, user-settings update, route) now live in
shared/src/plugins/plugins.schema.ts with their inferred types, alongside
the instance-config contracts. The journey-contract doctrine notes (#1842)
moved with them — each body stays deliberately loose so the handlers keep
the rejections and fallbacks they already own, and new spec tests pin that
looseness so a well-meant tightening breaks a test that explains itself.
plugins.dto.ts is now thin createZodDto wrappers over the shared schemas
(same class names, no controller changes), and the client's pluginInstall
types its options and body against PluginInstallRequest.
* fix(admin): offer Allowed hosts only to plugins that declared operatorEgress
The row's egress chip was already gated on the declaration, but the kebab
menu (desktop) and row action sheet (phone) offered Allowed hosts on every
installed plugin — inviting the admin to widen egress for plugins that
never asked for it. Both entries now render only when the plugin declares
operatorEgress; the dialog's unsupported-notice fallback stays as the
truth-teller for stale list data.
* test(admin): close the Sonar gaps on the instance-settings form
The PR gate flagged new-code coverage (76.4%) and duplication (10.9%).
- The two identical 39-line handler blocks were logic, not markup — they
move into one shared useInstanceSettings hook; each shell keeps its own
markup (the deliberate desktop/phone divergence stays)
- New tests per shell cover the untested branches: every field type
renders and round-trips into the save payload (select, checkbox/MToggle,
number, hint, no-label key fallback), a failed fetch is a toast, a
rejected save shows the server reason and keeps the edits, and the
dialog/sheet closes without saving
- INS-009 covers the controller's no-body '|| {}' fallback
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: michael-bohr <263459669+michael-bohr@users.noreply.github.com>
Co-authored-by: Maurice <mauriceboe@icloud.com>
tags and pros_cons are JSON held in TEXT columns. Three read paths decoded
them by hand and createEntry and updateEntry did not, so an edit answered
with the raw row: tags came back as '["beach","sunset"]', the store spread
it into an entry the client types as string[], and the journey page threw
"tags.map is not a function" behind an error boundary.
Any edit was enough to arm it. The update path writes JSON.stringify(val)
with no length check, so even clearing an entry's tags stores '[]' and
answers with that string.
Neither compiler could see it: JourneyEntry describes the row, tags and
all, and both sides read the same field name while only one of them was
right. So the decoded shape gets a type of its own and one decoder, which
is what turned up the third case - the early return for an update that
changes nothing handed back a raw row too.
Fixes#2085
Closes#2136
A stay was always exported as an all-day block across its whole range, and the
check-in and check-out markers came on top of it. When the stay records both
ends of its clock the markers already say everything the block does, and the
block is the half nobody can act on: it takes a week of calendar to repeat what
two one-hour events state precisely. Same shape as the parking and rental
windows in #2068.
Only where the markers actually stand in for the booking, though. They are
emitted once per stay and titled from its lowest-id reservation, so a second
room on the same stay would otherwise be left with nothing at all. A stay that
knows only one end keeps its block too, since nothing else carries the other
end's date.
CAL-025b covers the drop, -025c the one-sided stay, -025d the second room.
savePlace was not the only caller of findDuplicateCollectionPlace. The bulk copy
already carries google_place_id/google_ftid/osm_id into the row it writes but
asked without them, and the import picker never selected them at all, so a
renamed place was recognised on one path and not on the other two. The picker
answering differently from the import is the drift the method exists to prevent:
a row shown as new would come back refused.
COLLECTIONS-SVC-102 and -103 cover the bulk copy and the picker.
Three places answered "is this a place the trip already has?", not two:
PlacesService.findDuplicatePlace and isPlaceDuplicate got the shared strategy
list in #2130; CollectionsService.findDuplicateCollectionPlace was missed and
kept its own hand-rolled copy of the pre-#2130 order.
Unlike findDuplicatePlace, this one had no isPlaceDuplicate() guard in front of
it — savePlace calls it directly — so the coordinate fallback for a NAMED
candidate was live, not latent: two distinct places sharing an address (a
restaurant and the bar upstairs) would report each other as duplicates the
moment their coordinates round to the same ~11 m box, regardless of name.
It also never read google_place_id/google_ftid/osm_id at all, even though
every collection_places row stores them, so a place renamed since it was
saved could be saved again instead of being recognised by its provider id.
Now walks placeMatchStrategies() from @trek/shared, same as
findMatchingPlaceId: provider id, then name, then coordinates only when there
is no name. savePlace passes the provider id fields through since they are
already on the request body; the two bulk call sites (importablePlaces,
saveFromTripPlaces) are unchanged, since their source rows don't carry
provider ids to pass.
Closes#2137
`.meta()` advertises every node of the declaration; `nodeToZod` only walks the
ones it can type. Everything below follows from that gap, so the keyword
allowlist moves into `sanitiseNode`, which is the one walker that sees every
node.
A `pattern` was compiled straight into a RegExp and run on the main thread
against caller input, with no timeout and nothing to abort it: `^(a+)+$` against
33 characters stops the whole server, not just MCP. That is an escape from the
supervised child into the host event loop, and it does not take malice, since a
hand-written email pattern is the classic accident. Quantified groups that repeat
or alternate inside are refused, and the source is capped.
`mcp:tools` counted as a side effect, so `readOnlyHint` was structurally
unreachable: holding that grant is what lets a plugin advertise a tool at all.
Every plugin tool was therefore advertised as potentially destructive and every
client's auto-approval was off. It is a host entry point like `hook:*`, and is
classified as one.
Property names, `default` and `examples` were the strings that never met the
sanitiser. A key may carry newlines, so `\n## SYSTEM\n...` in one defeated the
newline collapse everything else goes through. Names are identifiers now; sample
values keep their own keys but have their strings bounded.
Also refused at parse time, which is what the rest of the file already does: an
object `enum` member carrying `$ref` past the deep scan, a non-boolean
`additionalProperties` advertised and checked by nothing, `enum` beside
`properties` skipping the children, a root `enum`/`const`/`nullable` producing a
non-object schema the SDK cannot read, and a draft-4 `exclusiveMinimum: true`
dropped from the check while staying in the advertisement.
`required` used `in`, so a field named `toString` read as present on every call.
Nodes are built with a null prototype, and the three names Zod cannot round-trip
are refused rather than silently lost.
A result block that had to be shortened got no marker and no log line, which
reads to the model like the whole answer. It is counted now, like a dropped one,
and `isError` survives every envelope path.
The security test file carried two raw NUL bytes, so git, GitHub and ripgrep all
treated it as binary: no diff, no blame, no review. Escaped, same runtime value.
Found in review of the branch. Each one is a case where the host told the
model or the admin something it did not enforce.
Advertised schemas are now enforced. buildToolInputSchema built a loose
object that checked only which keys were present, so a tool advertising
an integer, an enum and additionalProperties: false accepted a string, an
unlisted value and an extra property. .meta() still advertises the
declaration verbatim, and the schema underneath it is now built from that
same declaration, so the two cannot disagree. Anything the builder cannot
enforce (oneOf, unknown formats, non-primitive enum members) is refused by
parseManifest rather than advertised, and the failure lands at install
time where an admin sees it.
Writing the test for that turned up a live bug: z.custom() cannot be
represented in JSON Schema, so building enums that way would have thrown
inside tools/list for the whole session. Enums and consts are real literal
types now.
readOnlyHint classification fails closed. The side-effect list spelled out
db:write: and missed db:create:trips entirely, letting a plugin that can
create trips advertise a tool as read-only and idempotent, which is what
clients gate auto-approval on. It is an allowlist of read-only grants now,
and mcp-tool-annotations.test.ts walks the whole catalog so a new
permission fails a test rather than producing a wrong hint.
The result cap covers the whole result. It bounded each text block, so a
hundred blocks cost a hundred times the limit: 6.5M characters in a probe,
now 65K. Large objects are serialised with a budget instead of stringified
whole and then sliced, and the block count is capped.
The admin tool list was dead code. The registry preview carries only
widget and tripPage, so the section added for it never rendered, and the
UI hid the description behind the title when both existed. The preview
carries sanitised tools now (previewManifest is exported and tested, which
is how the gap survived), and both fields render, description always.
plugins:use is out of the DCR default. Adding it to ALL_SCOPES meant a
registration that names no scopes got it and had it pre-selected at
consent. Defaults come from DEFAULT_CLIENT_SCOPES now; asking for it by
name still works.
The public ManifestCapabilities gained mcpTools, so a typed manifest can
express what the runtime already validates.
MCPANN-001..005, MCPENF-001..010, MCPPREV-001..005, MCPTOOLS-020..022,
SDKP-015b/c.
The advertised tool surface is frozen at initialize, so anything that
changes which plugin tools exist has to end the sessions built before it.
tools/list_changed is not an alternative: no event store is configured,
and the surface would not be re-read anyway.
Unconditional, no diffing. The surface is per-session, since it depends
on the caller's scopes and the plugin's grants, so there is no ctx-free
set to compare against.
Five routes, mirroring what admin.controller.ts already does for addons:
activate, deactivate, update, retrust and uninstall. activate(id,
consent), update and retrust are also the grant-change paths, so "on an
mcp:tools grant change" needs nothing extra. A failed call invalidates
nothing, because nothing changed.
The addon cascade needed its own call. Disabling an addon can stop a
plugin that publishes tools, and the addon doing the cascading is not
necessarily one of MCP_RELEVANT_ADDONS, so mcpAffected does not cover it.
Still gated on a plugin actually stopping, which keeps the #1414 rule
that a config-only save kills nothing.
Imported from sessionManager rather than the src/mcp barrel, which
evaluates readEnv().mcp at module scope.
MCPINV-001 to MCPINV-007.
The advertise path, shaped like trip-warnings.mcp.ts: gate on the kill
switch, fan out over providersOf, let a failing provider contribute
nothing rather than fail the surface.
A tool is advertised only if the signed manifest declares it AND the
running build reported implementing it, the same two-sided check
callPlugin makes for exports. The manifest half is what makes the text
auditable: the child composes its loaded payload after onLoad, so without
it a sha256-pinned artifact could change what it publishes on every
restart with no re-consent.
The result envelope is built here, always. The registry returns a
handler's value verbatim and skips output validation without an
outputSchema, so a plugin returning {ok: true, data} would emit a result
with no content key and every conforming client would error. A failed or
timed-out invoke becomes isError rather than a JSON-RPC fault, so the
model can see it and try something else.
Gates in order: kill switch, then demo mode. The plugins domain had no
demo call site before, and the child has none of its own.
The service owns the process-level source rather than PluginRuntimeService
beside the other sinks, because it needs PluginHooks and PluginHooks
injects PluginRuntimeService. It clears the source on destroy, which the
existing setPluginChannelSource forgets to do: a torn-down buildApp()
would otherwise leave a live source closing over a dead runtime and bleed
into the next suite.
mcpToolCapabilities is a parallel reader rather than a capabilityList
generalisation, since that one returns string[]. It re-runs the manifest
parser instead of trusting the stored blob, which was written by whatever
parser was current at install time and whose caps are the security
control.
Two caps not in the brief: MCP_TOOLS_TOTAL_MAX across all plugins, since
8 each times twenty installed plugins would put 160 tools in front of a
model already choosing from ~200. Truncation is logged, because a
truncated surface reads exactly like a plugin that published nothing.
MCPTOOLS-001 to MCPTOOLS-019.
The child adds the tool names its mcpToolProvider hook implements to the
loaded payload, beside routes/jobs/exports, and the supervisor caches
them on Supervised like the rest. The hook itself already rides the
existing Object.keys(def.hooks) line, so the "did the child report the
hook too" half was free.
Declarative and synchronous: the host never asks the child what tools it
has. It intersects this list with the signed manifest, so a build that
reports a tool the manifest never declared is ignored rather than
trusted.
grantsOf comes along because the advertise path has to clamp what a
tool's annotations may claim against what the plugin was actually
granted, and sup.granted is already in memory. A per-tool DB read would
be the alternative.
plugin-host-entry.ts is excluded from coverage (it runs in a forked
subprocess), so the two supervisor integration cases are the only thing
proving any of this: the loaded payload, the acting-user-bound invoke,
and that both stop the moment the plugin is no longer active.
Tools are declared in the signed manifest, not only reported at load.
plugin-host-entry composes its loaded payload AFTER onLoad runs, so a
signed, sha256-pinned artifact could otherwise emit different tool text on
every restart with no version bump and no re-consent. The host will
advertise the intersection of the two.
It also gives the admin something to read. What mcp:tools will put in
front of every user is a manifest field now, so it can be shown on the
consent screen before the grant is given.
Rejects rather than drops, unlike most capability parsing: a malformed
tool should fail the install visibly, not become a tool that silently
never appears once the plugin is running. Caps and sanitising come from
mcp-tool-schema.ts, so the install path and the advertise path cannot
disagree about them.
Declaring tools without mcp:tools is an error on both sides. Otherwise
the consent screen lists tools that can never run.
No migration: install/discovery.ts already persists capabilities as JSON.
The SDK validator mirrors the checks it can make without the server's
normaliser, so `trek-plugin validate` catches a bad declaration before
publish rather than at install.
The security control for plugin MCP tools, landed on its own so it is
reviewable as one diff. Pure and Nest-free: no container, no DB, no
running plugin needed to test it.
The threat is not a plugin crashing. A tool name, title and description
are a persistent, admin-invisible write into every user's assistant
context, and .meta() puts the JSON Schema there verbatim. TREK's own code
proves the channel is obeyed: the static-token deprecation notice is
delivered in a tool result precisely because that is what an assistant
reliably surfaces.
sanitiseAssistantText is a sibling of stripEmoji, not a change to it.
stripEmoji preserves newlines on purpose (PDF paragraphs) and ~15 render
call sites depend on that. The new one collapses every whitespace run to
a single space, so a description cannot forge "## System" as a heading
that reads to the model like host text, and strips more than the brief
asked for: the bidi overrides and isolates and U+2028/9 are neither C0
controls nor \n, so they defeat both halves of the stated rule.
The schema is advertised rather than converted. z.looseObject({}).meta()
passes the plugin's own schema through byte for byte, keeping pattern,
oneOf and format, which a hand-rolled converter flattens to unknown. The
.check() keeps required enforced, so the handler cannot receive a call
missing arguments its own schema declared mandatory.
That only works with a normaliser in front, since .meta() is a
passthrough. It rejects a root type that is not object (which would
advertise a non-object inputSchema while the runtime still passes an
object), rejects $ref/$defs/definitions at any depth, and caps bytes,
properties and nesting. It strips a top-level id, which vanishes from the
advertisement and lands in z.globalRegistry._idmap, a strong Map unlike
the WeakMap holding the rest.
Sanitising recurses into every nested description, title and string enum
member. That is the part a top-level cap misses, and where a property
named trek_session_token with a plausible description would live.
Annotations are clamped asymmetrically. readOnlyHint gates client
auto-approval, so a plugin may lower its danger level and never raise it;
openWorldHint goes the other way, since egress makes a tool open-world
whether it admits it or not. db:own is deliberately not a side effect:
counting the plugin's private SQLite would make nearly every plugin
non-read-only and kill auto-approval for legitimate read tools.
MCPSCHEMA-001 to MCPSCHEMA-030.
Modelled on the hook:* family rather than on geolocation:read. The host
dispatches INTO the child for every tool call, so it needs what
HOOK_PERMISSION buys: providersOf()'s active-and-holds-the-grant check,
the second gate in invokeHook, and the acting-user binding. That last one
is the point. The user is bound host-side from the supervisor's
invocation map, so a tool called by user A reaches A's data whatever the
model puts in args.
The permission keeps no hook: prefix because the surface it opens is MCP,
not a TREK render slot, and nothing keys off that prefix. It is the first
HOOK_PERMISSION value shaped that way; `satisfies` still holds.
One commit, because it cannot be split. requireTotalCoverage makes a
HOOK_PERMISSION key with no @PluginHook consumer a boot failure, not a
test failure, and PluginRpcRegistryService.onModuleInit runs in every
buildApp() — so a half-landed version breaks every integration test in the
repo. The generator is chained to it too: it refuses to run without both
the en admin label and the CLI hint.
15s is the longest budget of any read hook. The others back a render
someone is waiting on and fall back when they time out; this one backs a
chat turn, and the plugin is likely calling a third-party API of its own.
Still under the supervisor's unused 30s default, which is a hung client
rather than a budget.
One callTool fn for every tool, since tool names are runtime data and
hookContracts() is keyed by (hook, fn).
PluginDefinition exists in two copies with no generator between them, so
both get McpToolProvider. grantGaps and the dev mock host need no change:
both read HOOK_PERMISSION, which is generated.
PLUGHOOK-010, plus the CONTRACTS row.
The importer's parsed item spells the provider id `googleFtid`; the match rule
reads `google_ftid`. `isPlaceDuplicate` was handed a mapped object, but
`findDuplicatePlace` got the raw one, so its id strategy was always empty. The
name then decided the match, and a second place sharing that name could take the
ftid belonging to the renamed one. One candidate now serves all three calls.
Sharing the strategy list shares the ORDER, not the comparison, and two
differences survive it. SQLite `lower()` is ASCII-only where JavaScript's is not,
so a row stored as CAFÉ CENTRAL does not match the candidate Café Central in SQL
while it does in memory. The coordinate fallback used to paper over that for a
named candidate, sometimes with the wrong row. And `buildDedupSet` collects
coordinates only for unnamed rows while the coordinate query considers every row,
which is the answer `findMatchingPlaceId` wants and the opposite of what the
in-memory half says. Both are now written down where someone will read them, and
pinned by tests, rather than implied away.
PLACE-SVC-028e covers the backfill landing on the right row. PLACES-SVC-014 to
-017 cover the id beating a misleading name, both remaining divergences, and the
tolerance as a number rather than as a reference to itself.
Two consumers decide whether a candidate place is one the trip already has,
and they spelled the rule out separately: the bulk importers ask an in-memory
set (isPlaceDuplicate), while findDuplicatePlace asks the database because its
caller needs the matched row's id. The two had drifted — the SQL copy falls
back to a coordinate match for a NAMED candidate, which the in-memory copy
deliberately never does, since that would merge the restaurant and the bar at
the same address.
The order now lives once in @trek/shared as placeMatchStrategies(), and each
consumer is a mechanical interpreter of it: provider id (the only part that
survives a rename, #1550), then name, then coordinates and only when there is
no name.
Adds PlacesService.findMatchingPlaceId as the public door for an importer that
has to link to the existing place rather than merely skip the candidate.
findDuplicatePlace stays private behind it — it also returns google_ftid for
the bulk importer's backfill, which is that caller's business and not part of
"which place is this?".
No behaviour change for either existing caller. The coordinate branch is
currently unreachable from the one call site, which sits inside an
isPlaceDuplicate() guard that has already matched on name and returned; the
drift only bites once something calls it standalone.
COORD_DEDUP_TOLERANCE and externalIdsOf move to shared and are re-exported from
places.helpers, so collections.service and places.service keep their imports.
Guests are trip-scoped accounts. Trip Members excludes them from its picker and
refuses them on the write path, and Collections does the same, but Vacay's two
directories did not: a guest stayed selectable in a plan's Invite User dialog
and in the shared-calendar picker, including after being removed from the trip
it was created for.
The queries are server-side, so the filter belongs there rather than in the
client, which never received the flag to filter on. Both write paths get it too:
the picker only hands out a list, the id comes back from the client, and the
Vacay MCP tools reach the same two service methods.
Fixes#2112
The Atlas hero counted more countries than the continent bars below it drew,
and more than the dashboard widget. The extra ones were invented: a place
without coordinates fell back to parsing its address, and any two-letter
uppercase segment at the end became a country code. "New York, NY" produced NY,
"Toronto, ON" produced ON.
Those phantoms only ever showed up in the one number. The continent strip files
an unknown code under 'Other' and never draws it, the map skips codes it cannot
resolve, and the dashboard builds its set from stored regions instead, which is
why it stayed right.
Filtering against a list of valid codes would not have been enough, and would
have hidden the worse half of this: 26 US state abbreviations are real ISO
country codes. CA is California far more often than Canada, DE is Delaware, IN
is Indiana. The existing comment above resolveCountryCode already describes that
collision and solves it by trusting coordinates first and sanity-checking the
address against the country's own bounding box. The gap it names in its last
line is this bug: with no coordinates there is nothing to check against.
So the bare-code branch is now only offered to callers that have coordinates. A
spelled-out country name still resolves either way. A lone "NY" or "CA" resolves
to nothing, which is the honest answer.
Fixes#2111
Pointing the instance at a different OIDC provider left two things from the old
one on every account. The avatar kept a URL on a host this instance no longer
talks to, so it rendered as a broken image and stayed that way: the sync only
ran when the new provider sent a picture claim, and having no picture for a user
is a perfectly normal answer.
The account also stayed pinned to the old provider. Login still worked, because
the verified-email lookup finds the row, but the relink sat behind a check for a
missing sub, so oidc_sub and oidc_issuer never moved. The SSO note in settings
went on naming a provider that was gone.
An uploaded avatar is a bare filename rather than a URL and is left alone on
both paths, exactly as before.
Fixes#2110
A journal importer can create the journey, create its entries and write their
stories, and then it stops: there was no way to give an entry a picture. The
seven journal.* methods cover text and nothing else, so an importer reading an
export archive could only upload its photos as trip files, which live on a trip
and never reach the journey's gallery.
journal.addEntryPhoto takes the bytes. Deliberately not a link to an existing
gallery photo and not a provider asset: an importer holding an archive has
neither, it has a file it just unpacked.
The access decision stays where it already was. addPhoto answers null unless the
acting user may edit the journey the entry belongs to, and null becomes
RESOURCE_FORBIDDEN here, so a plugin reaches exactly the entries its acting user
does. Everything the REST upload refuses, this refuses too: images only and no
SVG, the operator's allowed-file-types setting, 10MB decoded, demo mode. The
stored filename is the host's, never the plugin's, because the gallery dedupes
on file_path. The object is written before any row points at it and removed
again when the link is refused.
JournalRpc moves into its own module for this. It now needs StorageService and
the EXIF backfill out of MemoriesModule, and JourneyDomainModule exists
precisely so places, assignments and the plugin host can reach a few journey
functions without dragging both photo providers into their graphs.
Discussion #1365
Homepage's customapi widget and everything shaped like it want a handful
of numbers from one request; /api/v1/trips hands out the rows, but a
widget maps fields, it cannot aggregate a list.
The figures come from AtlasService.getTravelStats, the same call behind
the dashboard's passport card, rather than from a second count. Visited
countries carries years of rules — countries reached only by a flight,
layovers that do not count, countries hidden by hand — and a second
implementation would disagree with the card next to it within a release.
That is also why the controller sits in atlas/ and not in public-api/:
PublicApiModule stays a leaf, so the route comes to the data instead.
TravelStatsController next door already does this for
/api/auth/travel-stats. ApiTokenGuard is provided locally rather than
imported with PublicApiModule — @UseGuards instantiates a guard in the
declaring controller's module, so an export would still leave its
TokenService unresolvable here.
last_trip means the most recently started trip, not the most recently
created one, and is null while every trip is still ahead.
Closes#1367
Reading the surface back against real data turned up three things it dropped
without saying so.
Places with no day were invisible, because the payload only reached places
through day_assignments. On a live instance that is half of them, and they
are the ones carrying a coordinate and no claim about when — exactly what an
integration can act on. They come back under `unplanned_places`, minus the
hotel, which is already reported as an accommodation.
Bookings with no day were dropped by the day filter. reservations.day_id is
nullable and deleting a day detaches its bookings rather than removing them,
so this is not a hypothetical.
The bucket list had no endpoint at all. It hangs off the user rather than a
trip, so it gets its own, and it answers whether or not the Atlas addon is
switched on: the addon decides what TREK shows, not what a key can read.
Also fixes ?include=notes returning the bare trip: places, notes and
reservations are reported on days, so asking for one now implies days
instead of quietly answering with nothing.
The module stays a leaf. Reaching for AtlasService pulled AuthModule and the
storage registry in behind it and broke the surface's own boot, so the one
SELECT lives here like every other query in the file.
An MCP token drives every assistant tool; an API key reads trips over HTTP.
Handing the same credential to a third-party integration gives it the whole
MCP surface, so the two are now separate kinds on `mcp_tokens` and the kind
sits in the WHERE clause — a token of the wrong kind is indistinguishable
from one that does not exist.
Also adds `travellers` to the trip payload: display name and an owner flag,
no ids and no email addresses. An integration key reads its owner's
itinerary; it is not a way to enumerate the people around them.
Documents the surface in the wiki, since integrators need something to read
that is not the source.
Two gaps from the first pass, both about a client that polls.
Rate limiting is keyed by user rather than by IP. A self-hosted integration and
its owner's browser routinely leave the same address, and an IP bucket would let
one starve the other; 120 requests a minute is generous for a sync surface and
still bounded, so a runaway poll degrades its own integration instead of the
instance.
updated_at now ships with every trip, and its docblock says plainly what it is
not: a content timestamp. It moves when the trip's own fields change — title,
dates, currency, cover — and stays put when a place, a day or a note is edited,
because the child tables carry no updated_at of their own. That also rules out
the ?since= filter this was meant to enable: it would silently hide every trip
whose itinerary changed, which is worse than no filter at all. Making it honest
needs a schema change, not an API field, so the field is documented for what it
is and the filter is left out.
TREK's REST surface is internal: it answers a session cookie, carries no
version, and changes with the client. That is fine for our own frontend and
useless for anyone building against us — the Dawarich integration people have
been asking for since April needs something it can rely on not moving.
So this adds a small one rather than opening the existing 549 endpoints:
GET /api/v1/trips the trips the caller can reach
GET /api/v1/trips/:id one trip, ?include= picks the sections
Auth reuses the trek_ tokens users already manage under Settings, via Bearer
or X-API-Key. No new credential type, no new table — the guard calls the same
hash lookup the MCP transport does. It deliberately refuses a session JWT and,
for now, OAuth bearers: a credential whose scopes this surface does not yet
interpret is worse than one it rejects outright.
Access is decided per trip against canAccessTrip, the same owner-or-member
predicate as everywhere else, and a trip the caller may not read answers 404
identically to one that does not exist — a 403 would confirm the id is real and
turn the endpoint into a way to count someone else's trips.
The payload is not the rows. Ids, foreign keys and order_index stay inside;
accommodation day ids resolve to ISO dates, places arrive in planned order, and
every dated thing carries its date so a consumer can join on days rather than
on ids it has no way to look up.
fixArrivalDate normalized the arrival and then looked for the time behind a
leading date. The model usually sends the arrival as a bare clock, "13:00" with
no date on it, and slicing ten characters off that leaves nothing to match, so
the function returned early and stopped deriving the arrival date at all.
Try the bare 12-hour form first, then fall back to the normalized string, which
covers both shapes: "13:00", "01:11 pm", and a full date-time.
When neither the keyword table nor the model produced a type, the router wrote
"hotel" and cast whatever string it got to FlatType without checking it. A hotel
is a booking type, so a ferry voucher imported from the Transport tab opened the
booking form, which offers six chips and not one transport among them, and
"other" was the only honest pick left. An unrecognised string got through the
cast too and then vanished silently in nuExtractToKiReservations.
The type is validated against FLAT_TYPES now. When it cannot be read the item is
still extracted with the lodging shape so it survives mapping, but it says so,
and the tie-breaker #2080 added finally has something to fire on: isUnplaceableItem
only ever matched a type outside both form's vocabularies, and the server can
only emit its own eight, so on a real document it never matched at all.
The tab travels with the job instead of sitting in component state. The parse
deliberately outlives navigation and reload and the review is triggered by the
global widget on any page, so by the time it ran the state had remounted back to
its default and the context was gone.
splitIso reached the time by taking characters 11..16, so "2026-06-11T01:11 PM"
became 01:11. The hour survived and the half of the day did not, which is why
AM and 24-hour times in the same document imported correctly. The arrival then
sorted before the departure and the router rolled it onto the next day, so a
three-hour hop was rendered as an overnight.
The client has done this properly since #1725; this is the server half, which
was never pulled across. normalizeLocalDateTime rewrites a value only when it
actually finds a meridiem, so a date-only value, a value carrying an offset and
anything the parser does not know come back byte for byte.
The mapper has only two exits for a date, toIsoString and splitIso, so this
covers all eight types and every emitted field at once. It also stops
sameConnection comparing a NaN timestamp, which silently merged unrelated legs
into one booking.
The ICS feed is public with a token that never expires and prints the
confirmation number into the DESCRIPTION, and the shared-trip payload does a
SELECT * over reservations with share_bookings on by default. Neither filters
anything, so whatever lands on a trip is published the moment it is written.
That is fine while a person types every booking in, and it stops being fine as
soon as something writes on their behalf.
reservations.ingest_state is that gate. It defaults to live, SQLite backfills
every existing row on the ALTER, and no current writer names the column, so
every booking visible today stays visible. Nothing writes staged yet.
The predicate takes its table alias because share.service.ts queried
reservations without one. A stay is withheld only when bookings point at it and
all of them are staged: a hand-added hotel has no booking at all and has to keep
showing up.
Ten new controllers in the test registry, and one line in the session
instructions: without it nothing tells an assistant to read the display
settings before rendering a temperature, which is what makes that tool worth
having.
A plugin warning is a plugin telling the user something is wrong with their
trip, and get_trip_summary omitted them, so an assistant reviewed a trip
from a strictly smaller picture than the user was looking at.
The whole memories domain had no tools, so the Immich and Synology
libraries were invisible even though the attach body is plain JSON and the
photos never pass through the call. Search, album listing, album contents,
and the attach.
The routes that stream photo bytes stay out: those are img targets.
TREK's own bulk travel-history importer, and the closest thing in the
product to what this whole branch is about. Both halves are server-side
JSON, so neither was ever browser-bound; they simply had no tools. The list
tool's output carries what the import tool needs, so a model can run the
pair without a detour through the UI.
export_trip_ics is a snapshot of one trip, so subscribing was out of reach
and the all-trips feed had no counterpart at all. Both feed tokens can now
be read, minted, rotated and revoked.
The invite link is the one that grants membership, which create_share_link
is not, and the descriptions say so in both directions so a model does not
reach for the wrong one. The two consumer routes stay out: the ics URLs are
what gets subscribed to, and accepting an invite is a human act.
test-db's reset list was missing trip_invite_tokens, so a leftover invite
re-attached itself to the next test's trip once ids restarted.
Maurice