Refactoring

[giveen]

1) Refactoring Vision

Create a small, explicit command-processing pipeline with shared policy logic and thin platform adapters.

Core outcomes:

1. One unified flow for command handling.
2. One policy engine for safety/approval decisions.
3. One parser abstraction with platform implementations.
4. One execution abstraction with platform implementations.
5. Minimal branching in high-level orchestration.
6. Smaller test surface with stronger coverage per module.

2) Current Structural Pain (What to Eliminate)

1. Policy logic and platform logic are intertwined.
2. Similar concerns are implemented in multiple places:
   • command classification
   • confirmation gating
   • parsing/tokenization
   • routing decisions
3. Behavior is hard to reason about end-to-end because flow is distributed.
4. Tests are broad and repetitive instead of module-focused.
5. Platform-specific variations can grow duplication quickly.

3) Target Architecture (High-Level)

Use a pipeline architecture with explicit stages:

1. Normalize request
2. Parse and classify
3. Evaluate policy
4. Build execution plan
5. Execute with fallback strategy
6. Shape output and audit decision

Rule: this pipeline should be the only path for shell-like tool execution.

4) Proposed Module Layout

4.1 Command Domain Module

Purpose:

• Shared domain types used everywhere in the pipeline.

Types to centralize:

• CommandRequest
• ParsedCommand
• RiskSignals
• ApprovalDecision
• ExecutionRoute
• ExecutionPlan
• ExecutionResult

Outcome:

• Removes repeated ad hoc structs and makes behavior explainable.

4.2 Parser Module

Purpose:

• Parse command text into neutral structure.

Interface:

• Parse(command, platform) -> ParsedCommand + RiskSignals

Implementations:

• Unix parser
• PowerShell parser

Outcome:

• Parsing logic is no longer mixed into policy or execution.

4.3 Policy Engine Module

Purpose:

• Single source of truth for safe command policy.

Inputs:

• ParsedCommand
• RiskSignals
• cwd context
• platform policy config

Outputs:

• ApprovalDecision with reasons
• optional create-path carveout decision

Outcome:

• Approval behavior becomes deterministic and testable in isolation.

4.4 Route Planner Module

Purpose:

• Decide native execution vs shell execution using parsed/classified command.

Inputs:

• ParsedCommand
• policy outcome

Outputs:

• ExecutionPlan route + fallback strategy

Outcome:

• Route rules stop leaking into execution code.

4.5 Executor Module

Purpose:

• Execute plan using route-specific adapter.

Implementations:

• native executor
• shell executor
• Windows shell adapter

Outcome:

• Clear separation between “what to do” and “how to run”.

4.6 Output and Error Shaping Module

Purpose:

• Normalize exit status, stderr/stdout handling, truncation, and user-facing errors.

Outcome:

• Eliminates repeated formatting branches.

4.7 Policy Configuration Module

Purpose:

• Keep allowlists, denylists, carveout rules, and platform defaults in data tables.

Outcome:

• Behavior updates become config-heavy instead of branch-heavy.

4.8 Audit/Trace Module (Optional)

Purpose:

• Unified internal event model for decision visibility in debug mode.

Outcome:

• Faster diagnosis without instrumentation clutter in execution logic.

5) How to Shrink Code Size

1. Replace repeated conditional chains with table-driven rules.
2. Move risk checks into a single risk signal pass.
3. Convert platform branching from if chains into interface adapters.
4. Collapse similar tests into parameterized table tests with shared fixtures.
5. Standardize error/output shaping in one helper package.
6. Remove duplicate command splitting logic by forcing all modules to consume ParsedCommand.

6) Refactoring Template (Per Module)

Module Refactor Card

1. Module name
2. Responsibility statement
3. Public interface
4. Inputs and outputs
5. Dependencies allowed
6. Dependencies forbidden
7. Invariants
8. Migration steps
9. Test strategy
10. Rollback strategy

PR Template (Per Refactor Step)

1. Goal
2. Scope
3. Non-goals
4. API changes
5. Behavior changes
6. Test evidence
7. Risk assessment
8. Rollback notes

7) Migration Plan (Low-Risk Sequence)

Phase 0: Baseline and Safety Net

1. Capture current behavior with golden/table tests around existing flow.
2. Add explicit decision snapshots for representative commands by platform.

Exit criteria:

• Current behavior frozen in tests.

Phase 1: Introduce Domain Types

1. Add shared command and decision structs.
2. Replace local ad hoc structs gradually.

Exit criteria:

• Compilation unchanged, behavior unchanged, reduced local type duplication.

Phase 2: Extract Parser

1. Move parsing/tokenization into parser module.
2. Keep old call sites but route through parser facade.

Exit criteria:

• No behavior change in tests, parser unit tests cover edge cases.

Phase 3: Extract Policy Engine

1. Move approval logic to policy engine.
2. Keep platform policy in configuration tables.

Exit criteria:

• Approval outcomes identical to baseline suite.

Phase 4: Extract Route Planner

1. Move native-vs-shell decisions into planner.
2. Keep executor untouched initially.

Exit criteria:

• Route decisions match baseline.

Phase 5: Extract Executor and Fallback Strategy

1. Split execute path by route adapters.
2. Centralize fallback policy and output shaping.

Exit criteria:

• Execution tests pass on Unix and Windows compile targets.

Phase 6: Delete Dead Paths

1. Remove old branches and helpers no longer used.
2. Remove duplicated tests superseded by module tests.

Exit criteria:

• Net line-count reduction and simpler call graph.

Phase 7: Final Simplification Pass

1. Normalize naming and package boundaries.
2. Reduce public surface area to only required interfaces.

Exit criteria:

• Architecture docs and code structure align.

8) Architecture Rules to Enforce

1. Orchestrator layer may not parse commands.
2. Policy engine may not execute commands.
3. Executor may not decide policy.
4. Parsers must be side-effect free.
5. Policy should be table-driven where possible.
6. Cross-platform differences live in adapters, not core pipeline.

9) Testing Strategy (Refactor-Friendly)

1. Contract tests for parser:

   • command text -> ParsedCommand + RiskSignals

2. Contract tests for policy:

   • ParsedCommand -> ApprovalDecision

3. Contract tests for route planner:

   • parsed command + policy -> ExecutionPlan

4. Integration tests for full pipeline:

   • command request -> final result

5. Platform matrix:

   • Unix runtime tests
   • Windows compile + focused logic tests

6. Regression corpus:

   • real commands that previously caused routing/approval confusion

   10) Measurable Success Criteria

7. Reduce shell tool code path cyclomatic complexity by >= 30%.

8. Reduce duplicated branch logic across platforms by >= 40%.

9. Reduce total lines in command-handling area by ~15–25%.

10. Keep or improve coverage in command/policy/execution packages.

11. Zero behavior regressions on locked baseline scenarios.

11) Risk and Mitigation

1. Risk: behavior drift during extraction.

   • Mitigation: freeze behavior first with baseline tests and decision snapshots.

2. Risk: platform edge cases hidden by abstraction.

   • Mitigation: keep platform adapters explicit and heavily table-tested.

3. Risk: over-abstracting too early.

   • Mitigation: extract in vertical slices and validate each phase.

4. Risk: temporary complexity during migration.

   • Mitigation: short-lived compatibility wrappers with strict removal milestones.

   12) Recommended First 3 PR Slices

PR 1

Goal:

• Domain types + parser extraction facade (no behavior change).

PR 2

Goal:

• Policy engine extraction with existing outcomes preserved.

PR 3

Goal:

• Route planner and executor split + fallback centralization.

Why this sequence:

• Captures most simplification early with low operational risk.