Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/self_review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,4 @@ jobs:
github_token: ${{ steps.app-token.outputs.token }}
openrouter_api_key: ${{ secrets.OPENROUTER_KEY }}
model: ${{ vars.OPENROUTER_MODEL || 'anthropic/claude-sonnet-4-6' }}
request_timeout_seconds: ${{ vars.UMM_REQUEST_TIMEOUT_SECONDS }}
39 changes: 20 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,25 +74,26 @@ The `@umm review` comment trigger lets you re-request a review on any PR by comm

## Inputs

| Input | Default | Description |
| ----------------------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `github_token` | _(required)_ | Token for fetching the diff and posting the review. A GitHub App installation token keeps the bot identity. |
| `openrouter_api_key` | _(required)_ | OpenRouter API key |
| `model` | `anthropic/claude-sonnet-4-6` | OpenRouter model slug exactly as listed on openrouter.ai/models |
| `fallback_model` | `""` | Model to retry with if the primary model fails the structured-output ladder |
| `max_findings` | `""` _(uncapped)_ | Cap on posted findings, highest severity first. Empty = all validated findings post. |
| `severity_threshold` | `low` | Minimum severity to post: `low` \| `medium` \| `high` \| `critical` |
| `conventions_file` | `AGENTS.md` | Repo-relative path to the conventions file included in the prompt |
| `phases` | `combined` | Review phases to run. V1 supports: `combined` |
| `context_budget_tokens` | `80000` | Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) |
| `trace_related_files` | `true` | Enable heuristic context scanning — import-tracing for caller regressions and mention-matching for doc staleness detection. Does not affect `priority_docs` |
| `priority_docs` | `README.md` | Comma-separated repo-relative paths always included in review context, independent of `trace_related_files`. Shares the doc token budget with mention-matched docs when both are active. Empty = disabled |
| `max_scan_files` | `5000` | Maximum files to walk during workspace scan for related file and doc detection |
| `max_scan_bytes` | `262144` | Maximum byte size of a single file to include in the workspace scan |
| `max_related_files` | `8` | Maximum import-traced related files to include in review context |
| `max_related_docs` | `4` | Maximum mention-matched documentation files to include in review context (excludes priority docs) |
| `cost_summary` | `true` | Write a per-run cost report (model, prompt/completion tokens, USD) to the workflow step summary |
| `pr_number` | `""` | PR number override — required only when the triggering event does not identify a PR directly |
| Input | Default | Description |
| ------------------------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `github_token` | _(required)_ | Token for fetching the diff and posting the review. A GitHub App installation token keeps the bot identity. |
| `openrouter_api_key` | _(required)_ | OpenRouter API key |
| `model` | `anthropic/claude-sonnet-4-6` | OpenRouter model slug exactly as listed on openrouter.ai/models |
| `fallback_model` | `""` | Model to retry with if the primary model fails the structured-output ladder |
| `request_timeout_seconds` | `600` | Per-attempt timeout for a single model request, in seconds. A timed-out attempt aborts the HTTP call and advances the retry/fallback ladder instead of hanging the job |
| `max_findings` | `""` _(uncapped)_ | Cap on posted findings, highest severity first. Empty = all validated findings post. |
| `severity_threshold` | `low` | Minimum severity to post: `low` \| `medium` \| `high` \| `critical` |
| `conventions_file` | `AGENTS.md` | Repo-relative path to the conventions file included in the prompt |
| `phases` | `combined` | Review phases to run. V1 supports: `combined` |
| `context_budget_tokens` | `80000` | Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) |
| `trace_related_files` | `true` | Enable heuristic context scanning — import-tracing for caller regressions and mention-matching for doc staleness detection. Does not affect `priority_docs` |
| `priority_docs` | `README.md` | Comma-separated repo-relative paths always included in review context, independent of `trace_related_files`. Shares the doc token budget with mention-matched docs when both are active. Empty = disabled |
| `max_scan_files` | `5000` | Maximum files to walk during workspace scan for related file and doc detection |
| `max_scan_bytes` | `262144` | Maximum byte size of a single file to include in the workspace scan |
| `max_related_files` | `8` | Maximum import-traced related files to include in review context |
| `max_related_docs` | `4` | Maximum mention-matched documentation files to include in review context (excludes priority docs) |
| `cost_summary` | `true` | Write a per-run cost report (model, prompt/completion tokens, USD) to the workflow step summary |
| `pr_number` | `""` | PR number override — required only when the triggering event does not identify a PR directly |

## Outputs

Expand Down
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ inputs:
description: Model to retry with if the primary model fails the structured-output ladder
required: false
default: ""
request_timeout_seconds:
description: Per-attempt timeout for a single model request, in seconds. A timed-out attempt aborts the HTTP call and advances the retry/fallback ladder instead of hanging the job. Empty = the 600 default, so workflows can wire an unset repo variable directly
required: false
default: "600"
max_findings:
description: Cap on posted findings, highest severity first. Empty = uncapped (all validated findings post)
required: false
Expand Down
32 changes: 32 additions & 0 deletions src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const makeRawInputs = (overrides: Partial<RawInputs> = {}): RawInputs => ({
openrouterApiKey: "sk-or-testkey",
model: "anthropic/claude-sonnet-4-6",
fallbackModel: "",
requestTimeoutSeconds: "600",
maxFindings: "",
severityThreshold: "low",
conventionsFile: "AGENTS.md",
Expand All @@ -31,6 +32,7 @@ describe("parseConfig", () => {
openrouterApiKey: "sk-or-testkey",
model: "anthropic/claude-sonnet-4-6",
fallbackModel: "",
requestTimeoutSeconds: 600,
maxFindings: undefined,
severityThreshold: "low",
conventionsFile: "AGENTS.md",
Expand Down Expand Up @@ -73,6 +75,36 @@ describe("parseConfig", () => {
)
})

it("falls back to 600 for an empty request_timeout_seconds", () => {
// Workflows wiring a bare unset repo variable pass "" — that must mean
// "use the default", not a validation failure
const config = parseConfig(makeRawInputs({ requestTimeoutSeconds: "" }))

expect(config.requestTimeoutSeconds).toBe(600)
})

it("rejects a zero request_timeout_seconds", () => {
expect(() =>
parseConfig(makeRawInputs({ requestTimeoutSeconds: "0" })),
).toThrow('requestTimeoutSeconds: "0" is not a positive integer')
})

it("accepts a request_timeout_seconds at the timer-cap ceiling", () => {
const config = parseConfig(
makeRawInputs({ requestTimeoutSeconds: "2147483" }),
)

expect(config.requestTimeoutSeconds).toBe(2147483)
})

it("rejects a request_timeout_seconds whose milliseconds exceed the timer cap", () => {
expect(() =>
parseConfig(makeRawInputs({ requestTimeoutSeconds: "2147484" })),
).toThrow(
'requestTimeoutSeconds: "2147484" exceeds the 2147483-second cap (2^31−1 ms timer limit)',
)
})

it("rejects an empty context_budget_tokens", () => {
expect(() =>
parseConfig(makeRawInputs({ contextBudgetTokens: "" })),
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/orchestrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ const baseConfig: ActionConfig = {
openrouterApiKey: "sk-test",
model: "test/model",
fallbackModel: "",
requestTimeoutSeconds: 600,
maxFindings: undefined,
severityThreshold: "low",
conventionsFile: "AGENTS.md",
Expand Down
24 changes: 24 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,35 @@ const optionalPositiveInteger = z

const requiredPositiveInteger = z.string().transform(parsePositiveInteger)

/** Ceiling that keeps seconds × 1000 within the 2^31−1 ms timer cap —
* beyond it, timer implementations clamp the delay to ~1 ms and every
* request would time out instantly instead of being bounded. */
const maxTimeoutSeconds = 2_147_483

/** Mirrors the action.yml default — keep the two in sync. */
const defaultRequestTimeoutSeconds = 600

const timerSafeSeconds = z.string().transform((value, ctx) => {
// Empty string means "not provided": workflows wiring a bare unset repo
// variable pass "", which would otherwise override the action.yml default.
if (!value) return defaultRequestTimeoutSeconds
const parsed = parsePositiveInteger(value, ctx)
if (parsed > maxTimeoutSeconds) {
Comment thread
aliasunder marked this conversation as resolved.
ctx.addIssue({
code: "custom",
message: `"${value}" exceeds the ${maxTimeoutSeconds}-second cap (2^31−1 ms timer limit)`,
})
return z.NEVER
}
return parsed
})

const configSchema = z.object({
githubToken: z.string().min(1, "github_token is required"),
openrouterApiKey: z.string().min(1, "openrouter_api_key is required"),
model: z.string().min(1, "model must not be empty"),
fallbackModel: z.string(),
requestTimeoutSeconds: timerSafeSeconds,
maxFindings: optionalPositiveInteger,
// Shape-only, like phases: the value is validated by its domain owner
// (review/finding.ts resolveSeverityThreshold) at startup
Expand Down
6 changes: 5 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const collectRawInputs = (): RawInputs => ({
openrouterApiKey: core.getInput("openrouter_api_key", { required: true }),
model: core.getInput("model"),
fallbackModel: core.getInput("fallback_model"),
requestTimeoutSeconds: core.getInput("request_timeout_seconds"),
maxFindings: core.getInput("max_findings"),
severityThreshold: core.getInput("severity_threshold"),
conventionsFile: core.getInput("conventions_file"),
Expand Down Expand Up @@ -78,7 +79,10 @@ try {
generateFindings: createPromptedGenerateFindings(
{
openrouterClient: createOpenRouterClient(
{ sdk: new OpenRouter({ apiKey: config.openrouterApiKey }) },
{
sdk: new OpenRouter({ apiKey: config.openrouterApiKey }),
requestTimeoutMs: config.requestTimeoutSeconds * 1000,
Comment thread
aliasunder marked this conversation as resolved.
},
logger,
),
model: config.model,
Expand Down
30 changes: 21 additions & 9 deletions src/openrouter/__tests__/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,14 @@ const makeSdkStub = ({
sendResponses: StubResponse[]
generationResponses?: StubResponse[]
}) => {
const sendCalls: { chatRequest: ChatRequestSubset }[] = []
const generationCalls: { id: string }[] = []
const sendCalls: {
chatRequest: ChatRequestSubset
options: { timeoutMs?: number } | undefined
}[] = []
const generationCalls: {
id: string
options: { timeoutMs?: number } | undefined
}[] = []

const takeNext = (
queue: StubResponse[],
Expand All @@ -62,14 +68,14 @@ const makeSdkStub = ({

const sdk: OpenRouterLike = {
chat: {
send: async (request) => {
sendCalls.push(request)
send: async (request, options) => {
sendCalls.push({ ...request, options })
return takeNext(sendResponses, sendCalls.length, "chat.send")
},
},
generations: {
getGeneration: async (request) => {
generationCalls.push(request)
getGeneration: async (request, options) => {
generationCalls.push({ ...request, options })
return takeNext(
generationResponses,
generationCalls.length,
Expand All @@ -84,7 +90,10 @@ const makeSdkStub = ({

const makeClient = (stub: { sdk: OpenRouterLike }) => {
const logger = createTestLogger()
const client = createOpenRouterClient({ sdk: stub.sdk }, logger)
const client = createOpenRouterClient(
{ sdk: stub.sdk, requestTimeoutMs: 45_000 },
logger,
)
return { client, logger }
}

Expand All @@ -99,7 +108,7 @@ const requestParams = {
}

describe("requestReview", () => {
it("sends the strict json_schema response format with the review schema", async () => {
it("sends the strict json_schema response format with the review schema and per-attempt timeout", async () => {
const stub = makeSdkStub({ sendResponses: [{ value: acceptedChatResult }] })
const { client } = makeClient(stub)

Expand All @@ -124,6 +133,7 @@ describe("requestReview", () => {
},
stream: false,
},
options: { timeoutMs: 45_000 },
},
])
})
Expand Down Expand Up @@ -400,7 +410,9 @@ describe("requestReview", () => {

const result = await client.requestReview(requestParams)

expect(stub.generationCalls).toEqual([{ id: "gen-no-cost" }])
expect(stub.generationCalls).toEqual([
{ id: "gen-no-cost", options: { timeoutMs: 45_000 } },
])
expect(result.attempts[0]?.costUsd).toBe(0.0399)
})

Expand Down
Loading