Merge dev into main: model-policy fallback and clear review error surfacing - #107
Merged
Conversation
…he selected model When an org's model policy disallows the model chosen for review / security / fill flows (e.g. the review_depth presets gpt-5.2 or kimi-k2.6), runs previously died with an opaque 403 and the tracking comment only said 'Droid encountered an error'. - Pre-flight: prepare steps now fetch the org model policy via /api/organization/managed-settings and omit --model (falling back to the org default model) when the selected model is not allowed, surfacing a notice in the tracking comment. Fails open on any lookup error. - Reactive: base-action detects the model-policy 403 in the exec result and retries once without --model / --reasoning-effort. - Error surfacing: base-action now exposes the final result error as an error_message output, and update-comment-link renders it in the tracking comment with an actionable hint for model-policy 403s. Drive-bys: fix pre-existing duplicate type import in src/core/review/artifacts/write.ts (broke tsc) and prettier formatting in gitlab/examples/README.md (broke format:check). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
… tracking comment A failed automatic review kept 'Droid is reviewing code and running a security check…' below the error header, which made the failure comment read as still in progress. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…error dumps An unrecognized --model value now retries with the organization's default model (same as a policy-blocked model) instead of failing the run. When a model error still surfaces, the CLI's full available-models dump is condensed to the meaningful lines and the comment hints link to https://docs.factory.ai/models. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
feat(review): graceful fallback when org model policy blocks the selected model + clear review errors
Contributor
|
Droid finished @factory-nizar's task —— View job Overall looks solid and improves model-policy fallback plus error surfacing across the action. One correctness issue remains in the base-action retry loop: result-event state can leak across attempts and produce misleading fallback/error behavior. |
| const runDroidOnce = (): Promise<number> => { | ||
| const droidProcess = spawn(droidExecutable, config.droidArgs, { | ||
| stdio: ["ignore", "pipe", "inherit"], | ||
| stderrTail = ""; |
Contributor
There was a problem hiding this comment.
[P1] Reset lastResultEvent between retry attempts
lastResultEvent is not cleared per attempt, so a later retry that exits before emitting a type: "result" event can cause stale result data from a previous attempt to drive the retry decision and the final error_message. Reset it at the start of runDroidOnce so each attempt’s diagnostics reflect only that attempt.
Suggested change
| stderrTail = ""; | |
| stderrTail = ""; | |
| lastResultEvent = null; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the latest changes from
devintomain.Highlights
GET /api/organization/managed-settings) and omit--model/--reasoning-effortwhen the selected model is disallowed, sodroid execuses the org default. Fails open on any lookup error. Wired into review candidates + validator, security review/scan, PR description fill, and the GitLab prepare entrypoint.base-actiondetects the model-policy 403 (including the explicit opt-in variant) in the exec result and retries once without--model; invalid/unrecognized model ids fall back the same way instead of failing the run.base-actionexposes the final result error (or a condensed stderr tail) as anerror_messageoutput, rendered in the tracking comment with actionable TIP hints linking to https://docs.factory.ai/models; successful fallbacks render a NOTE so model switches are never silent; stale 'Droid is reviewing code…' progress text is stripped from final comments.review_depthpresets still selected blocked models.Commits included
Validation
base-actionPlease review and merge once CI passes.