Skip to content

Plugins: restore InstallSafeDITool download command for Xcodeproj users#273

Merged
dfed merged 15 commits intomainfrom
claude/restore-install-safeditool-plugin
Apr 20, 2026
Merged

Plugins: restore InstallSafeDITool download command for Xcodeproj users#273
dfed merged 15 commits intomainfrom
claude/restore-install-safeditool-plugin

Conversation

@dfed
Copy link
Copy Markdown
Owner

@dfed dfed commented Apr 20, 2026

Problem

Xcodeproj users hit a context.tool(named:).url = /\${BUILD_DIR}/\${CONFIGURATION}/SafeDITool template path during plugin setup — Xcode doesn't expose those build variables to plugin processes (verified via full env dump). The build plugin therefore can't execute SafeDITool at setup time and falls back to a regex-based output scanner that misses shapes the regex doesn't cover and produces false positives on string/comment matches.

swift build users don't have this problem: the default prebuilt trait downloads a release binary through the artifact bundle, and --traits sourceBuild is an explicit opt-in to building from source.

Approach

Bring back the 1.5.x InstallSafeDITool command plugin, scoped to Xcode only. It downloads the prebuilt SafeDITool release binary for the current SafeDI version into .safedi/<version>/safeditool next to the Xcode project. The Xcode build plugin prefers that path over the SPM-provided tool.

Chosen over alternatives: a symlink-to-DerivedData approach isn't reachable — SPM command plugins have no DerivedData access in their environment (confirmed by dumping the full env from a running plugin process).

Changes

  • Plugins/InstallSafeDITool/ — new command plugin, safedi-install-tool verb. The SPM CommandPlugin.performCommand emits an error explaining the plugin is Xcode-only (the conformance is required because Package.swift declares capability: .command). The XcodeCommandPlugin extension downloads SafeDITool-macos-<arch> from the SafeDI GitHub release matching the current version, installs it at .safedi/<version>/safeditool, marks it executable, and writes .safedi/.gitignore that excludes the per-version binaries from source control.
  • Plugins/Shared.swiftsafediFolder / expectedToolLocation / downloadedToolLocation / safeDIVersion / safeDIOrigin helpers on XcodePluginContext only. verifiedDownloadedToolLocation(_:) launches the cached binary with --version to confirm runnability before trusting it.
  • Build plugin (Xcode variant) — prefers verifiedDownloadedToolLocation(context.downloadedToolLocation). When the prebuilt is present, runs SafeDITool's scan subcommand (real parser) instead of falling back to PluginScanner. When missing, diagnoses with install instructions pointing at the right-click menu and falls back to PluginScanner as today.
  • Build plugin (SPM variant) — unchanged from main (uses context.tool(named: \"SafeDITool\") directly). No downloaded-tool lookup, no install warning.
  • Package.swift — registers the command plugin with .writeToPackageDirectory + .allowNetworkConnections permissions. Comment documents the plugin as Xcode-only.
  • Documentation/Manual.md — new "Installing the prebuilt SafeDITool binary" subsection under Xcode integration. Explains the warning, the install command, and the one-time-per-project-per-version step.

Edge cases handled

  • SPM invocation (swift package plugin safedi-install-tool) errors out with a clear explanation that the plugin is Xcode-only.
  • URLSession.download(for:) reports success for HTTP error bodies (404, 500); we check for 2xx and throw DownloadFailedError otherwise so the install doesn't silently install an error page as the tool.
  • Cached binary runnability verified via --version launch — avoids an opaque failure loop when a stale/corrupted binary is present.
  • .safedi/.gitignore written on first install so developers don't accidentally commit per-machine binaries.
  • Per-version subdirectory layout (.safedi/<version>/safeditool) lets multiple versions coexist during version bumps.
  • chmod +x on the downloaded file (macOS file-download preserves perms incorrectly by default).
  • Architecture selection (arm64 vs x86_64) at compile time via #if arch. Non-macOS hosts throw UnsupportedHostError.

Test plan

  • `swift build --traits sourceBuild` — plugin compiles clean
  • `./CLI/lint.sh`
  • Manual verification on a consuming Xcode project — needs either a test Xcodeproj that consumes SafeDI by git URL + version, or manual verification by someone with an existing SafeDI-consuming Xcodeproj.

Draft because I haven't been able to end-to-end-test the download path against a real versioned SafeDI release from a consumer Xcode project. The download URL + path math match the 1.5.4 shape that shipped and worked.

🤖 Generated with Claude Code

@dfed
Copy link
Copy Markdown
Owner Author

dfed commented Apr 20, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be6260c9f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Plugins/InstallSafeDITool/InstallSafeDITool.swift Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5076684) to head (56afc61).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #273   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           41        41           
  Lines         6796      6796           
=========================================
  Hits          6796      6796           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dfed
Copy link
Copy Markdown
Owner Author

dfed commented Apr 20, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a623a32048

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Plugins/InstallSafeDITool/InstallSafeDITool.swift Outdated
Comment thread Plugins/SafeDIGenerator/SafeDIGenerateDependencyTree.swift Outdated
dfed added a commit that referenced this pull request Apr 20, 2026
…P1 + P2)

Two review concerns on #273:

1. **P1: downloader always picked a macOS asset.** `downloadTool`
   branched only on CPU architecture, so a Linux host running
   `safedi-install-tool` downloaded `SafeDITool-macos-<arch>` and
   installed a Mach-O binary into `.safedi/<version>/safeditool`.
   The build plugin then preferred that path and every subsequent
   build failed with an exec-format error. Fix: branch on both OS
   and architecture (macOS or Linux, arm64 or x86_64); otherwise
   throw `UnsupportedHostError`.

2. **P2: build plugin trusted any `.safedi/<version>/safeditool`
   without verifying it runs.** A file at that path from a wrong-
   platform install (or a corrupted download that missed the exec
   bit) would become the authoritative tool — build commands
   invoked it, failed opaquely, and builds stayed broken until
   users manually removed the cached file. Fix: new
   `verifiedDownloadedToolLocation(_:)` helper in `Plugins/Shared.swift`
   launches the binary with `--version` and returns the URL only
   when the process exits with status 0. Launch/exec failures route
   the plugin back to the SPM-provided tool (with the existing
   install-command diagnostic in place).

Both call sites in the build plugin (SPM `createBuildCommands` and
`XcodeBuildToolPlugin.createBuildCommands`) now gate the
downloaded-tool path through the verifier.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dfed
Copy link
Copy Markdown
Owner Author

dfed commented Apr 20, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dfed
Copy link
Copy Markdown
Owner Author

dfed commented Apr 20, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a64dfee329

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +123 to +125
let (downloadedURL, response) = try await URLSession.shared.download(
for: URLRequest(url: githubDownloadURL),
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Verify downloaded tool integrity before installation

The installer downloads a binary from GitHub and immediately makes it executable/moves it into .safedi, but it never verifies checksum or code signature, so a replaced/compromised release asset would be executed by the build plugin. This is a regression in trust compared to the SafeDIToolBinary artifact path in Package.swift (which is checksum-pinned) and affects any Xcode user who runs safedi-install-tool.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged as a valid defense-in-depth gap, deferring to follow-up. Current controls: macOS binaries are codesigned + notarized by the publish workflow (Gatekeeper validates at exec time), and verifiedDownloadedToolLocation runs --version and matches the reported string against the expected version before trusting a cached tool. Adding a full checksum-pin equivalent to SafeDIToolBinary requires the publish workflow to also emit per-asset checksums and the plugin to embed them in Plugins/Shared.swift (alongside safeDIVersion), which is a meaningful surface change worth its own PR.

Comment thread Documentation/Manual.md Outdated
Comment on lines +50 to +53
swift package --package-path "/path/to/YourXcodeProject" \
--allow-network-connections all \
--allow-writing-to-package-directory \
safedi-install-tool
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fix install instructions to match plugin invocation mode

These docs tell users to run swift package ... safedi-install-tool, but InstallSafeDITool.performCommand(context: PluginContext, ...) always errors out and exits for SwiftPM invocation, so following this command path cannot work from the CLI. This will reliably block users who follow the manual outside Xcode, so the docs should direct only to the Xcode command-plugin flow (or a working CLI equivalent).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 042add4. The install section now directs users to Xcode's right-click plugin menu instead of the swift package CLI invocation (which now errors out on the SPM path).

Comment thread Documentation/Manual.md Outdated
@dfed dfed marked this pull request as ready for review April 20, 2026 20:01
Comment thread Documentation/Manual.md Outdated
@dfed dfed self-assigned this Apr 20, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 04bbb125c3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +277 to +279
} catch {
Diagnostics.warning("SafeDITool scan failed (\(error)). Falling back to in-process scan.")
// fall through to PluginScanner below
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Don't swallow SafeDITool scan process failures

This catch in the Xcode plugin path handles every runSafeDITool error as a recoverable warning, so real scan failures (for example, SafeDI validation/parsing errors from project sources) get downgraded to a PluginScanner fallback instead of failing fast like the SPM path. In projects where regex scanning misses outputs, that fallback can return no build command and silently keep stale generated code while hiding the original SafeDITool failure; fallback here should be limited to launch/runnability errors, not non-zero process exits.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in eac9480. The Xcode plugin's catch is now split to match the SPM variant: only SafeDIToolLaunchError (binary couldn't launch — template path, wrong platform) falls back to PluginScanner. SafeDIToolProcessError (scan exited non-zero — parse or validation error) bubbles up so real errors aren't masked by a regex-based fallback.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8d375ce98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Plugins/Shared.swift
dfed and others added 14 commits April 20, 2026 15:31
Brings back the command plugin we had in 1.5.x for downloading the
prebuilt SafeDITool release binary. Adapted for 2.x's macos-<arch>
asset naming and integrated with the existing build plugin's fallback
logic.

**Why**

SPM command plugins have no access to DerivedData (verified via full
env dump — Xcode exposes only the user's shell environment to plugin
processes), so we can't symlink to Xcode's downloaded artifact bundle.
A fresh download is the simplest reliable path.

Two scenarios benefit:

1. **Xcode + sourceBuild trait**: `context.tool(named:).url` returns an
   unresolved `${BUILD_DIR}/${CONFIGURATION}/SafeDITool` template that
   can't be executed during plugin setup. The plugin currently falls
   back to the regex-based `PluginScanner`. A downloaded prebuilt at
   `.safedi/<version>/safeditool` lets the plugin run the real parser's
   scan subcommand for authoritative output discovery.
2. **swift build + sourceBuild trait**: SafeDITool is built in debug
   config, ~15× slower than the release binary. Downloading the
   prebuilt release restores prod-speed codegen.

**What changes**

- `Plugins/InstallSafeDITool/` — new command plugin (`safedi-install-tool`
  verb). Downloads the arch-appropriate binary from
  `https://github.com/dfed/SafeDI/releases/download/<version>/SafeDITool-macos-<arch>`,
  installs it at `.safedi/<version>/safeditool`, marks it executable,
  and writes a `.safedi/.gitignore` excluding the binary (which is
  per-machine and per-version).
- `Plugins/Shared.swift` — `safediFolder` / `expectedToolLocation` /
  `downloadedToolLocation` / `safeDIVersion` helpers on both
  `PluginContext` and `XcodePluginContext`. Mirrors the 1.5.4 shape.
  Xcode's `safeDIVersion` is hardcoded (Xcode plugins can't read the
  package manifest) — acceptable since the binary format is
  forward-compatible within a minor release line.
- `Plugins/SafeDIGenerator/SafeDIGenerateDependencyTree.swift` — prefers
  `downloadedToolLocation` over the SPM-provided tool. Emits a
  `Diagnostics.warning` that points to the exact `swift package
  safedi-install-tool` invocation when the prebuilt isn't installed.
  The Xcode variant additionally runs the real `scan` subcommand when
  the prebuilt is available (instead of falling back to PluginScanner
  unconditionally).
- `Package.swift` — registers `InstallSafeDITool` as a command plugin
  with `.writeToPackageDirectory` + `.allowNetworkConnections` permissions.
- `Documentation/Manual.md` — "Installing the prebuilt SafeDITool binary"
  subsection under the Xcode project integration section. Explains the
  warning, the install command, the one-time-per-project step, and why
  Xcode requires it.

Local-path and root-package references (as in our example projects) are
rejected with a clear error — the install flow only makes sense with
versioned releases.

All 885 tests pass; lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`URLSession.download(for:)` resolves with `.success` for HTTP error
responses (404, 500, etc.) — the downloaded file contains the error
body, not the tool. Previously we'd `chmod +x` and move that body
into `.safedi/<version>/safeditool`, so the next build would try to
execute an HTML/JSON error page and fail opaquely.

Cast the `URLResponse` to `HTTPURLResponse` and require a 2xx status
before installing. On non-2xx: clean up the temp file and throw a
`DownloadFailedError` with the URL and status code so users can see
what went wrong (typically: the hardcoded SafeDI version in the
Xcode plugin path has drifted past what's published on GitHub).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Xcode 26.4's Swift 6 strict concurrency rejected the pattern of
capturing a `var capturedError: Error?` across a `Task.detached` —
the captured reference is non-Sendable. Match the 1.5.4 approach:
handle errors inline inside the Task (Diagnostics.error + exit(1))
so nothing crosses the Sendable boundary. Semantically identical,
zero-copy fix.
…P1 + P2)

Two review concerns on #273:

1. **P1: downloader always picked a macOS asset.** `downloadTool`
   branched only on CPU architecture, so a Linux host running
   `safedi-install-tool` downloaded `SafeDITool-macos-<arch>` and
   installed a Mach-O binary into `.safedi/<version>/safeditool`.
   The build plugin then preferred that path and every subsequent
   build failed with an exec-format error. Fix: branch on both OS
   and architecture (macOS or Linux, arm64 or x86_64); otherwise
   throw `UnsupportedHostError`.

2. **P2: build plugin trusted any `.safedi/<version>/safeditool`
   without verifying it runs.** A file at that path from a wrong-
   platform install (or a corrupted download that missed the exec
   bit) would become the authoritative tool — build commands
   invoked it, failed opaquely, and builds stayed broken until
   users manually removed the cached file. Fix: new
   `verifiedDownloadedToolLocation(_:)` helper in `Plugins/Shared.swift`
   launches the binary with `--version` and returns the URL only
   when the process exits with status 0. Launch/exec failures route
   the plugin back to the SPM-provided tool (with the existing
   install-command diagnostic in place).

Both call sites in the build plugin (SPM `createBuildCommands` and
`XcodeBuildToolPlugin.createBuildCommands`) now gate the
downloaded-tool path through the verifier.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
swift build users already get the prebuilt binary through the default
`prebuilt` trait, and `--traits sourceBuild` builds SafeDITool from
source on purpose. The install plugin only exists to work around Xcode's
`${BUILD_DIR}`-in-tool-path problem, so the SPM-side installer and
macOS/Linux host detection weren't pulling their weight.

The SPM `CommandPlugin.performCommand` stays (Package.swift's .command
capability requires CommandPlugin conformance) but now just emits an
error pointing users at the traits. The downloaded-tool lookup in the
SPM build plugin path is removed; the Xcode build plugin still prefers
it. Removes the Linux arch branches and the FoundationNetworking import.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The plugin target can't be excluded from the build — Package.swift
declares it unconditionally — so it has to compile on every platform
SPM supports, including Linux. Before this fix, removing the Linux
host detection and `FoundationNetworking` import left the
`URLSession.shared.download(for:)` call referencing APIs that don't
exist in Linux Foundation, breaking the Linux CI build.

Wrap the entire download body in `#if os(macOS)` with a throwing
`#else` branch. At runtime, non-macOS hosts still get
`UnsupportedHostError`; at compile time, Linux never sees
`URLSession.shared`, `URLRequest`, or `.posixPermissions` so the
compiler stays happy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ter errors

- **Atomic move**: replace `removeItem` + `moveItem` with `FileManager.replaceItemAt`.
  The old two-step sequence is a race under concurrent installs (e.g., a
  dev and CI running simultaneously): two processes can both `removeItem`,
  then only one `moveItem` succeeds because the destination exists again.
  `replaceItemAt` is atomic on HFS+/APFS.
- **Version match in verification**: `verifiedDownloadedToolLocation` now
  captures `--version` stdout and compares against `expectedVersion`.
  Catches the "stale binary from an older SafeDI left in `.safedi/`" case
  — a tool that launches cleanly but reports a mismatched version would
  previously be trusted indefinitely.
- **Wrap URLSession errors**: `URLError` stringifies to a useless generic
  ("The operation couldn't be completed."). `DownloadRequestFailedError`
  prepends the URL and passes through the underlying message so offline /
  DNS / release-tag-missing cases surface something actionable.
- **Manual**: update the install section to (a) direct users to Xcode's
  right-click → plugin menu (the CLI invocation no longer works since the
  SPM path errors out), and (b) tell users to commit `.safedi/.gitignore`
  so per-machine binaries stay ignored across the team.
- **Comment cleanup**: the `Task.detached` explanation now correctly
  notes that `exit(1)` on failure skips the deferred `leave()` — it's a
  hard process kill, not a graceful release.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…afeDIVersion

The InstallSafeDITool XcodeCommandPlugin can't read the package
manifest at runtime, so it consumes a hardcoded SafeDI version from
`Plugins/Shared.swift`. If the publish workflow doesn't keep that in
sync, every release ships a plugin that downloads the wrong release
asset.

Extend `Scripts/update-version.sh` to rewrite that version via a sed
range-address (the getter body is multi-line). Also add a
corresponding CI verification step so a future refactor that breaks
the sed regex is caught before a release.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The verb's description is what Xcode surfaces in its right-click
plugin menu and what `swift package plugin --list` prints. Both
audiences need to know this plugin only makes sense for Xcode
projects — swift build users already get the prebuilt tool through
the default `prebuilt` trait.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
update-version.sh now rewrites both Package.swift AND Plugins/Shared.swift
(the hardcoded safeDIVersion consumed by the Xcode command plugin).
The publish workflow's `git add Package.swift` was missing the second
file, so the release commit silently left the plugin pointing at the
previous version — confirmed in the 2.0.0-alpha-18-xcode-plugin-test
run: Package.swift got stamped, Plugins/Shared.swift did not.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rror gating, stderr drain)

**Revert test-release artifact pin in Package.swift.** The publish
workflow commit for the `2.0.0-alpha-18-xcode-plugin-test` release
stamped SafeDIToolBinary to point at that alpha asset. Revert to
`2.0.0-beta-4` + original checksum so this branch doesn't ship the
test artifact pin to main. The publish workflow will re-stamp on the
next real release.

**Xcode build plugin: distinguish launch from process failures.** The
Xcode variant's `catch {}` at the scan site downgraded every
`runSafeDITool` error to a PluginScanner fallback. Scan process
failures (parse errors, validation errors) were silently masked — the
regex scanner would then produce incomplete output, and users would
see cryptic downstream build errors. Now only `SafeDIToolLaunchError`
falls back (matching the SPM variant's pattern); `SafeDIToolProcessError`
bubbles up.

**Drain stderr via FileHandle.nullDevice in verifiedDownloadedToolLocation.**
The attached `Pipe()` was never read. An unread pipe deadlocks
`waitUntilExit()` once the buffer fills (~64 KB), and this helper
runs on the plugin-setup thread. Route stderr to /dev/null via
`FileHandle.nullDevice` — no read-side coupling, no deadlock.

**Tighten the update-version-check CI grep.** The bare
`grep '"99.99.99-test"' Plugins/Shared.swift` matched anywhere in the
file, so a future test-string literal elsewhere could satisfy the
assertion even if the sed pattern broke. Scope via
`grep -A 2 'var safeDIVersion: String {'` so only the getter's range
counts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dfed dfed force-pushed the claude/restore-install-safeditool-plugin branch from eac9480 to 0684609 Compare April 20, 2026 22:32
The single-quoted `'var safeDIVersion: String {'` in the `run:` value
contained a `{` that YAML parsed as a flow-mapping start. CI failed
before any job could execute: "Invalid workflow file... line 35." A
block scalar (`run: |`) scopes the whole command as a literal and
sidesteps the ambiguity.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dfed dfed merged commit 75846d3 into main Apr 20, 2026
18 checks passed
@dfed dfed deleted the claude/restore-install-safeditool-plugin branch April 20, 2026 22:49
dfed added a commit that referenced this pull request Apr 21, 2026
…plugin (#277)

## Summary
Style-guide follow-up to #273. CLAUDE.md bars bare \`if { return|throw
}\` patterns with implicit fall-through — the non-returning path has to
sit in an explicit \`else\`.

## Changes
- **\`Plugins/InstallSafeDITool/InstallSafeDITool.swift\`** — HTTP
status check for the downloaded asset. The old
  \`\`\`swift
  if let httpResponse = response as? HTTPURLResponse,
     !(200..<300).contains(httpResponse.statusCode)
  {
      throw DownloadFailedError(...)
  }
  \`\`\`
  falls through on 2xx. Restructured to
  \`\`\`swift
  if let httpResponse = response as? HTTPURLResponse {
guard (200..<300).contains(httpResponse.statusCode) else { throw ... }
  }
  \`\`\`
  Same runtime behavior.

- **\`Plugins/SafeDIGenerator/SafeDIGenerateDependencyTree.swift\`** —
Xcode variant's real-scan / \`PluginScanner\` split. The old \`if
runsRealScan { do-return-catch } ...\` block returned from inside the
\`if\` and fell through on \`SafeDIToolLaunchError\`. Extracted:
- \`buildCommandsUsingRealScan(...)\` — returns \`nil\` on recoverable
launch failure, lets \`SafeDIToolProcessError\` propagate
  - \`buildCommandsUsingPluginScanner(...)\` — regex-based fallback
  
  Caller is now
  \`\`\`swift
  if runsRealScan, let commands = try buildCommandsUsingRealScan(...) {
      return commands
  } else {
      return buildCommandsUsingPluginScanner(...)
  }
  \`\`\`
Both branches return. Runtime behavior unchanged: \`ProcessError\` still
surfaces, \`LaunchError\` still warns + falls to scanner.

## What this doesn't do
- Binary checksum / code signature verification for the downloaded tool
(codex P1 on #273). Current controls are Gatekeeper (macOS signing +
notarization at \`Process.run\` time) and the \`--version\` match in
\`verifiedDownloadedToolLocation\`. A full checksum-pin would mirror
\`SafeDIToolBinary\`'s checksum and needs publish-workflow coordination
— separate PR.

## Test plan
- [x] \`swift build --traits sourceBuild\` — builds clean
- [x] \`./CLI/lint.sh\`
- [x] Plugin code is non-unit-testable; no test changes. Existing CI
covers the \`buildCommandsUsingPluginScanner\` path via
\`spm-project-integration\` and \`spm-multi-project-integration\` (they
exercise the Xcode build plugin through \`xcodebuild\`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dfed added a commit that referenced this pull request Apr 21, 2026
## Summary
Rename the \`InstallSafeDITool\` command plugin verb from
\`safedi-install-tool\` (introduced in #273) back to
\`safedi-release-install\` (matches the 1.x verb). Removes migration
friction for users upgrading from 1.x and drops a clarifying note in the
manual that called out the drift.

Safe to rename: #273 hasn't shipped in any published release yet (only
the \`2.0.0-alpha-18-xcode-plugin-test\` test tag), so no consumer has
scripted around the 2.x verb.

## Changes
- \`Package.swift\`: verb string updated
- \`Plugins/InstallSafeDITool/InstallSafeDITool.swift\`: SPM-path error
message mentions the new verb
- \`Plugins/SafeDIGenerator/SafeDIGenerateDependencyTree.swift\`:
fallback warning now points users at \`SafeDI → Safedi Release Install\`
(Xcode-rendered capitalization)
- \`Documentation/Manual.md\`: install instructions + migration note
both updated; the \"1.x verb was X\" clarifying note is gone now that
they match

## Test plan
- [x] \`swift build --traits sourceBuild\` — builds clean
- [x] \`./CLI/lint.sh\`
- [x] \`grep -rn "safedi-install-tool|Safedi Install Tool"\` — no stale
references

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <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