-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add non-interactive support for transaction commands (Phase 4) #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add JSON output and password handling to transaction commands for automation support: Transaction Read Commands (JSON output): - tx list: Added --json output with transaction filtering - tx status: Added --json output with comprehensive tx data Transaction Write Commands (Password + JSON): - tx sign: Integrated getPassword utility, added JSON output - tx execute: Integrated getPassword utility, added JSON output Key Changes: - All commands check isNonInteractiveMode() and skip prompts/spinners - Error handling migrated to outputError with proper exit codes - Password input via getPassword (env var, file, or flag) - Consistent JSON output format across all tx commands - Non-interactive mode requires safeTxHash argument Related to Phase 4 implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
StoredTransaction type has executedAt field, not updatedAt. This fixes the TypeScript compilation error in CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this 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 pull request adds non-interactive mode support to transaction commands, enabling automation and CI/CD integration. The changes introduce conditional logic to handle both interactive terminal UI and JSON-based output modes.
- Adds non-interactive mode detection and JSON output for
tx status,tx sign,tx execute, andtx listcommands - Replaces direct
p.log.error/p.outrocalls with centralizedoutputErrorandoutputSuccesshelpers that respect output mode - Implements password handling via environment variables, files, or CLI flags using the centralized
getPasswordutility
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/commands/tx/status.ts | Adds non-interactive mode support with JSON output for transaction status retrieval and conditional UI rendering |
| src/commands/tx/sign.ts | Implements non-interactive signing with centralized password handling and JSON output for automation |
| src/commands/tx/execute.ts | Adds non-interactive execution support with centralized password handling and JSON output |
| src/commands/tx/list.ts | Implements JSON output mode for transaction listing with applied filters |
Comments suppressed due to low confidence (2)
src/commands/tx/status.ts:152
- Inconsistent error handling: The catch block uses the old error handling pattern with
p.log.errorandp.outro, which doesn't respect non-interactive mode. This should usehandleCommandError(error)for consistency with sign.ts (line 296) and execute.ts (line 186), or useoutputErrorto properly handle both interactive and non-interactive modes.
} catch (error) {
if (error instanceof SafeCLIError) {
p.log.error(error.message)
} else {
p.log.error(`Unexpected error: ${error instanceof Error ? error.message : 'Unknown error'}`)
}
p.outro('Failed')
}
src/commands/tx/list.ts:183
- Inconsistent error handling: The catch block uses the old error handling pattern with
p.log.errorandp.outro, which doesn't respect non-interactive mode. This should be updated to useoutputErrorfor consistency with the rest of the changes in this PR, especially since the function now supports non-interactive mode.
} catch (error) {
p.log.error(`Unexpected error: ${error instanceof Error ? error.message : 'Unknown error'}`)
p.outro('Failed')
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| outputError('No transactions found', ExitCode.ERROR) | ||
| } | ||
|
|
||
| if (isNonInteractiveMode()) { | ||
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: After outputting an error for no transactions, execution continues and will attempt to use non-interactive mode check and interactive prompt. The outputError function exits the process (returns never), but the code doesn't account for this. After line 32 outputs the error and exits, lines 35-53 become unreachable in that scenario. However, when there ARE transactions, line 35-37 will incorrectly exit even though the user could potentially select a transaction. This check should only apply when !selectedSafeTxHash is still true AND there are transactions available.
| outputError('No transactions found', ExitCode.ERROR) | |
| } | |
| if (isNonInteractiveMode()) { | |
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) | |
| outputError('No transactions found', ExitCode.ERROR) | |
| // outputError exits, but for clarity, return here as well | |
| return | |
| } | |
| if (isNonInteractiveMode()) { | |
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) | |
| // outputError exits, but for clarity, return here as well | |
| return |
| // Apply filters | ||
| let filtered = transactions | ||
| if (filterSafeAddress) { | ||
| filtered = filtered.filter( | ||
| (tx) => tx.safeAddress.toLowerCase() === filterSafeAddress!.toLowerCase() | ||
| ) | ||
| } | ||
| if (filterChainId) { | ||
| filtered = filtered.filter((tx) => tx.chainId === filterChainId) | ||
| } | ||
| if (statusFilter) { | ||
| filtered = filtered.filter((tx) => tx.status === statusFilter) | ||
| } |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code duplication: The filtering logic in lines 139-151 duplicates the filtering that happens in lines 76-78. The statusFilter is applied twice when in non-interactive mode with no safes: once at lines 76-78 and again (potentially) at line 149-150 if the code path continued. Consider refactoring to apply filters in a single location to avoid duplication and potential inconsistencies.
| // Apply filters | |
| let filtered = transactions | |
| if (filterSafeAddress) { | |
| filtered = filtered.filter( | |
| (tx) => tx.safeAddress.toLowerCase() === filterSafeAddress!.toLowerCase() | |
| ) | |
| } | |
| if (filterChainId) { | |
| filtered = filtered.filter((tx) => tx.chainId === filterChainId) | |
| } | |
| if (statusFilter) { | |
| filtered = filtered.filter((tx) => tx.status === statusFilter) | |
| } | |
| // Apply filters using helper | |
| const filtered = filterTransactions(transactions, filterSafeAddress, filterChainId, statusFilter) |
| txHash: tx.txHash, | ||
| explorerUrl, | ||
| createdAt: tx.createdAt, | ||
| executedAt: tx.executedAt, |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential undefined field reference: The updatedAt field is being accessed on line 127, but this field is not defined in the StoredTransaction interface. The interface only has createdAt and executedAt fields. This will result in updatedAt: undefined being included in the JSON output, which may confuse consumers of the API.
| executedAt: tx.executedAt, | |
| ...(tx.executedAt !== undefined ? { executedAt: tx.executedAt } : {}), |
| if (isNonInteractiveMode()) { | ||
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) | ||
| } |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: Similar issue as in status.ts. After checking for non-interactive mode and exiting with an error on line 38-40, the code continues to the interactive selectTransaction call on lines 41-47. The non-interactive mode check should be placed before attempting the interactive selection, and it should prevent the interactive prompt from being reached.
|
|
||
| if (!selectedSafeTxHash) { | ||
| if (isNonInteractiveMode()) { | ||
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: Similar issue as in status.ts and sign.ts. After checking for non-interactive mode and exiting with an error on line 36-38, the code continues to the interactive selectTransaction call on lines 39-47. The non-interactive mode check should prevent the interactive prompt from being reached.
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) | |
| outputError('Transaction hash is required in non-interactive mode', ExitCode.INVALID_ARGS) | |
| return |
| // Request password using centralized handler | ||
| const globalOptions = getGlobalOptions() | ||
| const password = await getPassword( | ||
| { | ||
| password: globalOptions.password, | ||
| passwordFile: globalOptions.passwordFile, | ||
| passwordEnv: 'SAFE_WALLET_PASSWORD', | ||
| }, | ||
| 'Enter wallet password' | ||
| ) | ||
|
|
||
| if (!password) { | ||
| outputError('Password is required', ExitCode.AUTH_FAILURE) | ||
| } | ||
|
|
||
| // Get private key | ||
| const spinner2 = p.spinner() | ||
| spinner2.start('Executing transaction') | ||
| const spinner2 = !isNonInteractiveMode() ? p.spinner() : null | ||
| spinner2?.start('Executing transaction') | ||
|
|
||
| let privateKey: string | ||
| try { | ||
| privateKey = ctx.walletStorage.getPrivateKey(activeWallet.id, password) | ||
| } catch { | ||
| spinner2.stop('Failed') | ||
| p.log.error('Invalid password') | ||
| p.outro('Failed') | ||
| return | ||
| spinner2?.stop('Failed') | ||
| outputError('Invalid password', ExitCode.AUTH_FAILURE) | ||
| } |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wallet type handling incomplete: The password is requested and used for private key wallets (line 125-138), but there's no check for Ledger wallets like in sign.ts. If the active wallet is a Ledger wallet, this code will attempt to get a password and decrypt a private key that doesn't exist, which will fail. The execute command should handle Ledger wallets similarly to how the sign command does (lines 122-165 in sign.ts).
| outputError( | ||
| `Failed to sign with Ledger: ${error instanceof Error ? error.message : 'Unknown error'}. Make sure your Ledger is connected, unlocked, and the Ethereum app is open`, | ||
| ExitCode.ERROR | ||
| ) |
Copilot
AI
Nov 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved error message clarity: The enhanced error message that combines the Ledger failure with troubleshooting guidance is good, but it's very long for a single error message. Consider whether the troubleshooting text should be a separate warning or info message rather than part of the error itself, to maintain clarity in JSON output mode where this entire string will be in the error field.
Summary
Adds non-interactive support to transaction commands with JSON output and password handling, enabling automation and scripting workflows.
Changes
Transaction Read Commands (JSON Output)
--jsonoutput mode with transaction filtering by Safe and status--jsonoutput with comprehensive transaction data including signatures, owners, and execution detailsTransaction Write Commands (Password Handling + JSON)
tx sign:
getPasswordutility for automated password inputSAFE_WALLET_PASSWORD), file, or CLI flagtx execute:
getPasswordutility for automated password inputImplementation Details
Non-Interactive Mode Detection
isNonInteractiveMode()to determine behaviorsafeTxHashargument for all commands in non-interactive modeError Handling
p.log.errortooutputErrorwith proper exit codesINVALID_ARGS,ERROR,SAFE_NOT_FOUND,NETWORK_ERROR,AUTH_FAILURE)Password Input Hierarchy
SAFE_WALLET_PASSWORD)--password-file)--password, visible in process list - use with caution)JSON Output Format
All commands return consistent JSON structure:
{ "success": true, "message": "Operation completed successfully", "data": { ... } }Testing
npm run build)npm run typecheck)Example Usage
Interactive Mode (unchanged)
Non-Interactive Mode (new)
Related
🤖 Generated with Claude Code