Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7c99804
feat(review): add OpenJDK repository context
wenshao Aug 2, 2026
0d4fb87
refactor(review): extract repository context foundation
wenshao Aug 3, 2026
4d5ce37
fix(review): repair CI type guard and add manifest repository context
wenshao Aug 3, 2026
78ae250
Merge branch 'main' into feat/review-openjdk-context
wenshao Aug 3, 2026
f27c77d
fix(review): harden repository context per maintainer review
wenshao Aug 3, 2026
b1dd7dc
Merge remote-tracking branch 'origin/main' into feat/review-openjdk-c…
wenshao Aug 3, 2026
0625585
fix(review): skip unsafe related paths in manifest context (#8401)
wenshao Aug 3, 2026
e36195c
Merge branch 'main' into feat/review-openjdk-context
qwen-code-dev-bot Aug 3, 2026
6654180
fix(ci): align review timeout helper test with externalized variables…
qwen-code-dev-bot Aug 3, 2026
fd339d5
Merge remote-tracking branch 'origin/main' into feat/review-openjdk-c…
qwen-code-dev-bot Aug 4, 2026
aeb7bc0
Merge branch 'main' into feat/review-openjdk-context
wenshao Aug 4, 2026
5e3adea
fix(review): harden repository context bounds and base identity reads…
qwen-code-dev-bot Aug 4, 2026
704e9f7
Merge branch 'main' into feat/review-openjdk-context
qwen-code-dev-bot Aug 4, 2026
34ebbac
chore(review): merge origin/main into feat/review-openjdk-context
qwen-code-dev-bot Aug 5, 2026
4bf8dc8
Merge remote-tracking branch 'origin/main' into feat/review-openjdk-c…
qwen-code-dev-bot Aug 5, 2026
e473346
Merge branch 'main' into feat/review-openjdk-context
qwen-code-dev-bot Aug 5, 2026
ba4571d
fix(review): bound manifest matching work and pin round-2 review gaps…
qwen-code-dev-bot Aug 5, 2026
fe06aa1
Merge branch 'main' into feat/review-openjdk-context
qwen-code-dev-bot Aug 5, 2026
a1366d9
fix(test): isolate serve streaming suite from stray workspace setting…
qwen-code-dev-bot Aug 6, 2026
aa8c7cb
fix(review): cap identity reads, bill match work by length, pin round…
qwen-code-dev-bot Aug 6, 2026
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
52 changes: 52 additions & 0 deletions docs/design/review-repository-context.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Review repository context

## Problem

The review pipeline needs a bounded way for repositories to declare review guidance without teaching shared roster, prompt, coverage, and composition code about individual projects. Repository metadata is security-sensitive for pull request reviews because the reviewed branch must not be able to opt into or remove trusted context.

## Manifest

A repository may provide strict JSON at `.qwen/review-context.json`:

```json
{
"version": 1,
"label": "Example repository",
"rules": [
{
"paths": ["packages/*/src/**"],
"relatedPaths": ["packages/cli/src/commands/review/**"],
"domains": ["runtime"],
"recommendedTests": ["test:runtime"],
"requiredConfigurations": ["debug"],
"requiredAgents": ["test-matrix"],
"unverifiedDimensions": ["Alternate configuration"],
"verificationNotes": ["Run the repository-native focused tests"]
}
]
}
```

The top-level fields are exactly `version`, `label`, and `rules`. Each rule requires `paths`; all other rule fields are optional. Unknown or missing required fields, comments, unsupported versions, oversized values, control characters, and duplicate array entries are rejected. Arrays are human-authored and may be written in any order; values from all matching rules are merged, deduplicated, and returned sorted and unique (the internal wire format keeps the strict sorted-and-unique check). Rule order is preserved. The total `paths` globs across all rules, the merged `relatedPaths` glob list, and every merged field are capped at the wire bounds and rejected fail-closed, so a matching burst cannot stall the step or outgrow the contract. Note the example's `relatedPaths` wildcard is scoped to one subsystem on purpose: wildcard `relatedPaths` are subject to the 128 resolved-file bound below, and a repository-wide scope like `packages/*/src/**` exceeds it on a repository this size.

`paths` and `relatedPaths` use repository-relative `/`-separated globs. Matching is case-sensitive on every platform and `?` consumes one UTF-16 code unit. The supported metacharacters are `*`, `?`, and a complete `**` path segment. Absolute paths, backslashes, empty or `.`/`..` segments, negation, brace expansion, character classes, and extended glob syntax are rejected.

A rule matches when any changed path matches one of its `paths` globs. If no rule matches, the provider returns no context. A matching rule's deduplicated `relatedPaths` globs are expanded from the worktree with dot files enabled, directory results disabled, symlink traversal disabled, and case-sensitive matching. Related globs containing wildcards must start with a non-wildcard directory segment so expansion cannot begin with a repository-wide wildcard; a completely static entry resolves to itself when it exists as a regular file. Globs whose path enters a dependency or build-output directory at any depth are rejected at validation (compared case-insensitively, on every platform), so the never-descend invariant holds for scan roots as well as recursion. Changed paths are removed from the result. Resolved files must remain inside the worktree. Expansion never descends into dependency and build-output trees (`node_modules`, `dist`, and the other conventional names the scan skips) and fails closed when any limit is exceeded: 16384 visited entries across the scan (files and directories, matching or not — calibrated on this repository's installed checkout, so a honestly scoped subtree, including all of `packages/`, never trips it), 128 resolved files in the result, and a matching-work budget charged per attempted pattern match (pattern length times path length) in both the rule filter and the expansion, which reports the matching-work limit and keeps a matching burst from stalling the step.

## Trust boundary

`repo-context` reads the fixed manifest path through `RepositoryContextProviderInput.readIdentityFile`. For pull request plans, the manifest therefore comes only from the trusted merge-base commit recorded by the fetch stage. The pull request head cannot opt in, opt out, or change the rules. For local plans, the manifest comes from the current worktree after safe-relative-path validation and realpath containment.

Identity reads return the same shape in both modes (CRLF normalised to LF, surrounding whitespace trimmed) and are capped at one megabyte, fail closed: an absent file yields `null`, but a file that is present, unreadable, or oversized throws rather than masquerading as "not this repository". Both modes follow a symlinked identity file itself under the same containment rule, and a directory yields nothing in both. A pull request plan whose merge base never resolved (`mergeBaseSha: null`) — or whose base fetch failed, leaving the recorded sha possibly stale — writes a `null` artifact without consulting the worktree at all: falling back to the worktree would read the manifest from the PR head, the exact read this boundary forbids, and a possibly stale sha is not a trusted source either.

Three residuals are recorded so the guarantee is not overstated. First, for pull request plans the RULES come from the merge base, but `relatedPaths` globs are expanded against the head worktree, so the head still decides which files the base's globs resolve to; impact is low because reviewers read the head tree anyway. Second, local reviews read the manifest from the current worktree, so reviewing an untrusted repository lets that repository put one bounded, control-character-free block of guidance — the label plus six capped arrays — into every code-reviewing brief; the one-megabyte read ceiling, the validation bounds, and inert rendering are the mitigation. Third, the two modes resolve identity symlinks with different engines: the pull request reader never descends through a symlinked intermediate path COMPONENT (the worktree reader does), and it caps identity symlink chains at 16 hops where the kernel resolves up to ~40 — throwing at the cap rather than degrading. A repository committing `.qwen` itself as a symlink to an in-tree directory therefore attaches context in local reviews and never in pull request reviews; the direction is fail-safe (strictly less, never more), but an operator diagnosing "context attaches locally but never on PRs" should know the asymmetry is by construction.

The manifest provider is statically registered in-process and returns the generic `RepositoryContext` shape with provider `manifest`. Its complete output passes through the shared `validateRepositoryContext` validator before downstream consumers use it. No dynamic plugin registry, shell execution, templates, or opaque payloads are supported.

## Review workflow

Medium- and high-effort local and same-repository pull request reviews invoke `repo-context` after the review plan is captured. The command receives absolute plan, worktree, and output paths. Low-effort reviews and cross-repository lightweight reviews skip repository context because they do not run the full local-tree workflow.

Code-review agents receive the generic context headed by its label. The build-and-test role receives recommended tests, required configurations, and verification notes. Required roles are merged into the normal roster without duplication, and only when the review's effort, topology, and mode already permit them — a manifest cannot inflate a medium review with the adversarial personas, re-add whole-diff walkers to a chunked fan-out, or demand a tree-grepping role from a review with no tree. Composition discloses unverified dimensions as non-blocking proof boundaries; a present-but-invalid context fails every consumer closed rather than being silently dropped anywhere.

Status: this is the foundation — the contract, the command, and the downstream consumers, exercised by unit tests and the review skill. No `.qwen/review-context.json` ships with this change, so nothing beyond tests runs end to end until a repository adopts one.
27 changes: 27 additions & 0 deletions docs/users/features/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,33 @@ You can customize review criteria per project. `/review` reads rules from these

