Skip to content
Open
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
16 changes: 12 additions & 4 deletions src/lib/handlers/AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
# HANDLER MODULE GUIDE

## OVERVIEW

Command handlers implement `/zai` behavior only after parsing + authorization; each module owns prompt construction, API call wiring, and response formatting. The `scheduled` handler is distinct: it executes scheduled tasks defined in `.zai-scheduled.yml` (and the manual `/zai update-agents` command) rather than responding to a standard review command.

## WHERE TO LOOK

| Command | File | Notes |
|---------|------|-------|
| `/zai ask` | `src/lib/handlers/ask.js` | Uses continuity state and broad PR context |
| `/zai review <path>` | `src/lib/handlers/review.js` | Targeted diff review, file-in-PR validation |
| `/zai explain <path>#Lx-Ly` | `src/lib/handlers/explain.js` | Range parsing + snippet extraction |
| `/zai describe` | `src/lib/handlers/describe.js` | File/directory description |
| `/zai explain <path>#Lx-Ly` | `src/lib/handlers/explain.js` | Range parsing + snippet extraction; auto-detects range from review comments when omitted |
| `/zai describe` | `src/lib/handlers/describe.js` | Generate a PR description from commit messages |
| `/zai impact` | `src/lib/handlers/impact.js` | Change impact analysis |
| `/zai help` | `src/lib/handlers/help.js` | Static help output with auth gate |
| `/zai update-agents` | `src/index.js` (`dispatchCommand`) | Manual AGENTS.md regen; reuses `handleUpdateAgentsTask` |
| scheduled tasks | `src/lib/handlers/scheduled.js` | Largest module; cron-driven `.zai-scheduled.yml` tasks; grounded + validated AGENTS.md upgrades |
| Handler registry | `src/lib/handlers/index.js` | Dispatcher map consumed by runtime (note: `scheduled` is exported but not in the `/zai` HANDLERS map) |

## SCHEDULED MODULE (`scheduled.js`) KEY SYMBOLS

- `handleScheduledEvent` (entry) → `executeScheduledTask` (per-task) → `buildExecutionContext` → `getScheduledHandler` (registry lookup).
- `handleUpdateAgentsTask` (grounded flow): gist command → `collectRepositoryContext` (`../repository-context.js`: real tree + existing AGENTS.md discovery + key files) → `buildAgentsUpgradePrompt` (embeds context, tells model it has NO live repo access) → `callZaiApiWithRetry` → `parseFileUpdatesFromResponse` (multi-format JSON) → `validateGeneratedAgentFiles` (`../agents-validation.js`: rejects non-AGENTS paths, out-of-scope writes, hallucinated content referencing non-existent files) → diff vs repo files → `createPR`.
- Registry: `SCHEDULED_HANDLERS` (const) + `registerScheduledHandler`/`getAllScheduledHandlers` for extension.
Expand All @@ -25,24 +28,29 @@ Command handlers implement `/zai` behavior only after parsing + authorization; e
- Scoping config (all optional, per-task in `.zai-scheduled.yml`): `context_paths`, `target_paths`, `exclude_paths`, `max_context_chars`, `max_file_chars`, `max_files_to_fetch`, `allow_create_new`, `update_existing_only`.

## CONVENTIONS

- Keep command argument parsing explicit and reject invalid formats early.
- Always use threaded replies (`replyToId`) for command results.
- Reactions should reflect lifecycle: acknowledge -> work -> success/failure.
- Reactions should reflect lifecycle: acknowledge work success/failure.
- Keep prompts bounded via context truncation helpers; never pass raw unbounded patches.
- Return user-safe failures; log internal details through shared logging helpers.

## TESTING
- Local handler unit coverage exists in `tests/handlers/`: `ask.test.js`, `explain.test.js`, `impact.test.js`, `review.test.js`, `scheduled.test.js`.

