Three breakages on the prior CI run:
1. PHPUnit 12 didn't propagate `@depends` data — chained tests got
ArgumentCountError because they expected `array $data` but PHPUnit
passed nothing. Convert all annotations to `#[Depends]` attributes,
matching the modern style already used by the Migrations and
VectorsDB suites.
2. `InsightsCustomConsoleTest` extends ProjectConsole, which doesn't
set up a project API key. The trait's `serverHeaders()` hardcodes
`x-appwrite-key`, so every Console test 401'd. Drop the Console
class entirely — the manager Create endpoint is KEY-only by
design, the Server class already exercises every code path, and a
Console-side variant adds no real coverage.
3. `testCreateRequiresManagerScope` called `getNewKey()`, which lives
on `ProjectCustom`. PHPStan flagged the call as undefined when the
trait was analyzed against the (no-longer-existing) Console class.
Move the test into `InsightsCustomServerTest.php` directly so it's
only ever resolved against `ProjectCustom`.
Plus PHP 8.4 + match-exhaustive cleanups PHPStan caught while I was
in there:
- `?array $headers = null` instead of `array $headers = null` on every
helper (PHP 8.4 deprecates implicit-nullable params).
- `?string $insightId = null` on `sampleInsight()`.
- `parentResourceType` match collapsed to `tablesDB => 'tables';
databases/documentsDB/vectorsDB => 'collections'`. The earlier
`$type` match already throws on unknown engine, so the fall-through
default was unreachable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
User Update only accepts severity + status now — analyzer-controlled
fields like summary flow through the manager Create endpoint. The
previous testUpdate sent `summary: 'Updated summary.'` and asserted
it changed; Utopia silently dropped the unknown param so the
assertion would have failed.
Trim the call to severity-only and verify the analyzer fields
(including summary) are preserved on the response.
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]
- InsightCTA model now exposes `service` (SDK namespace) and `method`
(function name) instead of a single `action` string. Drops the
string-splitting burden on every consumer and lets the console reach
the right SDK method directly.
- Validator requires both `service` and `method` non-empty; same
16-entry max still enforced.
- Endpoint normalization (Create + Update) splits the new shape into
the persisted CTA descriptor.
- Constants split: INSIGHT_CTA_SERVICE_* (databases / tablesDB /
documentsDB / vectorsDB) and INSIGHT_CTA_METHOD_* (createIndex).
- Insight model + InsightCTA model docs updated with the new field
semantics and per-engine examples.
- E2E factory `sampleCTA($id, $engine)` emits the correct service and
engine-appropriate params keys (tableId/columns for tablesDB;
collectionId/attributes everywhere else). Engine matrix asserts
`service` and `method` independently.
- Added e2e + unit coverage for the new failure modes (missing
service, missing method, empty service, empty method).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- 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.
- Restructure InsightsBase trait with explicit helper methods
(createInsight/getInsight/listInsights/updateInsight/deleteInsight,
createReport/getReport/listReports/updateReport/deleteReport, plus
sampleInsight/sampleCTA factories) — same shape ProxyBase uses.
- Add coverage for: report CRUD + duplicate-id rejection, invalid type
rejection, list filtering by all allowed attributes, cursor
pagination + missing-cursor, update preserving untouched fields,
CTA validation edge cases (duplicate ids, empty fields, count > 16),
dismissal round-trip + status filter, report cascade delete,
unauthorized access (no server key), empty-result list.
- Engine-specific insight types (tablesDBIndex, documentsDBIndex,
vectorsDBIndex, plus the legacy databaseIndex) so the CTA's `action`
can map to the matching public API: databases.createIndex,
tablesDB.createIndex, documentsDB.createIndex,
vectorsDB.createIndex. dataProvider drives the engine matrix and
asserts the right action lands in the persisted CTA. Constants for
each action name live in app/init/constants.php.
- InsightCTA model docs spell out which action belongs to which engine
and that the params keys differ between APIs (tableId/columns for
tablesDB vs collectionId/attributes for the legacy / DocumentsDB /
VectorsDB APIs).
- Insight model `type` description now lists every engine variant.
- CTAsTest gains coverage for object-shaped params, empty-action and
empty-label rejection, and the default 16-entry cap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
copilot-swe-agent[bot]andabnegate
Jake Barnby
Matej Bačo
ArnabChatterjee20k
Damodar Lohani
Damodar Lohani
Matej Bačo
Chirag Aggarwal