chennaji9 opened a new pull request, #6768:
URL: https://github.com/apache/rocketmq-dashboard/pull/6768
### Which Issue(s) This PR Fixes
None — small self-contained fix found by inspection (no issue tracked).
### Brief Description
`QoderAgentProvider` overrides neither streaming method, so a hosted Qoder
run falls back to the SPI default, which adapts the deprecated text channel —
i.e. `complete()` on the worker thread. `complete()` keeps its process private,
so `AgentStreamOptions.getProcessSink()` (wired by `AiRunExecutor` to the run
handle) was never called: a stop could not kill the real process tree, it
depended on a thread interrupt landing on the reader while any CLI-spawned
child kept running. `AgentRunHandle`'s whole attach-then-snapshot-descendants
design was dead code on this path, and the per-run `timeout` in the options was
ignored (the fixed 300s `complete()` budget applied instead).
This overrides `streamEvents` to run the same one-shot completion, but own
the process the way `ClaudeCodeAgentProvider` does:
- the subprocess is attached to the run handle immediately after `start()`
and before any output is read, so a stop that races the start still finds the
process;
- stdout (the answer) and stderr (diagnostics) are drained on background
threads — the merged-stream deadlock rationale is preserved without letting
stderr leak into the answer, and an answers-only-on-stderr run now fails the
empty-completion check instead of passing SDK warnings off as the completion;
- the wall-clock budget comes from `options.timeoutOr(...)`; the workspace
dir and per-run `extraEnv` are honoured per the `AgentStreamOptions` contract.
The SPI default's degradation behaviour ("a provider that only implements
`complete()` still streams") is now pinned by a test on a neutral provider,
since qoder no longer relies on it.
### How Did You Test This Change?
`cd server && mvn -B -ntp test
-Dtest='QoderAgentProviderTest,CliAgentProviderTest,ClaudeCodeAgentProviderTest,AiRunExecutorTest,AgentRunHandleStopTest,PromptEnhancerTest'
-DfailIfNoTests=false` → **Tests run: 79, Failures: 0**.
New tests in `QoderAgentProviderTest` (fake CLI subprocess / recorded
process, no network, no real qodercli):
- `streamEventsAttachesTheProcessBeforeReadingOutputTest` — the run handle
receives the process before the first stdout byte is consumed;
- `streamEventsTimeoutDestroysAndStillReportsTheAttachedProcessTest` — a run
over budget destroys the process and the sink still saw it;
- `streamEventsKeepsCliDiagnosticsOutOfTheAnswerTest` — stderr noise stays
out of the answer;
- `streamEventsRejectsACompletionThatNeverReachedStdoutTest` — stderr-only
output is an empty completion, not an answer.
Red side: the diagnostics and empty-completion tests were run against the
unfixed provider (same test class restricted to the seams that exist there) and
fail — the merged-stream fallback returns the warning text as the answer and
never throws.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes (4 new red-green tests
+ SPI-default contract pin)
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(n/a — no UI change)
- [x] Architecture constraints stay green (same module, no new dependency;
the new entry mirrors the existing Claude provider pattern)
- [x] New source files carry the ASF license header (n/a — no new source
file)
- [x] Documentation touched where behaviour changed (n/a — the provider now
honours the documented `AgentStreamOptions` contract instead of bypassing it)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