- Handler unit coverage in `tests/handlers/`: `ask.test.js`, `explain.test.js`, `impact.test.js`, `review.test.js`, `scheduled.test.js`.
- `describe` handler tested in `tests/describe.test.js` (at `tests/` level, not `tests/handlers/`).
- Scheduled pipeline coverage: `tests/handlers/scheduled.test.js` (registry, PR creation, parse, grounded `handleUpdateAgentsTask` flow incl. hallucination rejection), `tests/scheduled-config.test.js` (config + `validateAgentsConfig` scoping fields), `tests/repository-context.test.js` (tree/AGENTS.md discovery/budgets/globs), `tests/agents-validation.test.js` (path/hallucination/target-path guards incl. PR #15 regression).
- End-to-end command pipeline behavior is validated in `tests/integration/command-pipeline.test.js`.
- When changing parsing or output contracts, update both unit and integration assertions.

## ANTI-PATTERNS

- Parsing arguments with loose heuristics that silently alter user intent.
- Posting top-level comments for command replies (breaks conversational threading).
- Bypassing `auth.checkForkAuthorization` in a handler.
- Embedding duplicate parser/auth logic that already exists upstream.

## NOTES

- Prefer adding helper functions within a handler module before introducing cross-handler coupling.
- Keep marker constants stable once tests depend on them.
16 changes: 13 additions & 3 deletions tests/AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# TEST SUITE GUIDE

## OVERVIEW

Repository test coverage mixes module-focused tests in `tests/*.test.js` and scenario-driven flows in `tests/integration/*`.

## STRUCTURE

```text
tests/
├── *.test.js # Module-level tests for lib/runtime units
Expand All @@ -15,32 +17,40 @@ tests/
```

## WHERE TO LOOK

| Task | Location | Notes |
|------|----------|-------|
| Parser/auth/comment unit behavior | `tests/commands.test.js`, `tests/auth.test.js`, `tests/comments.test.js` | Fast regression checks |
| Runtime orchestration checks | `tests/action.test.js`, `tests/handlers.test.js`, `tests/index.test.js` | Entry flow and dispatch behavior |
| API and logging resilience | `tests/api.test.js`, `tests/logging.test.js` | Retry/error categorization |
| API and logging resilience | `tests/api.test.js`, `tests/logging.test.js` | Retry/error categorization, progressive timeout |
| Context, PR fetch, and batching | `tests/context.test.js`, `tests/pr-context.test.js`, `tests/changed-files.test.js`, `tests/auto-review.test.js` | Diff scoping, file-at-ref fetch, paginated/batched review |
| Continuity and events | `tests/continuity.test.js`, `tests/events.test.js` | Hidden-marker state, event-type detection |
| Code scope and window extraction | `tests/lib/code-scope.test.js` | Token budget, enclosing block, window extraction |
| Describe handler | `tests/describe.test.js` | PR description generation |
| Scheduled pipeline | `tests/handlers/scheduled.test.js`, `tests/scheduled-config.test.js`, `tests/repository-context.test.js`, `tests/agents-validation.test.js` | Config load + `validateAgentsConfig` scoping; `parseFileUpdatesFromResponse`; grounded `handleUpdateAgentsTask` flow incl. hallucination rejection; repo-context collection (tree/budgets/globs); validation guards incl. PR #15 regression |
| Scheduled pipeline (integration) | (pending) | End-to-end schedule event → context → Z.ai mock → validated PR is still a gap; unit coverage of the grounded flow exists via the `handleUpdateAgentsTask` seam (`__callZaiForTest`). |
| Full command pipeline | `tests/integration/command-pipeline.test.js` | Parse -> auth -> handler -> output contract |
| Full command pipeline | `tests/integration/command-pipeline.test.js` | Parse auth handler output contract |
| PR auto-review behavior | `tests/integration/pr-auto-review.test.js` | Marker upsert and PR event lifecycle |
| Shared mocks | `tests/helpers/mocks.js` | Reusable mock utilities for handlers and services |

## CONVENTIONS
- Test framework: Vitest v3 (uses vitest globals: describe/test/expect); configured via `vitest.config.js`.

- Test framework: Vitest v3 (uses vitest globals: `describe`/`test`/`expect`); configured via `vitest.config.js`.
- Keep tests deterministic with explicit mock payloads and marker assertions.
- Prefer scenario names that encode trigger + expected visible outcome.
- When changing comment markers or command UX, update integration snapshots/assertions immediately.
- Use `tests/helpers/mocks.js` for shared mock utilities instead of duplicating inline.
- Use `tests/fixtures/` for static event payloads instead of inline JSON.

## ANTI-PATTERNS

- Deleting integration assertions to make behavior changes pass.
- Asserting only internal calls without validating user-visible output.
- Duplicating large fixtures inline when reusable fixtures already exist.

## NOTES

- Test command: `npm test` → `vitest run --coverage`.
- Coverage uploaded to Codecov.
- Integration tests are the safety net for command threading and marker idempotency.
- Run specific test file: `npx vitest run <path>`.
13 changes: 12 additions & 1 deletion tests/integration/AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
# INTEGRATION TEST GUIDE

## OVERVIEW
Integration tests verify end-to-end GitHub event pipelines and visible bot behavior, not just isolated helper logic.

Integration tests verify end-to-end GitHub event pipelines and visible bot behavior, not just isolated helper logic. They are the primary safety net for command threading and marker idempotency.

## WHERE TO LOOK

| Scenario | File | Notes |
|----------|------|-------|
| Issue comment command pipeline | `tests/integration/command-pipeline.test.js` | Event classification, command parse, auth, dispatch, response shape |
| Pull request auto-review pipeline | `tests/integration/pr-auto-review.test.js` | PR event handling, marker create/update, no-change short-circuits |
| Integration fixture data | `tests/integration/fixtures/events.js` | Shared event payloads for realistic event simulation |

## CONVENTIONS

- Test framework: Vitest v3 with globals (`describe`/`test`/`expect`); configured via `vitest.config.js`.
- Test the public behavior chain from event input to final comment/reaction outcome.
- Cover both happy paths and safety gates (non-PR comments, unauthorized users, empty diffs).
- Keep marker expectations explicit so idempotent update regressions are caught quickly.
- Use shared fixtures from `tests/integration/fixtures/events.js` instead of duplicating event payloads inline.

## ANTI-PATTERNS

- Replacing integration assertions with unit-level mocks only.
- Ignoring failure-path expectations for auth and API errors.
- Coupling tests to unrelated implementation details that break harmless refactors.

## NOTES

- Run integration tests only: `npx vitest run tests/integration/`.
- Parent guide: `tests/AGENTS.md` for full test suite layout and conventions.