Skip to content

feat(adt): add INCL (PROG/I) write support for WriteSource, EditSource, CLI#121

Open
frd1201 wants to merge 3 commits into
oisee:mainfrom
frd1201:feat/incl-write-support
Open

feat(adt): add INCL (PROG/I) write support for WriteSource, EditSource, CLI#121
frd1201 wants to merge 3 commits into
oisee:mainfrom
frd1201:feat/incl-write-support

Conversation

@frd1201

@frd1201 frd1201 commented Apr 23, 2026

Copy link
Copy Markdown

Closes #116

Summary

  • New WriteInclude() workflow — full SyntaxCheck → Lock → UpdateSource → Unlock → Activate cycle for ABAP program includes
  • WriteSource / EditSource now accept object_type="INCL" (MCP tools and CLI vsp source write/edit)
  • vsp source edit INCL — CLI surgical string replacement for includes
  • DeployFromFile / ImportFromFile.incl.abap file extension recognized; object name extracted from filename (includes have no declaration statement)
  • Three bug fixes for the shared /includes/ URL segment that caused program includes to be misidentified as class sub-resources:
    • normalizeObjectURLForPackageCheck: was stripping /includes/NAME from program include URLs → SearchObject("PROGRAMS") → "package metadata not found"
    • SyntaxCheck artifactURI: was missing /source/main for program includes → SAP returned wrong content-type
    • EditSourceWithOptions isClassInclude: was missing /source/main in sourceURL → GET/PUT to XML object descriptor endpoint → HTTP 406
  • SyntaxCheck moved before Lock in UpdateFromFile — stateless SyntaxCheck between stateful Lock/Write broke the SAP session (HTTP 423 ExceptionResourceInvalidLockHandle)

Test plan

  • GetSource(object_type="INCL", name="ZZ_FABD1_DUSH8_CICD2") — read works
  • WriteSource(object_type="INCL", name="ZZ_FABD1_DUSH8_CICD2", source="...", mode="upsert") — create and update works
  • EditSource(object_url="/sap/bc/adt/programs/includes/ZZ_FABD1_DUSH8_CICD2", old_string="...", new_string="...") — surgical edit works
  • vsp source read INCL ZZ_FABD1_DUSH8_CICD2 — CLI read works
  • vsp source write INCL ZZ_FABD1_DUSH8_CICD2 < file.abap — CLI write works
  • vsp source edit INCL ZZ_FABD1_DUSH8_CICD2 --old "..." --new "..." — CLI edit works
  • ImportFromFile(file_path="zz_test.incl.abap") — deploy from file works (Updated PROG/I ZZ_FABD1_DUSH8_CICD2, activated)

@frd1201 frd1201 marked this pull request as draft April 23, 2026 20:31
frd1201 and others added 2 commits April 23, 2026 22:32
…e, CLI

Closes oisee#116. ABAP program includes were read-only — GetSource worked but
WriteSource, EditSource, and CLI `source edit/write` rejected object_type=INCL.

Core feature:
- pkg/adt/workflows.go: new WriteInclude() — SyntaxCheck→Lock→UpdateSource→Unlock→Activate
- pkg/adt/workflows_source.go: INCL in whitelist, existence check, create/update routing
- pkg/adt/fileparser.go: .incl.abap extension → ObjectTypeInclude
- internal/mcp/handlers_source.go: INCL in routeSourceAction, WriteSource/ImportFromFile/ExportToFile descriptions
- cmd/vsp/devops.go: INCL in buildObjectURL() + CLI source edit/test/atc commands

Bug fixes (program includes share /includes/ URL segment with class includes,
causing three functions to misidentify them as class sub-resources):
- pkg/adt/client.go: normalizeObjectURLForPackageCheck — don't strip /includes/NAME
  when prefix ends with /programs (was returning /sap/bc/adt/programs → SearchObject("PROGRAMS") → "package metadata not found")
- pkg/adt/devtools.go: SyntaxCheck artifactURI — append /source/main for program includes
  (was sending object descriptor URL → SAP returned wrong content-type)
- pkg/adt/workflows_edit.go: EditSourceWithOptions isClassInclude — require /oo/classes/ in URL
  (was setting sourceURL without /source/main → GET/PUT to XML endpoint → HTTP 406)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs in INCL deploy flow (ImportFromFile / vsp deploy):

1. fileparser.go: .incl.abap files have no declaration statement, so
   the name must be extracted from the filename (same pattern as class
   includes). Without this, ParseABAPFile failed with "could not parse
   object name from file".

2. workflows_deploy.go: SyntaxCheck is stateless and was running after
   Lock (stateful), which broke the SAP session and caused HTTP 423
   ExceptionResourceInvalidLockHandle. Moved SyntaxCheck to before Lock
   so the stateful session is established cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@frd1201 frd1201 force-pushed the feat/incl-write-support branch from c7890ea to 8f6c030 Compare April 23, 2026 20:32
