Skip to content

Merge dev into main: structured review session tags and no retry on usage limits - #143

Merged
factory-nizar merged 17 commits into
mainfrom
dev
Sep 19, 2026
Merged

factory-nizar merged 17 commits into
mainfrom
dev

Conversation

@factory-nizar

Copy link
Copy Markdown
Contributor

Promotes dev to main for the v8 release. Clean merge (git merge-tree reports no conflicts; every commit on main is already in dev).

Included

After merge

Create the v8 GitHub release on main (v7 is at #127; main also carries the unreleased #129, #131, #136, #140 dev merges).

factory-nizar and others added 17 commits August 24, 2026 16:36
`@octokit/graphql` resolves `response.data.data` and only throws when the
body carries an `errors` array, so an HTTP 200 whose body is not a GraphQL
envelope resolves `undefined`. `fetchPRBranchData` dereferenced that value
directly, turning a transient GitHub answer into
`TypeError: undefined is not an object (evaluating 'prResult.repository')`
that killed the `Prepare validator` step of Droid Auto Review and skipped
the review pass that posts the comments.

Route both fetchers in this module through one helper that checks the
response shape before reading it, retries the request via `utils/retry.ts`
per the repo convention for GitHub API calls, and reports the HTTP status,
content type and body preview captured through a request hook, since
`@octokit/graphql` otherwise discards the body that would explain the
failure. A present-but-null `repository` stays a stable "not found" answer
and is not retried.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Add a dedicated hidden marker to PR review tracking comments and preserve it across sanitized summary updates.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Replace the review-specific marker with a generic PR validation marker carrying an explicit source value for future validation comment types.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Resolve a typed run identity before tracking comments are created, pass it through each mode and MCP server, and derive PR validation metadata independently of comment presentation.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Check automatic security run-once state before creating the tracking comment. Skip comment creation for security-only no-op runs and downgrade dual-review comments when security will not run.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Replace the single-value validation source with exact run-type metadata. Mark code review, security review, and full security scan comments while keeping fill and CI Steward outside the PR validation marker.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Add a dedicated droid-default run type for bare @droid commands and return null when no command or automatic mode identifies a run. Propagate unresolved runs without exporting or marking a false review identity.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Distinguish PR issue and inline comment markers so review consumers can identify both the comment surface and originating run type.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Keep shared code/security classification across sticky reuse, progress updates, and final summaries instead of replacing or losing the marker.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
feat(review): tag tracking comments for classification
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…-the/2a92c804-520e-4c49-9184-ce275ab392d9

fix(github): survive GraphQL responses that carry no envelope
Every review `droid exec` carried a bare `--tag code-review`, so the
warehouse had to identify review sessions by matching prompt text and
pair the two passes by parsing "PR #N in owner/repo" out of the prompt.
That breaks whenever the prompt is reworded, never matched GitLab's
"MR !N", and truncates repo names at the first dot.

The tag now carries metadata: pass (candidates|validator), reviewType
(code|security), platform (github|gitlab), repo, pr, runId and
runAttempt. The name stays `code-review` so existing filters keep
working.

- GitHub: `githubReviewSessionTagArg` builds the fragment from the
  parsed context; quoted with shell-quote so `droid_args` splits it as
  one token.
- GitLab: gitlab-prepare writes the per-pass JSON into the resolved-env
  shim; the template passes it as its own quoted `--tag` argument with
  a bare-name fallback for shims from older prepare versions.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
runDroid retried every non-zero exit three times with backoff. A 402 from
the Factory API (5-hour/weekly/monthly usage limit, exhausted Extra Usage,
credit limit) or a BYOK provider's empty balance cannot clear in the
seconds between attempts, so each failing review opened three failed
sessions and the workflow reruns multiplied that further. Sep 10-17 review
telemetry showed an average of 4.7 pass-1 sessions per PR-day that ended on
a usage limit versus 1.5 otherwise.

In stream-json mode the CLI reports the fatal error as
{type:"error",source:"agent_loop",message:"402 {...}"}; there is no result
event. Capture that message per attempt, stop after the first attempt when
it is a usage limit, and surface the body's human-readable detail as the
step's error_message so the PR comment says which limit was hit. The same
captured message now also feeds the existing 403 model-policy fallback and
the generic error_message, which previously only looked at result events.

Test hygiene: run-droid-mcp.test.ts installs a process-wide
mock.module("child_process") that leaked into every later test file; it now
restores the real module in afterAll.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
fix(base-action): do not retry droid exec after a usage-limit failure
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
feat(review): send structured session tag on every review pass
@factory-droid

factory-droid Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

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


Validated two actionable classification bugs: sticky-comment fallback can reuse a combined marker on a new comment, and validator preparation converts an intentionally unclassified custom-trigger run into a code review.

2 inline comments posted

@factory-nizar
factory-nizar merged commit ad22b1d into main Sep 19, 2026
9 checks passed
repo,
issue_number: context.entityNumber,
body: initialBody,
body: issueCommentBody,

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.

[P2] Rebuild the marker when sticky-comment fallback creates a new comment

After a sticky comment is found, issueCommentBody is mutated to combine the previous and current run types. If updating that comment then fails, the fallback creates a separate comment from this mutated body, so a comment belonging only to the current run is incorrectly classified as a combined review-and-security comment. Build the fallback body from createInitialBody("issue-comment") instead.

throw new Error("prepare-validator requires a pull request context");
}
const runType =
parseDroidRunType(process.env.DROID_EXEC_RUN_TYPE) ?? DroidRunType.Review;

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.

[P2] Preserve an intentionally unclassified run type

A review triggered through a custom phrase can resolve to null, and the candidate pass intentionally leaves DROID_EXEC_RUN_TYPE unset, but this fallback converts that absence to droid-review for the validator and final posting steps. The same run therefore starts unclassified and ends classified as a code review, contrary to the new null-run-type path; carry an explicit null sentinel between passes instead of defaulting every missing value to Review.

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.

2 participants