Rules are injected into the LLM review agents (0-6) as additional criteria. For PR reviews, rules are read from the **base branch** to prevent a malicious PR from injecting bypass rules.

## Repository Context

Repositories can hand the reviewers bounded, repository-specific guidance by committing a strict JSON manifest to `.qwen/review-context.json`. At medium or high effort, `/review` reads the manifest after capturing the plan and attaches the matching guidance before any agent launches:

```json
{
"version": 1,
"label": "Example repository",
"rules": [
{
"paths": ["packages/*/src/**"],
"domains": ["runtime"],
"relatedPaths": ["packages/runtime/src/**"],
"recommendedTests": ["npm run test:runtime"],
"requiredConfigurations": ["debug"],
"requiredAgents": ["test-matrix"],
"unverifiedDimensions": ["Alternate runtime was not exercised"],
"verificationNotes": ["Use the repository native test runner"]
}
]
}
```

A rule applies when any changed file matches one of its `paths` globs (`*`, `?`, and `**` segments; case-sensitive). All matching rules merge their guidance: domains and related files for the review agents, recommended tests and required configurations for the build-and-test agent, extra reviewer roles (honoured only when the chosen effort and topology run them), and proof boundaries the final review discloses as unverified dimensions. Arrays may be written in any order; duplicate entries are rejected.

For PR reviews the manifest is read from the merge base, so the PR under review cannot opt itself into or out of guidance; local reviews read it from the current worktree. Low-effort and cross-repository reviews skip repository context. The full contract and trust model live in the [design doc](../../design/review-repository-context.md).

## Issue Fidelity

For bugfix PRs, the Issue Fidelity agent fetches issue evidence directly instead of relying on PR description text. It uses `gh pr view <pr> --repo <owner/repo> --json closingIssuesReferences` for GitHub's strong closing-issue metadata, then `gh issue view <number> --repo <issue_owner>/<issue_repo> --json title,body,comments` for the original report and discussion — the `--json` form includes the issue **body** (the reporter's original repro), which `--comments` alone omits, and the issue's own repository is read from each reference (a PR can close an issue in a different repo). This agent runs only for PR targets; local-diff and file-path reviews skip it.
Expand Down
39 changes: 23 additions & 16 deletions integration-tests/cli/qwen-serve-streaming.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const CLI_BIN =
process.env['TEST_CLI_PATH'] ??
path.resolve(__dirname, '../../packages/cli/dist/index.js');
const TOKEN = 'streaming-integ-secret';
const REPO_ROOT = path.resolve(__dirname, '../..');

// Windows: this suite shells out to `pgrep` / `kill -KILL` to simulate
// child-process crashes for the SIGKILL → `session_died` test, and those
Expand Down Expand Up @@ -78,6 +77,7 @@ let base = '';
let client: DaemonClient;
let fakeServer: FakeOpenAIServer;
let homeDir = '';
let workspaceDir = '';
let pendingWritePath = '';

beforeAll(async () => {
Expand Down Expand Up @@ -131,6 +131,7 @@ beforeAll(async () => {
ui: { enableFollowupSuggestions: false },
}),
);
workspaceDir = mkdtempSync(path.join(tmpdir(), 'qwen-serve-streaming-ws-'));
daemon = spawn(
process.execPath,
[
Expand All @@ -143,16 +144,19 @@ beforeAll(async () => {
'--hostname',
'127.0.0.1',
// Per #3803 §02 (1 daemon = 1 workspace), pin the bound
// workspace so every `createOrAttachSession({ workspaceCwd:
// REPO_ROOT })` below matches. Without this the daemon inherits
// the test runner's cwd (CI / IDE-launcher / direct vitest
// invocations all differ) and every session create returns
// 400 workspace_mismatch — the SSE / permission / Last-Event-ID
// tests below would all silently 404. Same fix the sibling routes test
// received earlier in this PR — missed in this file in the original §02
// pass.
// workspace so every `createOrAttachSession({ workspaceCwd })`
// below matches. Without this the daemon inherits the test
// runner's cwd (CI / IDE-launcher / direct vitest invocations
// all differ) and every session create returns 400
// workspace_mismatch — the SSE / permission / Last-Event-ID
// tests below would all silently 404. A scratch workspace (not
// the checkout) also keeps sessions hermetic: the daemon merges
// the workspace's `.qwen/settings.json` into every session, and
// a stray one on a shared runner (e.g. a `tools.sandbox` mode or
// a `tools.core` allowlist missing `todo_write`) silently breaks
// the Stop Guard flow below.
'--workspace',
REPO_ROOT,
workspaceDir,
],
{
stdio: ['ignore', 'pipe', 'pipe'],
Expand Down Expand Up @@ -211,6 +215,9 @@ afterAll(async () => {
if (homeDir) {
rmSync(homeDir, { recursive: true, force: true });
}
if (workspaceDir) {
rmSync(workspaceDir, { recursive: true, force: true });
}
}, 15_000);

/** Open an authenticated SSE stream and yield parsed frames. */
Expand Down Expand Up @@ -241,7 +248,7 @@ async function* sseFrames(
describePOSIX('qwen serve — child-crash recovery (real SIGKILL)', () => {
it('publishes session_died after the qwen --acp child is SIGKILL-ed', async () => {
const session = await client.createOrAttachSession({
workspaceCwd: REPO_ROOT,
workspaceCwd: workspaceDir,
});

// Find the daemon's direct `--acp` child PID.
Expand Down Expand Up @@ -295,7 +302,7 @@ describePOSIX('qwen serve — child-crash recovery (real SIGKILL)', () => {
);

// Listing must NOT show the dead session.
const remaining = await client.listWorkspaceSessions(REPO_ROOT);
const remaining = await client.listWorkspaceSessions(workspaceDir);
// Explicit `s` type for resilience against a stale dist .d.ts
// in the reviewer's tsc env (see same note in routes.test.ts).
expect(
Expand All @@ -306,7 +313,7 @@ describePOSIX('qwen serve — child-crash recovery (real SIGKILL)', () => {

// Retry must spawn fresh, not reuse the corpse.
const fresh = await client.createOrAttachSession({
workspaceCwd: REPO_ROOT,
workspaceCwd: workspaceDir,
});
expect(fresh.sessionId).not.toBe(session.sessionId);
expect(fresh.attached).toBe(false);
Expand All @@ -316,7 +323,7 @@ describePOSIX('qwen serve — child-crash recovery (real SIGKILL)', () => {
describePOSIX('qwen serve — multi-client first-responder permission', () => {
it('fans out permission_request to both subscribers; only one vote wins', async () => {
const session = await client.createOrAttachSession({
workspaceCwd: REPO_ROOT,
workspaceCwd: workspaceDir,
});

// Pin the session to `default` approval mode. The ACP child
Expand Down Expand Up @@ -448,7 +455,7 @@ describePOSIX('qwen serve — multi-client first-responder permission', () => {
describePOSIX('qwen serve — Last-Event-ID resume', () => {
it('reconnect with Last-Event-ID:N yields events with id > N', async () => {
const session = await client.createOrAttachSession({
workspaceCwd: REPO_ROOT,
workspaceCwd: workspaceDir,
});

// Fire a short prompt to populate the bus.
Expand Down Expand Up @@ -494,7 +501,7 @@ describePOSIX('qwen serve — Last-Event-ID resume', () => {
describePOSIX('qwen serve — daemon Todo Stop Guard replay', () => {
it('continues after prompt admission without an SSE client and replays the bounded attempts', async () => {
const session = await client.createOrAttachSession({
workspaceCwd: REPO_ROOT,
workspaceCwd: workspaceDir,
});
const requestStart = fakeServer.requests.length;
const guardMarker = `todo-guard-e2e-${requestStart}`;
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/commands/review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('reviewCommand', () => {
'fetch-pr',
'capture-local',
'plan-diff',
'repo-context',
'pr-context',
'comment-status',
'load-rules',
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { findingsCommand } from './review/findings.js';
import { fetchPrCommand } from './review/fetch-pr.js';
import { captureLocalCommand } from './review/capture-local.js';
import { planDiffCommand } from './review/plan-diff.js';
import { repoContextCommand } from './review/repo-context.js';
import { prContextCommand } from './review/pr-context.js';
import { commentStatusCommand } from './review/comment-status.js';
import { loadRulesCommand } from './review/load-rules.js';
Expand Down Expand Up @@ -49,6 +50,7 @@ export const reviewCommand: CommandModule = {
.command(fetchPrCommand)
.command(captureLocalCommand)
.command(planDiffCommand)
.command(repoContextCommand)
.command(prContextCommand)
.command(commentStatusCommand)
.command(loadRulesCommand)
Expand All @@ -74,7 +76,7 @@ export const reviewCommand: CommandModule = {
.command(cleanupCommand)
.demandCommand(
1,
'Specify a subcommand: run, parse-args, fetch-pr, capture-local, plan-diff, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
'Specify a subcommand: run, parse-args, fetch-pr, capture-local, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
)
.version(false),
handler: () => {
Expand Down
Loading
Loading