Skip to content

fix: harden write safety, ADT sessions, and result verification - #156

Draft
Augusto42 wants to merge 15 commits into
oisee:mainfrom
Augusto42:codex/vsp-bug-audit
Draft

fix: harden write safety, ADT sessions, and result verification#156
Augusto42 wants to merge 15 commits into
oisee:mainfrom
Augusto42:codex/vsp-bug-audit

Conversation

@Augusto42

@Augusto42 Augusto42 commented Aug 15, 2026

Copy link
Copy Markdown

Summary

This PR fixes ten independently reproduced defect groups across CLI safety, source-write result handling, installers, ADT stateful mutation sessions, activation workflows, recording history, and Windows test portability.

What changed

  • propagate persistent safety flags and resolved safety configuration to every CLI/MCP client
  • make Lua writeSource forward all supported options and return false with a diagnostic on logical failures
  • require successful activation and a valid completion marker in ExecuteABAP; propagate logical failure through CLI/MCP
  • verify package creation, deployment result, syntax, activation, and source read-back in installers
  • keep CSRF discovery/refresh in the same stateful session as lock/write/unlock
  • validate package scope before locking while continuing to enforce operation and transport policy
  • preserve MODIFICATION_SUPPORT as metadata and reuse lock corrNr under the normal transport safety whitelist
  • guarantee unique, time-sortable recording IDs even when the platform clock does not advance between calls
  • use native temporary directories for the JavaScript oracle test on Windows
  • make copy exit non-zero on failed objects, create packages only after a conclusive absence check, and skip unsupported or partial deployments before writing
  • surface logical WriteSource and Activate failures as MCP tool errors
  • stop deploy, table, batch, DSL, and rename workflows when activation reports success=false
  • write renamed source to the source endpoint, require unlock and activation, and never delete the old object after an unsuccessful activation
  • stop DeployFromFile before locking or writing when the existence probe is inconclusive

Root causes

The defects combined incomplete configuration propagation, logical failures being treated as transport success, inconclusive existence checks being treated as absence, and loss of session/transport context between ADT mutation requests. Some orchestration paths also advanced after unsuccessful activation or advertised deployment paths that were not implemented. Recording IDs relied on wall-clock resolution alone, causing rapid recordings to overwrite one another on Windows.

User impact

Write restrictions now apply consistently. Failed or partial writes, installations, executions, activations, copies, and renames are no longer reported as successful. Destructive rename does not touch the old object unless the replacement is written, unlocked, and activated. Stateful ADT edit flows retain their session and transport context without bypassing local safety gates.

Related issues and PRs

Refs #88, #92, #117, #141, #143, #144.
Overlaps with the intent of #106, #108, #120, #125, and #138 while retaining operation and transport validation after the package lookup is skipped.

Validation

  • focused tests for cmd/vsp, pkg/config, pkg/scripting, pkg/adt, pkg/dsl, internal/install, and internal/mcp
  • activation, rename, existence, copy, and MCP regressions passed 20 consecutive runs
  • history regression group passed 50 consecutive runs
  • pkg/jseval oracle comparison passes on Windows
  • go vet ./...
  • gofmt on every changed Go file
  • golangci-lint v1.64.8 run --new-from-rev=origin/main ./...
  • git diff origin/main..HEAD --check

The full go test ./... run passes for every package that does not require SQLite through CGO. The remaining failures are the existing cmd/vsp and pkg/cache SQLite tests under CGO_ENABLED=0.

All SAP-facing behavior is covered with mocks and synthetic fixtures; no live SAP system was contacted.

vinchacho added a commit to vinchacho/vibing-steampunk that referenced this pull request Aug 15, 2026
Post-cherry-pick fixups for upstream PR oisee#156 (Augusto42:codex/vsp-bug-audit):
module paths in new files, and the audit report renamed to our
YYYY-MM-DD-NNN convention (002; 001 is the landscape report).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vinchacho added a commit to vinchacho/vibing-steampunk that referenced this pull request Aug 15, 2026
Merge fixup between upstream PR oisee#150 (ActivateMultiple batch path) and
PR oisee#156 (stop on activation Success=false): when a batch activation
reports success=false without an inactive list, mark every requested
object Failed with the aggregate diagnostic instead of reporting them
activated — preserving ActivatePackage's classification contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vinchacho added a commit to vinchacho/vibing-steampunk that referenced this pull request Aug 15, 2026
reports/2026-08-15-001: mid-2026 ecosystem survey (official SAP, ARC-1,
skills packaging, agentic ABAP), competitive position, upstream PR oisee#150/oisee#156
review record, and a 4-theme borrow roadmap (security posture, plugin
evals, distribution, tool surface). CLAUDE.md counts re-derived after the
cherry-picks (focused 103, expert 154, tests 1363); stale cherry-pick
priority replaced. README_TOOLS.md: ActivateMultiple row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vinchacho added a commit to vinchacho/vibing-steampunk that referenced this pull request Aug 15, 2026
SafetyConfig gains ImpactGate (off default | advise | block) and
ImpactThreshold (high default | medium), wired through all three
config surfaces in the PR-oisee#156 resolution order (profile < env < flag):

- Persistent flags --impact-gate / --impact-threshold (inherited by
  all CLI subcommands, same block as the oisee#156 safety flags)
- Env vars SAP_IMPACT_GATE / SAP_IMPACT_THRESHOLD via viper
- Per-system .vsp.json fields impact_gate / impact_threshold

Invalid values are rejected at flag parse (root PersistentPreRunE,
mirroring --mode validation) with the valid choices listed; profile
values are normalized, invalid ones warn and fall back to defaults.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vinchacho added a commit to vinchacho/vibing-steampunk that referenced this pull request Aug 15, 2026
…plugin, impact gate

As-built documentation for the 2026-08-15 feature drops, per policy that
shipped features read as documentation, not proposals.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant