Skip to content

Conversation

@ThePlenkov
Copy link
Member

@ThePlenkov ThePlenkov commented Dec 15, 2025

User description

Overview

Implements ATC (ABAP Test Cockpit) checks integration for the CI/CD pipeline.

Context

This is the next step after the abapgit import milestone. ATC checks will enable:

  • Static code analysis for ABAP objects
  • Quality gates in CI pipeline
  • Integration with SAP's built-in code inspection tools

Tasks

  • Implement ATC check service in adt-client
  • Add ATC CLI commands
  • Create ATC plugin for abapify
  • Integration tests

Related

Part of the ABAP Code Review CI/CD pipeline implementation.


PR Type

Enhancement, Tests, Bug fix


Description

  • ATC (ABAP Test Cockpit) integration: Implements ATC checks service with command plugin for running static code analysis, creating worklists, and outputting results in multiple formats (console, JSON, GitLab, SARIF)

  • Generic save/load/lock operations: Adds base model methods for CRUD operations using contracts, lock/unlock lifecycle, and source code management with mode switching (create/update/upsert)

  • CRUD contract helper: Provides reusable helper for generating full CRUD contracts for ADT repository objects with support for lock/unlock, source code, and includes endpoints

  • Contract generation from discovery: Implements discovery parser and contract generator plugin to create type-safe ADT contracts from SAP discovery data with whitelist-based filtering

  • Plugin-based CLI architecture: Refactors CLI to dynamically load commands from configuration files, enabling extensibility without hardcoded command dependencies

  • abapGit deserialization: Implements bidirectional serialization for ABAP objects (CLAS, INTF, DEVC) with file-to-ADK object conversion and metadata mapping

  • XML parsing improvements: Enhances XML builder with namespace prefix resolution, attribute reference handling, and wrapped element format support for type discrimination

  • Object set bulk operations: Adds semantic layer for managing collections of ADK objects with bulk save, activate, deploy, and lock operations

  • Export/deploy command: Implements file deployment plugin supporting multiple format plugins (abapgit, oat) with dry-run mode and progress reporting

  • Error handling: Adds structured ADT error handling with exception parsing from SAP responses

  • Transport service: Implements CTS transport operations service layer for listing, getting, and deleting transports

  • Test framework enhancements: Adds typed contract scenario framework, mock HTTP adapter, and comprehensive test coverage for XML parsing, schema validation, and contract operations

  • Bug fixes: Fixes browser auth cookie clearing, transport response unwrapping, and property access issues


Diagram Walkthrough

flowchart LR
  Discovery["SAP ADT Discovery"]
  Parser["Discovery Parser"]
  ContractGen["Contract Generator"]
  Contracts["Generated Contracts"]
  
  AbapGit["abapGit Files"]
  Deserializer["Deserializer"]
  AdkObjects["ADK Objects"]
  
  Config["adt.config.ts"]
  PluginLoader["Plugin Loader"]
  Commands["CLI Commands"]
  
  BaseModel["Base Model"]
  CrudHelper["CRUD Helper"]
  SaveLoad["Save/Load/Lock Ops"]
  
  Discovery --> Parser
  Parser --> ContractGen
  ContractGen --> Contracts
  
  AbapGit --> Deserializer
  Deserializer --> AdkObjects
  
  Config --> PluginLoader
  PluginLoader --> Commands
  
  CrudHelper --> BaseModel
  BaseModel --> SaveLoad
  
  Contracts --> SaveLoad
  AdkObjects --> SaveLoad
  Commands --> SaveLoad
Loading

File Walkthrough

Relevant files
Enhancement
29 files
generate-contracts.ts
Contract generator plugin for ADT discovery data                 

packages/adt-codegen/src/plugins/generate-contracts.ts

  • New contract generator plugin that creates type-safe ADT contracts
    from discovery data
  • Supports whitelist-based generation with content-type to schema
    mapping
  • Generates CRUD contracts using helper and standard contracts with
    individual methods
  • Produces index file and documentation of available/enabled endpoints
+865/-0 
model.ts
Generic save/load/lock operations with CRUD contracts       

packages/adk/src/base/model.ts

  • Added SaveMode, SaveOptions, and ActivationResult types for save
    operations
  • Implemented generic load() method using CRUD contracts and wrapper
    keys
  • Added collectionUri property for creating new objects
  • Implemented save() method with lock/unlock lifecycle and mode
    switching (create/update/upsert)
  • Added lock() and unlock() methods using CRUD contracts with lock
    response parsing
  • Added setSource() and activate() methods for source code and
    activation operations
+364/-23
crud.ts
CRUD contract helper for ADT repository objects                   

packages/adt-contracts/src/helpers/crud.ts

  • New CRUD helper that generates full CRUD contracts for ADT repository
    objects
  • Supports standard CRUD operations (GET, POST, PUT, DELETE) with query
    parameters
  • Includes lock/unlock operations with stateful session handling
  • Supports optional source code endpoints (get/put for source types)
  • Supports optional includes endpoints (get/put for include types like
    definitions, implementations)
  • Provides repo() shorthand alias for common CRUD patterns
+446/-0 
atc.ts
ATC command plugin for ABAP Test Cockpit checks                   

packages/adt-atc/src/commands/atc.ts

  • New ATC command plugin for running ABAP Test Cockpit checks via CLI
  • Supports checking packages, objects, or transport requests
  • Integrates with ATC customizing to determine check variants
  • Creates worklists and runs analysis with configurable result limits
  • Outputs results in multiple formats (console, JSON, GitLab, SARIF)
  • Includes hyperlink support for ADT Eclipse integration
+333/-0 
clas.model.ts
Simplify class model with generic save/load operations     

packages/adk/src/objects/repository/clas/clas.model.ts

  • Simplified class model by removing property getters and implementing
    base class methods
  • Updated load() to use generic base implementation with CRUD contracts
  • Added saveMainSource() and saveIncludeSource() methods for source code
    persistence
  • Implemented savePendingSources() override for abapGit deserialization
    workflow
  • Added CRUD contract configuration via wrapperKey and crudContract
    properties
  • Removed explicit interface implementation in favor of data access via
    data property
+122/-114
intf.model.ts
Simplify interface model with generic save/load operations

packages/adk/src/objects/repository/intf/intf.model.ts

  • Simplified interface model by removing property getters
  • Updated load() to use generic base implementation with CRUD contracts
  • Added saveMainSource() method for source code persistence
  • Implemented savePendingSources() override for abapGit deserialization
    workflow
  • Added CRUD contract configuration via wrapperKey and crudContract
    properties
  • Removed explicit interface implementation in favor of data access via
    data property
+58/-34 
object-set.ts
ADK Object Set bulk operations service implementation       

packages/adk/src/base/object-set.ts

  • New AdkObjectSet class providing semantic layer for bulk operations on
    collections of ADK objects
  • Implements collection management methods (add, addAll, remove, clear,
    getAll, filterByType, filterByKind)
  • Provides bulk operations: saveAll(), activateAll(), deploy(),
    lockAll(), unlockAll(), loadAll()
  • Includes static factory methods from() and fromGenerator() for
    creating object sets from various sources
+406/-0 
export.ts
Export command plugin for file deployment to SAP                 

packages/adt-export/src/commands/export.ts

  • New export command plugin for deploying serialized files to SAP
    systems
  • Supports format plugins (abapgit, oat) with dynamic loading and
    shortcut resolution
  • Integrates with AdkObjectSet for bulk save and activation operations
  • Provides dry-run mode, object type filtering, and progress reporting
    with detailed result display
+280/-0 
build.ts
XML builder namespace and form attribute handling improvements

packages/ts-xsd/src/xml/build.ts

  • Added getPrefixForSchema() helper to resolve namespace prefixes for
    schema targetNamespaces
  • Enhanced element resolution to support wrapped data format from
    parse() with automatic unwrapping
  • Improved attribute namespace handling with per-attribute form
    attribute support and schema-aware prefix resolution
  • Fixed root element to never be self-closing (adds empty text node for
    SAP ADT compatibility)
  • Enhanced buildFieldWithTagName() to handle element-level form
    overrides and qualified/unqualified attribute forms
+94/-14 
classes.ts
Classes contract refactored to use crud() helper                 

packages/adt-contracts/src/adt/oo/classes.ts

  • Refactored classes contract from verbose manual definitions to concise
    crud() helper usage
  • Expanded ClassIncludeType to include testclasses and localtypes in
    addition to existing types
  • Updated documentation to reference crud() helper and list all
    supported operations (CRUD, lock/unlock, source, includes)
  • Reduced contract definition from 183 lines to 13 lines while
    maintaining full functionality
+19/-181
deserializer.ts
abapGit deserializer for file-to-ADK object conversion     

packages/adt-plugin-abapgit/src/lib/deserializer.ts

  • New deserializer module for converting abapGit format files to ADK
    objects using async generator pattern
  • Implements parseAbapGitFilename() to extract object metadata from file
    naming conventions
  • Provides deserialize() generator that yields AdkObject instances with
    pre-loaded data and sources
  • Handles XML parsing, source file mapping, and handler-based payload
    transformation
+191/-0 
base.ts
Handler base types extended for bidirectional serialization

packages/adt-plugin-abapgit/src/lib/handlers/base.ts

  • Added InferAdkData type helper to extract data type from AdkObject
    D> generics
  • Enhanced ObjectHandler interface with schema, suffixToSourceKey,
    fromAbapGit(), and setSources() members
  • Added ObjectPayload interface for deserialization context with name,
    description, sources, and additional data
  • Extended HandlerDefinition interface with fromAbapGit(),
    suffixToSourceKey, and setSources() for bidirectional serialization
  • Updated createHandler() to populate handler with schema and
    deserialization methods
+102/-7 
endpoint-config.ts
Endpoint configuration API for contract generation             

packages/adt-codegen/src/plugins/endpoint-config.ts

  • New endpoint configuration API for type-safe contract generation with
    defineEndpoint() and defineEndpoints() helpers
  • Supports simple patterns (string glob, regex) and advanced config
    objects with method filtering, schema override, and CRUD generation
  • Provides utility functions: isEndpointConfig(), normalizeEndpoint(),
    matchesPattern(), findMatchingEndpoint(), isPathEnabled(),
    isMethodEnabled()
  • Enables flexible endpoint discovery and filtering for code generation
+224/-0 
typed-scenario.ts
Typed contract scenario framework for type-safe testing   

packages/adt-contracts/tests/contracts/base/typed-scenario.ts

  • New typed contract scenario framework with generic type parameter for
    full type safety
  • Provides TypedContractScenario abstract base class with typed
    assertRequest() and assertResponse() methods
  • Includes type extractors: ExtractDescriptor, ExtractRequest,
    ExtractResponse, ExtractBody for inferring types from contract
    operations
  • Implements runTypedScenario() function with mock adapter integration
    for fully-typed client call testing
+195/-0 
clas.ts
Class handler bidirectional serialization with metadata mapping

packages/adt-plugin-abapgit/src/lib/handlers/objects/clas.ts

  • Added SUFFIX_TO_SOURCE_KEY reverse mapping for deserialization (Git →
    SAP)
  • Added VISIBILITY_TO_EXPOSURE and EXPOSURE_TO_VISIBILITY mappings for
    visibility enum conversion
  • Enhanced toAbapGit() to map comprehensive class metadata (category,
    visibility, final, abstract, references, language version)
  • Implemented fromAbapGit() for deserializing abapGit values to ADK
    class data with proper enum conversions
  • Implemented setSources() to store source files as pending during
    deserialization
+106/-12
cli.ts
CLI refactored to use plugin-based command loading             

packages/adt-cli/src/lib/cli.ts

  • Removed hardcoded ATC and export/deploy commands from main CLI
  • Added plugin loader integration via loadCommandPlugins() to
    dynamically load commands from adt.config.ts
  • Updated documentation comments to direct users to enable commands via
    config file
  • Removed imports for atcCommand, exportPackageCommand, and
    deployCommand
+13/-13 
auth-manager.ts
Auth manager plugin options extended with URL and client 

packages/adt-auth/src/auth-manager.ts

  • Added url and client properties to pluginOptions in session auth
    configuration
  • Enables plugins to access host URL and client code from authentication
    context
+2/-0     
transports.ts
Transport service layer for CTS operations                             

packages/adt-client/src/services/transports.ts

  • Implements TransportService class for orchestrating CTS transport
    operations
  • Provides methods for listing, getting, deleting transports with data
    transformation
  • Includes helper methods for extracting and mapping transport request
    data
  • Adds placeholder methods for create and release operations (not yet
    implemented)
+191/-0 
index.ts
ATC contract with manual and generated endpoints                 

packages/adt-contracts/src/adt/atc/index.ts

  • Combines generated contracts with manually-defined ATC endpoints not
    in discovery
  • Adds customizing contract for GET ATC customizing settings
  • Extends runs contract with POST endpoint for executing ATC checks
  • Adds worklistsExtended contract with POST create endpoint for new
    worklists
+63/-50 
plugin-loader.ts
CLI plugin loader for dynamic command registration             

packages/adt-cli/src/lib/plugin-loader.ts

  • Implements dynamic CLI command plugin loading from configuration files
  • Converts CLI-agnostic plugin definitions to Commander commands
  • Provides loadCliConfig() to find and load adt.config.ts from project
    hierarchy
  • Includes pluginToCommand() for translating plugin options/arguments to
    Commander format
+176/-0 
traces.types.ts
Extended traces schema types with new variants                     

packages/adt-schemas/src/schemas/generated/types/sap/traces.types.ts

  • Extends TracesSchema type with additional union variants for
    activation, traces, trace, and records
  • Adds comprehensive type definitions for trace activation, trace
    records, and trace properties
  • Includes nested structures for trace components, properties, and
    metadata
+196/-0 
cli-types.ts
CLI plugin type definitions and interfaces                             

packages/adt-plugin/src/cli-types.ts

  • Defines CLI-agnostic plugin interfaces: CliCommandPlugin, CliOption,
    CliArgument
  • Provides CliContext with logger, config, and ADT client factory
  • Includes AdtCliConfig for command plugin configuration
  • Enables plugins to work without depending on specific CLI frameworks
+198/-0 
contracts.ts
Contracts command plugin for discovery-based generation   

packages/adt-codegen/src/commands/contracts.ts

  • Implements contractsCommand plugin for generating type-safe contracts
    from ADT discovery
  • Handles discovery fetching from SAP with local caching for offline use
  • Validates configuration and provides helpful error messages for
    missing settings
  • Supports CLI options for discovery path, output directory, and force
    fetch
+142/-0 
index.ts
Contract testing framework using speci types                         

packages/adt-contracts/tests/contracts/base/index.ts

  • Refactors contract testing framework to use speci's
    RestEndpointDescriptor
  • Removes duplicate ContractDescriptor type in favor of speci types
  • Improves variable capture in test callbacks to avoid non-null
    assertions
  • Re-exports createClient and mock adapter utilities for contract tests
