fix(install): propagate Description, detect pre-existing package, stop silent success

[dme007]

Summary

Two independent defects in vsp install zadt-vsp (and its MCP-handler
twin, InstallZadtVsp). Surfaced running the installer end-to-end
against a fresh S/4HANA DEV client with VSP_HTTP_TRACE=1 — the tool
reported DEPLOYMENT COMPLETE, Deployed: 9 twice in a row while the
trace showed no object ever reached SAP. A follow-up rerun against an
already-installed system then escalated: the same code tried to LOCK +
DELETE the pre-existing $ZADT_VSP package and was only saved by SAP
refusing the delete because the package still contained objects
(PAK/051).

1. Propagate Description + surface !result.Success — per-object
   deploy silently reported OK when the object was never written.
2. Detect pre-existing package via direct probe — rerun tried to
   create a package that already exists, fell into the partial-create
   reconcile path, and attempted a destructive cleanup on foreign state.

A third, deeper defect — reconcileFailedCreate treating a 400
ExceptionResourceAlreadyExists as partial persistence and running
LOCK + DELETE on pre-existing state — is out of scope here and tracked
as a data-safety follow-up. The install flows no longer reach it
after fix #2, but the reconciler is still latently dangerous for any
other CreateObject caller that can race with pre-existing state.

1. Propagate Description; treat !result.Success as failure

Chained defect between the install driver and WriteSource:

• cmd/vsp/devops.go:3160 and internal/mcp/handlers_install.go:429
  both build WriteSourceOptions without Description, even though
  every embedded.ObjectInfo already carries one.
• In pkg/adt/workflows_source.go:300-303, writeSourceCreate short-
  circuits with result.Message = "Description is required for creating new objects" and returns nil as the error — it stuffs the
  failure into the result struct.
• Both loops ignore the returned *WriteSourceResult (_, err := …) and
  only branch on err. With err always nil, every object printed
  OK / ✓ Deployed and the summary reported full success.

Observable symptom in the HTTP trace: per object a single
GET /source/main → 404 (the existence probe) and then the next
object's probe. No POST, no PUT, no activation — and nine green ticks.

Fix:

• Pass obj.Description into opts.
• Inspect the returned result. If err != nil or res == nil or
  !res.Success, mark the object as FAILED and print res.Message.

The broader "return (result, nil) on failure" pattern in
writeSourceCreate / writeSourceUpdate is a bug class, not a single
bug, and is left for a follow-up; this change at least makes the
install loops fail loudly when they trip it.

2. Detect pre-existing package via direct probe

GetPackage reads the nodestructure API
(POST /repository/nodestructure?parent_name=$ZADT_VSP&parent_type=DEVC/K),
and its parser in pkg/adt/client.go never populates
PackageContent.URI. Both install flows gated their "create package"
step on pkg.URI != "":

cmd/vsp/devops.go:3063   — zadt-vsp install
cmd/vsp/devops.go:3315   — abapgit install

Because URI is never set, the gate always failed open. On first run
the package did not exist and the subsequent CreateObject succeeded
so nothing looked wrong. On second run:

POST /packages  → 400  ExceptionResourceAlreadyExists
GET  /packages/$ZADT_VSP  → 200      (reconcile existence probe)
POST /packages/$ZADT_VSP?_action=LOCK   → 200
DELETE /packages/$ZADT_VSP              → 400  PAK/051
    "Paket $ZADT_VSP enthält noch Entwicklungsobjekte oder andere Pakete"

The reconciler in pkg/adt/crud.go:reconcileFailedCreate interpreted
the 200 on the existence probe as "SAP partially persisted our create"
and ran the compensating cleanup — on a package full of the user's
prior objects. Only PAK/051 prevented data loss.

Why nodestructure cannot fix this: for both "package does not exist"
and "package exists but has no children" it returns the same empty
tree. There is no way to tell the two apart from its response.

Fix — add a public Client.PackageExists probe that hits
/sap/bc/adt/packages/{name} directly:

HTTP	meaning
200	exists
404	does not exist
else	transient / auth / 5xx — returned as error, caller decides

Both install flows switch to PackageExists. On a probe error we fall
through to the create path and let SAP's own error surface there,
preserving existing behaviour for transient failures.

After this change the rerun cleanly reports Package $ZADT_VSP exists
and skips CreateObject entirely; the dangerous reconcile branch
is not reached.

Files changed

cmd/vsp/devops.go                 — Description propagated; res.Success
                                    checked; PackageExists used in both
                                    zadt-vsp and abapgit install flows
internal/mcp/handlers_install.go  — same Description + res.Success fix
                                    for the MCP InstallZadtVsp tool
pkg/adt/client.go                 — new public Client.PackageExists

Related issues

None filed yet — both defects were caught during live install
verification on a fresh DEV client.

Non-goals / follow-ups

• Data-safety: reconcileFailedCreate guard on ExceptionResourceAlreadyExists.
  pkg/adt/crud.go:392 currently treats any CreateObject failure
  whose existence probe returns 200 as partial persistence, including
  the 400 ExceptionResourceAlreadyExists case where the object was
  there before our request. LOCK + DELETE on pre-existing state is
  categorically wrong and only failed safe here because SAP refused to
  delete a non-empty package. To track separately; requires sniffing
  the exception type from the original error body and short-circuiting
  cleanup when it is ExceptionResourceAlreadyExists.

• Bug class: writeSourceCreate / writeSourceUpdate return
  (result, nil) on failure. Every failure path in these functions
  stuffs the reason into result.Message and returns a nil error. Any
  caller that uses the Go-idiomatic _, err := … pattern silently sees
  success. The install loops are one known victim; a sweep for the
  pattern and a conversion to proper error returns is the real fix.