Skip to content

Merge dev into main: model-policy fallback and clear review error surfacing - #107

Merged
factory-nizar merged 4 commits into
mainfrom
dev
Jul 31, 2026
Merged

Merge dev into main: model-policy fallback and clear review error surfacing#107
factory-nizar merged 4 commits into
mainfrom
dev

Conversation

@factory-nizar

Copy link
Copy Markdown
Contributor

Brings the latest changes from dev into main.

Highlights

  • feat(review): Graceful fallback when the org model policy blocks the selected model (feat(review): graceful fallback when org model policy blocks the selected model + clear review errors #106)
    • Pre-flight: prepare steps fetch the org model policy (GET /api/organization/managed-settings) and omit --model/--reasoning-effort when the selected model is disallowed, so droid exec uses 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.
    • Reactive backstop: base-action detects 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.
    • Error surfacing: base-action exposes the final result error (or a condensed stderr tail) as an error_message output, 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.
    • Motivated by a customer whose review workflows died with an opaque 403 after tightening their model allowlist while review_depth presets still selected blocked models.

Commits included

  • 3a9aaeb feat(review): fall back on invalid model ids and condense model-list error dumps
  • 98bf871 fix(review): strip stale review/security progress text from the final tracking comment
  • 52c8513 feat(review): fall back gracefully when the org model policy blocks the selected model

Validation

  • 492 bun tests pass, typecheck and format:check clean in root and base-action
  • Verified end to end on factory-mono test PRs: policy/invalid-model fallback produces a passing check with a NOTE in the tracking comment (17310, 17311); genuine failures fail the check and surface the real error with hints (17303, 17305)

Please review and merge once CI passes.

factory-nizar and others added 4 commits July 30, 2026 17:54
…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
@factory-droid

factory-droid Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Droid finished @factory-nizar's task —— View job


Security Review

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.

@factory-nizar
factory-nizar merged commit 2041e7c into main Jul 31, 2026
3 checks passed
const runDroidOnce = (): Promise<number> => {
const droidProcess = spawn(droidExecutable, config.droidArgs, {
stdio: ["ignore", "pipe", "inherit"],
stderrTail = "";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant