Execution review auto open

[krystian-panek-vmltech]

fixes #302

[Copilot]

🟡 Changes recommended

Execution completion and auto-open state must be scoped and reset by execution ID.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds configurable automatic opening of review outputs after successful script executions.

Changes:

• Adds manual/auto policy configuration.
• Tracks completed executions for auto-open behavior.
• Opens the review dialog when outputs are available.
• Adds the corresponding OSGi setting.

File summaries

File	Summary
ui.frontend/src/types/main.ts	Adds review policy state and defaults.
ui.frontend/src/pages/ExecutionView.tsx	Determines when automatic opening applies.
ui.frontend/src/hooks/execution.ts	Tracks execution completion state.
ui.frontend/src/components/ExecutionReviewOutputsButton.tsx	Implements automatic dialog opening.
core/src/main/java/dev/vml/es/acm/core/gui/SpaSettings.java	Adds the OSGi configuration option.

Review details

Suppressed comments (1)

ui.frontend/src/hooks/execution.ts:17

• This completion flag is not scoped to executionId and is never reset when the route is reused for another execution. A previous execution can therefore make autoOpenReviewOutputs true for the next execution even though this hook did not observe that execution complete; associate the completion marker with the current ID and reset the polling state when the ID changes.

  const [justCompleted, setJustCompleted] = useState<boolean>(false);

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Moderate issues remain around execution-ID scoping, stale polling, and one-shot auto-open guards.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

ui.frontend/src/components/ExecutionReviewOutputsButton.tsx:39

• The local boolean still makes this guard once per mounted button, not once per execution. If the route supplies a new execution to the same component instance, the parent can pass autoOpen=true for the new ID, but this effect is blocked by the ref and does not depend on execution.id; key the ref by execution ID (or remove this second guard and let the parent own it).

    if (autoOpen && outputValues.length > 0 && !autoOpenedRef.current) {
      autoOpenedRef.current = true;
      setDialogOpen(true);
    }
  }, [autoOpen, outputValues.length]);

ui.frontend/src/hooks/execution.ts:17

• justCompleted is never reset or associated with an execution ID. ExecutionView is mounted on a parameterized route, so changing from one execution to another can leave this flag true; if the new execution is already succeeded, the autoOpenReview condition can open its outputs even though this execution was not observed transitioning to completion. Reset the completion state when executionId changes or return the completed ID and compare it with the current execution.

  const [justCompleted, setJustCompleted] = useState<boolean>(false);

ui.frontend/src/pages/ExecutionView.tsx:72

• Because route changes keep the same ExecutionView instance, an in-flight poll for the previous executionId can update execution after navigation. This condition can then see the old successful execution plus justCompleted and open its review dialog on the new URL. Tie the completion/open decision to the current ID (for example, require execution.id === executionId) and reset/cancel polling when executionId changes.

    appState.spaSettings.executionReviewOutputsPolicy === 'auto' &&
    isExecutableScript(execution.executable.id) &&
    execution.status === ExecutionStatus.SUCCEEDED &&
    justCompleted &&
    autoOpenedOutputsIdRef.current !== execution.id;

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved issues affect execution-specific auto-opening and the policy fallback behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

ui.frontend/src/pages/ExecutionView.tsx:30

• ExecutionStatus is not referenced in this file, so the frontend lint configuration reports a new @typescript-eslint/no-unused-vars warning. Remove it from the import.

ui.frontend/src/components/ExecutionReviewOutputsButton.tsx:39

• autoOpenedRef is scoped to the mounted button rather than to execution.id. When ExecutionView navigates to another execution without remounting, the new autoOpen signal can be true but this ref remains true, so the dialog is never opened for that execution. Track the last auto-opened execution ID here and include execution.id in the effect dependencies.

    if (autoOpen && outputValues.length > 0 && !autoOpenedRef.current) {
      autoOpenedRef.current = true;
      setDialogOpen(true);
    }
  }, [autoOpen, outputValues.length]);

ui.frontend/src/hooks/execution.ts:89

• This effect consumes the auto-open signal as soon as ExecutionView renders, but the actual dialog consumer is inside the Output tab. If the execution completes while another tab is selected and that panel is not mounted, switching to Output later receives autoOpen === false, so the configured auto-open is lost. Keep the completion pending per execution until the review control has actually opened/acknowledged it, rather than marking it consumed here.

  useEffect(() => {
    if (autoOpen && execution) {
      autoOpenedIdRef.current = execution.id;
    }
  }, [autoOpen, execution]);

ui.frontend/src/types/main.ts:56

• The client fallback is inconsistent with the server configuration: SpaSettings.Config.executionReviewOutputsPolicy() defaults to "auto", but this initial state uses "manual". If the state request fails or has not completed yet, the UI suppresses auto-open despite the configured/default policy. Keep the fallback aligned with the server default.

    executionReviewOutputsPolicy: 'manual',

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite