Skip to content

Conversation

@katspaugh
Copy link
Member

Summary

Fixes critical bugs where commands would hang indefinitely when run with --json or --quiet flags without providing required arguments.

Critical Bugs Fixed

1. getPassword() Hanging (Most Critical)

File: src/utils/password-handler.ts

  • Issue: Falls back to interactive p.password() prompt without checking non-interactive mode
  • Fix: Return null immediately in non-interactive mode, letting calling code handle the error
  • Impact: account deploy, tx sign, tx execute would hang forever without password

2. account create - Missing Required Argument Checks

File: src/commands/account/create.ts

  • Issue: Interactive prompts run without checking if required arguments are missing in non-interactive mode
  • Fixes Added:
    • Line 52: Require --chain-id
    • Line 103: Require --owners
    • Line 196: Require --threshold
    • Line 225: Require --name

3. account deploy - Missing Account Argument Check

File: src/commands/account/deploy.ts

  • Issue: Interactive Safe selector runs without checking non-interactive mode
  • Fix: Line 43: Require account address argument

Pattern Applied

All optional arguments that fall back to interactive prompts now follow this pattern:

if (options.argument) {
  // Use provided argument
} else {
  if (isNonInteractiveMode()) {
    outputError('Argument is required in non-interactive mode', ExitCode.INVALID_ARGS)
  }
  // Interactive prompt
}

Before Fix (Broken)

# This would hang indefinitely:
safe --json account create --owners "0x123..." --threshold 2 --name "test"
# Missing --chain-id, hangs on interactive chain selector

# This would hang indefinitely:
safe --json tx sign 0xabc...
# Missing password, hangs on interactive password prompt

After Fix (Works)

# Now fails fast with clear error:
$ safe --json account create --owners "0x123..." --threshold 2 --name "test"
{
  "success": false,
  "error": "Chain ID is required in non-interactive mode",
  "exitCode": 4
}

# Now fails fast with clear error:
$ safe --json tx sign 0xabc...
{
  "success": false,
  "error": "Password is required",
  "exitCode": 3
}

Testing

  • ✅ Build passes (npm run build)
  • ✅ Type checking passes (npm run typecheck)
  • ✅ Linting passes (via husky pre-commit hooks)
  • ✅ Commands now fail fast instead of hanging

Impact

High Priority Fix - These bugs make non-interactive mode unusable for automation:

  • CI/CD pipelines would hang indefinitely
  • Automation scripts would freeze waiting for input that will never come
  • No clear error message - just hangs
  • Must kill process manually

This PR ensures all commands fail fast with clear error messages when required arguments are missing in non-interactive mode.

Related

🤖 Generated with Claude Code

Fix critical bugs where commands would hang indefinitely when run
with --json or --quiet flags without required arguments.

Critical Fixes:
1. getPassword(): Return null instead of prompting in non-interactive mode
2. account create: Require --chain-id in non-interactive mode
3. account create: Require --owners in non-interactive mode
4. account create: Require --threshold in non-interactive mode
5. account create: Require --name in non-interactive mode
6. account deploy: Require account argument in non-interactive mode

Pattern Applied:
```typescript
if (options.argument) {
  // Use provided argument
} else {
  if (isNonInteractiveMode()) {
    outputError('Argument is required in non-interactive mode', ExitCode.INVALID_ARGS)
  }
  // Interactive prompt
}
```

Impact:
- Commands now fail fast with clear error messages instead of hanging
- Better user experience for automation and CI/CD pipelines
- Consistent error handling across all commands

Testing:
Running `safe --json account create` without required arguments now
immediately errors with "Argument is required in non-interactive mode"
instead of hanging indefinitely.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings November 7, 2025 16:22
Copy link
Contributor

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 adds non-interactive mode checks to prevent interactive prompts from appearing when running commands in JSON or quiet mode. The changes ensure that CLI commands fail early with clear error messages when required parameters are missing in non-interactive mode, rather than hanging while waiting for user input.

  • Added non-interactive mode check in getPassword() to return null instead of prompting
  • Added validation checks in deploySafe() and createSafe() to error early when required parameters are missing in non-interactive mode

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/utils/password-handler.ts Added non-interactive mode check to prevent password prompting when running in JSON/quiet mode
src/commands/account/deploy.ts Added validation to require account address parameter in non-interactive mode
src/commands/account/create.ts Added validation to require chain ID, owners, threshold, and name parameters in non-interactive mode

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


// Priority 4: Interactive prompt (fallback)
// Don't prompt in non-interactive mode - return null to trigger error
if (isNonInteractiveMode()) {
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

When getPassword() returns null in non-interactive mode, the calling code may not have sufficient context about why the password could not be obtained. Consider logging an error or warning message before returning null to help users understand that a password must be provided via environment variable, file, or CLI flag when running in non-interactive mode.

Suggested change
if (isNonInteractiveMode()) {
if (isNonInteractiveMode()) {
p.log.error(
'Password could not be obtained in non-interactive mode. Please provide a password via environment variable, file, or CLI flag.'
)

Copilot uses AI. Check for mistakes.
Add new section covering:
- How to enable non-interactive mode (--json, --quiet)
- Password handling for automation (env var, file, CLI flag)
- Required arguments for each command
- JSON output format and exit codes
- Complete CI/CD bash script example
- GitHub Actions workflow example
- Best practices for automation
- Command reference table with non-interactive support status

This documentation helps users:
- Set up automation workflows
- Integrate Safe CLI into CI/CD pipelines
- Handle passwords securely
- Parse JSON output correctly
- Understand exit codes for error handling

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

Co-Authored-By: Claude <noreply@anthropic.com>
@katspaugh katspaugh merged commit ebde124 into main Nov 7, 2025
4 checks passed
@katspaugh katspaugh deleted the fix/non-interactive-hanging-bugs branch November 7, 2025 16:28
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