Skip to content

Action execution: break handle_user_decision and execute_suggested_command into named steps #866

Open

Description

@smart-ssh-planner

Goal

The two other large functions of the execution path, handle_user_decision (~300 lines) and execute_suggested_command (~280 lines), are split into named steps like handle_action_proposed.

Context

crates/app-logic/src/orchestration/action_exec.rs has 2,478 lines of production code (tests already in 12 files under orchestration/action_exec/, declared at :42–84 and :2476). It is the execution path for AI and MCP command proposals: filter evaluation, risk escalation, injection check, confirmation, execution and logging. Big functions: handle_action_proposed (:116–833, ~717 lines, cyclomatic complexity ~75), handle_user_decision (:1455–1751), execute_suggested_command (:1905–2188). Four #[allow(clippy::too_many_arguments)] in this file. Function inventory (line numbers on develop, may have moved after the approved-command refactoring this issue waits for): PreparedDecision :95, handle_mcp_action_proposed :845, tags_for_session :891, auto_exec_rules_for :901, sftp_read_pseudo_command/sftp_write_pseudo_command :927–946, normalize_remote_path :947, pseudo_command_for_risk_classification :982, decision_kind :994, log_filter_escalation :1005, check_finding_confirm :1050, injection helpers with_injection_reason/sanitize_injection_reason/peek_injection_reason/take_injection_reason :1087–1137, state_independent_escalation_code :1138, risk helpers risk_assessment_for_action/red_risk_confirm_reason/red_risk_reason/escalate_data_risk :1198–1302, skip_auto_exec_after_stop :1303, describe_rejected_action :1350, evaluate_action :1379, execute_action :1752, sudo helpers detect_elevation_prefix/command_with_stdin_password_flag/uses_stored_sudo_password :1833–1882, emit_action_error :1883, logging truncate_for_log/log_command_execution/log_command_execution_failed :2189–2302, check_for_injected_instructions :2303, announce_second_opinion_setup_failure :2436.

After the module split, handle_user_decision lives in action_exec/decision.rs and execute_suggested_command in action_exec/execute.rs.

Requirements

  • Split both functions into private, documented steps (e.g. for the decision: validate pending action, apply edit-then-approve with re-evaluation, record decision, run or reject; for execution: prepare command incl. sudo stdin handling, start, stream live output, cancel handling, redact output, persist and report).
  • Same rules as for handle_action_proposed: side-effect order and results unchanged, no function above 200 lines, no new too_many_arguments allowances.

Acceptance Criteria

  • handle_user_decision and execute_suggested_command are each below 120 lines; no function in action_exec/decision.rs or action_exec/execute.rs is above 200 lines.
  • New unit tests for extracted steps with a decision branch: an edited command is evaluated again before it runs; a decision for a stale or unknown action id is rejected; output is redacted before it is persisted or logged; the stored sudo password is only sent on stdin when the command uses the stdin flag.
  • All existing action_exec/tests_* pass unchanged in number and result.
  • Gate green.

Technical Notes

  • Security-relevant (confirmation, edited commands, sudo password on stdin, output redaction; invariants 1–4 and 11 in .claude/skills/smart-ssh-coder/references/security-invariants.md); review priority elevated.
  • Restructuring only: behaviour, error codes, emitted events, log fields and messages stay identical. Fix nothing on the way; a bug you find goes into its own issue (mention it in the PR).
  • Tests: cargo test --workspace -- --list before and after; the number of tests per test binary does not decrease, and the same tests pass. Tests may move with the code they test (<module>/tests.rs or <module>/tests_<topic>.rs; no new *_tests.rs names).
  • Public items used from outside the crate keep their paths (re-export if an item moves); check with grep -rn over the workspace.
  • New code comments in English; existing German comments may move unchanged.
  • Update docs/architecture.md (module table "Ausnahmen von der Regel (A4)" and the list of app-shell modules) when a module moves between app-shell and app-logic.
  • Gate: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace, cargo build --workspace (the last one catches production calls into test-support-only items).

Spec impact: none

Related Work

Activity

  1. added this to the 0.8.0 milestone on Oct 26, 2026
  2. 20 remaining items

  3. marked this as blocking 15 issues on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Fields

    Priority

    P1 - High

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions