The prefix length 700 exceeded the resourceId/parentResourceId column
size of 255 (Database::LENGTH_KEY), crashing the container at startup.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add skipFilters to Reports/Get.php (was the only endpoint still
triggering the full N+1 subquery cascade)
- Scale CTA batch limit dynamically (insightCount * MAX_CTA_COUNT)
instead of fixed APP_LIMIT_SUBQUERY to prevent silent truncation
- Revert deleteReport to callback-based pagination so CTAs are not
orphaned when a report has more than APP_LIMIT_SUBQUERY insights
- Add explicit prefix lengths (700) to _key_project_resource and
_key_project_parent_resource indexes to stay under InnoDB 3072-byte limit
- Validate CTA service/method against ADVISOR_CTA_SERVICES and
ADVISOR_CTA_METHODS enums in the CTAs validator
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix N+1 in Reports/XList (51→4 queries) via skipFilters + batch fetch
- Add skipFilters to Reports/Delete and cursor fetch (avoid loading all
nested insights/CTAs just for ownership check)
- Fix N+1 in deleteReport worker (flat CTA deletion instead of per-insight)
- Add advisor entity cleanup on project deletion (reports, insights, CTAs)
- Remove resourceInternalId, parentResourceInternalId, $permissions from
Insight response model (internal IDs leak DB internals, permissions unused)
- Remove dead subQueryInsightCTAs filter registration
- Remove stale enum-value comments from platform schema
- Fix _key_dismissedAt index to include projectInternalId
- Fix scope category from 'Other' to 'Advisor'
- Switch action base class from Utopia\Platform\Action to Appwrite\Platform\Action
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The migrations worker (Appwrite→Appwrite migrations + CSV/JSON imports
& exports) reads `_APP_MIGRATION_HOST` to call back into this instance's
API. `_APP_MIGRATION_HOST` was introduced in #11229 (1.8.x / 1.9.x) but
was never added to `app/config/variables.php`, so `appwrite install` /
`appwrite upgrade` never write it into the generated `.env`. With the
var unset, the migrations worker fails — on a fresh self-hosted install
the first export hangs with no error (#11853). (Contributors and CI
don't hit it because the repo's hand-maintained `.env` already has
`_APP_MIGRATION_HOST=appwrite`; only the *installer-generated* `.env` is
missing it.)
Add `_APP_MIGRATION_HOST` to `app/config/variables.php` with default
`appwrite` — the API service name in the standard Docker Compose setup,
which is what the repo's own `.env` and the cloud Helm charts already
use, and what the migration endpoint was hardcoded to before #11229.
`appwrite install` and `appwrite upgrade` now write it into the
generated `.env`, so fresh installs and upgrades have it set and the
migration/import/export flows work.
Scope: this PR fixes the install & upgrade paths only — it deliberately
doesn't change the worker code.
Fixes#11853
The module's namespace and directory now match the top-level service
name (`advisor`) instead of one of its resource names (`insights`):
- src/Appwrite/Platform/Modules/Insights -> .../Modules/Advisor
- src/Appwrite/Insights -> src/Appwrite/Advisor
- tests/unit/Insights -> tests/unit/Advisor
- Route group label flipped from `'insights'` to `'advisor'`
- Section-header comments aligned
Resource names (`insights`, `reports`, `insightCTAs`) and the
`Insight*`/`Report` response models stay — those are the resources the
service exposes, not the service itself.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves conflict in app/init/constants.php (kept 4327 cache buster, took 1.9.4 stable version).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
target stores free-form URLs or resource IDs. URLs in the wild can
exceed the prior 2048-char cap, so switch the column to VAR_TEXT
(65535). The _key_project_target index already declares an explicit
700-char prefix length, so indexing still works on both MariaDB and
MongoDB.
Bump APP_CACHE_BUSTER for the schema change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch markdown summaries, JSON CTA params, and the embedded
insights/ctas subQuery payloads from sized VAR_STRING to VAR_TEXT.
None of these fields are indexed and they all hold free-form data, so
removing the artificial size cap means we never have to revisit the
limit when an analyzer surfaces a longer report.
- reports.summary, insights.summary: markdown, no upper bound enforced
- reports.insights, insights.ctas: subQuery virtuals carrying full
child rows
- insightCTAs.params: JSON parameter blob for arbitrary CTA actions
Bump APP_CACHE_BUSTER for the schema change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Register dedicated reports.write scope and switch deleteReport to it
so cloud can issue narrowly-scoped delete keys without granting
insights.write.
- Make insights.parentResourceInternalId optional with null default to
match its companion parentResourceType/parentResourceId fields and
unblock insights with no parent (e.g. database-level performance
insights).
- Tighten Insight.reportId model description: insights always belong to
a report, ad-hoc insights are not supported.
- Add reports.write to default test API key and admin role so existing
e2e tests using serverHeaders() can hit the delete endpoint.
- Bump APP_CACHE_BUSTER for the schema change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The reports `_key_project_target` index was failing to create on MongoDB
because target's full size (2048) plus targetType (64) exceeded the 1024
character maximum. Set explicit lengths to truncate target to 700 chars
(total 765, under both Mongo 1024 and MariaDB 768 limits).
Also drop redundant explicit lengths from VAR_ID positions in the
insights and insightCTAs indexes — the validator rejects explicit
lengths on VAR_ID attributes, and they default to length 1 anyway.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Executor\Exception\Timeout (with timeoutSeconds) and translates it at
each call site into BUILD_TIMEOUT, FUNCTION_SYNCHRONOUS_TIMEOUT, or
FUNCTION_ASYNCHRONOUS_TIMEOUT instead of always using the misleading sync
function error. Build timeouts now append to streamed buildLogs rather
than replacing them, and the build worker reports its timeout via Span.
Insights are children of reports — make the URL hierarchy reflect that.
Endpoints:
- POST /v1/manager/reports/:reportId/insights (manager Create)
- GET /v1/reports/:reportId/insights (List)
- GET /v1/reports/:reportId/insights/:insightId (Get)
- PATCH /v1/reports/:reportId/insights/:insightId (Update)
- DELETE /v1/reports/:reportId/insights/:insightId (Delete)
`reportId` moves from optional body field to required path param. All
endpoints fetch the report first (404 REPORT_NOT_FOUND if missing or
in another project), then verify the insight's `reportInternalId`
matches before doing anything else.
Side effects:
- Event names nested: `reports.[reportId].insights.[insightId].create`
etc. Top-level `insights.*` event tree removed from events.php.
- Realtime channel parser handles the nested form: a `reports.{rid}`
event lights up `reports`, `reports.{rid}` channels; a nested
`reports.{rid}.insights.{iid}` event also lights up
`reports.{rid}.insights` and `reports.{rid}.insights.{iid}`.
- Audit resource paths nested similarly:
`report/{request.reportId}/insight/{response.$id}`.
- listInsights query validator drops `reportId` from
ALLOWED_ATTRIBUTES — it's path-scoped now, not a query filter.
Tests:
- E2E helpers `createInsight`/`getInsight`/`listInsights`/
`updateInsight`/`deleteInsight` all take `reportId` as the first
argument.
- New `createFixtureReport()` helper for standalone validation tests
that need a parent.
- Dropped `testCreateWithoutReport` — reportId is mandatory now.
- `testCreateRejectsUnknownReport` now exercises the path-level 404
rather than a body-level check.
- `testGet` and `testUpdateMissing` exercise the
wrong-reportId-but-valid-insightId case (returns
`report_not_found`).
- `testList` asserts every result carries the path's reportId, plus a
404 case for a nonexistent parent.
- `testCreateForEachEngine` and the standalone create-rejection tests
inline-create their own fixture report and clean up after.
- `testListSurvivesEmptyDatabase` renamed to
`testListSurvivesEmptyReport` and uses a fresh fixture report.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`key` was a leftover from when CTAs were embedded JSON — there's no
remaining reason to require analyzers to invent a within-insight
identifier. The execution layer is gone (no `cta.key` event format),
insights are immutable from the user side (analyzers re-ingest by
delete + recreate, so idempotent matching never happens), and `label`
already covers human-facing identification. The console can group/sort
CTAs by `service`+`method` if needed.
- Schema: drop `key` attribute and the UNIQUE
`(insightInternalId, key)` index from insightCTAs. Required fields
are now `label`, `service`, `method` (+ optional `params`).
- Validator no longer requires `key`. Drop the dup-key normalization
loop in the manager Create endpoint — there's no semantic
uniqueness to enforce.
- Response model: `InsightCTA` keeps `$id` + standard headers,
`insightId` backref, and the four functional fields.
- E2E: drop sampleCTA's `$key` parameter, drop the
testCreateRejectsDuplicateCTAIds test entirely, rename empty-fields
test to testCreateRejectsCTAWithEmptyLabel and update the missing-
fields tests to drop `key` from their payloads.
- Unit tests rewritten to drop `key`.
- Comment on the `insights.ctas` virtual attribute updated to
reference the renamed `insightCTAs` collection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Disambiguate the platform-level collection name. Field/request-param
remains `ctas` (the embedded array on the insight response).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Embedding CTAs as a 16384-byte JSON blob on `insights` was the wrong
shape — they're real documents with their own lifecycle. Move them out.
Schema:
- New platform `ctas` collection. Each row carries `projectInternalId`,
`projectId`, `insightInternalId`, `insightId` (backref), plus the
CTA fields: `key`, `label`, `service`, `method`, `params`.
- Indexes: `(projectInternalId, insightInternalId)` for the subquery
lookup and a UNIQUE `(insightInternalId, key)` so the per-insight
uniqueness invariant lives at the DB layer (not just in PHP).
- The `ctas` field on `insights` becomes a virtual attribute backed by
a new `subQueryInsightCTAs` filter that joins child docs at read
time. Consumers still get CTAs embedded on the insight response —
one round-trip from their perspective.
- The CTA descriptor's within-insight identifier renamed `id` → `key`
(clashed with the document `$id`). Validator updated.
Endpoints:
- Manager Create now persists CTAs as separate `ctas` documents after
the parent insight, then re-fetches the insight so the response
carries the freshly-joined CTA list.
- User Update trimmed to user-controlled state only (`severity`,
`status`). `title`, `summary`, `payload`, `ctas`, and `analyzedAt`
are analyzer-controlled — analyzers re-ingest by deleting and
POSTing again to the manager endpoint.
- Insight Delete cascades to CTAs.
- Report Delete cascades through Insights → CTAs.
Response model:
- InsightCTA gains the standard document headers (`$id`,
`$createdAt`, `$updatedAt`) and an `insightId` backref. The
caller-supplied identifier is now `key`.
Tests:
- E2E sampleCTA factory uses `key` everywhere; testCreate asserts the
freshly-created CTA carries `$id`, `$createdAt`, `insightId`, and
the right shape.
- Dropped the testUpdate*CTA* tests — user Update no longer accepts
CTAs. testDismissViaUpdate now depends on testUpdate directly.
- Unit tests rewritten to validate `key` instead of `id`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eldad's review comment: insights about nested resources need a pointer
to the containing parent (the file-in-bucket pattern). Add three
optional fields:
- parentResourceType (plural noun, e.g. `tables`, `collections`)
- parentResourceId
- parentResourceInternalId
so an insight whose `resourceType=indexes` / `resourceId=_idx_status`
can also carry `parentResourceType=tables` / `parentResourceId=orders`
to identify the table that owns the index. All three are nullable for
top-level resources (e.g. a project-wide audit finding).
Schema, response model, manager Create endpoint, and the listInsights
query validator (parent fields are filterable). New compound index
`_key_project_parent_resource(projectInternalId, parentResourceType,
parentResourceId, $sequence)` to support the parent lookup pattern
the console will use ("show all insights for table X").
E2E factory generates a parent by default (engine-aware:
tables for tablesDB, collections for the others). New
testCreateWithoutParentResource exercises the top-level case;
testList gains a parent-resource filter assertion; testUpdate's
preserved-fields check picks up the new attributes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Insights are produced by internal Appwrite services (edge, executor,
background analyzers) — never by user clients. Move the ingestion
endpoint accordingly.
- Move Http/Insights/Create.php → Http/Manager/Insights/Create.php.
- Path: /v1/insights → /v1/manager/insights. SDK Method marked
`hide: true` and namespaced under `manager` so generated SDKs don't
expose it. Auth narrowed from [ADMIN, KEY] to [KEY] only.
- New scope `insights.manager`. Not granted by any user role
(app/config/roles.php) — Cloud/edge teams configure their internal
key issuance to grant it. `insights.write` description trimmed to
the user-facing surface (update/dismiss/delete) since create is now
manager-only.
- Reports, ListInsights, GetInsight, UpdateInsight, DeleteInsight
remain at /v1/insights/*. Existing scopes unchanged.
- Reports `categories` switched from JSON-encoded string to a native
array<string> column (size 64 per entry, up to 32 entries via the
endpoint validator). MySQL JSON-array indexes are weak and we never
query individual entries — read+rewrite only.
- E2E test API key in tests/e2e/Scopes/ProjectCustom.php gains
insights.read/write/manager + reports.read/write so the manager
endpoint is reachable from the test harness.
- E2E InsightsBase.createInsight() helper now POSTs /manager/insights.
- New testCreateRequiresManagerScope verifies a key with
insights.read/write but no insights.manager is rejected with 401.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
greptile flagged missing lengths but the codebase has two conventions:
- all-string composite: [LENGTH_KEY, LENGTH_KEY] (_key_unique, _key_provider_providerUid)
- mixed/non-string composite: [] (e.g. ('enabled', 'type'), ('region', 'accessedAt'),
('targetInternalId', 'topicInternalId'), ('period', 'time'), ('metric', 'period', 'time'))
(teamInternalId, confirm) is string+boolean; closest match is
('enabled', 'type') which uses lengths => [] and orders => [].
Aligning to that established pattern instead of inventing
[LENGTH_KEY, 0].
All four new indexes left lengths/orders as empty arrays; greptile
flagged the inconsistency vs every existing string-attribute index in
the file (e.g. _key_team uses [LENGTH_KEY], _key_unique uses
[LENGTH_KEY, LENGTH_KEY]).
- memberships._key_team_confirm: [LENGTH_KEY, 0] for (string, boolean)
+ [ORDER_ASC, ORDER_ASC]
- projects._key_teamInternalId: [LENGTH_KEY] + [ORDER_ASC]
- platforms._key_project_id: [LENGTH_KEY] + [ORDER_ASC]
- webhooks._key_project_id: [LENGTH_KEY] + [ORDER_ASC]
- memberships: _key_team_confirm on (teamInternalId, confirm) for team-membership confirm-state queries
- projects: _key_teamInternalId on teamInternalId for team-scoped project lookups
- platforms: _key_project_id on projectId for user-facing-id lookups
- webhooks: _key_project_id on projectId for user-facing-id lookups
Re-applies the indexes from the stale PR #9629 (1.7.x base, conflicting)
onto a fresh 1.9.x branch. None of these are in 1.9.x today; existing
similar indexes target projectInternalId / teamId rather than the
user-facing projectId / teamInternalId queries this addresses.
Address review feedback on PR #12194:
- Pivot CTAs to pure descriptors (id/label/action/params). Drop the
server-side execution layer: Action interface, registry, the
databases.indexes.create CTA action, the params validator, the
/v1/insights/:id/ctas/:id/executions endpoint, the InsightCTAExecution
model, the INSIGHT_CTA_* errors, and the corresponding events. The
console invokes the existing public API directly with the descriptor's
action + params.
- Restore Databases\Indexes\Action.php to its pre-CTA shape and inline
the index-create body back into Create.php (the createIndex helper
was added solely for CTA reuse).
- Move insights collection from project DB to platform DB and add a
parent reports collection alongside it. Insights carry projectId /
projectInternalId for tenant scoping and an optional reportId for
grouping. List endpoints filter by projectInternalId; Get/Update/
Delete also enforce project ownership before touching the document.
- New Reports module with full CRUD (Create/Get/XList/Update/Delete),
Report response model, Reports query validator, REPORT_NOT_FOUND /
REPORT_ALREADY_EXISTS errors, reports.read / reports.write scopes,
and reports.* event tree. Delete cascades to child insights.
- Update.php now mutates the loaded document via setAttribute (instead
of passing a partial new Document), reuses CTAsValidator (instead of
the looser ArrayList<JSON> + isset check), and rejects duplicate CTA
ids.
- Create.php enforces unique CTA ids during normalization.
- CTAsValidator gained a configurable maxCount (default 16) so the
Create path matches the Update path and the DB column size, and
oversized payloads return a clean 400.
- Validator\Queries\Insights adds status and reportId to
ALLOWED_ATTRIBUTES so dismissal / report workflows are filterable.
- Realtime channel parser guards $parts[1] for both insights and
reports event names.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Jake Barnby
Atharva Deosthale
Prem Palanisamy
copilot-swe-agent[bot]
Aditya Oberai
loks0n
Matej Bačo
Damodar Lohani
Matej Bačo