fix(adt): deploy session ordering + correct MODIFICATION_SUPPORT handling

[dme007]

Summary

Four independent but related fixes to the ADT layer. All surfaced while
verifying vsp deploy on SAP Business Application Studio destinations
(Cloud Connector + Principal Propagation). Live-tested against an on-prem
S/4HANA (DEV 2021 FPS05) for CLAS and INTF, both $TMP and transportable
customer packages. Full pkg/adt suite stays green.

1. SyntaxCheck before Lock in CreateFromFile / UpdateFromFile.
2. Drop the MODIFICATION_SUPPORT="NoModification" hard-fail at LOCK.
3. Preserve X-CSRF-Token and X-sap-adt-sessiontype on redirects.
4. Replace the cookie jar on ICMENOSESSION recovery.

Plus one opt-in diagnostic: VSP_HTTP_TRACE=1 dumps raw HTTP requests
and responses to stderr with Authorization / Cookie values redacted. It
made the root-cause analysis of (2) and (4) possible.

1. SyntaxCheck before Lock

SyntaxCheck carried the default stateless session-type; running it
after LockObject interleaves a stateless hop between the stateful
lock and write. SAP's ICM retires the stateful context on that hop
(Sap-Err-Id: ICMENOSESSION on the PUT), and the ensuing CSRF refresh
fails because the session that held the token is already gone.

Moving SyntaxCheck before LockObject keeps Lock → UpdateSource → Unlock → Activate as a single uninterrupted stateful block. The
per-request Stateful: true flags from 22517d4 are necessary but not
sufficient on their own — call ordering matters too.

2. MODIFICATION_SUPPORT="NoModification" is not "read-only"

Commit 22517d4 hard-failed LockObject when the response carried
MODIFICATION_SUPPORT=NoModification, reading it as "object is
read-only via ADT". That is a misreading of the SAP-standard interface
IF_ADT_LOCK_RESULT, which defines four values:

XML value	Constant	Documented meaning
ModifcationAssistant (sic)	CO_MOD_SUPPORT_MODASS	SAP/partner object, Modification Assistant on
ModificationsLoggedOnly	CO_MOD_SUPPORT_LOGGED_ONLY	SAP/partner object, changes only logged
NoModification	CO_MOD_SUPPORT_NOT_NEEDED	"Modification support is not needed because object is not in SAP/partner namespace in a customer system."
`` (empty)	CO_MOD_SUPPORT_NOT_SPECIFIED	not specified

The constant is NOT_NEEDED, not NOT_PERMITTED. Customer Z*/Y*
objects routinely return this value and the subsequent PUT succeeds —
confirmed live with VSP_HTTP_TRACE=1:

<LOCK_HANDLE>…</LOCK_HANDLE>
<CORRNR>WSDK920768</CORRNR> <CORRUSER>X60000469</CORRUSER>
<MODIFICATION_SUPPORT>NoModification</MODIFICATION_SUPPORT>
→ PUT …/source/main: HTTP 200, Etag returned; Activate: activationExecuted="true"

CORRNR / CORRUSER populated on the LOCK response means SAP accepted
the lock. The field is tracking metadata, not an authorisation verdict.

The "confusing HTTP 423 a few seconds later" the guard was meant to
pre-empt is fully explained by the session-affinity work in 27f4d7c +
the class-include completion in 22517d4 itself. The guard only ever
fires as a false positive on destinations whose SAP system populates
the flag — blocking legitimate customer edits.

LockObject now returns the parsed result verbatim for any value.
Tests move from the asymmetric _RejectsNoModification pair to a
table-driven _PassesThroughModificationSupport, plus the retained
read-lock variant.

3. CheckRedirect preserves ADT headers

Go's http.Client strips Authorization / WWW-Authenticate / Cookie on
cross-origin redirects (RFC 7235). The existing CheckRedirect
restores Authorization for #90. X-CSRF-Token and
X-sap-adt-sessiontype are not on Go's sensitive-header list and are
therefore forwarded by default — but if either silently vanishes
across a redirect (proxy, future library change, middleware), the
second hop lands stateless with a missing CSRF token and the in-flight
lock is invalidated. Re-set both explicitly from via[0], mirroring
the existing Authorization logic. Documented as defensive in the
source comment.

4. Replace the cookie jar on ICMENOSESSION recovery

Long-running processes (MCP server) hit HTTP 400 ICMENOSESSION on
every request after the first successful
Lock → Write → Unlock → Activate cycle. Captured trace:

POST …/activation?method=activate              → 200  (stateless; ends context)
POST …/repository/nodestructure                → 400 ICMENOSESSION
    + 6× Set-Cookie: sap-contextid=…
HEAD …/core/discovery                          → 400 ICMENOSESSION
    + 6× Set-Cookie: sap-contextid=…
…                                               (stuck)

Root cause: the stateless Activate closes SAP's stateful context
server-side, but sap-contextid cookies from the stateful phase stay
in Go's cookie jar — sometimes on multiple paths (/, /sap/,
/sap/bc/, /sap/bc/adt/). Every retry re-sends a matching dead
identifier, including the HEAD /core/discovery refetch that
IsSessionExpired recovery relies on. Short-lived CLI subcommands
escape because each spawns a fresh jar.

A first iteration of this fix expired cookies with
SetCookies(baseURL, {MaxAge: -1, Path: "/"}). It did not work in
practice: http.CookieJar keys entries by (name, domain, path) and
does not expose the stored path — an expire for Path="/" leaves
entries on deeper paths untouched, and that is exactly where SAP
stores the contextid.

Final fix replaces the entire cookie jar with a fresh one before the
CSRF refetch. User-provided cookies (config.Cookies, populated by
browser-auth / SAML / cookie-file flows) are attached per request via
addCookies() and survive untouched; only dynamically
server-deposited entries from the dead session are dropped —
the desired outcome. Transport gains a cached jar http.CookieJar
reference from the underlying *http.Client; a type-assertion on
HTTPDoer leaves the field nil for test mocks and the helper no-ops
there.

Files changed

pkg/adt/workflows_deploy.go     — SyntaxCheck before Lock
pkg/adt/crud.go                 — drop NoModification guard, add interface doc comment
pkg/adt/crud_reconcile_test.go  — _PassesThroughModificationSupport (table-driven)
pkg/adt/config.go               — CheckRedirect preserves X-CSRF-Token +
                                  X-sap-adt-sessiontype
pkg/adt/config_test.go          — _CheckRedirectPreservesADTHeaders
                                  _CheckRedirectHonoursLimit
pkg/adt/http.go                 — VSP_HTTP_TRACE env gate + trace helpers hooked
                                  into Request / retryRequest / fetchCSRFToken;
                                  Transport.jar field; clearSAPSessionCookies
                                  (jar-replace) helper; ICMENOSESSION recovery
                                  call site
pkg/adt/http_test.go            — _ClearSAPSessionCookies_ReplacesJar
                                  _ClearSAPSessionCookies_NonHTTPClientIsSafe

Related issues

• ExceptionResourceInvalidLockHandle, EditSource, WriteSource, CloneObject, ImportFromFile .. #88 / Bug Report: Invalid Lock Handle (Status 423) on Windows when Writing/Updating ABAP Objects #92 / [Bug] 423 "Invalid Lock Handle" when editing DDLS objects — LOCK and UPDATE_SOURCE executed in separate HTTP sessions #98 — session affinity. Fully closed by the combination
  of 27f4d7c + 22517d4's class-include completion + the ordering fix in
  this PR's commit 1.
• ExceptionResourceInvalidLockHandle (status 423) #91 — this PR reverses the NoModification interpretation. A real
  read-only case on BTP, if it exists, will now surface with SAP's
  authoritative error at the write step rather than spuriously at LOCK.

[frd1201]

Independent confirmation of the analysis from a different on-premise system.

Environment: S/4HANA on-prem (release D18, stack 001), customer namespace Z*, transportable package ZLO_PROMOTION_STAGING, transport task D18K..... (Aufgabe within order D18K923708).

Symptom (current main):
EditSource against ZCL_LO_PROMO_STAGING_BUILDER → Failed to lock object: object … is not modifiable via ADT on this system (SAP returned modificationSupport="NoModification" during LOCK). Identical class is fully editable in Eclipse ADT in the same client with the same user — confirming the flag is informational, not authoritative, exactly as you describe.

Side-effect we hit before reading your PR: because the existing guard returns the error before the LOCK_HANDLE is surfaced to the caller, the SAP-side ENQUEUE entry that the LOCK request created stays put (SM12 shows M...... editing the class until cleared manually). Removing the guard makes this moot since LockObject returns the handle and the normal unlock path runs — but it's also a useful hint that the guard was misclassified: SAP did acquire a real lock as part of the request.

What we did locally before finding #108: built a configurable opt-out (SAP_IGNORE_NO_MODIFICATION_GUARD) plus a defensive UnlockObject on the guard's error path. Both work, but your PR is the better fix — the guard was based on a misreading of IF_ADT_LOCK_RESULT.CO_MOD_SUPPORT_NOT_NEEDED, and removing it fixes the bug at the root rather than papering over it. We're going to drop our local branch in favour of merging this PR.

Verified after applying #108 logic locally: Lock → SyntaxCheck → UpdateSource → Unlock → Activate succeeds end-to-end on the customer class. activationExecuted="true", no orphan SM12 entry. Same evidence shape as your trace excerpt.

A merge of this PR (along with #125's mutation-gate ordering work) would unblock all on-prem EditSource/WriteSource flows on systems that populate MODIFICATION_SUPPORT for customer-namespace objects.