Skip to content

[codex] Avoid interactive Codex auth probe in installer preflight#56

Merged
nickbeau merged 3 commits into
mainfrom
symphony/193
Apr 7, 2026
Merged

[codex] Avoid interactive Codex auth probe in installer preflight#56
nickbeau merged 3 commits into
mainfrom
symphony/193

Conversation

@nickbeau
Copy link
Copy Markdown
Contributor

@nickbeau nickbeau commented Apr 7, 2026

Summary

  • replace the installer preflight codex login status probe with direct auth.json validation
  • report auth mode and authentication readiness without spawning an interactive login flow
  • add coverage for token auth, API-key auth, invalid auth files, and refresh the package guide

Why

ReleasedGroup/AgentSystem#193 reports the app triggering az login during setup. The only matching path in this repo was the installer preflight shelling out to codex login status. Reading the local Codex auth file keeps the preflight non-interactive while still validating that usable credentials exist.

Validation

  • dotnet restore Symphony.slnx
  • dotnet build Symphony.slnx
  • dotnet test Symphony.slnx --no-build

Spec

  • No direct SPEC.md section covers release-bundle installer auth probing; orchestration and runtime behavior are unchanged.

Closes ReleasedGroup/AgentSystem#193

Copilot AI review requested due to automatic review settings April 7, 2026 22:53
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the installer’s Codex CLI preflight to avoid running codex login status (which can trigger interactive external auth flows) by validating Codex authentication directly from the local auth.json file, and updates messaging/docs/tests accordingly.

Changes:

  • Replace codex login status probing with auth.json inspection and expose auth readiness + mode in preflight results.
  • Update installer output and remediation guidance to reflect non-interactive auth validation and API-key auth support.
  • Expand integration test coverage for login-token and API-key auth files, plus “no usable credentials” scenarios; update package guide documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Symphony.Integration.Tests/InstallCommandTests.cs Updates expected preflight result shape and remediation text for new auth fields.
tests/Symphony.Integration.Tests/CodexCliPreflightEvaluatorTests.cs Adjusts existing tests and adds coverage for API-key auth and unusable auth.json contents.
src/Symphony.Host/Setup/SymphonyInstallCommand.cs Updates installer preflight output to show auth mode and authentication readiness.
src/Symphony.Host/Setup/CodexCliPreflightEvaluator.cs Implements auth.json inspection, removes interactive CLI login probe, and extends preflight result model.
docs/PackageGuide.md Documents the new non-interactive auth validation behavior and acceptable credential formats.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +264 to +271
catch (IOException)
{
return new CodexCliAuthInspection(
true,
false,
null,
"Codex auth file exists but could not be read.");
}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

InspectAuthConfigurationAsync can throw UnauthorizedAccessException (and potentially other non-IOException exceptions) when opening/reading auth.json; currently only JsonException/IOException are caught, so preflight may crash instead of returning a warning + not-configured state. Consider catching UnauthorizedAccessException (and treating it similarly to IOException) so setup remains resilient on locked-down or permission-restricted machines.

Copilot uses AI. Check for mistakes.
Comment on lines +110 to +113
blockingIssues.Add(
hasAuthJson
? $"Codex auth file '{authJsonPath}' does not contain a usable authentication record."
: $"Codex auth file is missing: '{authJsonPath}'.");
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

When authentication inspection fails due to invalid/unreadable auth.json, the blocking issue is still reported as "does not contain a usable authentication record". Since the more specific reason is currently only surfaced via Notes (warnings), consider incorporating authInspection.Warning (or a more specific blocking issue) to make the actionable error clearer for users.

Suggested change
blockingIssues.Add(
hasAuthJson
? $"Codex auth file '{authJsonPath}' does not contain a usable authentication record."
: $"Codex auth file is missing: '{authJsonPath}'.");
var authenticationBlockingIssue = !hasAuthJson
? $"Codex auth file is missing: '{authJsonPath}'."
: !string.IsNullOrWhiteSpace(authInspection.Warning)
? authInspection.Warning
: $"Codex auth file '{authJsonPath}' does not contain a usable authentication record.";
blockingIssues.Add(authenticationBlockingIssue);

Copilot uses AI. Check for mistakes.
@nickbeau nickbeau merged commit faf4a7c into main Apr 7, 2026
2 checks passed
@nickbeau nickbeau deleted the symphony/193 branch April 7, 2026 23:02
nickbeau added a commit that referenced this pull request Apr 8, 2026
This reverts commit faf4a7c, reversing
changes made to 789fed0.
nickbeau added a commit that referenced this pull request Apr 8, 2026
Revert PR #56: Avoid interactive Codex auth probe in installer preflight
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.

2 participants