feat(adt-pilot): @abapify/adt-pilot — Mastra-based ABAP code review (workflow + harness)#116
Open
feat(adt-pilot): @abapify/adt-pilot — Mastra-based ABAP code review (workflow + harness)#116
Conversation
Agent-Logs-Url: https://github.com/abapify/adt-cli/sessions/8c6e39c2-3ae6-4d7f-8cef-5ff4f1b5904d Co-authored-by: ThePlenkov <6381507+ThePlenkov@users.noreply.github.com>
Agent-Logs-Url: https://github.com/abapify/adt-cli/sessions/8c6e39c2-3ae6-4d7f-8cef-5ff4f1b5904d Co-authored-by: ThePlenkov <6381507+ThePlenkov@users.noreply.github.com>
Agent-Logs-Url: https://github.com/abapify/adt-cli/sessions/8c6e39c2-3ae6-4d7f-8cef-5ff4f1b5904d Co-authored-by: ThePlenkov <6381507+ThePlenkov@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
ThePlenkov
April 29, 2026 04:50
View session
✅ Deploy Preview for adt-cli canceled.
|
Contributor
|
View your CI Pipeline Execution ↗ for commit d819a8d
☁️ Nx Cloud last updated this comment at |
Workflow mode (deterministic, no LLM): - Three-step pipeline: resolveObjects -> runAtcChecks -> buildReport - Discriminated-union input schema with 'package' or 'transport' modes - Package mode enumerates objects via list_package_objects MCP tool - Transport mode validates via cts_get_transport, uses transport URI as ATC target (server-side enumeration matches 'adt check --transport') - Per-object error capture: failed ATC runs become priority='error' findings; the workflow never aborts on a single failure Harness mode (interactive via Mastra Agent): - createAbapifyPilot(config) returns a Mastra Harness with a 'review' mode - Review agent wired with list_package_objects, atc_run, cts_get_transport - Optional custom instructions and pre-loaded MCP tools Type/build hygiene: - Type-erase Mastra workflow + step factories at the import boundary; Mastra's deeply generic types caused tsc OOM and DTS bloat - Public API surfaces narrow CodeReviewWorkflow / CodeReviewRun handles - typecheck target overridden to 'tsc -p tsconfig.lib.json --noEmit' so tsc does not follow project references (avoids adt-mcp typecheck OOM) - Move @abapify/adt-mcp from runtime deps to devDeps (only used in tests) - Promote @modelcontextprotocol/sdk to a runtime dep (used by mcp-client) Tests (28 total, all passing): - workflow.test.ts: rewritten for async createRun() API; covers package mode happy path + empty package, transport mode happy path + URI assertion, single + multi-object error paths, transport-lookup failure - types.test.ts: schema validation; credentials generated via randomBytes - harness.test.ts (new): factory smoke tests for Harness + Agent - mcp-client.test.ts: unchanged (still passing) Security: - All test credentials generated per-process with randomBytes (no more hardcoded 'secret'/'test-password' strings flagged by SonarCloud) Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…tspots SonarCloud (rule typescript:S5332) flagged 'http://sap:8000' string literals in tests/types.test.ts as security hotspots. Switch the example baseUrls in both tests and JSDoc to 'https://sap.example.com' / 'https://my-sap.example.com'. The localhost URL in workflow.test.ts is intentional (in-process mock server) and is not flagged by the rule. Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
devin-ai-integration
Bot
changed the title
Review Summary by Qodofeat(adt-pilot): Mastra-based ABAP code review workflow and harness
WalkthroughsDescription• Introduces @abapify/adt-pilot, a Mastra-powered ABAP code review package • Implements deterministic workflow with three-step pipeline: resolveObjects → runAtcChecks → buildReport • Supports package and transport modes with discriminated-union input schema • Provides interactive Harness with review agent wired to MCP tools • Includes comprehensive test suite (28 tests) with mock ADT server integration Diagramflowchart LR
Input["Input: Package or Transport"] --> Resolve["resolveObjects Step"]
Resolve --> ListPkg["list_package_objects MCP"]
Resolve --> CtsGet["cts_get_transport MCP"]
ListPkg --> Objects["Object URIs"]
CtsGet --> Objects
Objects --> RunAtc["runAtcChecks Step"]
RunAtc --> AtcRun["atc_run MCP per object"]
AtcRun --> Results["ATC Results + Errors"]
Results --> BuildReport["buildReport Step"]
BuildReport --> Report["CodeReviewReport"]
Report --> Output["findings, summary, mode, target"]
Input -.-> Harness["Harness + Review Agent"]
Harness --> Agent["Mastra Agent with MCP tools"]
File Changes1. packages/adt-pilot/src/types.ts
|
Code Review by Qodo
1. Same-package imports include .js
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Nx package @abapify/adt-pilot that provides ABAP ATC-based code review via Mastra in two forms: a deterministic 3-step workflow and an interactive Harness/Agent wrapper, plus OpenSpec documentation and lockfile updates.
Changes:
- Introduces
packages/adt-pilotwith Mastra workflow (resolveObjects → runAtcChecks → buildReport), MCP client wrapper, Harness/Agent factories, and Zod schemas. - Adds a full Vitest suite for workflow, types/schemas, harness smoke tests, and MCP tool-caller behavior.
- Registers the new package in workspace TS project references and updates OpenSpec docs +
bun.lock.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds TS project reference for the new adt-pilot package. |
| packages/adt-pilot/vitest.config.ts | Vitest config for the new package. |
| packages/adt-pilot/tsdown.config.ts | Package build configuration (notably disables DTS generation). |
| packages/adt-pilot/tsconfig.spec.json | Test TS config for Vitest typing. |
| packages/adt-pilot/tsconfig.lib.json | Library TS config + references to other workspace packages. |
| packages/adt-pilot/tsconfig.json | Project reference umbrella config. |
| packages/adt-pilot/tests/workflow.test.ts | End-to-end workflow integration tests with in-memory MCP transport. |
| packages/adt-pilot/tests/types.test.ts | Zod schema validation tests for input/output. |
| packages/adt-pilot/tests/mcp-client.test.ts | Unit tests for MCP tool-caller wrapper behavior. |
| packages/adt-pilot/tests/harness.test.ts | Smoke tests for Harness/Agent factories and defaults. |
| packages/adt-pilot/src/workflow.ts | Core workflow implementation + schemas + type-erased Mastra integration. |
| packages/adt-pilot/src/types.ts | Public/shared types for workflow and agent usage. |
| packages/adt-pilot/src/mcp-client.ts | MCP SDK client wrapper + connect helper. |
| packages/adt-pilot/src/index.ts | Public barrel exports for workflow, schemas, types, agent, harness, MCP helpers. |
| packages/adt-pilot/src/harness.ts | Mastra Harness factory with review mode. |
| packages/adt-pilot/src/agent.ts | Mastra Agent factory + default instructions. |
| packages/adt-pilot/project.json | Nx project config with custom typecheck command. |
| packages/adt-pilot/package.json | New publishable package manifest + dependencies. |
| openspec/changes/add-abapify-pilot/tasks.md | Implementation task checklist for the change. |
| openspec/changes/add-abapify-pilot/specs/code-review-workflow/spec.md | Workflow requirements/specification. |
| openspec/changes/add-abapify-pilot/specs/abapify-pilot-agent/spec.md | Harness/agent requirements/specification. |
| openspec/changes/add-abapify-pilot/proposal.md | High-level proposal and rationale. |
| openspec/changes/add-abapify-pilot/design.md | Design decisions and trade-offs. |
| openspec/changes/add-abapify-pilot/.openspec.yaml | OpenSpec metadata. |
| bun.lock | Lockfile updates including new package and Mastra dependency graph. |
Comment on lines
+9
to
+17
| "type": "module", | ||
| "exports": { | ||
| ".": "./dist/index.mjs", | ||
| "./package.json": "./package.json" | ||
| }, | ||
| "files": [ | ||
| "dist", | ||
| "README.md" | ||
| ], |
Comment on lines
+10
to
+13
| // @mastra/core has extremely complex generics + Zod schemas cannot be annotated | ||
| // for isolatedDeclarations. DTS generation is disabled to avoid OOM (same | ||
| // pattern as adt-mcp typecheck). TypeScript users can reference the source | ||
| // directly via moduleResolution:bundler. |
Comment on lines
+5
to
+6
| The `@abapify/adt-pilot` package SHALL export a `createAbapifyPilot(config)` factory that returns a configured Mastra `Harness` instance with the **review** mode wired to the `codeReviewWorkflow`. | ||
|
|
Comment on lines
+39
to
+43
| #### Scenario: Build succeeds | ||
|
|
||
| - **WHEN** `bunx nx build adt-pilot` is run | ||
| - **THEN** the build exits 0 and `packages/adt-pilot/dist/` is populated with `.mjs` and `.d.mts` files | ||
|
|
Comment on lines
+30
to
+33
| - **WHEN** the workflow is executed with `mode: 'transport'` and a valid `transportNumber` | ||
| - **THEN** the `resolveObjects` step calls the `cts_get_transport` MCP tool with that transport number | ||
| - **THEN** the step result contains the list of ABAP object URIs from that transport | ||
|
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (25 files)
Incremental Changes (f724665 → HEAD)Since the last review, only two trivial changes were made:
No code logic, types, or tests were modified. All 12 previously noted inline comments remain on unchanged lines. Key Highlights
Verification
RecommendationReady to merge. The implementation is complete, type-safe, well-tested, and follows monorepo best practices. Reviewed by ling-2.6-1t-20260423:free · 632,024 tokens |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+64
to
+95
| export type { | ||
| ConnectionParams, | ||
| AtcFinding, | ||
| AtcStepResult, | ||
| CodeReviewMode, | ||
| CodeReviewReport, | ||
| McpToolCaller, | ||
| } from './types.js'; | ||
|
|
||
| // Workflow | ||
| export { | ||
| createCodeReviewWorkflow, | ||
| codeReviewInputSchema, | ||
| codeReviewOutputSchema, | ||
| } from './workflow.js'; | ||
| export type { | ||
| CodeReviewInput, | ||
| CodeReviewWorkflow, | ||
| CodeReviewRun, | ||
| CodeReviewRunResult, | ||
| } from './workflow.js'; | ||
|
|
||
| // MCP client factory | ||
| export { createMcpToolCaller, connectMcpClient } from './mcp-client.js'; | ||
|
|
||
| // Agent | ||
| export { createReviewAgent, REVIEW_AGENT_INSTRUCTIONS } from './agent.js'; | ||
| export type { ReviewAgentConfig } from './agent.js'; | ||
|
|
||
| // Harness | ||
| export { createAbapifyPilot } from './harness.js'; | ||
| export type { AbapifyPilotConfig } from './harness.js'; |
There was a problem hiding this comment.
1. Same-package imports include .js 📘 Rule violation ✧ Quality
Multiple internal imports in @abapify/adt-pilot include file extensions (e.g., ./agent.js, ../src/index.js). This violates the rule requiring extensionless imports for same-package modules and can cause inconsistent import style across the repo.
Agent Prompt
## Issue description
Same-package relative imports include file extensions like `.js` (e.g., `./agent.js`, `../src/index.js`), which violates the repo rule for internal imports.
## Issue Context
This package is TypeScript/ESM, but the compliance requirement still mandates extensionless same-package imports.
## Fix Focus Areas
- packages/adt-pilot/src/index.ts[64-95]
- packages/adt-pilot/src/harness.ts[19-22]
- packages/adt-pilot/tests/workflow.test.ts[23-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+28
to
+37
| "dependencies": { | ||
| "@mastra/core": "^1.28.0", | ||
| "@mastra/mcp": "^1.5.2", | ||
| "@modelcontextprotocol/sdk": "^1.27.0", | ||
| "zod": "^3.24.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@abapify/adt-fixtures": "0.3.6", | ||
| "@abapify/adt-mcp": "0.3.6" | ||
| }, |
There was a problem hiding this comment.
2. Local deps not workspace:* 📘 Rule violation ✧ Quality
packages/adt-pilot/package.json declares local workspace packages with a plain version (0.3.6) instead of a workspace:* protocol. This violates the monorepo dependency versioning requirement and can break workspace resolution consistency.
Agent Prompt
## Issue description
Local monorepo dependencies are declared with plain versions instead of the required `workspace:` protocol.
## Issue Context
`@abapify/adt-fixtures` and `@abapify/adt-mcp` are local packages in this repo; the compliance rule requires `workspace:*` (or `workspace:^`/`workspace:~`) for such dependencies.
## Fix Focus Areas
- packages/adt-pilot/package.json[34-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+10
to
+14
| // @mastra/core has extremely complex generics + Zod schemas cannot be annotated | ||
| // for isolatedDeclarations. DTS generation is disabled to avoid OOM (same | ||
| // pattern as adt-mcp typecheck). TypeScript users can reference the source | ||
| // directly via moduleResolution:bundler. | ||
| dts: false, |
There was a problem hiding this comment.
3. Missing published typings 🐞 Bug ≡ Correctness
@abapify/adt-pilot disables DTS generation and its package.json does not expose any types, so TypeScript consumers will have no typings for the published package.
Agent Prompt
## Issue description
`packages/adt-pilot/tsdown.config.ts` sets `dts: false`, and `packages/adt-pilot/package.json` does not expose any types. As a result, consumers installing `@abapify/adt-pilot` from npm will not get TypeScript typings.
## Issue Context
The comment says consumers can “reference the source directly”, but the package publishes only `dist/` and `README.md`, so `src/` won’t be available to downstream users.
## Fix Focus Areas
- packages/adt-pilot/tsdown.config.ts[10-15]
- packages/adt-pilot/package.json[10-17]
## What to change
- Re-enable DTS generation (preferred) and add a `types` entry (and/or `exports` types condition) pointing to `./dist/index.d.mts`.
- If DTS truly must remain disabled, then publish `src/` and change `exports` accordingly (but note this changes the consumer contract).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…s rule Removes .js extensions from all internal relative imports in adt-pilot src and tests to comply with the mandatory bundler-imports rule (since tsconfig.base.json uses moduleResolution: bundler). Matches the convention used in every other package in the monorepo. Addresses Devin Review findings: - BUG_pr-review-job-f33d242b3be645e096ee9c409f2ad9e4_0001 (src/index.ts) - BUG_pr-review-job-f33d242b3be645e096ee9c409f2ad9e4_0002 (src/workflow.ts) - BUG_pr-review-job-f33d242b3be645e096ee9c409f2ad9e4_0003 (src/mcp-client.ts) - BUG_pr-review-job-f33d242b3be645e096ee9c409f2ad9e4_0004 (src/harness.ts) Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add `@abapify/adt-client` as a devDependency. Tests import `createAdtClient` from it (tests/workflow.test.ts), and the project reference list in tsconfig.lib.json already points at it. Without an explicit declaration the package was only resolvable via bun workspace hoisting, which fails in stricter resolvers. - Drop `@mastra/mcp` from runtime dependencies. No source file under `src/` imports it; all Mastra surfaces in use are from `@mastra/core/*`, and all MCP transport code is built on `@modelcontextprotocol/sdk`. JSDoc comments referencing `@mastra/mcp` describe an optional consumer choice for sourcing `mcpTools` and remain accurate. Addresses Devin Review findings: - BUG_pr-review-job-5092d9235b51447997e11b8d8766108a_0001 (missing @abapify/adt-client devDep) - BUG_pr-review-job-5092d9235b51447997e11b8d8766108a_0002 (phantom @mastra/mcp runtime dep) Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces
@abapify/adt-pilot, a Mastra-powered ABAP code-review package built on@abapify/adt-mcp. Two complementary modes share the same MCP-backed implementation:Workflow mode — deterministic, no LLM
createCodeReviewWorkflow(callTool)returns a Mastra workflow with exactly three steps:list_package_objectsand extracts everyobjects[].urito feedatc_run.cts_get_transportto validate the transport, then uses the transport URI (/sap/bc/adt/cts/transportrequests/{number}) as the ATC target so SAP's ATC server enumerates contained objects (matchesadt check --transport <number>).priority: 'error'findings; the workflow always completes with aCodeReviewReport.{ mode: 'package' | 'transport', ...connection }with full Zod validation.Harness mode — interactive via Mastra Agent
createAbapifyPilot(config)returns a Mastra Harness with areviewmode powered by an Agent wired tolist_package_objects,atc_run, andcts_get_transport. Optional custom instructions and pre-loaded MCP tools.Public API
Narrow handles
CodeReviewWorkflow/CodeReviewRun/CodeReviewRunResultdeliberately erase Mastra's deeply generic types, both to keep the consumer-facing surface readable and to avoidtscOOM during typecheck and DTS bloat during build.Build & test hygiene
typechecktarget overridden inproject.jsontotsc -p tsconfig.lib.json --noEmitso it does not follow project references —adt-mcp's typecheck (MCP SDK + Zod inference) is known to OOM and propagated throughtsc --build.@abapify/adt-mcpmoved from runtimedependenciestodevDependencies(it's only imported in tests; production consumers spawn their own MCP server via stdio/HTTP transport).@modelcontextprotocol/sdkpromoted to a runtime dependency (used bymcp-client.ts).tsconfig.spec.jsonadded so test-only deps live in the spec config.Tests (28 total, all passing)
workflow.test.ts— asynccreateRun()API; package mode (happy path + empty package), transport mode (happy path + URI assertion), single + multi-object error paths, transport-lookup failuretypes.test.ts— Zod schema parse/reject edge cases; credentials generated viarandomBytes; URLs usehttps://to clear SonarCloud hotspotsharness.test.ts(new) — factory smoke tests forcreateAbapifyPilotandcreateReviewAgentmcp-client.test.ts— MCP tool caller unit testsReview & Testing Checklist for Human
bunx nx run-many -t build test lint typecheck -p adt-pilotshould produce 0 failures (28 tests pass locally).createCodeReviewWorkflow(callTool)once withmode: 'package'and once withmode: 'transport', both pointing at a known package / transport, and confirmresult.result.findingsis non-empty when ATC has known issues.createAbapifyPilot({ mcpClientConfig }), send a natural-language"review package ZSOMETHING"message, and confirm the agent callslist_package_objectsthenatc_run.@abapify/adt-mcpbeing adevDependency(not a runtime dep) is acceptable for downstream consumers — production users now bring their own MCP server transport.Notes
WorkflowandSteptypes are intentionally type-erased at the import boundary insrc/workflow.ts(see "Internal type-erasure aliases"). Runtime correctness is enforced by Zod schemas; the public TypeScript API is narrowed viaCodeReviewWorkflow/CodeReviewRuninterfaces. Without this,tscOOMs at >4 GB even with--noEmit, androlldown-plugin-dtsproduces multi-megabyte declarations.bunx nx syncwill keeptsconfig.lib.jsonreferences to../adt-client,../adt-mcp,../adt-fixturesin sync because those packages appear in the project graph (via test imports). They are harmless because the overridden typecheck command does not follow references, andtsdownbuilds withdts: false.node:crypto.randomBytes— no hardcoded passwords remain.Link to Devin session: https://app.devin.ai/sessions/cccf1bebf740444aaf1d5d36b97a9f4d
Requested by: @ThePlenkov