+45/-43 
index.ts
Attribute reference resolution in schema walker                   

packages/ts-xsd/src/walker/index.ts

  • Adds schema field to AttributeEntry for namespace prefix resolution
  • Implements resolveAttribute() function to handle attribute references
  • Implements findAttribute() function to search attributes across schema
    hierarchy
  • Updates attribute walking to resolve refs and include schema context
+86/-10 
interfaces.ts
Interfaces contract using crud helper                                       

packages/adt-contracts/src/adt/oo/interfaces.ts

  • Refactors interfaces contract to use crud() helper with sources option
  • Simplifies contract definition by removing manual CRUD operation
    definitions
  • Includes source code management (main) through sources configuration
  • Reduces code duplication with helper-based approach
+15/-85 
discovery-parser.ts
Discovery XML parser for contract generation                         

packages/adt-codegen/src/plugins/discovery-parser.ts

  • Parses SAP ADT discovery XML into structured DiscoveryData format
  • Extracts workspaces, collections, and template links from discovery
  • Provides getAllCollections() helper to flatten collections across
    workspaces
  • Uses fast-xml-parser for robust XML parsing with namespace handling
+111/-0 
filetree.ts
FileTree implementations for file operations                         

packages/adt-export/src/utils/filetree.ts

  • Implements FsFileTree for file system backed file operations
  • Implements MemoryFileTree for in-memory testing
  • Provides glob pattern matching, file reading, and directory listing
  • Includes UTF-8 BOM stripping for text file reading
+114/-0 
packages.ts
Generated packages contract with multiple endpoints           

packages/adt-contracts/src/generated/adt/sap/bc/adt/packages.ts

  • Generated contract for package operations at /sap/bc/adt/packages
  • Includes methods for properties, tree, useaccesses, and value helps
  • Supports query parameters for correction number, lock handle, and
    version
  • Uses packagesV1 schema for response typing
+82/-0   
Tests
17 files
xml-parse.test.ts
Update XML parse tests for wrapped element format               

packages/ts-xsd/tests/unit/xml-parse.test.ts

  • Updated test assertions to match new XML parsing behavior that wraps
    results in element name
  • Changed expected results from flat structure to nested structure with
    root element wrapper
  • Updated 40+ test cases to reflect the wrapped format (e.g., { Person:
    { name: 'John' } })
+66/-52 
mock-e2e.test.ts
Update mock plugins to new serialization API                         

packages/adt-cli/src/lib/plugins/mock-e2e.test.ts

  • Updated mock plugins to use new serializeObject method instead of
    deprecated serialize
  • Changed from batch serialization to per-object serialization with
    SerializationContext
  • Removed deserialize method implementations from mock plugins
  • Simplified test setup and removed serialize/deserialize test cases
+54/-231
xml-build.test.ts
XML builder tests for namespaced types and inheritance     

packages/ts-xsd/tests/unit/xml-build.test.ts

  • Added comprehensive test suite for namespaced type references in XML
    building
  • Tests cover nested complex types with namespace prefixes, element refs
    from imported schemas, and deeply nested structures
  • Added tests for root element closing tags to ensure SAP ADT
    compatibility (no self-closing root elements)
  • Added tests for inherited attribute namespace prefix handling in
    derived schemas
+311/-1 
tm.test.ts
Transport management schema tests updated for wrapped parse format

packages/adt-schemas/tests/scenarios/tm.test.ts

  • Updated test assertions to match new wrapped data format from parse()
    (data wrapped with element name)
  • Changed all assertions to access content via root property instead of
    direct access
  • Added override keyword to validateBuilt() methods for consistency
+60/-48 
xml-cross-schema.test.ts
Cross-schema XML tests updated for wrapped parse format   

packages/ts-xsd/tests/unit/xml-cross-schema.test.ts

  • Updated all test assertions to match new wrapped data format from
    parse() (element name as wrapper key)
  • Changed assertions from direct property access to accessing via
    element name wrapper (e.g., result.Object.id instead of result.id)
  • Updated cross-schema parsing tests for element refs, inheritance, and
    deep schema chains
+55/-41 
atc.test.ts
ATC contract tests refactored to use generated contracts 

packages/adt-contracts/tests/contracts/atc.test.ts

  • Refactored ATC contract tests to use generated contracts from
    src/generated/adt/sap/bc/adt/atc/
  • Replaced manual atcContract definitions with worklistsContract and
    resultsContract imports
  • Updated test scenarios to validate generated contract definitions
    (method, path, headers, query parameters)
  • Simplified test structure and added comments explaining contract
    definition vs client call tests
+56/-60 
walker-attribute-ref.test.ts
Walker attribute reference resolution tests                           

packages/ts-xsd/tests/unit/walker-attribute-ref.test.ts

  • New test suite for walkAttributes() with attribute references (ref)
    support
  • Tests cover attribute ref resolution from same schema and from
    $imports, namespace-prefixed refs, and ref merging with use overrides
  • Tests validate mixed direct attributes and refs, refs in
    complexContent/extension, and fallback behavior for missing attributes
+187/-0 
atc.test.ts
ATC schema test scenarios for customizing and worklist     

packages/adt-schemas/tests/scenarios/atc.test.ts

  • New ATC schema test scenarios for customizing and worklist responses
  • AtcCustomizingScenario validates properties, exemption reasons, and
    justification requirements
  • AtcWorklistScenario validates object sets, objects with findings, and
    info entries with detailed attribute checks
  • Tests account for wrapped parse format and namespace-prefixed
    attributes
+133/-0 
interface-generator.test.ts
Update interface generator tests for new API                         

packages/ts-xsd/tests/unit/interface-generator.test.ts

  • Removes rootElement parameter from generateInterfaces() calls (now
    optional)
  • Adds type cast as unknown as Schema to test schemas for type
    compatibility
  • Simplifies test setup by removing redundant configuration options
+17/-28 
typed-client.test.ts
Typed contract client tests with schema inference               

packages/adt-contracts/tests/typed-client.test.ts

  • Demonstrates fully-typed contract client with response type inference
    from schemas
  • Tests GET operations for transport requests, packages, and ATC
    worklists
  • Verifies that mock adapter correctly parses XML responses using schema
    definitions
  • Validates compile-time type safety of contract method signatures
+168/-0 
deserializer.test.ts
Deserializer tests for ABAP object fixtures                           

packages/adt-plugin-abapgit/tests/deserializer.test.ts

  • Tests deserialization of ABAP objects (CLAS, INTF, DEVC) from fixture
    files
  • Implements mock FileTree that reads from fixtures directory with glob
    support
  • Verifies correct mapping of ABAP object types to human-readable kinds
  • Tests multiple object deserialization from mixed fixture directories
+142/-0 
type-parse-consistency.test.ts
Type and parse consistency validation tests                           

packages/ts-xsd/tests/unit/type-parse-consistency.test.ts

  • Tests consistency between parse() behavior and generated type
    definitions
  • Verifies that both wrap content with element name for type
    discrimination
  • Tests multi-root schema handling with union types
  • Validates roundtrip consistency (parse → build → parse)
+132/-0 
classes.test.ts
Update class scenario tests for wrapped response format   

packages/adt-schemas/tests/scenarios/classes.test.ts

  • Updates test to handle wrapped parse response format { elementName:
    content }
  • Changes validateParsed() to extract abapClass from wrapped response
  • Updates all property assertions to use unwrapped abapClass object
  • Adds override keyword to validateBuilt() method
+34/-34 
atc-typed.test.ts
Typed ATC worklist contract scenario example                         

packages/adt-contracts/tests/contracts/atc-typed.test.ts

  • Demonstrates fully-typed ATC worklist contract scenario with type
    inference
  • Shows how to use TypedContractScenario with contract and schema types
  • Validates request properties (method, path, query) with full type
    checking
  • Tests response parsing with deep nested property access and type
    narrowing
+120/-0 
xml-roundtrip.test.ts
XML roundtrip tests for wrapped parse format                         

packages/ts-xsd/tests/integration/xml-roundtrip.test.ts

  • Updates roundtrip tests to expect wrapped response format from parse()
  • Changes assertions to account for { elementName: content } wrapper
  • Verifies that buildXml() accepts both wrapped and unwrapped formats
  • Tests multi-roundtrip consistency with wrapped format
+16/-14 
mock-adapter.ts
Mock HTTP adapter for contract testing                                     

packages/adt-contracts/tests/contracts/base/mock-adapter.ts

  • Implements mock HTTP adapter for contract testing without real HTTP
    calls
  • Supports request matching by method and path (string or RegExp)
  • Automatically parses XML responses using schema definitions
  • Provides createMockAdapter() and createSimpleMockAdapter() factories
+124/-0 
packages.test.ts
Update package scenario tests for wrapped response format

packages/adt-schemas/tests/scenarios/packages.test.ts

  • Updates test to handle wrapped parse response format { elementName:
    content }
  • Extracts pkg from wrapped response before assertions
  • Updates all property assertions to use unwrapped pkg object
  • Adds override keyword to validateBuilt() method
+27/-23 
Configuration changes
3 files
abapgit-examples
Add abapgit-examples git submodule                                             

git_modules/abapgit-examples

  • Added git submodule reference to abapgit-examples repository
+1/-0     
content-type-mapping.ts
Content type to schema name mapping configuration               

packages/adt-contracts/config/contracts/content-type-mapping.ts

  • Maps SAP ADT content types to schema names for contract generation
  • Includes comprehensive mappings for ATC, OO, packages, and transport
    endpoints
  • Provides fallback mappings based on URL paths for content type
    resolution
  • Supports multiple content type variants (vnd.sap.atc.*, vnd.sap.adt.*)
+91/-0   
.nxignore
Nx ignore configuration for config files                                 

packages/adt-contracts/.nxignore

  • Ignores config directory and configuration files from Nx dependency
    analysis
  • Prevents Nx from treating config files as source dependencies
+4/-0     
Error handling
2 files
errors.ts
ADT error handling with exception parsing                               

packages/adt-client/src/errors.ts

  • Implements comprehensive ADT error handling with AdtError class for
    structured exception details
  • Adds parseAdtException() function to parse SAP ADT XML exceptions into
    typed AdtExceptionData
  • Provides createAdtError() factory function for creating errors from
    HTTP responses
  • Includes helper methods on AdtError for type checking and property
    access
+194/-0 
adapter.ts
Adapter error handling with ADT exception parsing               

packages/adt-client/src/adapter.ts

  • Integrates createAdtError() for structured error handling with
    exception parsing
  • Improves debug logging with string concatenation instead of template
    literals
  • Adds error handling for schema build() method with detailed error
    logging
  • Logs parsed ADT exception details (namespace, type) when available
+37/-14 
Bug fix
2 files
auth-core.ts
Browser auth with cookie clearing and validation                 

packages/browser-auth/src/auth-core.ts

  • Adds cookie clearing step before authentication to ensure fresh
    session
  • Combines 200 response check with cookie validation in single promise
  • Improves cookie matching logic to check for required patterns
  • Removes unused CookieData type import
+22/-38 
transport.ts
Transport response unwrapping and property fixes                 

packages/adk/src/objects/cts/transport/transport.ts

  • Fixes response unwrapping in load() method to access response.root
  • Updates static get() method to unwrap root element from response
  • Fixes AdkTransportTask.get() to unwrap response before creating
    instance
  • Fixes getConfigUri() to access configurations instead of configuration
+14/-6   
Additional files
101 files
package-versions.md +51/-0   
.gitmodules +3/-0     
.nxignore +5/-3     
adt.config.ts +20/-0   
export-architecture.md +277/-0 
eslint.config.mjs +5/-0     
package.json +4/-2     
package.json +2/-1     
adt.ts +43/-3   
index.ts +1/-1     
registry.ts +3/-2     
factory.ts +26/-0   
index.ts +10/-0   
transport-import.ts +10/-5   
transport.types.ts +7/-2     
devc.model.ts +14/-7   
tsconfig.lib.json +3/-0     
README.md +108/-0 
package.json +31/-0   
project.json +8/-0     
gitlab.ts +14/-6   
index.ts +6/-0     
sarif.ts +15/-7   
index.ts +19/-0   
types.ts +38/-0   
tsconfig.json +25/-0   
tsdown.config.ts +17/-0   
package.json +3/-0     
atc.ts +0/-145 
logout.ts +15/-7   
status.ts +21/-20 
search.ts +1/-1     
list.ts +2/-0     
adk-loader.ts +0/-191 
deploy.ts +0/-266 
index.ts +0/-1     
discovery.ts +6/-1     
package.ts +0/-118 
get.ts +8/-3     
index.ts +4/-2     
info.ts +19/-18 
outline.ts +4/-3     
search.ts +5/-5     
TreeConfigEditor.tsx +1/-1     
auth.ts +6/-4     
registry.ts +3/-3     
service.ts +0/-54   
service.ts +0/-247 
clients.ts +0/-72   
class.ts +2/-1     
discovery.ts +7/-5     
interface.ts +2/-1     
package.ts +8/-5     
logger-config.ts +1/-1     
package-hierarchy.ts +0/-127 
tsconfig.lib.json +12/-0   
package.json +2/-1     
client.ts +20/-0   
index.ts +24/-0   
index.ts +20/-0   
types.ts +3/-6     
package.json +3/-1     
codegen.ts +49/-0   
framework.ts +1/-1     
index.ts +19/-0   
tsconfig.json +9/-1     
tsdown.config.ts +1/-1     
config-loader.ts +7/-1     
index.ts +10/-1   
types.ts +53/-0   
AGENTS.md +60/-1   
adt.config.ts +54/-0   
enabled-endpoints.ts +41/-0   
adt-endpoints.md +1525/-0
package.json +2/-0     
project.json +71/-1   
generate-schemas.ts +63/-0   
base.ts +25/-0   
index.ts +30/-0   
results.ts +70/-0   
worklists.ts +34/-0   
transportrequests.ts +61/-0   
reference.ts +24/-0   
configurations.ts +24/-0   
metadata.ts +24/-0   
settings.ts +24/-0   
validation.ts +24/-0   
schemas.ts +52/-0   
speci-schema.ts +43/-0   
schemas.ts +6/-12   
cts.test.ts +4/-0     
discovery.test.ts +50/-2   
oo.test.ts +8/-4     
mock-adapter.ts +84/-0   
tsconfig.json +5/-1     
tsconfig.test.json +7/-0     
tsdown.config.ts +1/-1     
vitest.config.ts +7/-0     
README.md +82/-0   
package.json +32/-0   
Additional files not shown

- Add @abapify/adt-plugin-atc package
- Define types for ATC checks, findings, variants
- Stub plugin implementation
- Configure build with tsdown
ATC functionality will be implemented directly in adt-cli commands,
no need for a separate plugin package.
refactor(adk,adt-cli): unwrap root schema elements and improve type safety

BREAKING CHANGE: ADK object types now use unwrapped schema elements instead of full response types

ADK Changes:
- Unwrap root elements from schema responses (abapClass, package, abapInterface, etc.)
- Update type definitions to extract inner types (ClassXml = ClassResponse['abapClass'])
- Add explicit response unwrapping in load() methods across all objects
- Fix AdkObjectConstructor to use explicit any type for name
refactor(adt-contracts,adt-codegen): migrate from relative imports to tsconfig path aliases

- Update tsdown from 0.16.7 to 0.18.1 (supports path alias resolution in .d.ts files)
- Replace package self-references with #base and #schemas path aliases
- Update defaultResolveImports() to use aliases instead of calculating relative paths
- Regenerate all contract files with new import style
- Add generated contracts directory to ESLint ignores (intentionally uses package imports)
- Rename project
@qodo-code-review
Copy link

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: main

Failed stage: Run actions/setup-node@v3 [❌]

Failed test name: ""

Failure summary:

  • The workflow failed during dependency setup/caching because no lockfile was found in
    /home/runner/work/js/js (error at log line 143). The action expects one of: package-lock.json,
    npm-shrinkwrap.json, or yarn.lock, and without it the npm cache/dependency step cannot proceed
    correctly.
  • After that, npx nx-cloud fix-ci failed because it installed nx-cloud@19.1.0 on the fly and Nx Cloud
    reported a major-version incompatibility with the workspace’s @nrwl/* / @nx/* packages (log lines
    148–154), exiting with code 1.
Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

128:  with:
129:  node-version: 22
130:  cache: npm
131:  always-auth: false
132:  check-latest: false
133:  token: ***
134:  ##[endgroup]
135:  Found in cache @ /opt/hostedtoolcache/node/22.21.1/x64
136:  ##[group]Environment details
137:  node: v22.21.1
138:  npm: 10.9.4
139:  yarn: 1.22.22
140:  ##[endgroup]
141:  [command]/opt/hostedtoolcache/node/22.21.1/x64/bin/npm config get cache
142:  /home/runner/.npm
143:  ##[error]Dependencies lock file is not found in /home/runner/work/js/js. Supported file patterns: package-lock.json,npm-shrinkwrap.json,yarn.lock
144:  ##[group]Run npx nx-cloud fix-ci
145:  �[36;1mnpx nx-cloud fix-ci�[0m
146:  shell: /usr/bin/bash -e {0}
147:  ##[endgroup]
148:  npm warn exec The following package was not found and will be installed: nx-cloud@19.1.0
149:  NX CLOUD ERROR
150:  ---------------------------------------
151:  This version of Nx Cloud is incompatible with the @nrwl/* and @nx/* packages in your workspace, or Nx was not installed properly.
152:  Verify your install step was successful, and if it was,
153:  match your @nrwl/nx-cloud version to the same major version of your @nx/* and @nrwl/* packages and try again.
154:  ---------------------------------------
155:  ##[error]Process completed with exit code 1.
156:  Post job cleanup.

feat(adt-codegen): add endpoint-level method filtering and clean option

- Add endpoint-config module with defineEndpoint/defineEndpoints helpers
- Support method filtering per endpoint (GET, POST, PUT, DELETE, etc.)
- Add EndpointDefinition type for type-safe endpoint configuration
- Support both string patterns and config objects in enabled endpoints
- Add clean option to remove output directory before generation
- Export endpoint configuration API from package index
- Update enabled-endpoints.ts to use new
refactor(adt-contracts,adt-cli,adt-client,adt-codegen)!: remove /atc/runs endpoint and improve contract testing

BREAKING CHANGE: /sap/bc/adt/atc/runs endpoint removed from generated contracts (not in SAP discovery data)

ATC Changes:
- Remove runsContract from generated contracts (endpoint not in discovery)
- Update enabled-endpoints.ts with note about manual addition if needed
- Fix atc.ts command to not wrap data in 'run' root element (buildXml adds it automatically)
- Rewrite atc.test.ts to use generated contracts (
refactor(ts-xsd,adt-fixtures)!: change root type to match parse() behavior and update ATC fixtures

BREAKING CHANGE: Root schema types now represent element content directly instead of wrapping in element name object

ts-xsd Changes:
- Update generateRootType() to use element's content type directly (matches parse() return value)
- Remove element name wrapper from root types (was `{ elementName: Type }`, now just `Type`)
- Update all test assertions to expect unwrapped root types
- Add typecheck
refactor(adt-schemas)!: expand schema union types and add test typecheck task

BREAKING CHANGE: Schema types now include all possible root elements as union types instead of single element types

- Add typecheck:test task to project.json for test file type checking
- Expand atomExtended schema to include category and link elements
- Expand templatelinkExtended schema to include templateLink element
- Remove abapoo.types.ts (consolidated into other schemas)
- Add union types for syntaxConfiguration,
refactor(adt-contracts): combine generated and manual ATC contracts

- Add build, test, and typecheck:test tasks to project.json
- Restructure ATC contracts to use generated contracts as base
- Add manual endpoints for customizing, runs POST, and worklists.create (not in SAP discovery)
- Re-export generated resultsContract and worklistsContract
- Merge manual worklistsExtended with generated worklistsContract
- Add documentation explaining generated vs manual endpoint sources
```
refactor(adt-cli,adt-atc)!: extract ATC command to separate plugin package

BREAKING CHANGE: ATC command moved from adt-cli to @abapify/adt-atc plugin package

- Move atc.ts command implementation to @abapify/adt-atc package
- Remove ATC formatters (gitlab-formatter.ts, sarif-formatter.ts) from adt-cli
- Add @abapify/adt-codegen and @abapify/adt-atc as dependencies to adt-cli
- Update root adt.config.ts to load both codegen and atc command plugins
- Add config files (*.config.ts, *.config.mts
…st results

- Update status from BROKEN to WORKING after successful transport import
- Document successful import of 5 objects (2 CLAS, 2 DEVC, 1 INTF)
- Note 9 objects skipped due to unsupported types
- Add CLI output example showing successful import
- Clarify that build errors in adt-cli don't block import functionality
- Mark previous plugin API mismatch issue as RESOLVED
chore(git): remove abapgit-examples submodule

Remove github/abapgit-examples submodule and its .gitmodules entry
```
chore(git): remove abapgit-examples submodule

Remove github/abapgit-examples submodule and its .gitmodules entry
```
chore(git): remove abapgit-examples submodule

- Remove github/abapgit-examples submodule reference from .gitmodules
- Delete github/abapgit-examples submodule directory
```
@ThePlenkov ThePlenkov marked this pull request as ready for review December 23, 2025 16:06
@ThePlenkov ThePlenkov merged commit 5820f64 into main Dec 23, 2025
1 check failed
@ThePlenkov ThePlenkov deleted the feature/atc-checks branch December 23, 2025 16:06
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Path traversal write

Description: The generator derives dirPath from untrusted discovery href (and then uses join(outputDir,
dirPath + '.ts') with recursive mkdir/writeFile), so a crafted href containing sequences
like ../ could cause path traversal and write files outside outputDir when running codegen
on untrusted discovery inputs.
generate-contracts.ts [811-847]

Referred Code
// Generate relative path from href (e.g., /sap/bc/adt/atc/worklists -> sap/bc/adt/atc/worklists)
const dirPath = coll.href.replace(/^\//, '');
const contractName = dirPath.split('/').pop() || 'contract';

// Get endpoint config to check for CRUD mode
const endpointConfig = getEndpointConfig(coll.href);
const imports = importResolver(dirPath, outputDir);

let code: string;
let methodCount: number;

if (endpointConfig?.crud) {
  // Generate CRUD contract using crud() helper
  code = generateCrudContractFile(collJson, dirPath, imports, endpointConfig);
  methodCount = 4; // get, post, put, delete
  console.log('  + ' + dirPath + '.ts (CRUD: get, post, put, delete)');
} else {
  // Generate standard contract with individual methods
  const methods = processCollection(collJson);

  // Skip endpoints with no methods (e.g., method filter excluded all)



 ... (clipped 16 lines)
Sensitive data exposure

Description: parseLockResponse() throws an error that includes the first ~200 characters of the raw SAP
response body (responseText.substring(0, 200)), which can leak potentially sensitive
internal details into logs/CI output if SAP returns diagnostic or user/context data in the
lock response.
model.ts [374-389]

Referred Code
protected parseLockResponse(responseText: string): LockHandle {
  const lockHandleMatch = responseText.match(/<LOCK_HANDLE>([^<]+)<\/LOCK_HANDLE>/);

  if (!lockHandleMatch) {
    throw new Error(`Failed to parse lock handle from response: ${responseText.substring(0, 200)}`);
  }

  // Extract CORRNR (transport request) - this is the transport assigned to the object
  const corrNrMatch = responseText.match(/<CORRNR>([^<]+)<\/CORRNR>/);
  const corrUserMatch = responseText.match(/<CORRUSER>([^<]+)<\/CORRUSER>/);

  return { 
    handle: lockHandleMatch[1],
    correlationNumber: corrNrMatch?.[1],
    correlationUser: corrUserMatch?.[1],
  };
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Leaks raw response: parseLockResponse() throws an error including a substring of the raw SAP response body,
which can expose sensitive internal system details to callers.

Referred Code
protected parseLockResponse(responseText: string): LockHandle {
  const lockHandleMatch = responseText.match(/<LOCK_HANDLE>([^<]+)<\/LOCK_HANDLE>/);

  if (!lockHandleMatch) {
    throw new Error(`Failed to parse lock handle from response: ${responseText.substring(0, 200)}`);
  }

  // Extract CORRNR (transport request) - this is the transport assigned to the object
  const corrNrMatch = responseText.match(/<CORRNR>([^<]+)<\/CORRNR>/);
  const corrUserMatch = responseText.match(/<CORRUSER>([^<]+)<\/CORRUSER>/);

  return { 
    handle: lockHandleMatch[1],
    correlationNumber: corrNrMatch?.[1],
    correlationUser: corrUserMatch?.[1],
  };

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit logging: New critical actions (e.g., save(), lock(), unlock(), activate(), setSource()) perform
state-changing operations without any audit trail capturing user ID, action, timestamp,
and outcome in the code shown.

Referred Code
/** 
 * Lock the object for modification
 * 
 * Uses the CRUD contract's lock() method.
 * 
 * The lock response contains:
 * - LOCK_HANDLE: Required for subsequent PUT/unlock operations
 * - CORRNR: Transport request assigned to this object (use for PUT if no explicit transport)
 * - CORRUSER: User who owns the transport
 * 
 * @param transport - Optional transport request to use for locking.
 *                    Required when object is already in a transport.
 */
async lock(transport?: string): Promise<LockHandle> {
  if (this._lockHandle) return this._lockHandle;

  const contract = this.crudContract;
  if (!contract?.lock) {
    throw new Error(`Lock not supported for ${this.kind}. Provide crudContract with lock() method.`);
  }



 ... (clipped 299 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Errors may be skipped: The generator catches errors per JSON file and only console.errors then continues, which
can silently produce partial/incorrect outputs without failing the process.

Referred Code
for (const jsonFile of jsonFiles) {
  try {
    const content = await readFile(jsonFile, 'utf-8');
    const coll: CollectionJson = JSON.parse(content);

    const endpointInfo = { href: coll.href, title: coll.title, category: coll.category.term };

    if (!isEndpointEnabled(coll.href)) {
      skippedEndpointsInfo.push(endpointInfo);
      continue;
    }

    enabledEndpointsInfo.push(endpointInfo);

    const relPath = relative(collectionsDir, jsonFile);
    const dirPath = dirname(relPath);
    const contractName = dirPath.split('/').pop() || 'contract';

    // Get endpoint config to check for CRUD mode
    const endpointConfig = getEndpointConfig(coll.href);
    const imports = importResolver(dirPath, outputDir);



 ... (clipped 36 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured console logs: New code uses multiple console.log/console.error statements (including logging raw err)
which may be unstructured and could inadvertently emit sensitive details depending on
runtime context.

Referred Code
console.log('Collections: ' + collectionsDir);
console.log('Output: ' + outputDir);
console.log('Docs: ' + docsDir);

// Clean output directory if requested
if (clean && existsSync(outputDir)) {
  await rm(outputDir, { recursive: true });
  console.log('Cleaned: ' + outputDir);
}

// Load content type mapping - either from file or use object directly
if (typeof options.contentTypeMapping === 'string') {
  if (!existsSync(options.contentTypeMapping)) {
    throw new Error('Mapping file not found: ' + options.contentTypeMapping);
  }
  await loadMapping(options.contentTypeMapping);
} else {
  contentTypeMapping = options.contentTypeMapping;
}

// Load enabled endpoints - either from file or use object directly



 ... (clipped 94 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
External input parsing: The generator reads and JSON.parses external files (mapping, enabled endpoints,
collections) without schema validation, so malformed or malicious inputs could lead to
incorrect generation or unexpected behavior.

Referred Code
async function loadMapping(mappingPath: string): Promise<void> {
  const content = await readFile(mappingPath, 'utf-8');
  contentTypeMapping = JSON.parse(content);
}

async function loadEnabledEndpoints(path: string): Promise<void> {
  const content = await readFile(path, 'utf-8');
  const parsed = JSON.parse(content);
  enabledEndpointsList = normalizeEnabledEndpoints(parsed);
}

/** Check if endpoints config is legacy format */
function isLegacyFormat(endpoints: EnabledEndpoints): endpoints is LegacyEnabledEndpoints {
  return !Array.isArray(endpoints) && 'enabled' in endpoints;
}

/** Convert legacy or new format to EndpointDefinition[] */
function normalizeEnabledEndpoints(endpoints: EnabledEndpoints): EndpointDefinition[] {
  if (isLegacyFormat(endpoints)) {
    // Convert legacy { enabled: string[] } to EndpointDefinition[]
    return endpoints.enabled;



 ... (clipped 534 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Prevent path traversal security vulnerability

Add a security check to prevent path traversal attacks when generating contract
files from discovery data.

packages/adt-codegen/src/plugins/generate-contracts.ts [812-846]

 // Generate relative path from href (e.g., /sap/bc/adt/atc/worklists -> sap/bc/adt/atc/worklists)
 const dirPath = coll.href.replace(/^\//, '');
 const contractName = dirPath.split('/').pop() || 'contract';
 
 // Get endpoint config to check for CRUD mode
 const endpointConfig = getEndpointConfig(coll.href);
 const imports = importResolver(dirPath, outputDir);
 
 let code: string;
 let methodCount: number;
 
 if (endpointConfig?.crud) {
   // Generate CRUD contract using crud() helper
   code = generateCrudContractFile(collJson, dirPath, imports, endpointConfig);
   methodCount = 4; // get, post, put, delete
   console.log('  + ' + dirPath + '.ts (CRUD: get, post, put, delete)');
 } else {
   // Generate standard contract with individual methods
   const methods = processCollection(collJson);
   
   // Skip endpoints with no methods (e.g., method filter excluded all)
   if (methods.length === 0) {
     console.log('  - ' + dirPath + '.ts (skipped: no methods after filtering)');
     continue;
   }
   
   code = generateContractFile(collJson, methods, dirPath, imports);
   methodCount = methods.length;
   console.log('  + ' + dirPath + '.ts (' + methods.length + ' methods)');
 }
 
 totalMethods += methodCount;
 
 const outputPath = join(outputDir, dirPath + '.ts');
+
+// Security: Prevent path traversal attacks
+const resolvedOutputDir = relative(process.cwd(), outputDir);
+if (!relative(process.cwd(), outputPath).startsWith(resolvedOutputDir)) {
+  throw new Error(`Path traversal detected. Attempted to write outside of output directory: ${outputPath}`);
+}
+
 await mkdir(dirname(outputPath), { recursive: true });
 await writeFile(outputPath, code, 'utf-8');

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical path traversal security vulnerability where an attacker could write files outside the intended directory, and it provides a valid mitigation.

High
Escape XML attributes to prevent injection

Escape XML attribute values in activateAll to prevent malformed XML and
potential injection vulnerabilities.

packages/adk/src/base/object-set.ts [227-230]

+// Helper to escape XML attribute values
+const escapeXml = (str: string) =>
+  str
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&apos;');
+
 // Build activation request XML
 const objectRefs = this.objects.map(obj => 
-  `<adtcore:objectReference adtcore:uri="${obj.objectUri}" adtcore:type="${obj.type}" adtcore:name="${obj.name}"/>`
+  `<adtcore:objectReference adtcore:uri="${escapeXml(obj.objectUri)}" adtcore:type="${escapeXml(obj.type)}" adtcore:name="${escapeXml(obj.name)}"/>`
 ).join('\n  ');
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a potential XML injection vulnerability from building an XML string via interpolation without escaping attribute values, which could lead to malformed requests and is a critical security best practice.

High
Possible issue
Add correlationNumber property

Add the correlationNumber property to the LockHandle interface to match the
implementation.

packages/adk/src/base/model.ts [25-389]

 export interface LockHandle {
   handle: string;
+  correlationNumber?: string;
   correlationUser?: string;
 }
 
 protected parseLockResponse(responseText: string): LockHandle {
   ...
   return { 
     handle: lockHandleMatch[1],
     correlationNumber: corrNrMatch?.[1],
     correlationUser: corrUserMatch?.[1],
   };
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a type mismatch where the LockHandle interface is missing the correlationNumber property, which is returned by parseLockResponse and used in the save method.

Medium
Convert legacy endpoints correctly

Fix the normalizeEnabledEndpoints function to correctly map legacy string
endpoints to EndpointDefinition objects.

packages/adt-codegen/src/plugins/generate-contracts.ts [150-156]

 function normalizeEnabledEndpoints(endpoints: EnabledEndpoints): EndpointDefinition[] {
   if (isLegacyFormat(endpoints)) {
-    // Convert legacy { enabled: string[] } to EndpointDefinition[]
-    return endpoints.enabled;
+    return endpoints.enabled.map((href) => ({ href }));
   }
   return endpoints;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a type mismatch bug where an array of strings is returned instead of an array of EndpointDefinition objects, which would cause runtime errors.

Medium
Prevent memory leak by unregistering listener

Prevent a potential memory leak by unregistering the onResponse event listener
when the authentication promise is settled (resolved or rejected).

packages/browser-auth/src/auth-core.ts [74-110]

 const sapCookies = await new Promise<Awaited<ReturnType<typeof adapter.getCookies>>>((resolve, reject) => {
+  let cleanup: (() => void) | undefined;
+
+  const settle = (settleFn: () => void) => {
+    clearTimeout(timeoutId);
+    cleanup?.();
+    settleFn();
+  };
+
   const timeoutId = setTimeout(() => {
-    reject(new Error('Authentication timeout - SSO not completed'));
+    settle(() => reject(new Error('Authentication timeout - SSO not completed')));
   }, timeout);
 
   adapter.onPageClose(() => {
-    clearTimeout(timeoutId);
-    reject(new Error('Authentication cancelled - browser was closed'));
+    settle(() => reject(new Error('Authentication cancelled - browser was closed')));
   });
 
   // Check for both 200 response AND required cookies on every response
-  adapter.onResponse(async event => {
+  // Assuming onResponse returns a function to unregister the listener
+  cleanup = adapter.onResponse(async event => {
     // Only check on 200 responses from the target URL
     if (event.url === targetUrl && event.status === 200) {
       // Got 200 from target URL - now check if cookies are set
       const allCookies = await adapter.getCookies();
       const domainCookies = allCookies.filter(c =>
         c.domain.includes(sapHost) || sapHost.includes(c.domain.replace(/^\./, ''))
       );
 
       const matchedCookies = domainCookies.filter(c => cookieMatchesAny(c.name, cookiesToWait));
       const allFound = cookiesToWait.every(pattern =>
         matchedCookies.some(c => matchesCookiePattern(c.name, pattern))
       );
 
       if (allFound) {
-        clearTimeout(timeoutId);
         log('✅ Authentication complete!');
         log(`🍪 Found required cookies: ${matchedCookies.map(c => c.name).join(', ')}`);
-        resolve(matchedCookies);
+        settle(() => resolve(matchedCookies));
       }
       // If cookies not found yet, keep waiting for more responses
     }
   });
 
   // Navigate - events are already set up
   adapter.goto(targetUrl, { timeout: 30000 }).catch(() => {});
 });

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential memory leak by not unregistering an event listener. The proposed solution is a valid pattern for preventing such leaks, making the code more robust.

Medium
Handle empty lock response gracefully

Add a null check for the response in the lock method to handle cases where the
lock operation returns an empty response.

packages/adk/src/base/model.ts [335-351]

 async lock(transport?: string): Promise<LockHandle> {
   if (this._lockHandle) return this._lockHandle;
   
   const contract = this.crudContract;
   if (!contract?.lock) {
     throw new Error(`Lock not supported for ${this.kind}. Provide crudContract with lock() method.`);
   }
   
   // Use contract's lock method
   const response = await contract.lock(this.name, { corrNr: transport });
   
+  if (!response) {
+    throw new Error(`Failed to lock object '${this.name}'. The lock operation returned an empty response.`);
+  }
+
   // Parse lock response XML
   // Response format: <asx:abap>...<DATA><LOCK_HANDLE>xxx</LOCK_HANDLE><CORRNR>yyy</CORRNR>...</DATA>...</asx:abap>
   const responseText = String(response);
   this._lockHandle = this.parseLockResponse(responseText);
   return this._lockHandle;
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential null-reference issue if the lock operation returns an empty response and improves error handling by providing a more specific error message.

Medium
Handle missing worklist ID in response

Improve worklist ID parsing by adding a check to ensure an ID is successfully
extracted from a string response before proceeding.

packages/adt-atc/src/commands/atc.ts [274-287]

 // Extract worklist ID - SAP can return plain string or object
 let worklistId: string;
 if (typeof worklistCreateResponse === 'string') {
-  // Plain string response - extract ID from XML or use as-is
+  // Plain string response - extract ID from XML
   const idMatch = worklistCreateResponse.match(/id="([^"]+)"/);
-  worklistId = idMatch ? idMatch[1] : worklistCreateResponse.trim();
+  if (idMatch && idMatch[1]) {
+    worklistId = idMatch[1];
+  } else {
+    ctx.logger.error('❌ Failed to extract worklist ID from string response:', worklistCreateResponse);
+    process.exit(1);
+  }
 } else if ('worklist' in worklistCreateResponse && worklistCreateResponse.worklist?.id) {
   worklistId = worklistCreateResponse.worklist.id;
 } else if ('worklistRun' in worklistCreateResponse && worklistCreateResponse.worklistRun?.worklistId) {
   worklistId = worklistCreateResponse.worklistRun.worklistId;
 } else {
   ctx.logger.error('❌ Failed to get worklist ID from response');
   process.exit(1);
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential issue where an invalid worklistId could be used, leading to downstream errors, and proposes a more robust parsing logic.

Medium
Remove unexpected response field

Remove the unexpected response property from the object returned by activateAll
to align with the ActivationResult interface definition.

packages/adk/src/base/object-set.ts [248-253]

 return { 
   success: this.objects.length, 
   failed: 0, 
   messages: [], 
-  response: String(response), 
 };
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the returned object from activateAll includes a response property that is not defined in the ActivationResult interface, which is a type mismatch that should be corrected.

Medium
tighten exception detection

Improve the accuracy of ADT exception detection by using a regular expression to
match the element with its namespace prefix, reducing the chance of false
positives.

packages/adt-client/src/errors.ts [122-124]

 // Quick check if this looks like an ADT exception
-if (!xml.includes('exception') || !xml.includes('http://www.sap.com/abapxml/types/communicationframework')) {
+if (!/<\w*:exception\b/.test(xml) ||
+    !xml.includes('http://www.sap.com/abapxml/types/communicationframework')) {
   return undefined;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out a potential for false positives and provides a more precise regex-based check, improving the accuracy of the exception detection logic.

Low
General
Enhance missing module detection

Enhance module-not-found error detection by checking error.message in addition
to error.code to support different module systems.

packages/adt-export/src/commands/export.ts [39-45]

 } catch (error: unknown) {
-  const err = error as { code?: string };
-  if (err?.code === 'MODULE_NOT_FOUND') {
+  const err = error as Error;
+  if ((err as any).code === 'MODULE_NOT_FOUND' || err.message.includes(`Cannot find module '${packageName}'`)) {
     throw new Error(`Plugin package '${packageName}' not found. Install it with: bun add ${packageName}`);
   }
   throw error;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that checking only error.code might not be sufficient for all module systems (like ES modules) and proposes a more robust check by also inspecting the error message, improving error handling.

Medium
Restrict upsert fallback

In save method's upsert mode, restrict the fallback to 'create' mode to only
occur on a "not found" error.

packages/adk/src/base/model.ts [428-436]

 try {
   await this.lock(transport);
 } catch (e) {
-  // For upsert, lock failure means object doesn't exist - switch to create
-  if (mode === 'upsert') {
+  // Only fallback to create if it's a 404 Not Found error
+  if (mode === 'upsert' && this.isNotFoundError(e)) {
     return this.save({ ...options, mode: 'create' });
   }
   throw e;
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly refines the logic for upsert mode to only fall back to create on a "not found" error, making the error handling more precise and robust.

Low
Fix progress callback timing

Relocate the onProgress callback to after the saved counter is incremented to
ensure the progress report reflects the correct count.

packages/adk/src/base/object-set.ts [185-191]

+await obj.save(saveOptions);
+
+saved++;
 onProgress?.(saved, total, obj);
-
-await obj.save(saveOptions);
 
 result.success++;
 result.results.push({ object: obj, success: true });
-saved++;
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the onProgress callback is called before the saved counter is incremented, leading to an off-by-one progress report. Moving the call improves the accuracy of the progress reporting.

Low
Avoid using process.exit in actions

Remove the try/catch block and process.exit(1) call from the command action
handler to allow the Commander framework to manage error handling and process
termination gracefully.

packages/adt-cli/src/lib/plugin-loader.ts [97-129]

 cmd.action(async (...actionArgs: unknown[]) => {
   // Commander passes options as last argument before Command
   const cmdInstance = actionArgs.pop() as Command;
   const options = cmdInstance.opts();
   
   // Merge positional args into options
   const args: Record<string, unknown> = { ...options };
   if (plugin.arguments) {
     plugin.arguments.forEach((argDef, index) => {
       const argName = argDef.name.replace(/[<>\[\]]/g, '');
       args[argName] = actionArgs[index];
     });
   }
   
   const ctx: CliContext = {
     cwd: process.cwd(),
     config,
     logger: createSimpleLogger(),
     // Provide ADT client factory for plugins that need API access
     // Note: This is async - plugins must await the result
     getAdtClient: async () => await getAdtClientV2(),
     // Provide system name for ADT hyperlinks
     adtSystemName: getAdtSystem(),
   };
   
-  try {
-    await plugin.execute!(args, ctx);
-  } catch (err) {
-    console.error('Command failed:', err);
-    process.exit(1);
-  }
+  // The try/catch is removed to allow Commander to handle errors.
+  // If you need to log the error here, you can re-throw it.
+  await plugin.execute!(args, ctx);
 });

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: This is a good architectural suggestion that improves framework integration by removing process.exit(1) and allowing the commander library to handle errors, leading to more robust and consistent CLI behavior.

Low
Simplify object count logic for clarity

Refactor the object counting logic by removing the in-place modification of
result.discovered and calculating the total discovered count in the final report
for better clarity.

packages/adt-export/src/commands/export.ts [198]

-// Adjust discovered count (filter callback increments for skipped too)
-result.discovered += result.skipped;
+// The line `result.discovered += result.skipped;` is removed.
+// The `displayExportResults` function should be updated to calculate the total.
+// For example:
+// logger.info(`   📦 Discovered: ${result.discovered + result.skipped}`);
+// (Assuming `displayExportResults` can be modified)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that mutating result.discovered is confusing. Removing the mutation and calculating the total in the display function would improve clarity and maintainability.

Low
  • More

@ThePlenkov
Copy link
Member Author

Findings Acknowledged ✅

Thanks for the automated review!

Applied Fixes:

  1. Robust MODULE_NOT_FOUND check (Importance: 7) - Updated both format-loader.ts and export.ts to check error message in addition to error code for ES module compatibility:

    if ((err as any).code === 'MODULE_NOT_FOUND' || err.message?.includes(\`Cannot find module '${packageName}'\`))
  2. Secure Error Handling (Compliance) - Fixed parseLockResponse() in model.ts to not expose raw SAP response body in error messages:

    throw new Error('Failed to parse lock handle from SAP response. The response format may have changed.');
  3. Restrict upsert fallback (Importance: 6) - Updated save() in model.ts to only fallback to create mode on 404 errors, not any lock failure.

  4. Fix progress callback timing (Importance: 6) - Fixed saveAll() in object-set.ts to call onProgress after incrementing the saved counter.

Not Applied (Architectural Decisions):

  1. Avoid process.exit in plugin-loader - Keeping current behavior as it provides consistent error handling for CLI commands. Commander's default error handling doesn't give us the control we need.

  2. Simplify object count logic in export.ts - The current logic is intentional to show total discovered objects including skipped ones.


Reviewed by: @pplenkov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants