Overview - Adt-cli

adt-cli

7 days30 days

Latest CI Pipeline Executions

Fuzzy

Succeeded
add-adk-unit-tests
b3c0279e review(adk tests): fix toText expectations, isolate registry state Fetch-utils: - toText JSON-stringifies plain objects (source at fetch-utils.ts:26-28), so the two assertions that expected '[object Object]' were wrong and failed CI. Corrected to the actual JSON output. - Added a circular-reference test to cover the JSON.stringify catch branch, which is the only case that actually produces '[object Object]'. Registry: - Added __resetRegistryForTests() so tests get a clean singleton each time; called from beforeEach. Removes order-dependence and prevents bleed-in from module-load-time registerObjectType() calls in src/objects/**. - getRegisteredTypes empty-array test now asserts toEqual([]) instead of Array.isArray (meaningful only after reset is in place). - Extracted MockAdkObject -> AdkObjectConstructor cast into a single `mockCtor` helper; drops 10+ `as any` occurrences. - Renamed kinds_list -> kindsList (camelCase). - Fixed stray space in 'abap/ programs' -> 'abap/programs'. - Clarified isTypeRegistered test name to reflect what it actually asserts (full types are registered when main type is). Addresses findings from CodeRabbit, Copilot, Devin Review, and Qodo on #97.
by Petr Plenkov
Failed
add-adk-unit-tests
80518332 security: resolve adt binary by walking up node_modules/.bin Addresses SonarCloud security hotspot typescript:S4036 (Using shell interpreters when executing OS commands) introduced by the previous `bunx`-based invocation, which resolved the binary via the user's PATH. Add a tiny `findHoistedBin(name, startDir)` helper that walks up from `ctx.cwd` looking for `node_modules/.bin/<name>` (with the `.cmd` suffix on Windows) and returns an absolute, existsSync-verified path. The adt CLI is then launched with that absolute path, so: * PATH contents can no longer influence which binary is executed (resolves the Sonar S4036 hotspot). * bun-workspace monorepos still work — the helper walks up exactly as npm/bunx would, finding the hoisted binary at the workspace root (keeps Devin's original review fix). * Windows `.cmd` shims are still handled via `shell: true` gated to win32 per Node's child_process docs.
by Petr Plenkov
Succeeded
main
9659a89d security: resolve adt binary by walking up node_modules/.bin Addresses SonarCloud security hotspot typescript:S4036 (Using shell interpreters when executing OS commands) introduced by the previous `bunx`-based invocation, which resolved the binary via the user's PATH. Add a tiny `findHoistedBin(name, startDir)` helper that walks up from `ctx.cwd` looking for `node_modules/.bin/<name>` (with the `.cmd` suffix on Windows) and returns an absolute, existsSync-verified path. The adt CLI is then launched with that absolute path, so: * PATH contents can no longer influence which binary is executed (resolves the Sonar S4036 hotspot). * bun-workspace monorepos still work — the helper walks up exactly as npm/bunx would, finding the hoisted binary at the workspace root (keeps Devin's original review fix). * Windows `.cmd` shims are still handled via `shell: true` gated to win32 per Node's child_process docs.
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
cb10d1b5 security: resolve adt binary by walking up node_modules/.bin Addresses SonarCloud security hotspot typescript:S4036 (Using shell interpreters when executing OS commands) introduced by the previous `bunx`-based invocation, which resolved the binary via the user's PATH. Add a tiny `findHoistedBin(name, startDir)` helper that walks up from `ctx.cwd` looking for `node_modules/.bin/<name>` (with the `.cmd` suffix on Windows) and returns an absolute, existsSync-verified path. The adt CLI is then launched with that absolute path, so: * PATH contents can no longer influence which binary is executed (resolves the Sonar S4036 hotspot). * bun-workspace monorepos still work — the helper walks up exactly as npm/bunx would, finding the hoisted binary at the workspace root (keeps Devin's original review fix). * Windows `.cmd` shims are still handled via `shell: true` gated to win32 per Node's child_process docs.
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
ed22d7d5 review: use bunx for adt CLI invocation (monorepo hoisting) Addresses Devin Review finding on PR #114: resolving `node_modules/.bin/adt` relative to `ctx.cwd` fails in bun-workspaces monorepos where binaries are hoisted to the workspace root. Running `adt contracts` from a sub-package then throws ENOENT. Switch to `execFileSync('bunx', ['adt', …])` — `bunx` is the AGENTS.md-sanctioned convention for this repo and walks up the directory tree exactly as the old `npx` call did. `shell: true` stays platform-gated: enabled on Windows (bunx is a `.cmd` shim there), shell-free on POSIX.
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
e1146533 review: normalise resolver paths to POSIX separators Addresses Copilot Reviewer finding on PR #114: `collectSourceFiles()` produced paths joined with the platform separator (`\\` on Windows), but `ResolvedLocation.path` is documented as a git-relative path and consumed by the GitLab JSON + SARIF formatters, both of which expect POSIX (`/`) separators. Add a tiny `toPosix()` helper that only rewrites on platforms where `path.sep !== '/'`, and use it when inserting into the filename → path lookup. Downstream reads via `readFileSync` are unaffected (Node accepts `/` on Windows).
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
dc08479e quality: dedupe abapGit finding resolver (adt-plugin-abapgit → adt-atc) Addresses the SonarCloud duplication gate on PR #114 (new_duplicated_lines_density = 63% > 3% threshold). `packages/adt-plugin-abapgit/src/lib/finding-resolver.ts` had copy-pasted the entire `collectSourceFiles`, `getFileLines`, `parseMethodRanges`, `convertLine` and resolver-factory from `packages/adt-atc/src/resolvers/abapgit.ts` (adt-plugin-abapgit already depends on @abapify/adt-atc for the `FindingResolver` type, so the copy is pure duplication). * Export `createAbapGitResolver` from `@abapify/adt-atc` so it becomes part of the package's public API. * Replace the plugin file with a one-line re-export: `export { createAbapGitResolver as createFindingResolver } from '@abapify/adt-atc'`. Public API of @abapify/adt-plugin-abapgit (`createFindingResolver`) is unchanged. This also implicitly resolves the two MINOR S7772 `node:path` findings and the second CRITICAL S2871 `.sort()` finding that existed in the plugin copy — a single canonical implementation now carries the fixes from the previous commit.
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
d5b85f6a review: harden detectGithubRepo (SCP vs URL, dotted repos, shell-safe owner/repo) Addresses two bot review findings on PR #115: * Devin Review — the SCP-style regex `^[^@\s]+@([^:]+):(.+)$` falsely matched URL-form remotes that contain both `user@` and a `:port` segment, e.g. `ssh://git@github.com:22/abapify/adt-cli.git`. The port was captured as part of the path and then rejected by parseOwnerRepo, silently disabling trusted-publisher auto-detection. The SCP branch is now skipped whenever the input contains `://`, so URL-form remotes fall through to `new URL()`. * CodeRabbit — the previous shape regex `[^/]+/[^/.]+?` rejected valid GitHub repos with dots in the name (e.g. `owner/repo.name`) and accepted shell metacharacters in the owner segment. The value flows unquoted into a shell command at `plugin.ts` line ~126 (`--trust-repo=${trustRepo}`), so a strict allow-list is enforced before return: `^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`. This accepts dotted repos and rejects anything that could be interpreted by a shell.
by Petr Plenkov
Succeeded
main
3f27dad9 review: harden detectGithubRepo (SCP vs URL, dotted repos, shell-safe owner/repo) Addresses two bot review findings on PR #115: * Devin Review — the SCP-style regex `^[^@\s]+@([^:]+):(.+)$` falsely matched URL-form remotes that contain both `user@` and a `:port` segment, e.g. `ssh://git@github.com:22/abapify/adt-cli.git`. The port was captured as part of the path and then rejected by parseOwnerRepo, silently disabling trusted-publisher auto-detection. The SCP branch is now skipped whenever the input contains `://`, so URL-form remotes fall through to `new URL()`. * CodeRabbit — the previous shape regex `[^/]+/[^/.]+?` rejected valid GitHub repos with dots in the name (e.g. `owner/repo.name`) and accepted shell metacharacters in the owner segment. The value flows unquoted into a shell command at `plugin.ts` line ~126 (`--trust-repo=${trustRepo}`), so a strict allow-list is enforced before return: `^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`. This accepts dotted repos and rejects anything that could be interpreted by a shell.
by Petr Plenkov
Succeeded
fix/main-health
d414b35f review: harden detectGithubRepo (SCP vs URL, dotted repos, shell-safe owner/repo) Addresses two bot review findings on PR #115: * Devin Review — the SCP-style regex `^[^@\s]+@([^:]+):(.+)$` falsely matched URL-form remotes that contain both `user@` and a `:port` segment, e.g. `ssh://git@github.com:22/abapify/adt-cli.git`. The port was captured as part of the path and then rejected by parseOwnerRepo, silently disabling trusted-publisher auto-detection. The SCP branch is now skipped whenever the input contains `://`, so URL-form remotes fall through to `new URL()`. * CodeRabbit — the previous shape regex `[^/]+/[^/.]+?` rejected valid GitHub repos with dots in the name (e.g. `owner/repo.name`) and accepted shell metacharacters in the owner segment. The value flows unquoted into a shell command at `plugin.ts` line ~126 (`--trust-repo=${trustRepo}`), so a strict allow-list is enforced before return: `^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`. This accepts dotted repos and rejects anything that could be interpreted by a shell.
by Petr Plenkov
Succeeded
fix/main-health
25e4846e quality: simplify detectGithubRepo to cut regex complexity Split host extraction into two small paths: `new URL()` for proper URL schemes and a trivial SCP-style regex for `user@host:path`. The combined regex in the previous commit had cyclomatic complexity 31 (> 20 allowed by SonarCloud `typescript:S5843`). Behaviour and the `js/incomplete-url-substring-sanitization` fix are preserved — host match is now anchored via `URL.hostname` or an explicit SCP host capture, so substrings like `github.com.attacker.com` still cannot match. Resolves SonarCloud issue typescript:S5843 on fix/main-health.
by Petr Plenkov
Succeeded
fix/main-health
ca1cc788 security: rename report.checks.mfa to mfaPolicy to reflect semantics The field stores the configured npm publish MFA *policy* (e.g. `none`, `automatic`), not a user authentication factor or secret. Logging it in the structured JSON report is safe, but the CodeQL heuristic `js/clear-text-logging` fired on the field name `mfa`. Rename clarifies intent and removes the false-positive signal. Resolves code-scanning alert #42 (js/clear-text-logging).
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
b960756e fix(aclass): qualified method names + keyword method names Responses to the 3 Devin findings that arrived after my previous fix batch. Root cause of both: `parseMethodDecl` / `parseMethodImpl` consumed only a single identifier token as the method name. ## 🔴 Method name = ABAP keyword (e.g. `to`) Both `parseMethodImpl` and `parseMethodDecl` used the strict `nameTok.tokenType.name !== 'Identifier'` check. The word `to` is the `To` keyword (used in `REF TO`, `OF TABLE`, …), so `METHOD to.` — present in `zcl_petstore3.clas.locals_imp.abap:152` — silently dropped the method from the AST. Fix: both sites now use the existing `isNameLike()` type-guard (same approach already applied to attribute / parameter / struct-field names in the previous commit). ## 🟡 Qualified method names (`zif_foo~bar`) truncated `parseMethodImpl` read only `header.tokens[1]`, so `METHOD zif_petstore3~add_pet.` landed with `name: 'zif_petstore3'`. All 19 interface method implementations in the generated `zcl_petstore3.clas.abap` were affected; the body was fine (offset- based slicing) but the AST `MethodImpl.name` was wrong, breaking any consumer that indexes methods by name. Fix: extracted a `readQualifiedName(toks, start)` helper that walks `<name>(~|=>)<name>…` chains (reusing the same pattern `consumeTypeRef` uses for qualified type names), and applied it to both `parseMethodImpl` and `parseMethodDecl` (the latter also handles `METHODS zif_foo~bar REDEFINITION.`). ## Tests - New test: `MethodImpl` of `METHOD zif_petstore3~add_pet.` yields `name === 'zif_petstore3~add_pet'`. - New test: `METHODS zif_foo~bar REDEFINITION.` yields `name === 'zif_foo~bar'` with `isRedefinition === true`. Verified on the petstore3 corpus: the 20 `MethodImpl` nodes in `zcl_petstore3.clas.abap` now show `constructor` + 19 fully-qualified `zif_petstore3~<op>` names. Live TRL (CB9980002179, us10): deploy → activate → 3/3 AUnit pass. Tests: aclass 51/51, openai-codegen 159/159, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
main
7824bbcc fix(aclass): qualified method names + keyword method names Responses to the 3 Devin findings that arrived after my previous fix batch. Root cause of both: `parseMethodDecl` / `parseMethodImpl` consumed only a single identifier token as the method name. ## 🔴 Method name = ABAP keyword (e.g. `to`) Both `parseMethodImpl` and `parseMethodDecl` used the strict `nameTok.tokenType.name !== 'Identifier'` check. The word `to` is the `To` keyword (used in `REF TO`, `OF TABLE`, …), so `METHOD to.` — present in `zcl_petstore3.clas.locals_imp.abap:152` — silently dropped the method from the AST. Fix: both sites now use the existing `isNameLike()` type-guard (same approach already applied to attribute / parameter / struct-field names in the previous commit). ## 🟡 Qualified method names (`zif_foo~bar`) truncated `parseMethodImpl` read only `header.tokens[1]`, so `METHOD zif_petstore3~add_pet.` landed with `name: 'zif_petstore3'`. All 19 interface method implementations in the generated `zcl_petstore3.clas.abap` were affected; the body was fine (offset- based slicing) but the AST `MethodImpl.name` was wrong, breaking any consumer that indexes methods by name. Fix: extracted a `readQualifiedName(toks, start)` helper that walks `<name>(~|=>)<name>…` chains (reusing the same pattern `consumeTypeRef` uses for qualified type names), and applied it to both `parseMethodImpl` and `parseMethodDecl` (the latter also handles `METHODS zif_foo~bar REDEFINITION.`). ## Tests - New test: `MethodImpl` of `METHOD zif_petstore3~add_pet.` yields `name === 'zif_petstore3~add_pet'`. - New test: `METHODS zif_foo~bar REDEFINITION.` yields `name === 'zif_foo~bar'` with `isRedefinition === true`. Verified on the petstore3 corpus: the 20 `MethodImpl` nodes in `zcl_petstore3.clas.abap` now show `constructor` + 19 fully-qualified `zif_petstore3~<op>` names. Live TRL (CB9980002179, us10): deploy → activate → 3/3 AUnit pass. Tests: aclass 51/51, openai-codegen 159/159, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
feat/aclass-parser
66a8c9db fix(aclass): qualified method names + keyword method names Responses to the 3 Devin findings that arrived after my previous fix batch. Root cause of both: `parseMethodDecl` / `parseMethodImpl` consumed only a single identifier token as the method name. ## 🔴 Method name = ABAP keyword (e.g. `to`) Both `parseMethodImpl` and `parseMethodDecl` used the strict `nameTok.tokenType.name !== 'Identifier'` check. The word `to` is the `To` keyword (used in `REF TO`, `OF TABLE`, …), so `METHOD to.` — present in `zcl_petstore3.clas.locals_imp.abap:152` — silently dropped the method from the AST. Fix: both sites now use the existing `isNameLike()` type-guard (same approach already applied to attribute / parameter / struct-field names in the previous commit). ## 🟡 Qualified method names (`zif_foo~bar`) truncated `parseMethodImpl` read only `header.tokens[1]`, so `METHOD zif_petstore3~add_pet.` landed with `name: 'zif_petstore3'`. All 19 interface method implementations in the generated `zcl_petstore3.clas.abap` were affected; the body was fine (offset- based slicing) but the AST `MethodImpl.name` was wrong, breaking any consumer that indexes methods by name. Fix: extracted a `readQualifiedName(toks, start)` helper that walks `<name>(~|=>)<name>…` chains (reusing the same pattern `consumeTypeRef` uses for qualified type names), and applied it to both `parseMethodImpl` and `parseMethodDecl` (the latter also handles `METHODS zif_foo~bar REDEFINITION.`). ## Tests - New test: `MethodImpl` of `METHOD zif_petstore3~add_pet.` yields `name === 'zif_petstore3~add_pet'`. - New test: `METHODS zif_foo~bar REDEFINITION.` yields `name === 'zif_foo~bar'` with `isRedefinition === true`. Verified on the petstore3 corpus: the 20 `MethodImpl` nodes in `zcl_petstore3.clas.abap` now show `constructor` + 19 fully-qualified `zif_petstore3~<op>` names. Live TRL (CB9980002179, us10): deploy → activate → 3/3 AUnit pass. Tests: aclass 51/51, openai-codegen 159/159, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
feat/aclass-parser
afea756f fix(aclass): qualified method names + keyword method names Responses to the 3 Devin findings that arrived after my previous fix batch. Root cause of both: `parseMethodDecl` / `parseMethodImpl` consumed only a single identifier token as the method name. ## 🔴 Method name = ABAP keyword (e.g. `to`) Both `parseMethodImpl` and `parseMethodDecl` used the strict `nameTok.tokenType.name !== 'Identifier'` check. The word `to` is the `To` keyword (used in `REF TO`, `OF TABLE`, …), so `METHOD to.` — present in `zcl_petstore3.clas.locals_imp.abap:152` — silently dropped the method from the AST. Fix: both sites now use the existing `isNameLike()` type-guard (same approach already applied to attribute / parameter / struct-field names in the previous commit). ## 🟡 Qualified method names (`zif_foo~bar`) truncated `parseMethodImpl` read only `header.tokens[1]`, so `METHOD zif_petstore3~add_pet.` landed with `name: 'zif_petstore3'`. All 19 interface method implementations in the generated `zcl_petstore3.clas.abap` were affected; the body was fine (offset- based slicing) but the AST `MethodImpl.name` was wrong, breaking any consumer that indexes methods by name. Fix: extracted a `readQualifiedName(toks, start)` helper that walks `<name>(~|=>)<name>…` chains (reusing the same pattern `consumeTypeRef` uses for qualified type names), and applied it to both `parseMethodImpl` and `parseMethodDecl` (the latter also handles `METHODS zif_foo~bar REDEFINITION.`). ## Tests - New test: `MethodImpl` of `METHOD zif_petstore3~add_pet.` yields `name === 'zif_petstore3~add_pet'`. - New test: `METHODS zif_foo~bar REDEFINITION.` yields `name === 'zif_foo~bar'` with `isRedefinition === true`. Verified on the petstore3 corpus: the 20 `MethodImpl` nodes in `zcl_petstore3.clas.abap` now show `constructor` + 19 fully-qualified `zif_petstore3~<op>` names. Live TRL (CB9980002179, us10): deploy → activate → 3/3 AUnit pass. Tests: aclass 51/51, openai-codegen 159/159, abap-ast 115/115. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
feat/aclass-parser
dfdfd27a feat(aclass): typed EventDecl + forward-class decl + abaplint gate Addresses the 5 CodeRabbit review comments on PR #111 that arrived after the initial fix batch. ## Typed `EventDecl` node (`ast.ts:102` thread) `EVENTS name [EXPORTING VALUE(p) TYPE t].` and `CLASS-EVENTS …` now parse into a proper `EventDecl` AST node instead of falling through to `RawMember`. New interface wired into the `ClassMember` union. `parser.ts` gains a `parseEventDecl()` that reuses `consumeMethodParam()` for exporting parameters so the shape mirrors `MethodDecl.exporting`. ## Stricter fixture gate (`fixtures.test.ts:37` thread) The fixture test previously filtered to lex-only errors. Tightened to `expect(errors).toEqual([])` — the fixture contract is "these generated files parse cleanly", so any new diagnostic (missing ENDCLASS, unknown shape, …) flips the gate red. ## Test-data refresh (`parse-interface.test.ts:211` thread) `EVENTS` used to be the example of an unknown member. Now that it's typed, the old test was locking in stale behaviour. Split into: - a positive `EventDecl` test (name + isClassEvent + exporting param). - a genuinely-unknown `WILDCARD something_unrecognised_by_mvp.` test that still validates the `RawMember` fallback path. ## Diagnostic for unterminated classes (`parse-interface.test.ts:237` thread) Previously `parse('CLASS x DEFINITION.\n PUBLIC SECTION.\n')` silently returned an empty-errors AST. That's a real information leak. `parseClassDef` / `parseClassImpl` now emit an "unterminated CLASS …" diagnostic when `ENDCLASS.` never arrives, and the test requires at least one error with `severity === 'error'`. ## Forward class declarations (tripping the stricter gate) The stricter fixture gate immediately caught that `zcl_petstore3.clas.locals_def.abap` contains CLASS lcl_http DEFINITION DEFERRED. — a valid ABAP forward declaration with no body and no ENDCLASS. `parseClassDef` now short-circuits when it sees `DEFERRED` or `LOAD` in the modifier list, producing an empty-sections `ClassDef` and requiring no ENDCLASS. Same file also uses method names like `METHODS to IMPORTING target TYPE REF TO data.` — `to` tokenises as the `To` keyword (part of `REF TO`), so `isNameLike()` is now applied to method-name positions in both `parseMethodDecl` and `parseMethodImpl` too. ## abaplint second opinion (`aclass-parse-gate.test.ts:44` thread) Parse-gate now runs every generator output through BOTH `aclass` AND `@abaplint/core`. We gate only on `parser_error` keys — abaplint's default rule set includes stylistic rules (`description_empty`, `in_statement_indentation`, `global_class` filename check) that aren't relevant to structural parseability. ## Tests - `aclass`: 49/49 (5 suites — added EventDecl tests, unterminated-class diagnostic test, real-unknown RawMember test) - `openai-codegen`: 159/159 (15 suites — +6 abaplint-gate assertions on top of the existing 5 aclass-gate assertions) - `abap-ast`: 115/115 (no regressions) - typecheck ✅ / lint ✅ Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
feat/aclass-parser
bd1e7fdf refactor(aclass): extract parseNameTypeStatement helper to fix SonarCloud duplication gate SonarCloud quality gate flagged `new_duplicated_lines_density` at 3.47% (threshold 3.0%). The only duplicated-block in the aclass parser was the shared `<head> <name> TYPE <typeref>` preamble in `parseAttributeDecl` (DATA / CLASS-DATA) and the simple-form branch of `parseTypeDecl` (TYPES `<x>` TYPE `<y>`). Extracted `parseNameTypeStatement(c, { name, type })` that collects the statement, validates the name + TYPE keyword, consumes the type ref, and returns the shared bag `{ nameTok, typeRes, toks, stmt, tailIdx }` — `tailIdx` points at the token after the type ref so callers can still inspect trailing modifiers like `READ-ONLY`. No behaviour change — both `parseAttributeDecl` and the simple `parseTypeDecl` path produce identical AST shapes as before. 48/48 aclass tests still pass; 153/153 openai-codegen tests still pass. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
feat/aclass-parser
fdba5474 fix(aclass+openai-codegen): address PR #111 review findings Responses to the inline review comments on PR #111 + the two findings my own /github-pr-review surfaced that bots missed. ## Parser correctness ### `consumeTypeRef` + `parseFieldRun` + `consumeMethodParam`: accept keyword tokens as identifier-shaped names (Devin 🔴, two threads) ABAP grammar lets many reserved words serve as identifiers in declaration positions — `DATA data TYPE i.`, `REF TO data`, struct fields named `type` / `data`, parameters named `value`. The lexer classifies those as keyword tokens; the parser now reinterprets any token whose `image` is identifier-shaped (`/^[A-Za-z_][A-Za-z0-9_/]*$/`) as a name via a new `isNameLike()` type-guard. Applied to: - type-reference head (`REF TO data`, `TYPE <keyword-named-type>`) - qualified type-reference parts (`zif_y=>data`) - structure-field names in `TYPES: BEGIN OF / END OF` - method-parameter names (plain and `VALUE(…)` forms) New tests (`tests/parse-interface.test.ts`): parameter named `data`; `REF TO data`; struct fields named `type`/`data`/`value`; qualified type ref with keyword tail. ### `MethodImpl.bodySpan.startLine/startColumn` (Copilot 🟠) Previously pointed at the `METHOD` keyword. Now resolves the first token at/after `bodyStart` via new `Cursor.firstTokenAtOrAfter()` so the span refers to the body content. New tests cover a normal body and the empty-body edge case. ### `parse()` top-level try/catch (my review) Documented contract: "never throws for malformed input". `Cursor .current()` still throws past-EOF. Wrapped the parse loop in a try/catch that turns any unexpected exception into a normalised `ParseError` so the contract holds even if a future refactor forgets an `eof()` guard. ### Remove `void T;` marker (Copilot 🟡) Dropped the unused `import * as T from './tokens'` and the corresponding `void T;` silencer. It added noise and would mask real unused-import issues in future edits. ## Reusable consumer gate New `@abapify/aclass/assert.ts`: - `assertCleanParse(source, fileLabel?)` — throws `AclassParseError` with file:line messages if lex or parse errors exist. - `AclassParseError` carries `{ source, errors }` for tools that want to inspect the diagnostics programmatically. Refactored `packages/openai-codegen/tests/aclass-parse-gate.test.ts` to consume the helper instead of inlining filter logic. Future packages that need the same invariant (e.g. `adt-plugin-abapgit`) can reach for the same helper without reimplementing it. ## Docs / spec alignment ### `openspec/changes/add-aclass-parser` - **`proposal.md`** — describe the parser as a statement splitter (which is what shipped) instead of the Chevrotain `CstParser` form the original proposal described. Rename `cdsClassDef` / `cdsClassImplementation` / `cdsInterfaceDef` to `classDef` / `classImpl` / `interfaceDef` (amazon-q). - **`specs/aclass/spec.md`** — fix the run-on `SourceSpan` docstring (amazon-q). Move `errors: ParseError[]` out of `AbapSourceFile` and into a new `ParseResult` block — the impl always returned `{ ast, errors }` and the spec was the only place they ever lived together (Copilot). - **`tasks.md`** — flip every item to `[x]`; Waves 0–3 are all shipped in this PR. Fix the reversed INTERFACE/INTERFACES keyword ordering note so it matches both the lexer source and `packages/aclass/AGENTS.md` (Copilot). ### `packages/aclass/AGENTS.md` - **"Current status"** — rewrite from "Wave 0 only" to "Waves 0–3 shipped" with a brief per-wave breakdown (Copilot). - **"Architecture"** — replace the `Chevrotain CstParser + Visitor` diagram with the actual statement-splitter pipeline and a short rationale for the choice. - **Key-files table** — list the real source files (`src/parser.ts`, `src/assert.ts`) instead of phantom `src/visitor.ts` / CstParser references. - **Anti-patterns table** — drop the "no dynamic RegExp per keyword" row (Copilot). The `kw()` helper intentionally uses `new RegExp(word, 'i')` with `REGEX_META` escaping; the guidance was contradicting the shipped code. ### PR body Rewritten to reflect what actually landed (Waves 0–3 + live TRL verification). Previously described as "docs-only" which was false from `fc76f504` onward (Copilot). ## Tests & live verification - `aclass`: 48/48 (5 suites) - `openai-codegen`: 153/153 (15 suites) - `abap-ast`: 115/115 (no regressions) - Typecheck ✅ / Lint ✅ / Build ✅ - Live TRL (CB9980002179, us10): deploy → activation pre-audit clean → `adt aunit -c ZCL_PS3_CLIENT_TESTS` → 3/3 pass. ## Intentionally deferred (tracked in `openspec/changes/add-aclass-parser/tasks.md`) - **Fixture corpus location** (my review, low) — stays as live generator output; documented as deliberate coupling. - **`bun.lock` in diff** — expected side-effect of adding the `@abapify/aclass` workspace dev-dep to `@abapify/openai-codegen`. - **Byte-exact roundtrip printer** — spec task; current structural roundtrip is sufficient for the parse-gate invariant. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov
Succeeded
copilot/fix-unix-specific-find-command
eed8fb35 ci: split Nx Cloud token into RO/RW repo secrets, drop environments Using GitHub Environments to scope the read-write Nx Cloud token to `main` pushes caused every workflow run to be registered as a "deployment" in the GitHub UI, which was misleading — these workflows don't deploy anything. Switch to two plain repository secrets: * `NX_CLOUD_ACCESS_TOKEN_RO` — read-only token, safe to expose on PRs and feature branches. * `NX_CLOUD_ACCESS_TOKEN_RW` — read-write token, injected only when we actually need to write to the remote cache. In `ci.yml` the token is selected via a gated expression so PRs get RO and pushes to `main` get RW. `release.yml`, `publish.yml` and `publish-gpr.yml` always run against `main`/tags, so they hardcode RW. The `protected-branch-pipeline` and `unprotected-branch-pipeline` environments have been deleted along with this change. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
by Petr Plenkov