fix: save engine stderr to temp log and show --exclude hint on failure#20
fix: save engine stderr to temp log and show --exclude hint on failure#20
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 309efbbd3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _, _ = f.WriteString(content) | ||
| return f.Name() |
There was a problem hiding this comment.
Check temp log writes before reporting saved output
writeTempLog unconditionally returns the temp file path even when WriteString fails, so buildEngineFailureError can tell users “Full engine output saved to …” when the log is actually empty or truncated. This is reproducible on ENOSPC or other write errors and removes the only place stderr is preserved in this flow, making failures harder to debug in exactly the environments where diagnostics are most needed.
Useful? React with 👍 / 👎.
| for _, f := range filterResult.ExcludedFiles { | ||
| if !excluded[f] { | ||
| candidates = append(candidates, f) |
There was a problem hiding this comment.
Exclude hint must avoid already pattern-excluded files
buildExcludeCandidates appends filterResult.ExcludedFiles directly, but those files were already excluded by git.Filter and therefore cannot be further reduced by adding --exclude again. When engine execution fails, this produces no-op remediation steps (for example default-excluded lockfiles), which can generate long misleading hints and distract from the actionable files that are actually still in the prompt.
Useful? React with 👍 / 👎.
Closes #19
Problem
When the engine subprocess fails on a large diff, the error shown to the user is not actionable:
engine command failed: exit status 1:(no information)Both cases left the user with no idea what went wrong or how to fix it.
Solution
engine.EngineError(new type ininternal/engine/cli.go)CLI.Generate()now returns a structured*EngineErroron non-zero exit instead of embedding stderr into the error string. TheStderrfield preserves the full output separately, accessible viaerrors.As.Actionable error message (
internal/app/app.go)When the engine fails,
buildEngineFailureError():/tmp/git-ai-commit-stderr-*.log) — even when stderr is empty. The file is kept for the user to inspect.--excludehint when the diff had truncated or pattern-excluded files, suggesting the likely culprits for a context window overflow.Example output:
Changes
internal/engine/cli.go—EngineErrortype;Generate()returns*EngineErroron failureinternal/engine/cli_test.go— unit tests forEngineErrorinternal/app/app.go—buildEngineFailureError,writeTempLog,buildExcludeCandidateshelpers;commitDiffnow also returnsgit.Resultinternal/app/app_test.go— unit tests for the new helpers