From 8fbe9aa963c4b693533e13163bdbeae14cd02857 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Thu, 22 Jan 2026 10:36:56 +0000 Subject: [PATCH] Add step command refinements: --here, --id, and help commands - Add --here position option to insert steps before the current step - Add --id option to specify custom step IDs for expression references - Add --help flag support for all step commands with detailed usage info - Update browser extension UI with ID field and improved position dropdown --- .../plans/dap-step-commands-refinements.md | 853 ++++++++++++++++++ browser-ext/content/content.css | 7 + browser-ext/content/content.js | 23 +- .../Dap/StepCommands/StepCommandHandler.cs | 222 +++++ .../Dap/StepCommands/StepCommandParser.cs | 91 +- .../Dap/StepCommands/StepCommandResult.cs | 2 + .../Dap/StepCommands/StepFactory.cs | 10 +- .../Dap/StepCommands/StepManipulator.cs | 158 ++++ .../Dap/StepCommands/StepCommandParserL0.cs | 40 + 9 files changed, 1396 insertions(+), 10 deletions(-) create mode 100644 .opencode/plans/dap-step-commands-refinements.md diff --git a/.opencode/plans/dap-step-commands-refinements.md b/.opencode/plans/dap-step-commands-refinements.md new file mode 100644 index 000000000..0386d113e --- /dev/null +++ b/.opencode/plans/dap-step-commands-refinements.md @@ -0,0 +1,853 @@ +# Step Commands Refinements: --here, --id, and Help + +**Status:** Draft +**Author:** GitHub Actions Team +**Date:** January 2026 +**Prerequisites:** dap-step-manipulation.md (completed) + +## Progress Checklist + +- [x] **Chunk 1:** `--here` Position Option +- [x] **Chunk 2:** `--id` Option for Step Identification +- [x] **Chunk 3:** Help Commands (`--help`) +- [x] **Chunk 4:** Browser Extension UI Updates + +## Overview + +This plan addresses three refinements to the step manipulation commands based on user feedback: + +1. **`--here` position option**: Insert a step before the current step (the one you're paused at), so it runs immediately when stepping forward +2. **`--id` option**: Allow users to specify a custom step ID for later reference (e.g., `steps..outputs`) +3. **Help commands**: Add `--help` flag support to all step commands for discoverability + +## Problem Statement + +### Issue 1: "First pending position" inserts in the wrong place + +When paused before a step (e.g., checkout at position 1), using `--first` inserts the new step *after* the current step, not before it: + +``` +Before (paused at step 1): +▶ 1. Checkout + +After "steps add run 'echo hello' --first": +▶ 1. Checkout + 2. hello [ADDED] <-- Wrong! Should be before Checkout +``` + +**Root cause:** `PositionType.First` returns index 0 of the `JobSteps` queue, which contains steps *after* the current step. The current step is held separately in `_currentStep`. + +**Expected behavior:** User wants to insert a step that will run immediately when they continue, i.e., before the current step. + +### Issue 2: No way to specify step ID + +Dynamically added steps get auto-generated IDs like `_dynamic_`, making them impossible to reference in expressions like `steps..outputs.foo`. + +### Issue 3: Command options are hard to remember + +With growing options (`--name`, `--shell`, `--after`, `--before`, `--at`, `--first`, `--last`, etc.), users need a way to quickly see available options without consulting documentation. + +--- + +## Chunk 1: `--here` Position Option + +**Goal:** Add a new position option that inserts a step before the current step (the one paused at a breakpoint). + +### Design + +| Flag | Meaning | +|------|---------| +| `--here` | Insert before the current step, so it becomes the next step to run | + +**Behavior:** +- Only valid when paused at a breakpoint +- Returns error if not paused: "Can only use --here when paused at a breakpoint" +- Inserts the new step such that it will execute immediately when the user continues/steps forward + +**Example:** +``` +Before (paused at step 1): +▶ 1. Checkout + 2. Build + 3. Test + +After "steps add run 'echo hello' --here": +▶ 1. hello [ADDED] <-- New step runs next + 2. Checkout + 3. Build + 4. Test +``` + +### Files to Modify + +| File | Changes | +|------|---------| +| `StepCommandParser.cs` | Add `Here` to `PositionType` enum; add `StepPosition.Here()` factory; parse `--here` flag in add/move commands | +| `StepManipulator.cs` | Handle `PositionType.Here` in `CalculateInsertIndex()` and `CalculateMoveTargetIndex()` | + +### Implementation Details + +**StepCommandParser.cs:** + +```csharp +// Add to PositionType enum +public enum PositionType +{ + At, + After, + Before, + First, + Last, + Here // NEW: Insert before current step (requires paused state) +} + +// Add factory method to StepPosition +public static StepPosition Here() => new StepPosition { Type = PositionType.Here }; + +// Update ToString() +PositionType.Here => "here", + +// In ParseReplAddRunCommand and ParseReplAddUsesCommand, add case: +case "--here": + cmd.Position = StepPosition.Here(); + break; + +// Same for ParseReplMoveCommand +``` + +**StepManipulator.cs:** + +```csharp +// In CalculateInsertIndex(): +case PositionType.Here: +{ + // "Here" means before the current step + // Since current step is held separately (not in JobSteps queue), + // we need to: + // 1. Verify we're paused (have a current step) + // 2. Insert at position 0 of pending AND move current step after it + + if (_currentStep == null) + { + throw new StepCommandException(StepCommandErrors.InvalidPosition, + "Can only use --here when paused at a breakpoint."); + } + + // The new step goes at index 0, and we need to re-queue the current step + // Actually, we need a different approach - see "Special handling" below +} +``` + +**Special handling for `--here`:** + +The current architecture has `_currentStep` held separately from `JobSteps`. To insert "before" the current step, we need to: + +1. Insert the new step at position 0 of `JobSteps` +2. Move `_currentStep` back into `JobSteps` at position 1 +3. Set the new step as `_currentStep` + +Alternative (simpler): Modify `InsertStep` to handle `Here` specially: + +```csharp +public int InsertStep(IStep step, StepPosition position) +{ + // Special case: --here inserts before current step + if (position.Type == PositionType.Here) + { + if (_currentStep == null) + { + throw new StepCommandException(StepCommandErrors.InvalidPosition, + "Can only use --here when paused at a breakpoint."); + } + + // Re-queue current step at the front + var pending = _jobContext.JobSteps.ToList(); + pending.Insert(0, _currentStep); + + // Insert new step before it (at position 0) + pending.Insert(0, step); + + // Clear and re-queue + _jobContext.JobSteps.Clear(); + foreach (var s in pending) + _jobContext.JobSteps.Enqueue(s); + + // New step becomes current + _currentStep = step; + + // Track change and return index + var newIndex = _completedSteps.Count + 1; + // ... track change ... + return newIndex; + } + + // ... existing logic for other position types ... +} +``` + +### Testing + +- [ ] `steps add run "echo test" --here` when paused at step 1 inserts at position 1 +- [ ] New step becomes the current step (shows as `▶` in list) +- [ ] Original current step moves to position 2 +- [ ] Stepping forward runs the new step first +- [ ] `--here` when not paused returns appropriate error +- [ ] `steps move 3 --here` moves step 3 to before current step + +--- + +## Chunk 2: `--id` Option for Step Identification + +**Goal:** Allow users to specify a custom ID for dynamically added steps. + +### Design + +| Flag | Meaning | +|------|---------| +| `--id ` | Set the step's ID (used in `steps..outputs`, etc.) | + +**Validation:** +- ID must be a non-empty string +- No format restrictions (matches YAML behavior - users can use any string) + +**Duplicate handling:** +- If a step with the same ID already exists, return error: "Step with ID '' already exists" + +**Default behavior (unchanged):** +- If `--id` is not provided, auto-generate `_dynamic_` as before + +### Files to Modify + +| File | Changes | +|------|---------| +| `StepCommandParser.cs` | Add `Id` property to `AddRunCommand` and `AddUsesCommand`; parse `--id` flag | +| `StepFactory.cs` | Add `id` parameter to `CreateRunStep()` and `CreateUsesStep()`; use provided ID or generate one | +| `StepCommandHandler.cs` | Pass `Id` from command to factory; validate uniqueness | +| `StepManipulator.cs` | Add `HasStepWithId(string id)` method for uniqueness check | + +### Implementation Details + +**StepCommandParser.cs:** + +```csharp +// Add to AddRunCommand and AddUsesCommand classes: +public string Id { get; set; } + +// In ParseReplAddRunCommand and ParseReplAddUsesCommand: +case "--id": + cmd.Id = GetNextArg(tokens, ref i, "--id"); + break; +``` + +**StepFactory.cs:** + +```csharp +// Update method signatures: +ActionStep CreateRunStep( + string script, + string id = null, // NEW + string name = null, + // ... rest unchanged +); + +ActionStep CreateUsesStep( + string actionReference, + string id = null, // NEW + string name = null, + // ... rest unchanged +); + +// In implementation: +public ActionStep CreateRunStep(string script, string id = null, ...) +{ + var stepId = Guid.NewGuid(); + var step = new ActionStep + { + Id = stepId, + Name = id ?? $"_dynamic_{stepId:N}", // Use provided ID or generate + DisplayName = name ?? "Run script", + // ... + }; + // ... +} +``` + +**StepManipulator.cs:** + +```csharp +// Add method to check for duplicate IDs: +public bool HasStepWithId(string id) +{ + if (string.IsNullOrEmpty(id)) + return false; + + // Check completed steps + foreach (var step in _completedSteps) + { + if (step is IActionRunner runner && runner.Action?.Name == id) + return true; + } + + // Check current step + if (_currentStep is IActionRunner currentRunner && currentRunner.Action?.Name == id) + return true; + + // Check pending steps + foreach (var step in _jobContext.JobSteps) + { + if (step is IActionRunner pendingRunner && pendingRunner.Action?.Name == id) + return true; + } + + return false; +} +``` + +**StepCommandHandler.cs:** + +```csharp +// In HandleAddRunCommand and HandleAddUsesCommand: +if (!string.IsNullOrEmpty(cmd.Id) && _manipulator.HasStepWithId(cmd.Id)) +{ + throw new StepCommandException(StepCommandErrors.DuplicateId, + $"Step with ID '{cmd.Id}' already exists."); +} + +var actionStep = _factory.CreateRunStep( + cmd.Script, + cmd.Id, // NEW + cmd.Name, + // ... +); +``` + +### Command Examples + +```bash +# Add step with custom ID +steps add run "echo hello" --id greet --name "Greeting" + +# Reference in later step +steps add run "echo ${{ steps.greet.outputs.result }}" + +# Duplicate ID returns error +steps add run "echo bye" --id greet +# Error: Step with ID 'greet' already exists +``` + +### Testing + +- [ ] `steps add run "echo test" --id my_step` creates step with ID `my_step` +- [ ] Step ID appears correctly in `steps list` output +- [ ] Attempting duplicate ID returns clear error +- [ ] Omitting `--id` still generates `_dynamic_` IDs +- [ ] ID is correctly set on the underlying `ActionStep.Name` property + +--- + +## Chunk 3: Help Commands (`--help`) + +**Goal:** Add `--help` flag support to provide usage information for all step commands. + +### Design + +| Command | Output | +|---------|--------| +| `steps` | List of available subcommands | +| `steps --help` | Same as above | +| `steps add --help` | Help for `add` command (shows `run` and `uses` subcommands) | +| `steps add run --help` | Help for `add run` with all options | +| `steps add uses --help` | Help for `add uses` with all options | +| `steps edit --help` | Help for `edit` command | +| `steps remove --help` | Help for `remove` command | +| `steps move --help` | Help for `move` command | +| `steps list --help` | Help for `list` command | +| `steps export --help` | Help for `export` command | + +**Output format:** Text only (no JSON support needed) + +### Files to Modify + +| File | Changes | +|------|---------| +| `StepCommandParser.cs` | Add `HelpCommand` class; detect `--help` flag and return appropriate help command | +| `StepCommandHandler.cs` | Add `HandleHelpCommand()` with help text for each command | + +### Implementation Details + +**StepCommandParser.cs:** + +```csharp +// Add new command class: +public class HelpCommand : StepCommand +{ + /// + /// The command to show help for (null = top-level help) + /// + public string Command { get; set; } + + /// + /// Sub-command if applicable (e.g., "run" for "steps add run --help") + /// + public string SubCommand { get; set; } +} + +// Modify ParseReplCommand to detect --help: +private StepCommand ParseReplCommand(string input) +{ + var tokens = Tokenize(input); + + // Handle bare "steps" command + if (tokens.Count == 1 && tokens[0].Equals("steps", StringComparison.OrdinalIgnoreCase)) + { + return new HelpCommand { Command = null }; + } + + if (tokens.Count < 2 || !tokens[0].Equals("steps", StringComparison.OrdinalIgnoreCase)) + { + throw new StepCommandException(StepCommandErrors.ParseError, + "Invalid command format. Expected: steps [args...]"); + } + + // Check for --help anywhere in tokens + if (tokens.Contains("--help") || tokens.Contains("-h")) + { + return ParseHelpCommand(tokens); + } + + var subCommand = tokens[1].ToLower(); + // ... existing switch ... +} + +private HelpCommand ParseHelpCommand(List tokens) +{ + // Remove --help/-h from tokens + tokens.RemoveAll(t => t == "--help" || t == "-h"); + + // "steps --help" or "steps" + if (tokens.Count == 1) + { + return new HelpCommand { Command = null }; + } + + // "steps add --help" + var cmd = tokens[1].ToLower(); + + // "steps add run --help" + string subCmd = null; + if (tokens.Count >= 3 && (cmd == "add")) + { + subCmd = tokens[2].ToLower(); + if (subCmd != "run" && subCmd != "uses") + subCmd = null; + } + + return new HelpCommand { Command = cmd, SubCommand = subCmd }; +} +``` + +**StepCommandHandler.cs:** + +```csharp +private StepCommandResult HandleHelpCommand(HelpCommand cmd) +{ + string helpText = (cmd.Command, cmd.SubCommand) switch + { + (null, _) => GetTopLevelHelp(), + ("add", null) => GetAddHelp(), + ("add", "run") => GetAddRunHelp(), + ("add", "uses") => GetAddUsesHelp(), + ("edit", _) => GetEditHelp(), + ("remove", _) => GetRemoveHelp(), + ("move", _) => GetMoveHelp(), + ("list", _) => GetListHelp(), + ("export", _) => GetExportHelp(), + _ => $"Unknown command: {cmd.Command}" + }; + + return new StepCommandResult + { + Success = true, + Message = helpText + }; +} + +private string GetTopLevelHelp() => @" +steps - Manipulate job steps during debug session + +COMMANDS: + list Show all steps with status + add Add a new step (run or uses) + edit Modify a pending step + remove Delete a pending step + move Reorder a pending step + export Generate YAML for modified steps + +Use 'steps --help' for more information about a command. +".Trim(); + +private string GetAddHelp() => @" +steps add - Add a new step to the job + +USAGE: + steps add run