Stop the GUI and RFC source readers targeting different SAP systems

[KexiHe]

Summary

Two source-read paths returned different text for the same active program Z_EXCEPTION_1. The root cause was not a stale or lossy rendition — the two legs were reading two different SAP systems.

RPY_PROGRAM_READ is authoritative and faithful. Verified live against both systems:

	S4D/100	S4H/400
line 8	data:gv_file_name type string value 'P17E.txt',	data:gv_file_name type string value 'D:\SAP Item\P17E.txt',"…
PROGDIR	STATE=A UNAM=RICO UDAT=20240829	STATE=A UNAM=KN789 UDAT=20251019

The text attributed to "the RFC path" is exactly S4D/100's active source; the text attributed to "the SE38 GUI path" is exactly S4H/400's. Both systems host an independently-maintained program of that name, diverged since 2024.

Same-system control (both readers, S4H/400, no alignment fudge): differing_lines=0 / 54, multi-byte (Chinese) inline comments included. There is no general pretty-printer rewrite between storage and display.

Why the legs split

Each leg resolves its SAP target through a different chain, and neither stated which system it used:

Leg	Resolution order
RFC — Connect-SapRfc	pin → GUI-active → default → sole profile
GUI — AttachSapSession	hint → SAPDEV_SESSION_PATH → sole connection → refuse

Two compounding defects:

1. SAPDEV_SESSION_PATH was dead. The check-and-download generator exported it and hardcoded %%SESSION_PATH%% to '', but cscript runs from a later, separate block — the env var was already gone. Confirmed in repro: the VBS reported attached ... (sole connection, sole session) (Strategy 3), never Strategy 2. The AI-session pin was silently discarded.
2. Attach Strategy 3 never checks the pin. "Sole connection" is unambiguous about which session, silent about which system. Pin on S4D/100 + only GUI window on S4H/400 → it returned S4H/400. (Get-SapCurrentSessionPath behaved correctly here — it returned ''; its comment calls that degrade "safe", which holds only when no pin exists.)

A check-fix-upload loop on that basis downloads one system's source and uploads it over another's — silently reverting a year of live edits. Affects /sap-check-abap, /sap-fix-abap, /sap-git serialize, and the m365-copilot sap_propose_program_fix tool.

What changed

File	Change
shared/scripts/sap_attach_lib.vbs	New AssertSapGuiTarget. Every success path emits GUI_TARGET: system=… client=… user=… path=… via=… (from GuiSession.Info — IDs, locale-independent per language_independence_rules.md) and refuses hard (exit 2) when SAPDEV_EXPECT_SYSTEM/SAPDEV_EXPECT_CLIENT are set and mismatch. Unset = legacy behaviour, still stamped.
shared/scripts/sap_connection_lib.ps1	New Set-SapGuiTargetExpectation — resolves the same profile Connect-SapRfc picks and exports it. Clears rather than guesses when nothing resolves.
shared/scripts/sap_rfc_read_source.ps1	Result carries System/Client/Host; writes a source.meta.json provenance sidecar next to source.txt. Header documents the fidelity finding.
skills/sap-se38,se37/*_check_and_download.vbs	1-based read, truncate at last non-blank, emit SOURCE_LINES: <n>, WARN on hard cap.
skills/sap-se38,se37/SKILL.md	Generator bakes the resolved session path into %%SESSION_PATH%%; execute block calls Set-SapGuiTargetExpectation in the same process as cscript.
skills/sap-se24/..._check_and_download.vbs	Header comment corrected — se24 downloads via the editor Download menu, so the padding gotcha never applied.
contributing/parallel_safe_session_attach.md, CLAUDE.md	Gotcha 4 corrected; new gotcha 5; canonical wrapper pattern updated.

Secondary fix — download padding and off-by-one

AbapEditor.GetLineText on SAP GUI 7700 / S/4HANA is 1-based (index 0 is a phantom empty line) and returns "" out-of-range without raising, so For i = 0 To 9999 ... If Err.Number <> 0 Then Exit For never terminated early — 10,001 lines for a 54-line program. The control exposes no line-count property (LineCount/LinesCount/NumberOfLines → error 438; .Text returns 19 chars). Now: 43,104 → 3,320 bytes, SOURCE_LINES: 54, and file line N == SAP source line N (the phantom line was silently offsetting syntax-check line numbers by one).

Reviewer notes

• Identity source matters. Provenance is read from RfcDestination.SystemAttributes, not .SystemID (blank on a direct -Server/-Sysnr connection — would have produced a silent blank stamp) and not .Attributes (does not exist on RfcDestination). Both were dead ends I hit before landing on the working one.
• AssertSapGuiTarget has no leading underscore on purpose. VBScript identifiers must start with a letter, and because the lib is pulled in via ExecuteGlobal, a _-prefixed Sub fails to compile on the caller's ExecuteGlobal line — a confusing failure. Noted inline.
• The blank-run stop is a heuristic (500 consecutive blank lines = past end of source). Real ABAP source has no such run — comment lines are not blank — and the emitted SOURCE_LINES makes any truncation visible rather than silent. A hard cap of 99,999 emits a WARN.
• The RCA lives at temp/testReport/source_read_divergence_rca_20260806.md, which is gitignored (temp/), so it is not in this diff — its substance is reproduced above.

Verification

Guard behaviour, live on S4H/400:

### Case 1: expectation = S4D/100 (the original incident)
GUI_TARGET: system=S4H client=400 user=KM717 path=/app/con[0]/ses[0] via=explicit-hint
ERROR: SAP GUI target mismatch. Expected S4D/100 but attached session ... is S4H/400/KM717.
exit=2

### Case 2: expectation = S4H/400 (matches)
SOURCE_LINES: 54 / RESULT: SYNTAX_OK / exit=0

Reconcile-then-diff:

RFC  : status=OK system=S4H client=400 host=vhcalhdbdb lines=54
GUI  : system=S4H client=400
PROVENANCE: MATCH (S4H/400) -- a text delta from here is a real delta
LINES: gui=54 rfc=54
DIFF : differing_lines=0 / 54
VERDICT: PASS - the two readers agree.

node scripts/check-consistency.mjs → OK: 4 plugins, 123 skills, all manifests aligned at version 0.8.1, Tier 3 attach contract clean, screen-baseline coverage 136/136.

Deliberately out of scope

• The guard is wired only into the three read/download paths. The write paths (se38/se37/se11/se24 create/update/delete) resolve their target the same two ways and carry the identical hazard with worse consequences — deploying to the wrong system.
• The dead-SAPDEV_SESSION_PATH pattern (env exported in the generator block, cscript launched later) recurs in ~12 other wrapper blocks across se38/se37 and likely other skills. Each is a latent pin-discarding retarget.
• A CI gate could catch both: flag any SKILL.md block that sets $env:SAPDEV_SESSION_PATH without invoking cscript in the same block.

🤖 Generated with Claude Code