frd1201 added a commit to frd1201/vibing-steampunk that referenced this pull request Apr 23, 2026
Merges PR oisee#121 (pending upstream) into fork/main so the fix is available
locally. Will be removed from fork/main once upstream merges the PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@txape10

txape10 commented May 27, 2026

Copy link
Copy Markdown

Hi! I opened PR #134 (now closed) targeting the same root cause described in #133,
which overlaps with this one on client.go and workflows_edit.go.

Since your fix covers the same ground and does more, I closed mine. The one thing
I'd suggest adding here is a unit test that pins the normalizeObjectURLForPackageCheck
cases explicitly — program includes must not be truncated, class includes must be:

func TestNormalizeObjectURLForPackageCheck(t *testing.T) {
	cases := []struct {
		input string
		want  string
	}{
		// Program includes (INCL) — must not be mangled
		{"/sap/bc/adt/programs/includes/ZTEST_INCL", "/sap/bc/adt/programs/includes/ZTEST_INCL"},
		{"/sap/bc/adt/programs/includes/ZTEST_INCL/source/main", "/sap/bc/adt/programs/includes/ZTEST_INCL"},
		// Regular programs — unchanged
		{"/sap/bc/adt/programs/programs/ZTEST/source/main", "/sap/bc/adt/programs/programs/ZTEST"},
		// Class includes — strip to parent class
		{"/sap/bc/adt/oo/classes/ZCL_FOO/includes/testclasses", "/sap/bc/adt/oo/classes/ZCL_FOO"},
		{"/sap/bc/adt/oo/classes/ZCL_FOO/includes/definitions", "/sap/bc/adt/oo/classes/ZCL_FOO"},
		// Class source — strip /source/main only
		{"/sap/bc/adt/oo/classes/ZCL_FOO/source/main", "/sap/bc/adt/oo/classes/ZCL_FOO"},
	}
	for _, tc := range cases {
		got := normalizeObjectURLForPackageCheck(tc.input)
		if got != tc.want {
			t.Errorf("normalizeObjectURLForPackageCheck(%q)\n  got  %q\n  want %q", tc.input, got, tc.want)
		}
	}
}

Verified on-prem S/4HANA 758: matchCount: 1 reading ZREPORT_PRUEBA_TOP (package ZABAP01)
after the fix; write reaches SAP and returns corrNr not found instead of the pre-flight abort.
Also links to #133 for the on-prem reproduction case.
Also found the same overly-broad /includes/ check in SyntaxCheck() (devtools.go line 42) — program includes need /source/main in artifactURI or SAP can't locate errors. Same one-line fix: require /oo/classes/ alongside /includes/.

txape10 pushed a commit to txape10/vibing-steampunk that referenced this pull request May 27, 2026
…ee#121

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
txape10 added a commit to txape10/vibing-steampunk that referenced this pull request Jun 2, 2026
…nts fixes

Bug B — INCL write/update (issue oisee#116):
- Add WriteInclude() workflow (SyntaxCheck→Lock→UpdateSource→Unlock→Activate)
- Add INCL case in writeSourceCreate and writeSourceUpdate
- Add INCL to WriteSource validation switch and exists check
- Resolve INCL→PROG/I in CanonicalObjectType (removes TODO pending PR oisee#121)
- Wire INCL in routeSourceAction MCP handler

Bug A — DELETE session affinity (issue oisee#88):
- Add DeleteObjectWithAutoLock(): tries accessMode=DELETE, falls back to MODIFY;
  performs lock+delete atomically in a single stateful session
- Make lock_handle optional in handleDeleteObject; auto-lock path used when absent

RunQuery / tableContents (previous session):
- GetTableContents with sqlFilter now routes to freestyle endpoint (DDIC endpoint
  ignores request bodies)
- Auto-constructs SELECT * FROM <table> WHERE <filter> when filter is not a full SELECT
- Add panic recovery in callHandler so internal panics surface as tool errors (not -32603)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
frd1201 added a commit to frd1201/vibing-steampunk that referenced this pull request Jun 19, 2026
@frd1201

frd1201 commented Jun 19, 2026

Copy link
Copy Markdown
Author

Thanks @txape10 — added TestNormalizeObjectURLForPackageCheck with exactly those cases (program includes stay intact, class includes strip to the parent class). Good catch on SyntaxCheck() too — that /includes/ fix is already part of this PR (devtools.go: isClassInclude now requires /oo/classes/ alongside /includes/, so the artifactURI gets /source/main for program includes). Pushed.

@frd1201 frd1201 marked this pull request as ready for review June 19, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WriteSource in the SAP MCP servers does not currently support ABAP include objects (INCL)

2 participants