Skip to content

Update code review agent instructions #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
YunchuWang merged 2 commits into from
Mar 2, 2026
Merged

Update code review agent instructions #119

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
370b8fd
Update review agent instructions
YunchuWang Mar 2, 2026
5b691d7
docs: enhance daily code review agent instructions for testing requir…
YunchuWang Mar 2, 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
20 changes: 11 additions & 9 deletions .github/agents/daily-code-review.agent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,12 @@ invariants, task hierarchy, entity system, error handling patterns, and where bu
tend to hide. This context is essential for distinguishing real bugs from intentional
design decisions.

## Step 1: Deduplication (MANDATORY — Do This Second)
## Step 1: Review Exclusion List (MANDATORY — Do This Second)

Before analyzing any code, you MUST check what's already in-flight:
The workflow has already collected open PRs, open issues, recently merged PRs, and bot PRs
with the `copilot-finds` label. This data is injected below as **Pre-loaded Deduplication Context**.

1. **List all open PRs** with the `copilot-finds` label.
2. **List all open issues** with the `copilot-finds` label.
3. **List all open PRs** created by `copilot` or `github-actions[bot]` in the last 30 days.
4. **List recently merged PRs** (last 14 days) to avoid re-raising recently fixed items.

Build an **exclusion list** of:
Review it and build a mental exclusion list of:
- File paths already touched by open PRs
- Problem descriptions already covered by open issues
- Areas recently fixed by merged PRs
Expand Down Expand Up @@ -131,10 +127,16 @@ Example: `copilot-finds/bug/fix-unhandled-promise-rejection`

### Code Changes
- Fix the actual problem
- Add new test(s) that:
- Add new **unit test(s)** that:
- Would have caught the bug (for bug fixes)
- Cover the previously uncovered path (for missing tests)
- Verify the improvement works (for improvements)
- **If the change affects orchestration, activity, entity, or client/worker behavior:**
Add **Azure Managed e2e test(s)** in `test/e2e-azuremanaged/` as well. Follow the
existing patterns there (uses `DurableTaskAzureManagedClientBuilder` /
`DurableTaskAzureManagedWorkerBuilder`, reads `DTS_CONNECTION_STRING` or
`ENDPOINT`/`TASKHUB` env vars). Add the new test case to the appropriate existing
Comment on lines +137 to +138
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

The env var guidance here doesn’t match the existing test/e2e-azuremanaged patterns. Several specs use DURABLE_TASK_SCHEDULER_CONNECTION_STRING (e.g., query-apis.spec.ts, rewind.spec.ts) and entity.spec.ts uses AZURE_DTS_CONNECTION_STRING, so stating it “reads DTS_CONNECTION_STRING” is misleading. Please update this wording to reflect the actual accepted env vars (or standardize the e2e tests and then document the standardized names).

Suggested change
`DurableTaskAzureManagedWorkerBuilder`, reads `DTS_CONNECTION_STRING` or
`ENDPOINT`/`TASKHUB` env vars). Add the new test case to the appropriate existing
`DurableTaskAzureManagedWorkerBuilder`, and reads the same supported env vars as
the existing specs, such as `DURABLE_TASK_SCHEDULER_CONNECTION_STRING` or
`AZURE_DTS_CONNECTION_STRING`). Add the new test case to the appropriate existing

Copilot uses AI. Check for mistakes.
spec file (e.g., `orchestration.spec.ts`, `entity.spec.ts`, `retry-advanced.spec.ts`).
- Keep changes minimal and focused — one concern per PR

### Labels
Expand Down
Loading