SD-10196: Open PRs in skills.new

[inesp]

The push fails and we are asked if we want to create a PR:

After answering Yes, a PR is opened:

[linear-code]

SD-10196

[claude]

Claude finished @inesp's task in 2m 23s —— View job

---

This PR cleanly extends the existing PR-fallback flow to the Sleuth/skills.new vault and is well-commented and tested, with a few edge cases around 403 handling and partial PR creation worth tightening.

[MEDIUM] Any 403 from the upload endpoint is treated as an edit-permission denial, misrouting auth failures into the PR-offer flow

Location: internal/vault/sleuth.go:300

Issue: The condition if uploadResp.PermissionDenied || resp.StatusCode == http.StatusForbidden turns every 403 into an AssetEditPermissionError. A 403 that is not about asset-edit permission (e.g. an invalid/expired token, or any other forbidden response) produces an AssetEditPermissionError with an empty AssetGID. sx add then prompts "Open a pull request with your changes instead?", and if the user accepts (or runs with --yes, which auto-accepts), OpenAssetPullRequest immediately fails with cannot open pull request: missing asset id — a confusing dead end instead of a clear auth error.

Suggestion: Gate the fallback on the explicit machine-readable signal rather than the bare status code — e.g. treat it as an edit denial only when uploadResp.PermissionDenied is true (the server's documented signal), or additionally require uploadResp.SkillID != "". Fall through to the existing httpStatusError/uploadResp.Error handling otherwise so genuine 403s surface as themselves.

[MEDIUM] A mid-upload failure leaves a created PR with only partial file changes

Location: internal/vault/sleuth_pr.go:62 (addPullRequestFiles)

Issue: OpenAssetPullRequest first creates the PR, then adds files one-by-one. If any AddAssetPullRequestFileChange call fails after some have succeeded, the function returns an error and sx add reports failure — but the PR already exists on the server containing a partial subset of the asset's files. Unlike the git-vault flow (which commits the asset atomically to a branch), there's no cleanup, so a maintainer could see/merge an incomplete PR. A retry also opens a second PR rather than continuing the first.

Suggestion: If the server supports it, send the file changes in a single batched mutation, or attempt to delete/close the PR on partial failure. At minimum, document this non-atomic behavior in docs/rbac.md and make the returned error mention that an incomplete PR may have been created and should be closed before retrying.

[LOW] An asset whose zip contains only `metadata.toml` would open an empty pull request

Location: internal/vault/sleuth_pr.go:30 (OpenAssetPullRequest)

Issue: metadata.toml is intentionally skipped from file changes. If the uploaded zip has no other files, the PR is created with zero addAssetPullRequestFileChange calls — an empty PR that can't meaningfully be merged. This is an unlikely edge case but currently passes silently.

Suggestion: After unpacking, if no non-metadata files were added, return an error (and ideally avoid creating the PR), e.g. cannot open pull request: asset has no files to propose.

[LOW] No command-level test covers the new Sleuth PR-offer wiring

Location: internal/commands/add.go:633-672 (addViaProposedPR, promptOpenPR generic branch)

Issue: The vault layer is well tested (sleuth_pr_test.go), but the new add.go wiring — type-asserting to assetPRProposer, the errors.As denial branch, the --yes auto-accept path, and the new teams-empty generic prompt message — has no coverage. This is where the routing bug in the MEDIUM finding above would manifest.

Suggestion: Add a command-level test that feeds an AssetEditPermissionError (with and without AssetGID) through addNewAsset and asserts the prompt/PR behavior, including the --yes path.

• `SD-10196-skills-vault-pr`