/sap-review-abap

name: sap-review-abap
description: |
  AI semantic + security code review for an EXISTING ABAP object or a local .abap
  file. Reads the active source, builds a structure + call/data map, then reasons
  over the code across a fixed dimension checklist — security (dynamic-SQL
  injection, missing/wrong AUTHORITY-CHECK, client handling), correctness (unchecked
  SY-SUBRC, READ TABLE guards), performance (SELECT-in-LOOP, nested loops, SELECT *),
  robustness/LUW (unhandled exceptions, COMMIT in loop), and maintainability. This
  is the SEMANTIC review that complements — does NOT replace — the deterministic
  /sap-check-abap parser and the in-system /sap-atc rule engine. Every finding cites
  a line + excerpt and is adversarially re-verified before emission (false positives
  dropped), written through the shared finding model to <NAME>.review.tsv/.json for
  /sap-evidence-pack. Read-only.
  Prerequisites: pinned /sap-login connection for object-name input; class download
  and --callers need a GUI session; a .abap file needs no SAP connection.
argument-hint: "<OBJECT_NAME | path-to.abap> [--type program|include|fm|class|auto] [--dimensions all|security,perf,correctness,robustness,maintainability] [--callers] [--gate advisory|block] [--no-gui]"

SAP Review ABAP Skill

You produce a **semantic, security-aware code review** of an existing ABAP object (or a local `.abap` file). You read the real source, reason about what it actually does, and emit prioritized, line-cited findings that a rule engine cannot derive. You are **read-only** — you never deploy, activate, or edit.

This skill observes `shared/rules/skill_operating_rules.md` (reads only — no SQL writes, no unsolicited deployment) and `shared/rules/language_independence_rules.md` (the GUI download / where-used VBS it reuses identify controls by ID, status by `MessageType`).

Task: $ARGUMENTS

> **Positioning (state this to the user when relevant).** `/sap-review-abap` is > the *judgment* stage of the quality lane, distinct from its neighbours: > `gen-abap → check-abap → **review-abap** → atc → fix-abap → deploy`. > - `/sap-check-abap` — deterministic parse: naming, DDIC types, SQL field > existence, unused vars. Cheap, exact, no false judgement. > - `/sap-atc` — SAP's in-system Code Inspector rule set; the hard gate. > - `/sap-review-abap` (you) — LLM reasoning over logic, security and > performance the other two structurally cannot see. Advisory by default.

---

Shared Resources

| File / token | Path | Purpose | |---|---|---| | `sap_settings_lib.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_settings_lib.ps1` | `Get-SapSettingValue`, settings merge | | `sap_connection_lib.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_connection_lib.ps1` | `Get-SapWorkDir`, `Get-SapCurrentSessionPath` | | `sap_rfc_lib.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_rfc_lib.ps1` | `Connect-SapRfc` | | `sap_object_resolver.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_object_resolver.ps1` | `Resolve-SapObject` — type + TADIR object code + scope identity | | `sap_rfc_read_source.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_rfc_read_source.ps1` | `Read-SapAbapSource` (RPY source + include tree) | | `sap_explain_parse.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\..\skills\sap-explain-object\references\sap_explain_parse.ps1` | offline source → `map.json` (units / externals / db reads+writes) | | `sap_finding_lib.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_finding_lib.ps1` | `New-SapFinding` / `Export-SapFindings*` / `Get-SapVerdict` | | `sap_gate_policy.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_gate_policy.ps1` | `Get-SapGatePolicy` / `Set-SapFindingGates` (reads the brief's Quality bar) | | `sap_artifact_lib.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_artifact_lib.ps1` | `New-SapScopeKey` / `Register-SapArtifact` (best-effort) | | `sap_attach_lib.vbs` (`%%ATTACH_LIB_VBS%%`) | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_attach_lib.vbs` | `AttachSapSession` (class download / where-used) | | SE24 download VBS | `<SAP_DEV_CORE_SHARED_DIR>\..\skills\sap-se24\references\sap_se24_check_and_download.vbs` | class source (GUI) | | where-used VBS | `<SAP_DEV_CORE_SHARED_DIR>\..\skills\sap-where-used-list\references\sap_where_used_list.vbs` | callers (GUI, `--callers`) | | `abap_code_quality_rules.md` | `<SAP_DEV_CORE_SHARED_DIR>\rules\abap_code_quality_rules.md` | The §-numbered quality rules the dimension checklist anchors to | | `customer_brief.md` | `{custom_url}\customer_brief_<LANG>.md` → `{custom_url}\customer_brief.md` → `<SAP_DEV_CORE_SHARED_DIR>\templates\customer_brief_<LANG>.md` → `...\templates\customer_brief.md` (`<LANG>`: `userConfig.template_language` → `userConfig.sap_language` → `EN`; `EN` skips the `_<LANG>` probes) | Release / `MODE_*` / Quality bar — drives gating | | `sap_log_helper.ps1` | `<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_log_helper.ps1` | structured logging |

`<SAP_DEV_CORE_SHARED_DIR>` resolves to `plugins/sap-dev-core/shared` — from this skill, go **3 levels up** from `<SKILL_DIR>` (skill → `skills/` → plugin dir → `plugins/`), then into `sap-dev-core\shared`.

---

Step 0 — Resolve Work Directory

Resolve `work_dir` via the env-aware helper — do NOT read `settings.json` directly (that ignores `SAPDEV_AI_WORK_DIR` / `userconfig.json`):

powershell -NoProfile -ExecutionPolicy Bypass -Command ". '<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_settings_lib.ps1'; . '<SAP_DEV_CORE_SHARED_DIR>\scripts\sap_connection_lib.ps1'; Write-Output ('WORK_DIR=' + (Get-SapWorkDir)); Write-Output ('CUSTOM_URL=' + (Get-SapSettingValue 'custom_url' ((Get-SapWorkDir) + '\custom')))"

Settings reads/writes follow `<SAP_DEV_CORE_SHARED_DIR>/rules/settings_lookup.md`.

| Setting | Default if blank | |---|---| | `work_dir` | `C:\sap_dev_work` | | `custom_url` | `{work_dir}\custom` |

Set `{WORK_TEMP}` = `{work_dir}\temp` and `{OUT}` = `{WORK_TEMP}\review\{OBJECT}` (for file input use the file stem as `{OBJECT}`). Ensure `{OUT}` exists:

cmd /