Skip to content

Action execution: break handle_action_proposed into named steps #863

Open

Description

@smart-ssh-planner

Goal

handle_action_proposed reads as a short sequence of named steps instead of one 700-line function, so each step (prepare, evaluate, escalate, decide, confirm, execute, report) can be reviewed and tested on its own.

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 previous issue handle_action_proposed lives in orchestration/action_exec/propose.rs.

Requirements

  • Split handle_action_proposed into private functions (or a small step struct with methods), each with a doc comment stating its input, output and the safety property it keeps, e.g. prepare_action, evaluate_filter, apply_escalations, resolve_auto_exec, await_confirmation, run_approved, report_outcome (names your choice).
  • The order of side effects (events, log lines, ledger writes, confirmation registration) stays identical; early-return paths keep their exact results.
  • The function handle_action_proposed itself is below 150 lines; no step function is above 200 lines; no new #[allow(clippy::too_many_arguments)] (use a context struct).

Acceptance Criteria

  • handle_action_proposed < 150 lines; no function in action_exec/propose.rs > 200 lines.
  • The number of #[allow(clippy::too_many_arguments)] in orchestration/action_exec* does not increase.
  • At least one new unit test per extracted step that has a decision branch (e.g. escalation from auto-exec to confirm for MCP origin, red risk always confirm, injection suspected), asserting the step's result directly.
  • All existing action_exec/tests_* pass unchanged in number and result.
  • Gate green.

Technical Notes

  • Security-relevant (only execution path; invariants 1–7 in .claude/skills/smart-ssh-coder/references/security-invariants.md); review priority elevated. A step may only keep or raise the required confirmation, never lower it; the stop check before auto-exec (Spec 0066) stays where it is in the sequence.
  • 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