Conversation
Greptile SummaryThis is a large experimental refactoring that replaces direct LLM API calls (Eino framework, TEI embedder/reranker) with subprocess invocations of installed AI CLI tools (Claude Code, Gemini CLI, Codex CLI). The architecture shift means users no longer need to configure API keys separately — TaskWing orchestrates whichever AI CLI is already on Key changes:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Bootstrap/Plan as Bootstrap / Plan cmd
participant Runner as runner.Pool
participant Claude as claudeRunner
participant Gemini as geminiRunner
participant Codex as codexRunner
participant Knowledge as knowledge.Service
participant SQLite as memory.SQLiteStore
User->>Bootstrap/Plan: taskwing bootstrap / plan
Bootstrap/Plan->>Runner: DetectAndCreateRunners()
Runner-->>Bootstrap/Plan: []Runner (Claude > Gemini > Codex priority)
Bootstrap/Plan->>Runner: Pool.Execute(jobs) [parallel analysis]
par Focus area 1
Runner->>Claude: Invoke(BootstrapAnalysisPrompt)
Claude-->>Runner: InvokeResult (JSON)
and Focus area 2
Runner->>Gemini: Invoke(BootstrapAnalysisPrompt)
Gemini-->>Runner: InvokeResult (JSON)
and Focus area 3
Runner->>Codex: Invoke(BootstrapAnalysisPrompt)
Codex-->>Runner: InvokeResult (JSON)
end
Runner-->>Bootstrap/Plan: []JobResult
Bootstrap/Plan->>Bootstrap/Plan: result.Decode() → BootstrapAnalysis
Bootstrap/Plan->>Knowledge: AddNode(finding) per result
Knowledge->>SQLite: UpsertNodeBySummary(node)
SQLite-->>Knowledge: (nodeID, error)
Bootstrap/Plan->>Runner: Pool.ExecuteSequential(taskJobs) [task execution]
Runner->>Claude: InvokeWithFiles(TaskExecutionPrompt)
Claude-->>Runner: InvokeResult
Runner-->>Bootstrap/Plan: []JobResult
Bootstrap/Plan-->>User: Summary panel (findings, symbols, duration)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| // ExecuteSequential runs jobs one at a time using the first runner. | ||
| // Use this for file-modifying operations (task execution) where order matters. | ||
| func (p *Pool) ExecuteSequential(ctx context.Context, jobs []Job) []JobResult { | ||
| if len(p.runners) == 0 { | ||
| return []JobResult{{Error: fmt.Errorf("no runners available")}} | ||
| } | ||
|
|
||
| runner := p.runners[0] | ||
| results := make([]JobResult, len(jobs)) | ||
|
|
||
| for i, job := range jobs { | ||
| if ctx.Err() != nil { | ||
| results[i] = JobResult{ | ||
| JobID: job.ID, | ||
| Error: ctx.Err(), | ||
| } | ||
| break | ||
| } | ||
|
|
||
| result, err := runner.InvokeWithFiles(ctx, job.Request) | ||
| results[i] = JobResult{ | ||
| JobID: job.ID, | ||
| Result: result, | ||
| Error: err, | ||
| RunnerType: runner.Type(), | ||
| } | ||
|
|
||
| // Stop on failure | ||
| if err != nil { | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Zero-value results for unexecuted jobs are misleading
When ExecuteSequential breaks early — either due to context cancellation or a job error — the remaining entries in results are left as zero-value JobResult{} values: nil error, nil result, and an empty JobID. Callers that iterate over all returned results and check only result.Error != nil will silently treat those unexecuted jobs as succeeded.
Consider explicitly marking unexecuted entries, e.g.:
// After the loop, fill in unexecuted jobs
for j := i + 1; j < len(jobs); j++ {
results[j] = JobResult{
JobID: jobs[j].ID,
Error: fmt.Errorf("job not executed: previous job failed"),
}
}Or at minimum document that callers must check for an empty JobID to detect un-run jobs.
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Scanner error is never checked
After the scan loop exits, scanner.Err() is never inspected. If the process's stdout pipe closes with an I/O error (e.g., the subprocess was killed), the error is silently swallowed and lastAssistantText may be incomplete with no indication of failure.
| } | |
| } | |
| } | |
| if err := scanner.Err(); err != nil { | |
| return nil, fmt.Errorf("codex stdout scan: %w", err) | |
| } | |
| waitErr := cmd.Wait() |
| // isRetryable returns true for errors that warrant a retry. | ||
| func isRetryable(err error) bool { | ||
| if err == nil { | ||
| return false | ||
| } | ||
| msg := err.Error() | ||
| // Retry on timeouts, CLI failures, and JSON parse errors | ||
| for _, keyword := range []string{"timeout", "timed out", "JSON", "json", "exit status", "signal"} { | ||
| if strings.Contains(msg, keyword) { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
Overly broad retry keywords catch permanent failures
"exit status" and "signal" are matched as retryable, but virtually every subprocess failure wraps one of those phrases (e.g., "claude invocation failed (exit 1)", "exit status 127" for a missing binary, or "signal: killed" after a manual kill). This means auth failures, invalid arguments, missing binaries, and out-of-memory kills are all retried up to defaultRetries (4) times before finally failing — wasting minutes of wall-clock time.
Consider narrowing the set to genuinely transient signals/codes, or at minimum removing "exit status" and keeping only specific known-transient patterns:
| // isRetryable returns true for errors that warrant a retry. | |
| func isRetryable(err error) bool { | |
| if err == nil { | |
| return false | |
| } | |
| msg := err.Error() | |
| // Retry on timeouts, CLI failures, and JSON parse errors | |
| for _, keyword := range []string{"timeout", "timed out", "JSON", "json", "exit status", "signal"} { | |
| if strings.Contains(msg, keyword) { | |
| return true | |
| } | |
| } | |
| for _, keyword := range []string{"timeout", "timed out", "connection refused", "connection reset"} { | |
| if strings.Contains(msg, keyword) { | |
| return true | |
| } | |
| } |
1 participant
No description provided.