feat(core): implement ADR-010 reporting as an admin operation#628
Conversation
Treat reporting as a sibling of the Admin Script Runner (ADR-005) on shared primitives, replacing PR #607's standalone reporting silo. Reports register via `reports: []` in the app definition (plus `admin.includeBuiltinReports` for core built-ins), extend `ReportBase`, and run through the same runner, admin API key, async/SQS execution, and EventBridge scheduling as admin scripts. Run modes: live (compute inline, persist nothing), recorded (persist an execution record), snapshot (recorded run tagged into a named series). All three use the isolated `AdminScriptExecution` store (type: 'REPORT', no user/ integration FK), so a user-scoped query can never return a report record. Highlights: - ReportBase + built-in `integrations` report (PR #607 aggregation preserved, schemaVersion 1) reading via the admin command bundle, not a repository. - Fold the retired reporting repository triad into the canonical integration / integration-mapping repositories (findAllForReport, countByIntegrationIds); hoist the DocumentDB cursor-drain into documentdb-utils (findManyDrained / aggregateDrained) so deployment-wide scans never truncate at the first batch. - report-commands (type:'REPORT', guarded findExecutionById, findSnapshotSeries), report-runner, report-router (live + async + snapshots + executions + schedule), report-executor-handler, bootstrap wiring with a script/report name-collision guard. - devtools AdminScriptBuilder: dedicated ReportQueue + report executor Lambda, report router, artifact bucket + scheduler resources gated on reports; remove the always-on standalone reporting function. - Artifact storage (S3 + signed URL, private + SSE) for non-JSON report output. - Credential-refresh generic (countActiveByType) reading only a non-secret projection — never decrypts credential secrets. Retire the silo: delete reporting-router, reporting/repositories/*, use-cases/list-integrations-report, handlers/routers/reporting; collapse the dedicated reporting API key into the admin API key. Reviewed across three adversarial passes; confirmed findings fixed (artifact dev-stage selection, DocumentDB truncation, execution-record compensation on enqueue/requeue failure). NOTE: adds @aws-sdk/client-s3 and @aws-sdk/s3-request-presigner to packages/core/package.json — package-lock.json must be regenerated with the repo's canonical npm 10 before merge (do not commit an npm 11 lockfile). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
Add a Reports (ADR-010) section to the frigg skill covering the reports registry, ReportBase, run modes (live/recorded/snapshot), endpoints, artifact output, and scheduling, with two examples: a snapshot+scheduled JSON report reading via the frigg command bundle, and a recorded CSV report writing to artifact storage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5477c3a6aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| router.post('/:name/run', async (req, res) => { | ||
| try { | ||
| const { name } = req.params; | ||
| const { mode = 'live', params = {} } = req.body || {}; |
There was a problem hiding this comment.
When callers omit mode, this forces every report through live instead of using the report's declared default (Definition.runModes[0], which ReportRunner already implements). For reports that default to snapshot/recorded or do not support live, POST /api/v2/reports/:name/run without a mode now either fails with INVALID_MODE or runs inline when the report author intended an async recorded run.
Useful? React with 👍 / 👎.
| mode, | ||
| input: params, |
There was a problem hiding this comment.
For recorded/snapshot requests this path creates a persisted execution and sends an SQS message without first checking that the requested mode and params satisfy the report definition. A request such as mode: "recorded" with invalid input, or mode: "bogus", receives 202 QUEUED and only later becomes a failed execution, unlike live runs and admin scripts which fail fast with 400; validate against the report definition before createExecution.
Useful? React with 👍 / 👎.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
…isolation) Follow-up to ADR-010 reporting, resolving verified code-review findings: - Artifact retrieval: report execution + snapshot reads now mint a signed download URL (report-commands) so stored non-JSON artifacts are reachable via the API; snapshot artifactUrl is a real URL, not the raw storage ref. - Async run validation: recorded/snapshot runs validate mode + inputSchema before persisting/enqueueing, returning 400 up front instead of a 202 that fails later in the worker. - Isolation: admin-script findExecutionById now rejects REPORT rows, symmetric with report findExecutionById rejecting non-REPORT rows. - Report stays protocol-agnostic: integrations-report throws a coded INVALID_INPUT error instead of Boom (no HTTP in the application layer). - Add zip artifact content type; remove orphan findMany import; correct the local artifact adapter comment. - Docs: clarify that a Definition-declared schedule only supplies the default mode; the recurring trigger is activated via PUT /:name/schedule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
…unwrap helper Reduce PR-added code comments across the reporting stack so the code speaks for itself (~700 -> ~290 comment lines), keeping only non-obvious "why": invariants, ordering/consistency reasons, and gotchas. Removed route-path JSDoc banners, architecture-tag headers, and param/flow restatements. Comment-only changes; no runtime behavior altered. Also drop the unwrap() helper in integrations-report: read commands directly so the report body reads plainly. A failed read still fails the report (the runner records FAILED), just without unwrap's synthesized message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
1 participant
Treat reporting as a sibling of the Admin Script Runner (ADR-005) on shared primitives, replacing PR #607's standalone reporting silo. Reports register via
reports: []in the app definition (plusadmin.includeBuiltinReportsfor core built-ins), extendReportBase, and run through the same runner, admin API key, async/SQS execution, and EventBridge scheduling as admin scripts.Run modes: live (compute inline, persist nothing), recorded (persist an execution record), snapshot (recorded run tagged into a named series). All three use the isolated
AdminScriptExecutionstore (type: 'REPORT', no user/ integration FK), so a user-scoped query can never return a report record.Highlights:
integrationsreport (PR feat(reporting): read-only integrations reporting API in core #607 aggregation preserved, schemaVersion 1) reading via the admin command bundle, not a repository.Retire the silo: delete reporting-router, reporting/repositories/*, use-cases/list-integrations-report, handlers/routers/reporting; collapse the dedicated reporting API key into the admin API key.
Reviewed across three adversarial passes; confirmed findings fixed (artifact dev-stage selection, DocumentDB truncation, execution-record compensation on enqueue/requeue failure).
NOTE: adds @aws-sdk/client-s3 and @aws-sdk/s3-request-presigner to packages/core/package.json — package-lock.json must be regenerated with the repo's canonical npm 10 before merge (do not commit an npm 11 lockfile).