fix: check process start for stdin-less commands so the Executor queue can't stall

[annejan]

Summary

From the full src/ review — a HIGH-severity queue stall in Executor.

The bug

Executor::executeNext() only called waitForStarted() for commands that carry stdin input:

startProcess(i.app, i.args);
if (!i.input.isEmpty()) {
  if (!m_process.waitForStarted(-1)) { /* handle failure */ }
  ...write input...
}

A command without stdin — which is most git / gpg / pass invocations — that fails to start (missing binary, bad path, permissions) emits QProcess::errorOccurred(FailedToStart) but never finished(). The Executor advances its FIFO queue from the finished handler (which clears running), so that handler never ran: running stayed true forever and the entire queue stalled, freezing all subsequent password-store operations with no user feedback.

(The stdin case was already handled — see #1598.)

The fix

Run the start check for every command regardless of stdin. On failure, emit an error (deferred via a queued call so we don't re-enter a caller still on the stack), dequeue, clear running, and continue to the next item. The stdin-write path is unchanged.

Test

tst_executor::executeAsyncFailedToStartNoInputDoesNotStall queues a failing no-input command followed by a valid one and asserts the error is surfaced and the second command still completes. Verified to fail without the fix (the second command never runs — the queue stalls until the test times out).

Full executor suite (35) and integration suite (22) pass; zero doxygen warnings.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where a queued command that failed to start could leave the executor stuck and prevent later tasks from running.
  • Improved handling of failed process starts so the queue continues processing remaining items.
  • Ensured process input is handled consistently and the write channel is closed after each attempt.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: a3edb224-06f1-4460-91b9-b1c69c655cf7

📥 Commits

Reviewing files that changed from the base of the PR and between e450c23 and 394e29f.

📒 Files selected for processing (2)

• src/executor.cpp
• tests/auto/executor/tst_executor.cpp

---

📝 Walkthrough

Walkthrough

Refactors Executor::executeNext() so process-start failures are detected unconditionally (not only when stdin input exists), resetting internal state, emitting an error asynchronously, and recursing to process the remaining queue. A regression test verifies the queue no longer stalls when a no-stdin command fails to start.

Changes

Executor Queue Stall Fix

Layer / File(s)	Summary
executeNext refactor and failure handling src/executor.cpp	Adds early-return guard, unconditional waitForStarted check, failure cleanup (close write channel, clear running, dequeue, emit error via QueuedConnection, recurse), and stdin write handling before closing the write channel.
Regression test for no-stdin failure case tests/auto/executor/tst_executor.cpp	Adds executeAsyncFailedToStartNoInputDoesNotStall test slot verifying a single error emission and that a subsequent queued command still completes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• IJHack/QtPass#1497: Adds similar async Executor failure-handling test coverage ensuring queued commands still run instead of stalling.
• IJHack/QtPass#1599: Modifies the same Executor::executeNext() failure-to-start path with accompanying regression tests.
• IJHack/QtPass#1569: Relies on Executor::executeNext() failure-handling behavior via executeBlocking("wsl", ...) calls.

Suggested labels: size:M

Poem

A queue once stuck, refused to budge,
One failed launch and gave a nudge—
Now errors fly, the write shuts tight,
The next task hops in, ready to fight! 🐇
No more stalls beneath the moon,
Just hopping tasks, done real soon.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: checking startup for stdin-less commands to prevent the Executor queue from stalling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/executor-failed-to-start

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coveralls]

coverage: 56.72% (-0.006%) from 56.726% — fix/executor-failed-to-start into main

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.97%. Comparing base (e450c23) to head (394e29f).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1606      +/-   ##
==========================================
- Coverage   52.98%   52.97%   -0.02%     
==========================================
  Files          52       52              
  Lines        4371     4370       -1     
==========================================
- Hits         2316     2315       -1     
  Misses       2055     2055              

Flag	Coverage Δ
qtpass	52.97% <100.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.