From 96d6c7d933e39fec7be4da3c9dbe6870adcd07e0 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 4 Apr 2026 14:25:30 +0200 Subject: [PATCH 1/4] Add initial githuman ask command and plan --- ASK.md | 690 ++++++++++++++++++++++++++++++++++++++++ src/cli/commands/ask.ts | 116 +++++++ src/cli/index.ts | 6 + tests/cli/index.test.ts | 48 +++ 4 files changed, 860 insertions(+) create mode 100644 ASK.md create mode 100644 src/cli/commands/ask.ts diff --git a/ASK.md b/ASK.md new file mode 100644 index 0000000..b817a2e --- /dev/null +++ b/ASK.md @@ -0,0 +1,690 @@ +# `githuman ask` plan + +## Goal + +Turn `githuman ask` into a synchronous handoff command for agent ↔ human collaboration: + +1. The agent runs `githuman ask`. +2. GitHuman makes the review UI available and prints the URL, optionally opening the browser. +3. The human reviews the changes and leaves feedback in GitHuman. +4. When the human signals they are done, `githuman ask` exits by printing the human's feedback: + - todo items + - review comments + - review status changes +5. The agent reads that output and applies fixes. +6. The feature ships with clear documentation and website updates so people immediately understand why it is useful and how to use it. + +This is intentionally different from the current "create one todo and exit" behavior. + +--- + +## Proposed UX + +### Basic flow + +```bash +githuman ask +``` + +Behavior: + +- start GitHuman if needed, using serve-like options +- print the review URL +- optionally open the browser +- wait until the human marks the review request as complete +- print a structured summary of the human feedback +- exit with success + +### With a prompt for the human + +```bash +githuman ask "Please review the parser refactor" +``` + +This creates a visible review-request marker in GitHuman so the human knows what the agent is asking for. + +### With explicit network settings + +```bash +githuman ask --host 0.0.0.0 --port 4000 --open +``` + +Options should closely match `githuman serve`. + +--- + +## Proposed completion signal + +The command needs a concrete way to know when the human is done. + +### Recommended approach + +Use a dedicated request todo created by `githuman ask`. + +Example content: + +- `AI review request: Please review the parser refactor` +- `AI review request: Please review the current changes` + +The human completes their turn by marking that todo as done in the UI. + +### Why this is the best initial design + +- no new database schema is required +- no new UI concept is required to ship v1 +- works with the existing todo system +- gives the CLI an unambiguous "done" signal +- keeps the review loop explicit for both human and agent + +### Optional future improvement + +Later we can add a dedicated UI button like: + +- `Finish human review` +- `Send feedback back to agent` + +That would be cleaner, but it requires a new UI/backend concept. + +--- + +## What `githuman ask` should do + +### 1. Resolve runtime config + +Load settings in this priority order: + +1. CLI flags +2. config file +3. existing defaults + +Relevant settings: + +- `host` +- `port` +- `https` +- `auth` +- `open` + +### 2. Ensure GitHuman is available + +Two possible strategies: + +#### Option A: start an in-process server from `ask` + +Pros: +- one command does everything +- simple user experience + +Cons: +- `ask` becomes responsible for lifecycle management +- more care needed for shutdown and polling + +#### Option B: connect to an already-running server, otherwise start one + +Pros: +- nicer if the user already has GitHuman open +- reuses existing running session + +Cons: +- slightly more branching in the logic + +### Recommendation + +Implement **Option B**: + +1. check `GET /api/health` +2. if reachable, reuse it +3. otherwise start a local server with the resolved config + +--- + +## URL behavior + +`githuman ask` should always print the URL to stdout/stderr in a copyable form. + +Examples: + +- `http://localhost:3847` +- `https://192.168.1.10:3847?token=...` + +If `--open` is enabled, also open the browser. + +Default behavior should match `serve` semantics as closely as possible. + +--- + +## Config file + +## Goal + +Avoid forcing the agent/user to pass `--host` and `--port` every time. + +### Proposed file location + +Repository-local: + +- `.githuman/config.json` + +This keeps the settings next to the repository's GitHuman data. + +### Example + +```json +{ + "host": "localhost", + "port": 3847, + "https": false, + "open": true +} +``` + +### Why repo-local first + +- matches the existing `.githuman/` data model +- works well for dogfooding inside one repository +- easy for agents to discover +- avoids surprising machine-global behavior + +### Future extension + +If needed later, support both: + +1. `~/.githuman/config.json` +2. `.githuman/config.json` + +with repo-local taking precedence. + +### Required behavior + +- `serve` should also read the config file +- `ask` should reuse the same config loader +- CLI flags always override config file values + +--- + +## Documentation and website changes + +This feature is important enough that it should ship with a documentation and marketing pass, not just code. + +### Product story to communicate + +`githuman ask` is the handoff point between an LLM and a human reviewer: + +1. the agent asks for review +2. GitHuman opens a human-friendly review surface +3. the human leaves comments and todos +4. the command returns those comments to the agent +5. the agent fixes the issues + +That loop is the headline. It is novel, useful, and easy to demo. + +### Documentation updates + +We should update at least: + +- `README.md` +- CLI help output examples where relevant +- any docs page that explains the review workflow +- release notes / changelog entry if we keep one + +### README changes + +The README should include: + +- a short feature callout near the top +- a concrete `githuman ask` example +- a short explanation of the human-in-the-loop flow +- the new config file location and example +- how completion works in v1 (mark the request todo as done) + +### CLI reference changes + +Document: + +- `githuman ask` options +- config precedence +- how `ask` differs from `serve` +- JSON output mode for agent integrations + +### Workflow docs changes + +Add or update a how-to guide covering: + +- how to ask a human for review from an agent session +- how the human finishes their turn +- how the agent resumes from the printed feedback + +Also add an explanation page or README subsection covering: + +- why `githuman ask` exists +- why the request todo is used as the completion signal in v1 +- how this creates a tight human ↔ agent review loop + +### Website changes + +The website should treat this as a flagship feature. + +Recommended changes: + +- add a hero or feature section for `githuman ask` +- explain the "agent asks → human reviews → agent resumes" loop visually +- include a terminal example showing `githuman ask` +- include one short UI screenshot of the review experience +- add a concise "why this matters" explanation for teams using AI coding agents + +### Screenshots and demo assets + +Because this is a very visual feature, we should refresh screenshots after implementation. + +At minimum: + +- capture a screenshot showing the request todo / review workflow +- update README screenshots if the feature is shown there +- update website assets if the landing page references the new flow + +If needed, refresh screenshots with: + +```bash +node scripts/screenshots.ts +``` + +### Messaging guidance + +The tone should frame this feature as: + +- human-in-the-loop review for AI coding agents +- a simple bridge between coding agents and human reviewers +- a workflow feature, not just another CLI command + +--- + +## Feedback collected at the end + +When the human completes the request, `githuman ask` should print: + +1. the request todo status +2. all current pending todos relevant to the review session +3. all comments added by the human during the session +4. the latest review status + +### Recommended output shape + +Plain text, optimized for agent consumption: + +```text +GitHuman feedback ready +URL: http://localhost:3847 +Review status: changes_requested + +Todos: +- Fix null handling in parser.ts +- Add a test for empty input + +Comments: +- src/parser.ts:42 + "This should reject undefined explicitly." +- src/parser.test.ts + "Please add a regression test for whitespace-only input." +``` + +### Important detail + +The command should only print **new feedback from this ask session**, not old historical comments. + +That means `ask` must record a session start timestamp and filter by: + +- todos created or updated after the session start +- comments created or updated after the session start + +Potentially also filter by review ID when available. + +--- + +## Review targeting + +`githuman ask` needs to know what the human is reviewing. + +### v1 recommendation + +Operate on the current repository/session and do not force a review ID. + +Behavior: + +- if there is an active in-progress review, reuse it +- otherwise open the GitHuman UI and let the human inspect staged changes or create/select the right review + +### Better v1.1 + +Support optional targeting: + +```bash +githuman ask --review +``` + +If `--review` is provided, the end-of-command output should scope comments/todos to that review when possible. + +--- + +## Polling / waiting model + +`githuman ask` should wait until the request todo is completed. + +### Polling loop + +Every 1-2 seconds: + +- fetch the ask todo by ID +- if completed, gather feedback and exit +- if deleted, treat as cancelled +- if server disappears, show a clear error + +### Nice-to-have later + +Use SSE/events for lower-latency updates instead of polling. + +For v1, polling is simpler and good enough. + +--- + +## Cancellation behavior + +### Human cancellation + +If the request todo is deleted, exit non-zero with: + +- `Review request was cancelled` + +### Agent cancellation + +If the process is interrupted: + +- stop any server that `ask` started itself +- leave the request todo in place +- print the URL again before exit if possible + +--- + +## CLI options + +`githuman ask` should support most of the `serve` options: + +- `-p, --port ` +- `--host ` +- `--https` +- `--no-https` +- `--cert ` +- `--key ` +- `--auth [token]` +- `--open` +- `--no-open` +- `-v, --verbose` +- `-h, --help` + +Additional ask-specific options: + +- `--review ` — optional review to watch/scope +- `--interval ` — optional polling interval +- `--json` — machine-readable final output + +--- + +## JSON output mode + +For agent integrations, `--json` is important. + +Example: + +```json +{ + "url": "http://localhost:3847", + "reviewStatus": "changes_requested", + "todos": [ + { + "id": "...", + "content": "Fix null handling in parser.ts", + "reviewId": null + } + ], + "comments": [ + { + "id": "...", + "reviewId": "...", + "filePath": "src/parser.ts", + "lineNumber": 42, + "content": "This should reject undefined explicitly.", + "resolved": false + } + ] +} +``` + +--- + +## Implementation checklist + +## Milestone 1 — Shared config loading + +### Code + +- [ ] add a shared CLI config loader for `.githuman/config.json` +- [ ] support at least: + - [ ] `host` + - [ ] `port` + - [ ] `https` + - [ ] `open` + - [ ] auth-related defaults where appropriate +- [ ] define precedence clearly: + - [ ] CLI flags override config file values + - [ ] config file values override built-in defaults +- [ ] update `githuman serve` to use the shared config loader +- [ ] keep existing env-var behavior working unless we intentionally replace it + +### Acceptance criteria + +- [ ] `githuman serve` works exactly as before when no config file exists +- [ ] `githuman serve` respects `.githuman/config.json` +- [ ] `--host` and `--port` no longer need to be passed every time when config is present + +## Milestone 2 — Rework `githuman ask` + +### Code + +- [ ] remove the current fire-and-forget todo-creation behavior +- [ ] make `ask` resolve config using the same path as `serve` +- [ ] make `ask` try to connect to an existing server first +- [ ] if no server is running, start one with serve-like options +- [ ] print the final URL in a copyable form +- [ ] support browser opening with serve-like semantics +- [ ] create a dedicated request todo for the ask session +- [ ] store enough local session state to know: + - [ ] the request todo ID + - [ ] the session start time + - [ ] whether `ask` started the server itself + +### Acceptance criteria + +- [ ] `githuman ask` can be run in a fresh repo with GitHuman configured +- [ ] the command shows the human where to review +- [ ] the human has a visible request marker in the UI + +## Milestone 3 — Wait for human completion + +### Code + +- [ ] poll the request todo until it is completed +- [ ] treat deletion of the request todo as cancellation +- [ ] handle server unavailability with a clear error +- [ ] support configurable polling interval if we keep `--interval` + +### Acceptance criteria + +- [ ] `githuman ask` stays open while the human is reviewing +- [ ] `githuman ask` exits when the request todo is marked done +- [ ] `githuman ask` exits non-zero if the request todo is deleted + +## Milestone 4 — Print human feedback for the agent + +### Code + +- [ ] record the session start timestamp before waiting +- [ ] collect feedback created or updated during the session: + - [ ] todos + - [ ] comments + - [ ] review status +- [ ] scope by review ID when available +- [ ] print a plain-text summary for agent consumption +- [ ] add `--json` output mode + +### Acceptance criteria + +- [ ] the final output contains only feedback relevant to this ask session +- [ ] the output is useful enough for an agent to continue work without extra manual lookup +- [ ] JSON output is stable and machine-readable + +## Milestone 5 — Polish and edge cases + +### Code + +- [ ] improve terminal messaging during startup and waiting +- [ ] handle Ctrl+C cleanly +- [ ] stop any server that `ask` started itself +- [ ] leave user-started servers untouched +- [ ] optionally support `--review ` in the initial implementation or clearly defer it +- [ ] decide whether SSE is out of scope for v1 and document that decision + +### Acceptance criteria + +- [ ] interruption behavior is predictable +- [ ] startup/wait/final-summary messages are easy to understand +- [ ] there is no ambiguity about whether the review request finished, was cancelled, or failed + +## Milestone 6 — Documentation updates + +### README + +- [ ] add a feature callout for `githuman ask` +- [ ] add a concrete CLI example +- [ ] explain the human-in-the-loop flow +- [ ] document `.githuman/config.json` +- [ ] document the completion signal used in v1 + +### Reference / guides + +- [ ] update CLI reference/help examples for `ask` +- [ ] add or update a workflow guide for the full agent ↔ human loop +- [ ] add or update explanatory docs for why `ask` exists and how it works +- [ ] add release notes / changelog entry if applicable + +### Acceptance criteria + +- [ ] a new user can understand `githuman ask` from the README alone +- [ ] a returning user can look up flags and config precedence quickly +- [ ] docs reflect the exact shipped behavior, not an idealized version + +## Milestone 7 — Website updates + +### Website content + +- [ ] add `githuman ask` as a flagship feature on the landing page +- [ ] explain the `agent asks → human reviews → agent resumes` loop +- [ ] add a terminal snippet using `githuman ask` +- [ ] add a short explanation of why this matters for AI-assisted development + +### Visuals + +- [ ] capture or update screenshots for the ask flow +- [ ] refresh any demo assets used by the README or website +- [ ] ensure screenshots match the current UI + +### Acceptance criteria + +- [ ] the website makes the feature easy to understand in a few seconds +- [ ] the screenshots and copy match the shipped implementation +- [ ] the feature feels prominent, not buried + +## Suggested delivery order + +- [ ] ship Milestone 1 first +- [ ] ship Milestones 2 and 3 together +- [ ] ship Milestone 4 before calling the feature complete +- [ ] ship Milestones 6 and 7 in the same PR or release window as the code +- [ ] validate all examples and screenshots immediately before merge + +--- + +## Backend/API needs + +### Can be done with existing APIs + +Mostly yes. + +Existing endpoints already cover: + +- health +- todos +- comments +- reviews + +### Small additions that may help + +Potential optional additions: + +- `GET /api/todos/:id` is already available and useful for polling +- query filters by timestamp may be useful later, but are not required for v1 +- a dedicated `ask session` API is not required for the first version + +--- + +## Testing plan + +### CLI tests + +Add tests for: + +- config file loading +- `ask --help` +- URL printing +- creating the request todo +- waiting until the request todo is completed +- final output includes new todos/comments +- `--json` output +- reuse running server vs start new server + +### Integration tests + +Add a test that: + +1. starts the app +2. runs `githuman ask` +3. simulates the human by creating comments/todos and completing the ask todo +4. asserts the command exits with the expected summary + +### Optional e2e test + +A future Playwright test could validate the full handoff loop in the browser. + +### Documentation / website validation + +Before shipping, verify that: + +- README examples match the real CLI output +- the documented config file format matches the implementation +- the website copy matches the final completion signal semantics +- screenshots reflect the actual UI and current branding + +--- + +## Suggested first cut + +To keep this shippable, the first implementation should: + +- use `.githuman/config.json` +- make `serve` and `ask` share config loading +- have `ask` create a dedicated request todo +- treat "request todo marked done" as the completion signal +- poll with `GET /api/todos/:id` +- print new todos/comments since session start +- support `--json` +- update the README and website copy alongside the implementation + +This gives us the full agent → human → agent loop without requiring a major UI redesign, while also making the feature easy to discover. diff --git a/src/cli/commands/ask.ts b/src/cli/commands/ask.ts new file mode 100644 index 0000000..004d973 --- /dev/null +++ b/src/cli/commands/ask.ts @@ -0,0 +1,116 @@ +/** + * Ask command - request human review by creating a todo item + */ +import { parseArgs } from 'node:util' +import { randomUUID } from 'node:crypto' +import { initDatabase, closeDatabase, getDatabase } from '../../server/db/index.ts' +import { createConfig } from '../../server/config.ts' +import { TodoRepository } from '../../server/repositories/todo.repo.ts' + +/** + * Notify the running server that todos have changed. + * This is fire-and-forget - if the server isn't running, we silently continue. + */ +async function notifyServer (config: { port: number; host: string; authToken: string | null }) { + try { + const headers: Record = { 'Content-Type': 'application/json' } + if (config.authToken) { + headers.Authorization = `Bearer ${config.authToken}` + } + + const res = await fetch(`http://${config.host}:${config.port}/api/events/notify`, { + method: 'POST', + headers, + body: JSON.stringify({ type: 'todos', action: 'updated' }), + signal: AbortSignal.timeout(1000), + }) + await res.text() + } catch { + // Server not running or unreachable - silently continue + } +} + +function printHelp () { + console.log(` +Usage: githuman ask [message] [options] + +Ask a human to review something by creating a visible todo item. + +Arguments: + message Optional request for the human reviewer + +Options: + --review Scope the request to a specific review + --json Output the created todo as JSON + -h, --help Show this help message + +Examples: + githuman ask + githuman ask "Please review the parser changes" + githuman ask "Does this migration look safe?" --review abc123 +`) +} + +function buildRequestContent (message: string): string { + const trimmed = message.trim() + if (!trimmed) { + return 'Review request: Please review the current changes.' + } + + return `Review request: ${trimmed}` +} + +export async function askCommand (args: string[]) { + const { values, positionals } = parseArgs({ + args, + allowPositionals: true, + options: { + review: { type: 'string' }, + json: { type: 'boolean', default: false }, + help: { type: 'boolean', short: 'h' }, + }, + }) + + if (values.help) { + printHelp() + process.exit(0) + } + + const content = buildRequestContent(positionals.join(' ')) + const config = createConfig() + + try { + initDatabase(config.dbPath) + const db = getDatabase() + const repo = new TodoRepository(db) + + const todo = repo.create({ + id: randomUUID(), + content, + completed: false, + reviewId: values.review ?? null, + }) + + if (values.json) { + console.log(JSON.stringify(todo, null, 2)) + } else { + console.log(`Created review request: ${todo.id.slice(0, 8)}`) + console.log(` ${todo.content}`) + if (todo.reviewId) { + console.log(` Review: ${todo.reviewId.slice(0, 8)}`) + } + } + + await notifyServer(config) + closeDatabase() + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + console.error('Error: Database does not exist yet. Run "githuman serve" first.') + process.exit(1) + } else { + throw err + } + } +} + +export { buildRequestContent } diff --git a/src/cli/index.ts b/src/cli/index.ts index 7790e3e..aac8070 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -42,6 +42,7 @@ Commands: list List all saved reviews for the current repository export Export a review to markdown resolve Mark a review as approved and resolve all comments + ask Ask a human to review by creating a todo item todo Manage todo items for tracking tasks Options: @@ -92,6 +93,11 @@ switch (command) { await resolveCommand(process.argv.slice(3)) break } + case 'ask': { + const { askCommand } = await import('./commands/ask.ts') + await askCommand(process.argv.slice(3)) + break + } case 'todo': { const { todoCommand } = await import('./commands/todo.ts') await todoCommand(process.argv.slice(3)) diff --git a/tests/cli/index.test.ts b/tests/cli/index.test.ts index d325911..1b57354 100644 --- a/tests/cli/index.test.ts +++ b/tests/cli/index.test.ts @@ -52,6 +52,7 @@ describe('CLI', () => { assert.ok(result.stdout.includes('Usage:')) assert.ok(result.stdout.includes('serve')) assert.ok(result.stdout.includes('list')) + assert.ok(result.stdout.includes('ask')) }) it('should show help with -h flag', async () => { @@ -384,6 +385,53 @@ describe('CLI', () => { }) }) + describe('ask command', () => { + it('should show help with --help flag', async () => { + const result = await runCli(['ask', '--help']) + + assert.strictEqual(result.exitCode, 0) + assert.ok(result.stdout.includes('Usage: githuman ask')) + assert.ok(result.stdout.includes('--review ')) + assert.ok(result.stdout.includes('--json')) + }) + + it('should create a review request with the default message', async (t) => { + const askTempDir = await createTestRepoWithDb(t) + const result = await runCli(['ask'], { cwd: askTempDir }) + + assert.strictEqual(result.exitCode, 0) + assert.ok(result.stdout.includes('Created review request')) + assert.ok(result.stdout.includes('Review request: Please review the current changes.')) + + const listResult = await runCli(['todo', 'list', '--all'], { cwd: askTempDir }) + assert.strictEqual(listResult.exitCode, 0) + assert.ok(listResult.stdout.includes('Review request: Please review the current changes.')) + }) + + it('should create a review request with a custom message', async (t) => { + const askTempDir = await createTestRepoWithDb(t) + const result = await runCli(['ask', 'Please review the parser changes'], { cwd: askTempDir }) + + assert.strictEqual(result.exitCode, 0) + assert.ok(result.stdout.includes('Review request: Please review the parser changes')) + + const listResult = await runCli(['todo', 'list', '--all'], { cwd: askTempDir }) + assert.strictEqual(listResult.exitCode, 0) + assert.ok(listResult.stdout.includes('Review request: Please review the parser changes')) + }) + + it('should output JSON when requested', async (t) => { + const askTempDir = await createTestRepoWithDb(t) + const result = await runCli(['ask', 'Does this look correct?', '--json'], { cwd: askTempDir }) + + assert.strictEqual(result.exitCode, 0) + const data = JSON.parse(result.stdout) + assert.strictEqual(data.content, 'Review request: Does this look correct?') + assert.strictEqual(data.completed, false) + assert.strictEqual(data.reviewId, null) + }) + }) + describe('resolve command', () => { it('should show help with --help flag', async () => { const result = await runCli(['resolve', '--help']) From edc1e37e59f8fc1bd317c809d9ccc387f5ffd25a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 4 Apr 2026 18:18:50 +0200 Subject: [PATCH 2/4] Implement initial githuman ask workflow --- ASK.md | 804 ++++++++++++++------------------------ README.md | 84 +++- src/cli/commands/ask.ts | 287 +++++++++++--- src/cli/commands/serve.ts | 151 ++----- src/cli/config.ts | 107 +++++ src/cli/index.ts | 2 +- src/cli/server-runtime.ts | 234 +++++++++++ src/server/app.ts | 2 +- tests/cli/ask.test.ts | 227 +++++++++++ tests/cli/config.test.ts | 59 +++ tests/cli/index.test.ts | 38 +- website/index.html | 24 +- 12 files changed, 1281 insertions(+), 738 deletions(-) create mode 100644 src/cli/config.ts create mode 100644 src/cli/server-runtime.ts create mode 100644 tests/cli/ask.test.ts create mode 100644 tests/cli/config.test.ts diff --git a/ASK.md b/ASK.md index b817a2e..5057192 100644 --- a/ASK.md +++ b/ASK.md @@ -1,690 +1,468 @@ -# `githuman ask` plan +# `githuman ask` redesign plan -## Goal +## Status -Turn `githuman ask` into a synchronous handoff command for agent ↔ human collaboration: +The current implementation proved that a synchronous agent ↔ human handoff is valuable, but it also exposed a major UX problem: -1. The agent runs `githuman ask`. -2. GitHuman makes the review UI available and prints the URL, optionally opening the browser. -3. The human reviews the changes and leaves feedback in GitHuman. -4. When the human signals they are done, `githuman ask` exits by printing the human's feedback: - - todo items - - review comments - - review status changes -5. The agent reads that output and applies fixes. -6. The feature ships with clear documentation and website updates so people immediately understand why it is useful and how to use it. +- using todos as the control mechanism is wrong +- the human has to understand internal workflow details +- the UI is too generic for a handoff flow +- there is no obvious single action that says “give this back to the assistant” -This is intentionally different from the current "create one todo and exit" behavior. +This document replaces the earlier todo-driven plan. --- -## Proposed UX +## Product goal -### Basic flow +`githuman ask` should feel like a first-class handoff between an assistant and a human reviewer. -```bash -githuman ask -``` - -Behavior: - -- start GitHuman if needed, using serve-like options -- print the review URL -- optionally open the browser -- wait until the human marks the review request as complete -- print a structured summary of the human feedback -- exit with success - -### With a prompt for the human - -```bash -githuman ask "Please review the parser refactor" -``` - -This creates a visible review-request marker in GitHuman so the human knows what the agent is asking for. - -### With explicit network settings +Desired flow: -```bash -githuman ask --host 0.0.0.0 --port 4000 --open -``` +1. the assistant runs `githuman ask` +2. GitHuman opens a dedicated review/handoff page +3. the human reviews the changes and leaves feedback +4. the human clicks a clear primary action: **Continue assistant** +5. `githuman ask` exits and prints the feedback for the assistant -Options should closely match `githuman serve`. +The completion signal must be explicit and built for this feature, not borrowed from todos. --- -## Proposed completion signal +## Core product decisions -The command needs a concrete way to know when the human is done. +## 1. `ask` must be a first-class object -### Recommended approach +Do **not** model ask sessions as todos. -Use a dedicated request todo created by `githuman ask`. +Introduce a dedicated persisted entity, for example `ask_sessions`. -Example content: +Suggested fields: -- `AI review request: Please review the parser refactor` -- `AI review request: Please review the current changes` +- `id` +- `repository_path` +- `review_id` nullable +- `message` +- `status` + - `waiting_for_human` + - `ready_for_agent` + - `cancelled` +- `assistant_context` nullable +- `created_at` +- `updated_at` +- `completed_at` nullable -The human completes their turn by marking that todo as done in the UI. +Optional later fields: -### Why this is the best initial design +- `feedback_summary` +- `url_token` or access metadata if needed +- `agent_name` / `agent_id` -- no new database schema is required -- no new UI concept is required to ship v1 -- works with the existing todo system -- gives the CLI an unambiguous "done" signal -- keeps the review loop explicit for both human and agent +## 2. Completion must be explicit -### Optional future improvement +The human should finish the handoff by clicking a primary UI button: -Later we can add a dedicated UI button like: +- **Continue assistant** -- `Finish human review` -- `Send feedback back to agent` +Optional secondary button: -That would be cleaner, but it requires a new UI/backend concept. +- `Cancel request` ---- +This button should update the ask session status to `ready_for_agent`. -## What `githuman ask` should do +That status change is the only completion signal `githuman ask` should wait on. -### 1. Resolve runtime config +## 3. Todos remain useful, but not for control flow -Load settings in this priority order: +Todos can still exist as review artifacts. -1. CLI flags -2. config file -3. existing defaults +They may be part of the feedback returned to the assistant, but they must not be used to: -Relevant settings: - -- `host` -- `port` -- `https` -- `auth` -- `open` - -### 2. Ensure GitHuman is available - -Two possible strategies: - -#### Option A: start an in-process server from `ask` - -Pros: -- one command does everything -- simple user experience - -Cons: -- `ask` becomes responsible for lifecycle management -- more care needed for shutdown and polling - -#### Option B: connect to an already-running server, otherwise start one - -Pros: -- nicer if the user already has GitHuman open -- reuses existing running session - -Cons: -- slightly more branching in the logic - -### Recommendation - -Implement **Option B**: - -1. check `GET /api/health` -2. if reachable, reuse it -3. otherwise start a local server with the resolved config +- represent the ask session itself +- determine whether the human is done +- force the human to understand internal mechanics --- -## URL behavior +## UX requirements -`githuman ask` should always print the URL to stdout/stderr in a copyable form. +## Dedicated ask page -Examples: +`githuman ask` should send the human to a focused page, not a generic dashboard. -- `http://localhost:3847` -- `https://192.168.1.10:3847?token=...` +Example routes: -If `--open` is enabled, also open the browser. +- `/ask/:id` +- or `/handoff/:id` -Default behavior should match `serve` semantics as closely as possible. +This page should be massively simpler than the normal review UI. ---- +### Page goals -## Config file +The page should answer three questions immediately: -## Goal +1. What is the assistant asking for? +2. Where should I leave feedback? +3. How do I hand this back to the assistant? -Avoid forcing the agent/user to pass `--host` and `--port` every time. +### Page content -### Proposed file location +Recommended layout: -Repository-local: +- title: `Assistant review request` +- assistant message +- review context + - linked review, if available + - repository / branch context +- concise instructions +- current feedback state + - comments count + - unresolved comments count + - todos count +- large sticky primary button: + - **Continue assistant** +- smaller secondary button: + - `Cancel request` -- `.githuman/config.json` +### Interaction model -This keeps the settings next to the repository's GitHuman data. +The human should be able to: -### Example +1. inspect the review +2. leave comments +3. add todos if useful +4. click **Continue assistant** -```json -{ - "host": "localhost", - "port": 3847, - "https": false, - "open": true -} -``` - -### Why repo-local first - -- matches the existing `.githuman/` data model -- works well for dogfooding inside one repository -- easy for agents to discover -- avoids surprising machine-global behavior - -### Future extension - -If needed later, support both: - -1. `~/.githuman/config.json` -2. `.githuman/config.json` - -with repo-local taking precedence. - -### Required behavior - -- `serve` should also read the config file -- `ask` should reuse the same config loader -- CLI flags always override config file values +No hidden rules. +No “mark this special todo as done.” +No generic workflow leakage. --- -## Documentation and website changes - -This feature is important enough that it should ship with a documentation and marketing pass, not just code. - -### Product story to communicate - -`githuman ask` is the handoff point between an LLM and a human reviewer: - -1. the agent asks for review -2. GitHuman opens a human-friendly review surface -3. the human leaves comments and todos -4. the command returns those comments to the agent -5. the agent fixes the issues - -That loop is the headline. It is novel, useful, and easy to demo. - -### Documentation updates - -We should update at least: - -- `README.md` -- CLI help output examples where relevant -- any docs page that explains the review workflow -- release notes / changelog entry if we keep one - -### README changes - -The README should include: - -- a short feature callout near the top -- a concrete `githuman ask` example -- a short explanation of the human-in-the-loop flow -- the new config file location and example -- how completion works in v1 (mark the request todo as done) - -### CLI reference changes - -Document: - -- `githuman ask` options -- config precedence -- how `ask` differs from `serve` -- JSON output mode for agent integrations - -### Workflow docs changes - -Add or update a how-to guide covering: - -- how to ask a human for review from an agent session -- how the human finishes their turn -- how the agent resumes from the printed feedback - -Also add an explanation page or README subsection covering: - -- why `githuman ask` exists -- why the request todo is used as the completion signal in v1 -- how this creates a tight human ↔ agent review loop +## CLI behavior -### Website changes - -The website should treat this as a flagship feature. - -Recommended changes: - -- add a hero or feature section for `githuman ask` -- explain the "agent asks → human reviews → agent resumes" loop visually -- include a terminal example showing `githuman ask` -- include one short UI screenshot of the review experience -- add a concise "why this matters" explanation for teams using AI coding agents - -### Screenshots and demo assets - -Because this is a very visual feature, we should refresh screenshots after implementation. - -At minimum: - -- capture a screenshot showing the request todo / review workflow -- update README screenshots if the feature is shown there -- update website assets if the landing page references the new flow - -If needed, refresh screenshots with: +## Command flow ```bash -node scripts/screenshots.ts +githuman ask "Please review the parser refactor" ``` -### Messaging guidance - -The tone should frame this feature as: - -- human-in-the-loop review for AI coding agents -- a simple bridge between coding agents and human reviewers -- a workflow feature, not just another CLI command - ---- - -## Feedback collected at the end - -When the human completes the request, `githuman ask` should print: +Behavior: -1. the request todo status -2. all current pending todos relevant to the review session -3. all comments added by the human during the session -4. the latest review status +1. resolve config like `serve` +2. reuse a running GitHuman server if available, otherwise start one +3. create an ask session +4. print the dedicated ask URL +5. optionally open the browser +6. wait for ask session status to become `ready_for_agent` or `cancelled` +7. collect feedback from the session +8. print plain text or JSON output for the assistant -### Recommended output shape +## Suggested output -Plain text, optimized for agent consumption: +Plain text: ```text GitHuman feedback ready -URL: http://localhost:3847 +URL: https://host:port/ask/123 +Ask status: ready_for_agent Review status: changes_requested Todos: -- Fix null handling in parser.ts -- Add a test for empty input +- Add a regression test for whitespace-only input Comments: - src/parser.ts:42 - "This should reject undefined explicitly." -- src/parser.test.ts - "Please add a regression test for whitespace-only input." + "Please reject undefined explicitly." ``` -### Important detail - -The command should only print **new feedback from this ask session**, not old historical comments. +JSON: -That means `ask` must record a session start timestamp and filter by: - -- todos created or updated after the session start -- comments created or updated after the session start - -Potentially also filter by review ID when available. +```json +{ + "url": "https://host:port/ask/123", + "askStatus": "ready_for_agent", + "reviewStatus": "changes_requested", + "todos": [], + "comments": [] +} +``` --- -## Review targeting +## Feedback semantics -`githuman ask` needs to know what the human is reviewing. +When `githuman ask` completes, it should return feedback created during that ask session. -### v1 recommendation +Recommended scope: -Operate on the current repository/session and do not force a review ID. - -Behavior: +- comments created or updated after ask session start +- todos created or updated after ask session start +- review status at completion time +- optionally unresolved comments for the targeted review -- if there is an active in-progress review, reuse it -- otherwise open the GitHuman UI and let the human inspect staged changes or create/select the right review - -### Better v1.1 - -Support optional targeting: - -```bash -githuman ask --review -``` - -If `--review` is provided, the end-of-command output should scope comments/todos to that review when possible. +If the ask session is tied to a review, filter primarily by that review. --- -## Polling / waiting model - -`githuman ask` should wait until the request todo is completed. - -### Polling loop +## Config behavior -Every 1-2 seconds: +Keep the new config work. -- fetch the ask todo by ID -- if completed, gather feedback and exit -- if deleted, treat as cancelled -- if server disappears, show a clear error +Use repo-local config: -### Nice-to-have later - -Use SSE/events for lower-latency updates instead of polling. +- `.githuman/config.json` -For v1, polling is simpler and good enough. +Supported defaults: ---- +- `host` +- `port` +- `https` +- `open` +- `authToken` -## Cancellation behavior +Precedence: -### Human cancellation +1. CLI flags +2. `.githuman/config.json` +3. built-in defaults -If the request todo is deleted, exit non-zero with: +This part of the implementation is still valid and should remain. -- `Review request was cancelled` +--- -### Agent cancellation +## Backend/API changes -If the process is interrupted: +## New persistence -- stop any server that `ask` started itself -- leave the request todo in place -- print the URL again before exit if possible +Add an `ask_sessions` table. ---- +Suggested migration: -## CLI options +- create table +- add indexes for `status`, `review_id`, `repository_path`, `created_at` -`githuman ask` should support most of the `serve` options: +## Repository/service layer -- `-p, --port ` -- `--host ` -- `--https` -- `--no-https` -- `--cert ` -- `--key ` -- `--auth [token]` -- `--open` -- `--no-open` -- `-v, --verbose` -- `-h, --help` +Add: -Additional ask-specific options: +- `AskSessionRepository` +- `AskSessionService` -- `--review ` — optional review to watch/scope -- `--interval ` — optional polling interval -- `--json` — machine-readable final output +Capabilities: ---- +- create ask session +- fetch ask session by ID +- list ask sessions if needed +- mark as ready for agent +- cancel ask session +- compute session feedback summary -## JSON output mode +## API routes -For agent integrations, `--json` is important. +Add endpoints such as: -Example: +- `POST /api/asks` +- `GET /api/asks/:id` +- `PATCH /api/asks/:id` +- `POST /api/asks/:id/continue` +- `POST /api/asks/:id/cancel` +- optional `GET /api/asks/:id/feedback` -```json -{ - "url": "http://localhost:3847", - "reviewStatus": "changes_requested", - "todos": [ - { - "id": "...", - "content": "Fix null handling in parser.ts", - "reviewId": null - } - ], - "comments": [ - { - "id": "...", - "reviewId": "...", - "filePath": "src/parser.ts", - "lineNumber": 42, - "content": "This should reject undefined explicitly.", - "resolved": false - } - ] -} -``` +This makes the flow explicit and testable. --- -## Implementation checklist +## Frontend changes -## Milestone 1 — Shared config loading +## New ask page -### Code +Add a dedicated page and route for the ask handoff. -- [ ] add a shared CLI config loader for `.githuman/config.json` -- [ ] support at least: - - [ ] `host` - - [ ] `port` - - [ ] `https` - - [ ] `open` - - [ ] auth-related defaults where appropriate -- [ ] define precedence clearly: - - [ ] CLI flags override config file values - - [ ] config file values override built-in defaults -- [ ] update `githuman serve` to use the shared config loader -- [ ] keep existing env-var behavior working unless we intentionally replace it +Likely additions: -### Acceptance criteria +- `src/web/pages/AskPage.tsx` +- route in `src/web/App.tsx` +- API client methods in `src/web/api/...` +- hooks for ask session loading and completion -- [ ] `githuman serve` works exactly as before when no config file exists -- [ ] `githuman serve` respects `.githuman/config.json` -- [ ] `--host` and `--port` no longer need to be passed every time when config is present +## Simplified UI requirements -## Milestone 2 — Rework `githuman ask` +The ask page should: -### Code +- prioritize the assistant message +- make the next action obvious +- reduce navigation noise +- prominently show the **Continue assistant** button -- [ ] remove the current fire-and-forget todo-creation behavior -- [ ] make `ask` resolve config using the same path as `serve` -- [ ] make `ask` try to connect to an existing server first -- [ ] if no server is running, start one with serve-like options -- [ ] print the final URL in a copyable form -- [ ] support browser opening with serve-like semantics -- [ ] create a dedicated request todo for the ask session -- [ ] store enough local session state to know: - - [ ] the request todo ID - - [ ] the session start time - - [ ] whether `ask` started the server itself +The page may link into the full review UI, but the handoff page itself must stay simple. -### Acceptance criteria +## Continue button behavior -- [ ] `githuman ask` can be run in a fresh repo with GitHuman configured -- [ ] the command shows the human where to review -- [ ] the human has a visible request marker in the UI +Clicking **Continue assistant** should: -## Milestone 3 — Wait for human completion +- update ask session status to `ready_for_agent` +- optionally confirm the action +- show immediate success state +- allow `githuman ask` to exit -### Code - -- [ ] poll the request todo until it is completed -- [ ] treat deletion of the request todo as cancellation -- [ ] handle server unavailability with a clear error -- [ ] support configurable polling interval if we keep `--interval` +--- -### Acceptance criteria +## Documentation updates -- [ ] `githuman ask` stays open while the human is reviewing -- [ ] `githuman ask` exits when the request todo is marked done -- [ ] `githuman ask` exits non-zero if the request todo is deleted +This is still a flagship feature and needs docs alongside implementation. -## Milestone 4 — Print human feedback for the agent +## README -### Code +Update README to describe the new first-class handoff flow: -- [ ] record the session start timestamp before waiting -- [ ] collect feedback created or updated during the session: - - [ ] todos - - [ ] comments - - [ ] review status -- [ ] scope by review ID when available -- [ ] print a plain-text summary for agent consumption -- [ ] add `--json` output mode +- `githuman ask` creates a dedicated assistant review request +- the human completes the handoff with **Continue assistant** +- the feature is distinct from todos +- config file usage and precedence -### Acceptance criteria +## CLI docs -- [ ] the final output contains only feedback relevant to this ask session -- [ ] the output is useful enough for an agent to continue work without extra manual lookup -- [ ] JSON output is stable and machine-readable +Document: -## Milestone 5 — Polish and edge cases +- `githuman ask` options +- server reuse/start behavior +- JSON output +- cancellation behavior -### Code +## Explanation docs -- [ ] improve terminal messaging during startup and waiting -- [ ] handle Ctrl+C cleanly -- [ ] stop any server that `ask` started itself -- [ ] leave user-started servers untouched -- [ ] optionally support `--review ` in the initial implementation or clearly defer it -- [ ] decide whether SSE is out of scope for v1 and document that decision +Add a short explanation of why this feature exists: -### Acceptance criteria +- AI agents need a clean human checkpoint +- GitHuman provides a true human-in-the-loop handoff +- the assistant resumes only when the human explicitly returns control -- [ ] interruption behavior is predictable -- [ ] startup/wait/final-summary messages are easy to understand -- [ ] there is no ambiguity about whether the review request finished, was cancelled, or failed +--- -## Milestone 6 — Documentation updates +## Website updates -### README +The website should highlight this redesigned flow, not the old todo workaround. -- [ ] add a feature callout for `githuman ask` -- [ ] add a concrete CLI example -- [ ] explain the human-in-the-loop flow -- [ ] document `.githuman/config.json` -- [ ] document the completion signal used in v1 +Messaging should emphasize: -### Reference / guides +- assistant asks for review +- human reviews in a focused UI +- human clicks **Continue assistant** +- assistant resumes with structured feedback -- [ ] update CLI reference/help examples for `ask` -- [ ] add or update a workflow guide for the full agent ↔ human loop -- [ ] add or update explanatory docs for why `ask` exists and how it works -- [ ] add release notes / changelog entry if applicable +Recommended updates: -### Acceptance criteria +- hero/feature callout for `githuman ask` +- simple visual 3- or 4-step flow +- screenshot of the simplified ask page +- terminal example showing `githuman ask` -- [ ] a new user can understand `githuman ask` from the README alone -- [ ] a returning user can look up flags and config precedence quickly -- [ ] docs reflect the exact shipped behavior, not an idealized version +--- -## Milestone 7 — Website updates +## Testing plan -### Website content +## Backend tests -- [ ] add `githuman ask` as a flagship feature on the landing page -- [ ] explain the `agent asks → human reviews → agent resumes` loop -- [ ] add a terminal snippet using `githuman ask` -- [ ] add a short explanation of why this matters for AI-assisted development +Add tests for: -### Visuals +- ask session creation +- status transitions +- continue/cancel actions +- feedback collection semantics -- [ ] capture or update screenshots for the ask flow -- [ ] refresh any demo assets used by the README or website -- [ ] ensure screenshots match the current UI +## CLI tests -### Acceptance criteria +Add tests for: -- [ ] the website makes the feature easy to understand in a few seconds -- [ ] the screenshots and copy match the shipped implementation -- [ ] the feature feels prominent, not buried +- creating an ask session +- waiting for `ready_for_agent` +- handling `cancelled` +- JSON output +- config precedence +- server reuse vs startup -## Suggested delivery order +## Frontend tests -- [ ] ship Milestone 1 first -- [ ] ship Milestones 2 and 3 together -- [ ] ship Milestone 4 before calling the feature complete -- [ ] ship Milestones 6 and 7 in the same PR or release window as the code -- [ ] validate all examples and screenshots immediately before merge +Add tests for: ---- +- ask page rendering +- Continue button visibility +- Continue button action +- cancellation flow +- simplified page state -## Backend/API needs +## E2E tests -### Can be done with existing APIs +Add Playwright coverage for: -Mostly yes. +1. run `githuman ask` +2. open ask page +3. leave comment(s) +4. click **Continue assistant** +5. assert CLI exits with collected feedback -Existing endpoints already cover: +--- -- health -- todos -- comments -- reviews +## Delivery plan -### Small additions that may help +## Phase 1 — Preserve useful infrastructure -Potential optional additions: +Keep and refine: -- `GET /api/todos/:id` is already available and useful for polling -- query filters by timestamp may be useful later, but are not required for v1 -- a dedicated `ask session` API is not required for the first version +- shared config loading +- serve/ask runtime sharing +- CLI output formatting +- docs/website momentum ---- +## Phase 2 — Introduce first-class ask sessions -## Testing plan +- add DB migration +- add repository/service/routes +- update CLI to use ask sessions instead of todos -### CLI tests +## Phase 3 — Build the dedicated ask page -Add tests for: +- add new route and page +- simplify UI aggressively +- add **Continue assistant** button -- config file loading -- `ask --help` -- URL printing -- creating the request todo -- waiting until the request todo is completed -- final output includes new todos/comments -- `--json` output -- reuse running server vs start new server +## Phase 4 — Wire completion and feedback -### Integration tests +- CLI waits for ask session status +- collect scoped feedback +- finalize text and JSON output -Add a test that: +## Phase 5 — Docs and website -1. starts the app -2. runs `githuman ask` -3. simulates the human by creating comments/todos and completing the ask todo -4. asserts the command exits with the expected summary +- update README +- update CLI docs +- update landing page +- add screenshot/demo assets -### Optional e2e test +--- -A future Playwright test could validate the full handoff loop in the browser. +## Non-goals for v1 of the redesign -### Documentation / website validation +Avoid these until the core flow feels right: -Before shipping, verify that: +- overly smart automation for detecting completion +- making todos mandatory +- embedding too much general review UI into the ask page +- multi-agent orchestration -- README examples match the real CLI output -- the documented config file format matches the implementation -- the website copy matches the final completion signal semantics -- screenshots reflect the actual UI and current branding +The first priority is a simple, obvious, trustworthy handoff. --- -## Suggested first cut +## Acceptance criteria -To keep this shippable, the first implementation should: +This redesign is successful when: -- use `.githuman/config.json` -- make `serve` and `ask` share config loading -- have `ask` create a dedicated request todo -- treat "request todo marked done" as the completion signal -- poll with `GET /api/todos/:id` -- print new todos/comments since session start -- support `--json` -- update the README and website copy alongside the implementation +- `githuman ask` no longer depends on todos for completion +- the human sees a dedicated, simplified handoff page +- there is a clear **Continue assistant** button +- the assistant reliably resumes with structured feedback +- the flow is understandable without explanation -This gives us the full agent → human → agent loop without requiring a major UI redesign, while also making the feature easy to discover. +If a first-time user can complete the handoff without being told about implementation details, the redesign worked. diff --git a/README.md b/README.md index 3587c65..f464db7 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ Before `git commit`, you get a proper review interface—not a wall of terminal ## Features - **Visual diff review** - Review staged changes in a clean, GitHub-like interface +- **Human handoff for agents** - Run `githuman ask` to ask a human for review, wait, and hand their feedback back to the agent - **Inline comments** - Add comments to specific lines with code suggestions - **Review workflow** - Track status: in progress, approved, or changes requested - **Todo tracking** - Create tasks for follow-up work via CLI or web interface @@ -56,6 +57,8 @@ npx githuman@latest serve ## Quick Start +### Review staged changes before commit + ```bash # Stage your changes (from AI agent or manual edits) git add . @@ -66,6 +69,16 @@ githuman serve This opens a web interface at `http://localhost:3847` where you can review your staged changes before committing. +### Ask a human to review and wait for feedback + +```bash +githuman ask "Please review the parser refactor" +``` + +`githuman ask` starts or reuses GitHuman, prints the review URL, optionally opens the browser, waits for the human to mark the request todo as done, and then prints the new todos, comments, and review status updates for the agent. + +In v1, the completion signal is simple and explicit: the human finishes their turn by marking the ask todo as done in the UI. + ## Agent Skills GitHuman provides an agent skill that teaches AI coding agents when and how to use GitHuman for reviewing changes. @@ -86,8 +99,30 @@ githuman serve [options] Options: -p, --port Port to listen on (default: 3847) --host Host to bind to (default: localhost) + --open Auto-open browser --no-open Don't open browser automatically - --token Require authentication token + --https Enable HTTPS + --no-https Disable HTTPS + --cert Use a custom TLS certificate + --key Use a custom TLS private key + --auth [token] Enable auth, optionally with a provided token + -v, --verbose Enable verbose logging + -h, --help Show help +``` + +### Ask a Human to Review + +```bash +githuman ask [message] [options] + +Options: + -p, --port Port to use for GitHuman + --host Host to bind to when starting GitHuman + --open Open the browser automatically + --no-open Don't open the browser automatically + --review Scope feedback to a specific review + --interval Polling interval in milliseconds + --json Output final feedback as JSON -h, --help Show help ``` @@ -140,6 +175,8 @@ Options: ## Workflow +### Staging-area review + 1. **AI agent makes changes** - Claude, Copilot, Cursor, or any tool stages code 2. **Run `githuman serve`** - Opens the review interface 3. **Review the diff** - See exactly what changed, file by file @@ -148,6 +185,14 @@ Options: 6. **Decide** - Approve and commit, or request changes from the agent 7. **Export** - Optionally save the review as documentation +### Human-in-the-loop agent handoff + +1. **The agent runs `githuman ask`** +2. **GitHuman starts or reconnects** and prints the review URL +3. **The human reviews in the browser** and leaves comments or todos +4. **The human marks the ask todo as done** when feedback is ready +5. **`githuman ask` exits with the new feedback** so the agent can continue + ## Web Interface ### Creating a Review @@ -183,26 +228,53 @@ Set a token to require authentication: ```bash # Via CLI flag -githuman serve --token mysecrettoken +githuman serve --auth mysecrettoken-that-is-at-least-32-characters + +# Auto-generate a token +githuman serve --auth # Via environment variable -GITHUMAN_TOKEN=mysecrettoken githuman serve +GITHUMAN_TOKEN=mysecrettoken-that-is-at-least-32-characters githuman serve ``` Clients must include the token in the `Authorization` header: ``` -Authorization: Bearer mysecrettoken +Authorization: Bearer mysecrettoken-that-is-at-least-32-characters ``` +## Configuration + +GitHuman can read repo-local defaults from `.githuman/config.json`. + +```json +{ + "host": "localhost", + "port": 3847, + "https": false, + "open": true +} +``` + +Config precedence is: + +1. CLI flags +2. `.githuman/config.json` +3. built-in defaults + ## Data Storage -Reviews and comments are stored in a SQLite database at: +GitHuman stores local data in: ``` -/.githuman/reviews.db +/.githuman/ ``` +Important files include: + +- `reviews.db` - reviews, comments, and todos +- `config.json` - optional repo-local CLI defaults + This directory is typically gitignored. ## Development diff --git a/src/cli/commands/ask.ts b/src/cli/commands/ask.ts index 004d973..c683bd0 100644 --- a/src/cli/commands/ask.ts +++ b/src/cli/commands/ask.ts @@ -1,72 +1,170 @@ /** - * Ask command - request human review by creating a todo item + * Ask command - start or reuse GitHuman, wait for human review, then print feedback */ -import { parseArgs } from 'node:util' import { randomUUID } from 'node:crypto' -import { initDatabase, closeDatabase, getDatabase } from '../../server/db/index.ts' -import { createConfig } from '../../server/config.ts' +import { parseArgs } from 'node:util' +import open from 'open' +import { getDatabase } from '../../server/db/index.ts' import { TodoRepository } from '../../server/repositories/todo.repo.ts' +import { CommentRepository } from '../../server/repositories/comment.repo.ts' +import { ReviewRepository } from '../../server/repositories/review.repo.ts' +import type { Comment, Review, Todo } from '../../shared/types.ts' +import { ensureServerRunning, extractAuthArg, getAccessUrl, resolveServerRuntime, stripAuthArg } from '../server-runtime.ts' -/** - * Notify the running server that todos have changed. - * This is fire-and-forget - if the server isn't running, we silently continue. - */ -async function notifyServer (config: { port: number; host: string; authToken: string | null }) { - try { - const headers: Record = { 'Content-Type': 'application/json' } - if (config.authToken) { - headers.Authorization = `Bearer ${config.authToken}` - } +const DEFAULT_INTERVAL_MS = 1500 - const res = await fetch(`http://${config.host}:${config.port}/api/events/notify`, { - method: 'POST', - headers, - body: JSON.stringify({ type: 'todos', action: 'updated' }), - signal: AbortSignal.timeout(1000), - }) - await res.text() - } catch { - // Server not running or unreachable - silently continue - } +interface AskResult { + url: string; + requestTodo: Todo; + reviewStatus: Review['status'] | null; + todos: Todo[]; + comments: Comment[]; } function printHelp () { console.log(` Usage: githuman ask [message] [options] -Ask a human to review something by creating a visible todo item. +Ask a human to review, wait for them to finish, then print their feedback. Arguments: message Optional request for the human reviewer Options: - --review Scope the request to a specific review - --json Output the created todo as JSON + -p, --port Port to use for GitHuman + --host Host to bind to when starting GitHuman + --https Enable HTTPS + --no-https Disable HTTPS + --cert Path to TLS certificate file (PEM format) + --key Path to TLS private key file (PEM format) + --auth [token] Enable auth (auto-generate token if omitted) + --open Open the browser automatically + --no-open Don't open the browser automatically + --review Scope feedback to a specific review + --interval Polling interval in milliseconds (default: 1500) + --json Output the final feedback as JSON + -v, --verbose Enable verbose logging while starting GitHuman -h, --help Show this help message +Config file: + - .githuman/config.json can provide default host, port, https, open, and authToken + - CLI flags override config file values + Examples: githuman ask githuman ask "Please review the parser changes" - githuman ask "Does this migration look safe?" --review abc123 + githuman ask --review abc123 --no-open `) } function buildRequestContent (message: string): string { const trimmed = message.trim() if (!trimmed) { - return 'Review request: Please review the current changes.' + return 'AI review request: Please review the current changes.' + } + + return `AI review request: ${trimmed}` +} + +async function notifyServer (url: string, authToken: string | null) { + try { + const headers: Record = { + 'Content-Type': 'application/json', + } + + if (authToken) { + headers.Authorization = `Bearer ${authToken}` + } + + await fetch(new URL('/api/events/notify', url), { + method: 'POST', + headers, + body: JSON.stringify({ type: 'todos', action: 'updated' }), + signal: AbortSignal.timeout(1000), + }) + } catch { + // Best effort only + } +} + +function sleep (ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)) +} + +function wasTouchedDuringSession (record: { createdAt: string; updatedAt: string }, sessionStartedAt: number): boolean { + return Math.max(Date.parse(record.createdAt), Date.parse(record.updatedAt)) >= sessionStartedAt +} + +function formatTodo (todo: Todo): string { + const suffix = todo.reviewId ? ` (review ${todo.reviewId.slice(0, 8)})` : '' + return `- ${todo.content}${suffix}` +} + +function formatComment (comment: Comment): string { + const location = comment.lineNumber == null + ? comment.filePath + : `${comment.filePath}:${comment.lineNumber}` + + return `- ${location}\n ${JSON.stringify(comment.content)}` +} + +function formatPlainTextResult (result: AskResult): string { + const lines = [ + 'GitHuman feedback ready', + `URL: ${result.url}`, + `Request todo: completed (${result.requestTodo.id.slice(0, 8)})`, + `Review status: ${result.reviewStatus ?? 'unknown'}`, + '', + 'Todos:', + ] + + if (result.todos.length === 0) { + lines.push('- None') + } else { + lines.push(...result.todos.map(formatTodo)) } - return `Review request: ${trimmed}` + lines.push('', 'Comments:') + + if (result.comments.length === 0) { + lines.push('- None') + } else { + lines.push(...result.comments.map(formatComment)) + } + + return lines.join('\n') +} + +function getLatestReviewStatus (reviews: Review[], sessionStartedAt: number, reviewId?: string): Review['status'] | null { + if (reviewId) { + return reviews.find(review => review.id === reviewId)?.status ?? null + } + + const recent = reviews + .filter(review => wasTouchedDuringSession(review, sessionStartedAt)) + .sort((a, b) => Date.parse(b.updatedAt) - Date.parse(a.updatedAt))[0] + + return recent?.status ?? null } export async function askCommand (args: string[]) { + const authValue = extractAuthArg(args) + const filteredArgs = stripAuthArg(args, authValue) const { values, positionals } = parseArgs({ - args, + args: filteredArgs, + allowNegative: true, allowPositionals: true, options: { + port: { type: 'string', short: 'p' }, + open: { type: 'boolean' }, + host: { type: 'string' }, + https: { type: 'boolean' }, + cert: { type: 'string' }, + key: { type: 'string' }, review: { type: 'string' }, + interval: { type: 'string' }, json: { type: 'boolean', default: false }, + verbose: { type: 'boolean', short: 'v' }, help: { type: 'boolean', short: 'h' }, }, }) @@ -76,40 +174,123 @@ export async function askCommand (args: string[]) { process.exit(0) } - const content = buildRequestContent(positionals.join(' ')) - const config = createConfig() + const intervalMs = values.interval ? parseInt(values.interval, 10) : DEFAULT_INTERVAL_MS + if (!Number.isFinite(intervalMs) || intervalMs <= 0) { + console.error('Error: --interval must be a positive integer') + process.exit(1) + } + + const runtime = await resolveServerRuntime(args, { + port: values.port, + open: values.open, + host: values.host, + https: values.https, + cert: values.cert, + key: values.key, + verbose: values.verbose, + }) + + const server = await ensureServerRunning(runtime) + const url = getAccessUrl(runtime.config) + + let interrupted = false + const onInterrupt = () => { + interrupted = true + } + process.once('SIGINT', onInterrupt) + process.once('SIGTERM', onInterrupt) try { - initDatabase(config.dbPath) + if (runtime.openBrowser) { + await open(url) + } + const db = getDatabase() - const repo = new TodoRepository(db) + const todoRepo = new TodoRepository(db) + const commentRepo = new CommentRepository(db) + const reviewRepo = new ReviewRepository(db) + + if (values.review) { + const review = reviewRepo.findById(values.review) + if (!review) { + console.error(`Error: Review not found: ${values.review}`) + process.exitCode = 1 + return + } + } - const todo = repo.create({ + const sessionStartedAt = Date.now() + + const requestTodo = todoRepo.create({ id: randomUUID(), - content, + content: buildRequestContent(positionals.join(' ')), completed: false, reviewId: values.review ?? null, }) - if (values.json) { - console.log(JSON.stringify(todo, null, 2)) - } else { - console.log(`Created review request: ${todo.id.slice(0, 8)}`) - console.log(` ${todo.content}`) - if (todo.reviewId) { - console.log(` Review: ${todo.reviewId.slice(0, 8)}`) - } + await notifyServer(url, runtime.config.authToken) + + if (!values.json) { + console.error(`GitHuman available at: ${url}`) + console.error(`Waiting for human review: ${requestTodo.content}`) + console.error('Mark the request todo as done in GitHuman when feedback is ready.') } - await notifyServer(config) - closeDatabase() - } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') { - console.error('Error: Database does not exist yet. Run "githuman serve" first.') - process.exit(1) - } else { - throw err + while (true) { + if (interrupted) { + console.error(`\nInterrupted. GitHuman is still available at: ${url}`) + process.exitCode = 1 + return + } + + const currentTodo = todoRepo.findById(requestTodo.id) + if (!currentTodo) { + console.error('Review request was cancelled') + process.exitCode = 1 + return + } + + if (currentTodo.completed) { + const allTodos = todoRepo.findAll() + const filteredTodos = allTodos + .filter(todo => todo.id !== requestTodo.id) + .filter(todo => !todo.completed) + .filter(todo => !values.review || todo.reviewId === values.review) + .filter(todo => wasTouchedDuringSession(todo, sessionStartedAt)) + + const reviews = reviewRepo.findAll({ + repositoryPath: runtime.config.repositoryPath, + pageSize: 1000, + }).data + + const comments = (values.review + ? commentRepo.findByReview(values.review) + : reviews.flatMap(review => commentRepo.findByReview(review.id))) + .filter(comment => wasTouchedDuringSession(comment, sessionStartedAt)) + .sort((a, b) => Date.parse(a.createdAt) - Date.parse(b.createdAt)) + + const result: AskResult = { + url, + requestTodo: currentTodo, + reviewStatus: getLatestReviewStatus(reviews, sessionStartedAt, values.review), + todos: filteredTodos, + comments, + } + + if (values.json) { + console.log(JSON.stringify(result, null, 2)) + } else { + console.log(formatPlainTextResult(result)) + } + return + } + + await sleep(intervalMs) } + } finally { + process.removeListener('SIGINT', onInterrupt) + process.removeListener('SIGTERM', onInterrupt) + await server.close() } } diff --git a/src/cli/commands/serve.ts b/src/cli/commands/serve.ts index dba9ede..a918557 100644 --- a/src/cli/commands/serve.ts +++ b/src/cli/commands/serve.ts @@ -2,10 +2,9 @@ * Serve command - start the review server */ import { parseArgs } from 'node:util' -import { readFileSync } from 'node:fs' import open from 'open' -import { startServer, createConfig, generateToken } from '../../server/index.ts' -import { loadOrCreateCertificates, getCertificatePaths } from '../../server/tls/certificates.ts' +import { startServer } from '../../server/index.ts' +import { getAccessUrl, resolveServerRuntime, extractAuthArg, stripAuthArg } from '../server-runtime.ts' function printHelp () { console.log(` @@ -15,83 +14,49 @@ Start the review server and open web interface. Options: -p, --port Port to run server on (default: 3847) + --open Auto-open browser (default: true) --no-open Don't auto-open browser --host Host to bind to (default: localhost) --https Force HTTPS (auto-generates self-signed cert if needed) - --no-https Disable HTTPS (only applies when --host is non-localhost) + --no-https Disable HTTPS --cert Path to TLS certificate file (PEM format) --key Path to TLS private key file (PEM format) --auth [token] Enable auth (auto-generates token if no value given) -v, --verbose Enable verbose logging (full pino-pretty output) -h, --help Show this help message +Config file: + - .githuman/config.json can provide default host, port, https, open, and authToken + - CLI flags override config file values + HTTPS: - - localhost (default): HTTP - no encryption needed for local development - - localhost + --https: HTTPS with self-signed certificate - - --host specified: HTTPS auto-enabled with self-signed certificate - - --no-https: Force HTTP even when using --host + - localhost (default): HTTP unless --https or config enables HTTPS + - non-localhost host: HTTPS auto-enabled with a self-signed certificate unless --no-https is used - --cert/--key: Use custom certificates (both required together) - Auto-generated certificates stored in ~/.githuman/cert.pem and ~/.githuman/key.pem Authentication: - - localhost (default): No auth required - safe for local development - - --host specified: Auth auto-enabled with generated token + - localhost (default): No auth required unless configured + - non-localhost host: Auth auto-enabled with generated token - --auth: Explicitly enable auth (auto-generate or provide your own 32+ char token) `) } -/** - * Check if --auth flag is present and if it has a value. - * Returns: - * - undefined: --auth not present - * - null: --auth present without value (auto-generate) - * - string: --auth present with value - */ -function extractAuthArg (args: string[]): string | null | undefined { - const authIndex = args.findIndex(arg => arg === '--auth') - if (authIndex === -1) { - return undefined // --auth not present - } - - const nextArg = args[authIndex + 1] - if (!nextArg || nextArg.startsWith('-')) { - return null // --auth present but no value - } - - return nextArg // --auth with value -} - export async function serveCommand (args: string[]) { - // Pre-process --auth to handle optional value const authValue = extractAuthArg(args) - - // Check if --https was explicitly passed (to force HTTPS on localhost) - const httpsExplicitlyEnabled = args.includes('--https') - - // Remove --auth and its value from args for parseArgs (if present with value) - let filteredArgs = args - if (authValue !== undefined) { - const authIndex = args.findIndex(arg => arg === '--auth') - if (authValue === null) { - // Just --auth without value, remove only --auth - filteredArgs = [...args.slice(0, authIndex), ...args.slice(authIndex + 1)] - } else { - // --auth with value, remove both - filteredArgs = [...args.slice(0, authIndex), ...args.slice(authIndex + 2)] - } - } + const filteredArgs = stripAuthArg(args, authValue) const { values } = parseArgs({ args: filteredArgs, allowNegative: true, options: { - port: { type: 'string', short: 'p', default: '3847' }, - open: { type: 'boolean', default: true }, - host: { type: 'string', default: 'localhost' }, - https: { type: 'boolean', default: true }, + port: { type: 'string', short: 'p' }, + open: { type: 'boolean' }, + host: { type: 'string' }, + https: { type: 'boolean' }, cert: { type: 'string' }, key: { type: 'string' }, - verbose: { type: 'boolean', short: 'v', default: false }, + verbose: { type: 'boolean', short: 'v' }, help: { type: 'boolean', short: 'h' }, }, }) @@ -101,73 +66,23 @@ export async function serveCommand (args: string[]) { return } - const host = values.host! - const isNonLocalhost = host !== 'localhost' && host !== '127.0.0.1' - - // Validate --cert and --key must be used together - if ((values.cert && !values.key) || (!values.cert && values.key)) { - throw new Error('--cert and --key must be specified together') - } - - // Determine auth token: - // - If --auth was used with a value, use that value - // - If --auth was used without value, auto-generate - // - If --host is specified (non-localhost), auto-generate - // - Otherwise (localhost, no --auth), no auth needed - let authToken: string | undefined - let tokenAutoGenerated = false - if (typeof authValue === 'string') { - // --auth provided - authToken = authValue - } else if (authValue === null || isNonLocalhost) { - // --auth without value OR non-localhost host → auto-generate - authToken = generateToken() - tokenAutoGenerated = true - } - // else: localhost without --auth → no auth (authToken remains undefined → null in config) - - // Determine HTTPS: - // - Custom certs provided → HTTPS enabled - // - --https explicitly passed → HTTPS enabled (force on localhost) - // - Non-localhost host + values.https (not --no-https) → HTTPS enabled - // - Otherwise → HTTP - const hasCustomCerts = !!(values.cert && values.key) - const enableHttps = hasCustomCerts || httpsExplicitlyEnabled || (isNonLocalhost && values.https) - let tlsCert: string | undefined - let tlsKey: string | undefined - let certificatePath: string | undefined - - if (enableHttps) { - if (hasCustomCerts) { - // Load custom certificates from files - tlsCert = readFileSync(values.cert!, 'utf-8') - tlsKey = readFileSync(values.key!, 'utf-8') - certificatePath = values.cert! - } else { - // Use auto-generated self-signed certificates - const { cert, key } = await loadOrCreateCertificates() - tlsCert = cert - tlsKey = key - certificatePath = getCertificatePaths().certPath - } - } - - const config = createConfig({ - port: parseInt(values.port!, 10), - host, - authToken, - https: enableHttps, - tlsCert, - tlsKey, + const runtime = await resolveServerRuntime(args, { + port: values.port, + open: values.open, + host: values.host, + https: values.https, + cert: values.cert, + key: values.key, + verbose: values.verbose, }) - // Start the server - await startServer(config, { verbose: values.verbose, tokenAutoGenerated, certificatePath }) + await startServer(runtime.config, { + verbose: runtime.verbose, + tokenAutoGenerated: runtime.tokenAutoGenerated, + certificatePath: runtime.certificatePath, + }) - // Open browser if requested - if (values.open) { - const protocol = config.https ? 'https' : 'http' - const url = `${protocol}://${config.host}:${config.port}` - await open(url) + if (runtime.openBrowser) { + await open(getAccessUrl(runtime.config)) } } diff --git a/src/cli/config.ts b/src/cli/config.ts new file mode 100644 index 0000000..368151c --- /dev/null +++ b/src/cli/config.ts @@ -0,0 +1,107 @@ +/** + * Shared CLI config loading + */ +import { existsSync, readFileSync } from 'node:fs' +import { execSync } from 'node:child_process' +import { join, resolve } from 'node:path' + +export interface CliConfigFile { + host?: string; + port?: number; + https?: boolean; + open?: boolean; + authToken?: string; +} + +export interface LoadedCliConfig { + repositoryPath: string; + configPath: string; + values: CliConfigFile; +} + +function isPlainObject (value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function getRepositoryPath (cwd: string): string { + const resolvedCwd = resolve(cwd) + + try { + const result = execSync('git rev-parse --show-toplevel', { + cwd: resolvedCwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }) + return result.trim() + } catch { + return resolvedCwd + } +} + +export function getCliConfigPath (cwd = process.cwd()): string { + return join(getRepositoryPath(cwd), '.githuman', 'config.json') +} + +export function loadCliConfig (cwd = process.cwd()): LoadedCliConfig { + const repositoryPath = getRepositoryPath(cwd) + const configPath = join(repositoryPath, '.githuman', 'config.json') + + if (!existsSync(configPath)) { + return { + repositoryPath, + configPath, + values: {}, + } + } + + const raw = readFileSync(configPath, 'utf-8') + const parsed = JSON.parse(raw) as unknown + + if (!isPlainObject(parsed)) { + throw new Error(`Invalid GitHuman config in ${configPath}: expected a JSON object`) + } + + const values: CliConfigFile = {} + + if (parsed.host !== undefined) { + if (typeof parsed.host !== 'string' || !parsed.host.trim()) { + throw new Error(`Invalid GitHuman config in ${configPath}: "host" must be a non-empty string`) + } + values.host = parsed.host + } + + if (parsed.port !== undefined) { + const portValue = parsed.port + if (typeof portValue !== 'number' || !Number.isInteger(portValue) || portValue <= 0) { + throw new Error(`Invalid GitHuman config in ${configPath}: "port" must be a positive integer`) + } + values.port = portValue + } + + if (parsed.https !== undefined) { + if (typeof parsed.https !== 'boolean') { + throw new Error(`Invalid GitHuman config in ${configPath}: "https" must be a boolean`) + } + values.https = parsed.https + } + + if (parsed.open !== undefined) { + if (typeof parsed.open !== 'boolean') { + throw new Error(`Invalid GitHuman config in ${configPath}: "open" must be a boolean`) + } + values.open = parsed.open + } + + if (parsed.authToken !== undefined) { + if (typeof parsed.authToken !== 'string' || !parsed.authToken.trim()) { + throw new Error(`Invalid GitHuman config in ${configPath}: "authToken" must be a non-empty string`) + } + values.authToken = parsed.authToken + } + + return { + repositoryPath, + configPath, + values, + } +} diff --git a/src/cli/index.ts b/src/cli/index.ts index aac8070..3515cda 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -42,7 +42,7 @@ Commands: list List all saved reviews for the current repository export Export a review to markdown resolve Mark a review as approved and resolve all comments - ask Ask a human to review by creating a todo item + ask Ask a human to review, wait, and print their feedback todo Manage todo items for tracking tasks Options: diff --git a/src/cli/server-runtime.ts b/src/cli/server-runtime.ts new file mode 100644 index 0000000..741e4fb --- /dev/null +++ b/src/cli/server-runtime.ts @@ -0,0 +1,234 @@ +/** + * Shared server runtime resolution for serve-like commands + */ +import { request as httpRequest } from 'node:http' +import { request as httpsRequest } from 'node:https' +import { readFileSync } from 'node:fs' +import type { FastifyInstance } from 'fastify' +import { loadCliConfig } from './config.ts' +import { buildApp } from '../server/app.ts' +import { createConfig, generateToken, type ServerConfig } from '../server/config.ts' +import { initDatabase, closeDatabase } from '../server/db/index.ts' +import { loadOrCreateCertificates, getCertificatePaths } from '../server/tls/certificates.ts' + +export interface ResolvedServerRuntime { + config: ServerConfig; + openBrowser: boolean; + verbose: boolean; + tokenAutoGenerated: boolean; + certificatePath?: string; +} + +export interface StartedServerHandle { + app: FastifyInstance; + startedByCommand: boolean; + close: () => Promise; +} + +function getConnectHost (host: string): string { + if (host === '0.0.0.0') return '127.0.0.1' + if (host === '::') return 'localhost' + return host +} + +export function getAccessUrl (config: ServerConfig): string { + const protocol = config.https ? 'https' : 'http' + const host = getConnectHost(config.host) + const base = `${protocol}://${host}:${config.port}` + + if (!config.authToken) { + return base + } + + return `${base}?token=${encodeURIComponent(config.authToken)}` +} + +async function healthCheck (config: ServerConfig): Promise { + const host = getConnectHost(config.host) + + return await new Promise((resolve) => { + const onResponse = (res: { statusCode?: number | undefined; resume: () => void }) => { + res.resume() + resolve((res.statusCode ?? 0) >= 200 && (res.statusCode ?? 0) < 300) + } + + const req = config.https + ? httpsRequest({ + protocol: 'https:', + hostname: host, + port: config.port, + path: '/api/health', + method: 'GET', + timeout: 1000, + rejectUnauthorized: false, + }, onResponse) + : httpRequest({ + protocol: 'http:', + hostname: host, + port: config.port, + path: '/api/health', + method: 'GET', + timeout: 1000, + }, onResponse) + + req.on('timeout', () => { + req.destroy() + resolve(false) + }) + + req.on('error', () => { + resolve(false) + }) + + req.end() + }) +} + +/** + * Check if --auth flag is present and if it has a value. + * Returns: + * - undefined: --auth not present + * - null: --auth present without value (auto-generate) + * - string: --auth present with value + */ +export function extractAuthArg (args: string[]): string | null | undefined { + const authIndex = args.findIndex(arg => arg === '--auth') + if (authIndex === -1) { + return undefined + } + + const nextArg = args[authIndex + 1] + if (!nextArg || nextArg.startsWith('-')) { + return null + } + + return nextArg +} + +export function stripAuthArg (args: string[], authValue: string | null | undefined): string[] { + if (authValue === undefined) { + return args + } + + const authIndex = args.findIndex(arg => arg === '--auth') + if (authIndex === -1) { + return args + } + + if (authValue === null) { + return [...args.slice(0, authIndex), ...args.slice(authIndex + 1)] + } + + return [...args.slice(0, authIndex), ...args.slice(authIndex + 2)] +} + +export async function resolveServerRuntime (args: string[], values: { + port?: string; + open?: boolean; + host?: string; + https?: boolean; + cert?: string; + key?: string; + verbose?: boolean; +}): Promise { + const authValue = extractAuthArg(args) + const cliConfig = loadCliConfig() + const fileConfig = cliConfig.values + + const host = values.host ?? fileConfig.host ?? 'localhost' + const port = parseInt(values.port ?? String(fileConfig.port ?? 3847), 10) + const openBrowser = values.open ?? fileConfig.open ?? true + const verbose = values.verbose ?? false + const isNonLocalhost = host !== 'localhost' && host !== '127.0.0.1' + + if (!Number.isInteger(port) || port <= 0) { + throw new Error('--port must be a positive integer') + } + + if ((values.cert && !values.key) || (!values.cert && values.key)) { + throw new Error('--cert and --key must be specified together') + } + + let authToken: string | undefined + let tokenAutoGenerated = false + + if (typeof authValue === 'string') { + authToken = authValue + } else if (authValue === null) { + authToken = generateToken() + tokenAutoGenerated = true + } else if (fileConfig.authToken) { + authToken = fileConfig.authToken + } else if (isNonLocalhost) { + authToken = generateToken() + tokenAutoGenerated = true + } + + const hasCustomCerts = !!(values.cert && values.key) + const httpsPreference = values.https ?? fileConfig.https + const enableHttps = hasCustomCerts || (httpsPreference ?? isNonLocalhost) + let tlsCert: string | undefined + let tlsKey: string | undefined + let certificatePath: string | undefined + + if (enableHttps) { + if (hasCustomCerts) { + tlsCert = readFileSync(values.cert!, 'utf-8') + tlsKey = readFileSync(values.key!, 'utf-8') + certificatePath = values.cert! + } else { + const { cert, key } = await loadOrCreateCertificates() + tlsCert = cert + tlsKey = key + certificatePath = getCertificatePaths().certPath + } + } + + const config = createConfig({ + port, + host, + authToken, + https: enableHttps, + tlsCert, + tlsKey, + }) + + return { + config, + openBrowser, + verbose, + tokenAutoGenerated, + certificatePath, + } +} + +export async function ensureServerRunning (runtime: ResolvedServerRuntime): Promise { + const running = await healthCheck(runtime.config) + + if (running) { + initDatabase(runtime.config.dbPath) + return { + app: null as unknown as FastifyInstance, + startedByCommand: false, + close: async () => { + closeDatabase() + }, + } + } + + initDatabase(runtime.config.dbPath) + const app = await buildApp(runtime.config, { + logger: runtime.verbose, + verbose: runtime.verbose, + }) + await app.listen({ port: runtime.config.port, host: runtime.config.host }) + + return { + app, + startedByCommand: true, + close: async () => { + await app.close() + closeDatabase() + }, + } +} diff --git a/src/server/app.ts b/src/server/app.ts index 1d3f4ae..ea95a09 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -140,7 +140,7 @@ export async function buildApp ( bearerAuth: { type: 'http', scheme: 'bearer', - description: 'Optional Bearer token authentication. Set via GITHUMAN_TOKEN env var or --token flag.', + description: 'Optional Bearer token authentication. Set via GITHUMAN_TOKEN env var or --auth flag.', }, }, }, diff --git a/tests/cli/ask.test.ts b/tests/cli/ask.test.ts new file mode 100644 index 0000000..26200d7 --- /dev/null +++ b/tests/cli/ask.test.ts @@ -0,0 +1,227 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert' +import { spawn } from 'node:child_process' +import { join } from 'node:path' +import { randomUUID } from 'node:crypto' +import { DatabaseSync } from 'node:sqlite' +import { createTestRepoWithDb } from './test-utils.ts' +import { initDatabase, closeDatabase, getDatabase } from '../../src/server/db/index.ts' +import { buildApp } from '../../src/server/app.ts' +import { createConfig } from '../../src/server/config.ts' +import { ReviewRepository } from '../../src/server/repositories/review.repo.ts' +import { TodoRepository } from '../../src/server/repositories/todo.repo.ts' +import { CommentRepository } from '../../src/server/repositories/comment.repo.ts' + +const CLI_PATH = join(import.meta.dirname, '../../src/cli/index.ts') + +interface ExecResult { + stdout: string; + stderr: string; + exitCode: number | null; +} + +function openTestDatabase (repoDir: string): DatabaseSync { + return new DatabaseSync(join(repoDir, '.githuman', 'reviews.db'), { + enableForeignKeyConstraints: true, + timeout: 1000, + }) +} + +function createReview (repoDir: string): string { + initDatabase(join(repoDir, '.githuman', 'reviews.db')) + const db = getDatabase() + const reviewRepo = new ReviewRepository(db) + const reviewId = randomUUID() + + reviewRepo.create({ + id: reviewId, + repositoryPath: repoDir, + baseRef: 'main', + sourceType: 'branch', + sourceRef: 'feature/test', + snapshotData: JSON.stringify({ + repository: { + name: 'test-repo', + branch: 'feature/test', + remote: null, + path: repoDir, + }, + version: 2, + }), + status: 'in_progress', + }) + + closeDatabase() + return reviewId +} + +async function waitForRequestTodo (repoDir: string, content: string): Promise<{ id: string; content: string }> { + const deadline = Date.now() + 10000 + + while (Date.now() < deadline) { + const db = openTestDatabase(repoDir) + const todoRepo = new TodoRepository(db) + const todo = todoRepo.findAll().find(item => item.content === content) + db.close() + + if (todo) { + return { id: todo.id, content: todo.content } + } + + await new Promise(resolve => setTimeout(resolve, 50)) + } + + throw new Error('Timed out waiting for ask request todo') +} + +async function completeAskSession (repoDir: string, reviewId: string, requestTodoId: string) { + const db = openTestDatabase(repoDir) + const todoRepo = new TodoRepository(db) + const commentRepo = new CommentRepository(db) + const reviewRepo = new ReviewRepository(db) + + todoRepo.create({ + id: randomUUID(), + content: 'Add a regression test for whitespace-only input', + completed: false, + reviewId, + }) + + commentRepo.create({ + id: randomUUID(), + reviewId, + filePath: 'src/parser.ts', + lineNumber: 42, + lineType: 'added', + content: 'Please reject undefined explicitly.', + suggestion: null, + resolved: false, + }) + + reviewRepo.update(reviewId, { + status: 'changes_requested', + }) + + todoRepo.update(requestTodoId, { completed: true }) + db.close() +} + +async function startAskServer (repoDir: string, port: number) { + initDatabase(join(repoDir, '.githuman', 'reviews.db')) + const config = createConfig({ + repositoryPath: repoDir, + dbPath: join(repoDir, '.githuman', 'reviews.db'), + host: 'localhost', + port, + https: false, + authToken: null, + }) + const app = await buildApp(config, { logger: false }) + await app.listen({ port, host: 'localhost' }) + + return { + close: async () => { + await app.close() + closeDatabase() + }, + } +} + +async function runAsk (args: string[], cwd: string): Promise { + return await new Promise((resolve, reject) => { + const child = spawn('node', [CLI_PATH, 'ask', ...args], { + cwd, + env: { ...process.env }, + }) + + let stdout = '' + let stderr = '' + const timeout = setTimeout(() => { + child.kill('SIGTERM') + reject(new Error('Timed out waiting for ask command to finish')) + }, 15000) + + child.stdout.on('data', data => { + stdout += data.toString() + }) + + child.stderr.on('data', data => { + stderr += data.toString() + }) + + child.on('close', exitCode => { + clearTimeout(timeout) + resolve({ stdout, stderr, exitCode }) + }) + }) +} + +describe('githuman ask', { concurrency: 1 }, () => { + it('waits for the human to finish and prints plain-text feedback', async (t) => { + const repoDir = await createTestRepoWithDb(t) + const reviewId = createReview(repoDir) + const port = 45000 + Math.floor(Math.random() * 1000) + const requestContent = 'AI review request: Please review the parser refactor' + const server = await startAskServer(repoDir, port) + + try { + const askPromise = runAsk([ + '--review', reviewId, + '--no-open', + '--interval', '50', + '--port', String(port), + 'Please review the parser refactor', + ], repoDir) + + const requestTodo = await waitForRequestTodo(repoDir, requestContent) + await completeAskSession(repoDir, reviewId, requestTodo.id) + + const result = await askPromise + + assert.strictEqual(result.exitCode, 0, `${result.stderr}\n${result.stdout}`) + assert.ok(result.stderr.includes('GitHuman available at: http://localhost:')) + assert.ok(result.stdout.includes('GitHuman feedback ready')) + assert.ok(result.stdout.includes('Review status: changes_requested')) + assert.ok(result.stdout.includes('Add a regression test for whitespace-only input')) + assert.ok(result.stdout.includes('src/parser.ts:42')) + assert.ok(result.stdout.includes('Please reject undefined explicitly.')) + } finally { + await server.close() + } + }) + + it('supports machine-readable JSON output', async (t) => { + const repoDir = await createTestRepoWithDb(t) + const reviewId = createReview(repoDir) + const port = 46000 + Math.floor(Math.random() * 1000) + const requestContent = 'AI review request: Please review the current changes.' + const server = await startAskServer(repoDir, port) + + try { + const askPromise = runAsk([ + '--review', reviewId, + '--no-open', + '--interval', '50', + '--port', String(port), + '--json', + ], repoDir) + + const requestTodo = await waitForRequestTodo(repoDir, requestContent) + await completeAskSession(repoDir, reviewId, requestTodo.id) + + const result = await askPromise + assert.strictEqual(result.exitCode, 0, `${result.stderr}\n${result.stdout}`) + + const data = JSON.parse(result.stdout) + assert.ok(data.url.startsWith('http://localhost:')) + assert.strictEqual(data.reviewStatus, 'changes_requested') + assert.strictEqual(data.requestTodo.content, requestContent) + assert.strictEqual(data.todos.length, 1) + assert.strictEqual(data.comments.length, 1) + assert.strictEqual(data.todos[0].content, 'Add a regression test for whitespace-only input') + assert.strictEqual(data.comments[0].content, 'Please reject undefined explicitly.') + } finally { + await server.close() + } + }) +}) diff --git a/tests/cli/config.test.ts b/tests/cli/config.test.ts new file mode 100644 index 0000000..f4f8136 --- /dev/null +++ b/tests/cli/config.test.ts @@ -0,0 +1,59 @@ +import { after, describe, it } from 'node:test' +import assert from 'node:assert' +import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { execSync } from 'node:child_process' +import { loadCliConfig } from '../../src/cli/config.ts' + +function createRepo (): string { + const dir = mkdtempSync(join(tmpdir(), 'githuman-cli-config-')) + execSync('git init', { cwd: dir, stdio: 'ignore' }) + execSync('git config user.email "test@test.com"', { cwd: dir, stdio: 'ignore' }) + execSync('git config user.name "Test"', { cwd: dir, stdio: 'ignore' }) + after(() => { + rmSync(dir, { recursive: true, force: true }) + }) + return dir +} + +describe('cli config loader', () => { + it('loads repo-local .githuman/config.json', () => { + const repoDir = createRepo() + mkdirSync(join(repoDir, '.githuman'), { recursive: true }) + writeFileSync(join(repoDir, '.githuman', 'config.json'), JSON.stringify({ + host: '0.0.0.0', + port: 4010, + https: true, + open: false, + }, null, 2)) + + const loaded = loadCliConfig(repoDir) + + assert.strictEqual(loaded.repositoryPath, repoDir) + assert.strictEqual(loaded.values.host, '0.0.0.0') + assert.strictEqual(loaded.values.port, 4010) + assert.strictEqual(loaded.values.https, true) + assert.strictEqual(loaded.values.open, false) + }) + + it('returns empty values when no config file exists', () => { + const repoDir = createRepo() + const loaded = loadCliConfig(repoDir) + + assert.deepStrictEqual(loaded.values, {}) + assert.strictEqual(loaded.repositoryPath, repoDir) + assert.strictEqual(loaded.configPath, join(repoDir, '.githuman', 'config.json')) + }) + + it('rejects invalid config types', () => { + const repoDir = createRepo() + mkdirSync(join(repoDir, '.githuman'), { recursive: true }) + writeFileSync(join(repoDir, '.githuman', 'config.json'), JSON.stringify({ port: 'bad' })) + + assert.throws( + () => loadCliConfig(repoDir), + /"port" must be a positive integer/ + ) + }) +}) diff --git a/tests/cli/index.test.ts b/tests/cli/index.test.ts index 1b57354..e6d0c11 100644 --- a/tests/cli/index.test.ts +++ b/tests/cli/index.test.ts @@ -393,42 +393,8 @@ describe('CLI', () => { assert.ok(result.stdout.includes('Usage: githuman ask')) assert.ok(result.stdout.includes('--review ')) assert.ok(result.stdout.includes('--json')) - }) - - it('should create a review request with the default message', async (t) => { - const askTempDir = await createTestRepoWithDb(t) - const result = await runCli(['ask'], { cwd: askTempDir }) - - assert.strictEqual(result.exitCode, 0) - assert.ok(result.stdout.includes('Created review request')) - assert.ok(result.stdout.includes('Review request: Please review the current changes.')) - - const listResult = await runCli(['todo', 'list', '--all'], { cwd: askTempDir }) - assert.strictEqual(listResult.exitCode, 0) - assert.ok(listResult.stdout.includes('Review request: Please review the current changes.')) - }) - - it('should create a review request with a custom message', async (t) => { - const askTempDir = await createTestRepoWithDb(t) - const result = await runCli(['ask', 'Please review the parser changes'], { cwd: askTempDir }) - - assert.strictEqual(result.exitCode, 0) - assert.ok(result.stdout.includes('Review request: Please review the parser changes')) - - const listResult = await runCli(['todo', 'list', '--all'], { cwd: askTempDir }) - assert.strictEqual(listResult.exitCode, 0) - assert.ok(listResult.stdout.includes('Review request: Please review the parser changes')) - }) - - it('should output JSON when requested', async (t) => { - const askTempDir = await createTestRepoWithDb(t) - const result = await runCli(['ask', 'Does this look correct?', '--json'], { cwd: askTempDir }) - - assert.strictEqual(result.exitCode, 0) - const data = JSON.parse(result.stdout) - assert.strictEqual(data.content, 'Review request: Does this look correct?') - assert.strictEqual(data.completed, false) - assert.strictEqual(data.reviewId, null) + assert.ok(result.stdout.includes('--interval ')) + assert.ok(result.stdout.includes('--host ')) }) }) diff --git a/website/index.html b/website/index.html index 9597195..c846a85 100644 --- a/website/index.html +++ b/website/index.html @@ -421,7 +421,7 @@

Review AI Code Before Commit

-

GitHub revolutionized how humans collaborate on code.
GitHuman defines how humans review code written by AI.

+

GitHuman gives AI coding agents a clean handoff to human reviewers.
Review staged changes, collect feedback, and send it back to the agent.

@@ -432,7 +432,7 @@

Review AI Code Before Commit
- cd /your/repo && npx githuman@latest serve + cd /your/repo && npx githuman@latest ask "Please review these changes"
@@ -465,6 +465,10 @@

Features

Visual Diff Review

Review staged changes in a clean, syntax-highlighted interface. No more squinting at terminal output.

+
+

Agent-to-Human Handoff

+

Run githuman ask to open GitHuman, wait for a human review, and hand the resulting comments and todos back to the agent.

+

Inline Comments

Add comments to specific lines with optional code suggestions. Track issues before they become commits.

@@ -522,29 +526,29 @@

AI agent makes changes

2
-

Run githuman serve

-

Opens the review interface in your browser

+

Run githuman ask

+

GitHuman starts or reconnects, prints the URL, and can open the browser automatically

3
-

Review the diff

-

See exactly what changed, file by file, with syntax highlighting

+

Human reviews the changes

+

They inspect the diff, add comments, and create follow-up todos in the UI

4
-

Add comments & todos

-

Note issues, questions, or follow-up work

+

Mark the request todo as done

+

That explicit action tells GitHuman the human's turn is complete

5
-

Decide

-

Approve and commit, or request changes from the agent

+

The agent gets structured feedback back

+

githuman ask exits with the new todos, comments, and review status so the agent can continue

From 93afb0154c9e55fe6f5589da1fad1e4f3cfd83e5 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 4 Apr 2026 23:51:18 +0200 Subject: [PATCH 3/4] Implement ask session handoff flow --- README.md | 6 +- src/cli/commands/ask.ts | 218 +++++++++------- src/cli/server-runtime.ts | 15 +- src/server/app.ts | 17 +- src/server/db/migrations.ts | 23 ++ src/server/repositories/ask-session.repo.ts | 112 ++++++++ src/server/routes/asks.ts | 217 ++++++++++++++++ src/server/routes/events.ts | 4 +- src/server/services/ask-session.service.ts | 190 ++++++++++++++ src/shared/types.ts | 48 ++++ src/web/App.tsx | 2 + src/web/api/asks.ts | 27 ++ src/web/components/layout/AskSessionDock.tsx | 47 ++++ src/web/components/layout/Layout.tsx | 2 + src/web/hooks/useAsks.ts | 110 ++++++++ src/web/hooks/useServerEvents.ts | 2 +- src/web/pages/AskPage.tsx | 204 +++++++++++++++ tests/cli/ask.test.ts | 44 ++-- tests/server/app.test.ts | 43 ++++ tests/server/routes/asks.test.ts | 254 +++++++++++++++++++ tests/server/routes/events.test.ts | 13 + tests/web/components/AskSessionDock.test.tsx | 64 +++++ tests/web/hooks/useAsks.test.ts | 117 +++++++++ website/index.html | 2 +- 24 files changed, 1650 insertions(+), 131 deletions(-) create mode 100644 src/server/repositories/ask-session.repo.ts create mode 100644 src/server/routes/asks.ts create mode 100644 src/server/services/ask-session.service.ts create mode 100644 src/web/api/asks.ts create mode 100644 src/web/components/layout/AskSessionDock.tsx create mode 100644 src/web/hooks/useAsks.ts create mode 100644 src/web/pages/AskPage.tsx create mode 100644 tests/server/routes/asks.test.ts create mode 100644 tests/web/components/AskSessionDock.test.tsx create mode 100644 tests/web/hooks/useAsks.test.ts diff --git a/README.md b/README.md index f464db7..8f2f7e2 100644 --- a/README.md +++ b/README.md @@ -75,9 +75,9 @@ This opens a web interface at `http://localhost:3847` where you can review your githuman ask "Please review the parser refactor" ``` -`githuman ask` starts or reuses GitHuman, prints the review URL, optionally opens the browser, waits for the human to mark the request todo as done, and then prints the new todos, comments, and review status updates for the agent. +`githuman ask` starts or reuses GitHuman, opens a dedicated ask page, waits for the human to finish reviewing, and then prints the new todos, comments, and review status updates for the agent. -In v1, the completion signal is simple and explicit: the human finishes their turn by marking the ask todo as done in the UI. +The handoff is completed in the UI with a dedicated **Continue assistant** button. ## Agent Skills @@ -190,7 +190,7 @@ Options: 1. **The agent runs `githuman ask`** 2. **GitHuman starts or reconnects** and prints the review URL 3. **The human reviews in the browser** and leaves comments or todos -4. **The human marks the ask todo as done** when feedback is ready +4. **The human clicks `Continue assistant`** when feedback is ready 5. **`githuman ask` exits with the new feedback** so the agent can continue ## Web Interface diff --git a/src/cli/commands/ask.ts b/src/cli/commands/ask.ts index c683bd0..60561b6 100644 --- a/src/cli/commands/ask.ts +++ b/src/cli/commands/ask.ts @@ -1,24 +1,32 @@ /** - * Ask command - start or reuse GitHuman, wait for human review, then print feedback + * Ask command - create an assistant↔human handoff session and wait for completion */ -import { randomUUID } from 'node:crypto' import { parseArgs } from 'node:util' import open from 'open' -import { getDatabase } from '../../server/db/index.ts' -import { TodoRepository } from '../../server/repositories/todo.repo.ts' -import { CommentRepository } from '../../server/repositories/comment.repo.ts' -import { ReviewRepository } from '../../server/repositories/review.repo.ts' -import type { Comment, Review, Todo } from '../../shared/types.ts' -import { ensureServerRunning, extractAuthArg, getAccessUrl, resolveServerRuntime, stripAuthArg } from '../server-runtime.ts' +import type { AskFeedback, AskSession, AskSessionDetails, Comment, CreateAskSessionRequest, Todo } from '../../shared/types.ts' +import { ensureServerRunning, extractAuthArg, getAppUrl, resolveServerRuntime, stripAuthArg } from '../server-runtime.ts' const DEFAULT_INTERVAL_MS = 1500 -interface AskResult { +interface AskResult extends AskFeedback { url: string; - requestTodo: Todo; - reviewStatus: Review['status'] | null; - todos: Todo[]; - comments: Comment[]; +} + +interface ApiErrorResponse { + error?: string; + code?: string; +} + +class AskCommandApiError extends Error { + status: number + code?: string + + constructor (message: string, status: number, code?: string) { + super(message) + this.name = 'AskCommandApiError' + this.status = status + this.code = code + } } function printHelp () { @@ -57,29 +65,74 @@ Examples: `) } -function buildRequestContent (message: string): string { +function buildRequestMessage (message: string): string { const trimmed = message.trim() if (!trimmed) { - return 'AI review request: Please review the current changes.' + return 'Please review the current changes.' } - return `AI review request: ${trimmed}` + return trimmed } -async function notifyServer (url: string, authToken: string | null) { - try { - const headers: Record = { - 'Content-Type': 'application/json', - } +function getHeaders (authToken: string | null): Record { + const headers: Record = { + 'Content-Type': 'application/json', + } - if (authToken) { - headers.Authorization = `Bearer ${authToken}` - } + if (authToken) { + headers.Authorization = `Bearer ${authToken}` + } + + return headers +} + +async function parseResponse (response: Response): Promise { + if (!response.ok) { + const error = (await response.json().catch(() => ({}))) as ApiErrorResponse + throw new AskCommandApiError( + error.error ?? `HTTP ${response.status}`, + response.status, + error.code + ) + } + + return await response.json() as T +} + +async function createAskSession (baseUrl: string, authToken: string | null, request: CreateAskSessionRequest): Promise { + const response = await fetch(new URL('/api/asks', baseUrl), { + method: 'POST', + headers: getHeaders(authToken), + body: JSON.stringify(request), + }) + + return await parseResponse(response) +} - await fetch(new URL('/api/events/notify', url), { +async function getAskSession (baseUrl: string, authToken: string | null, id: string): Promise { + const response = await fetch(new URL(`/api/asks/${id}`, baseUrl), { + method: 'GET', + headers: getHeaders(authToken), + }) + + return await parseResponse(response) +} + +async function getAskFeedback (baseUrl: string, authToken: string | null, id: string): Promise { + const response = await fetch(new URL(`/api/asks/${id}/feedback`, baseUrl), { + method: 'GET', + headers: getHeaders(authToken), + }) + + return await parseResponse(response) +} + +async function notifyServer (baseUrl: string, authToken: string | null, type: 'asks' | 'todos' | 'reviews' | 'comments') { + try { + await fetch(new URL('/api/events/notify', baseUrl), { method: 'POST', - headers, - body: JSON.stringify({ type: 'todos', action: 'updated' }), + headers: getHeaders(authToken), + body: JSON.stringify({ type, action: 'updated' }), signal: AbortSignal.timeout(1000), }) } catch { @@ -91,13 +144,10 @@ function sleep (ms: number) { return new Promise(resolve => setTimeout(resolve, ms)) } -function wasTouchedDuringSession (record: { createdAt: string; updatedAt: string }, sessionStartedAt: number): boolean { - return Math.max(Date.parse(record.createdAt), Date.parse(record.updatedAt)) >= sessionStartedAt -} - function formatTodo (todo: Todo): string { + const status = todo.completed ? '[done] ' : '' const suffix = todo.reviewId ? ` (review ${todo.reviewId.slice(0, 8)})` : '' - return `- ${todo.content}${suffix}` + return `- ${status}${todo.content}${suffix}` } function formatComment (comment: Comment): string { @@ -112,7 +162,7 @@ function formatPlainTextResult (result: AskResult): string { const lines = [ 'GitHuman feedback ready', `URL: ${result.url}`, - `Request todo: completed (${result.requestTodo.id.slice(0, 8)})`, + `Ask status: ${result.ask.status}`, `Review status: ${result.reviewStatus ?? 'unknown'}`, '', 'Todos:', @@ -135,18 +185,6 @@ function formatPlainTextResult (result: AskResult): string { return lines.join('\n') } -function getLatestReviewStatus (reviews: Review[], sessionStartedAt: number, reviewId?: string): Review['status'] | null { - if (reviewId) { - return reviews.find(review => review.id === reviewId)?.status ?? null - } - - const recent = reviews - .filter(review => wasTouchedDuringSession(review, sessionStartedAt)) - .sort((a, b) => Date.parse(b.updatedAt) - Date.parse(a.updatedAt))[0] - - return recent?.status ?? null -} - export async function askCommand (args: string[]) { const authValue = extractAuthArg(args) const filteredArgs = stripAuthArg(args, authValue) @@ -191,7 +229,7 @@ export async function askCommand (args: string[]) { }) const server = await ensureServerRunning(runtime) - const url = getAccessUrl(runtime.config) + const baseUrl = getAppUrl(runtime.config, '/') let interrupted = false const onInterrupt = () => { @@ -201,80 +239,66 @@ export async function askCommand (args: string[]) { process.once('SIGTERM', onInterrupt) try { - if (runtime.openBrowser) { - await open(url) - } - - const db = getDatabase() - const todoRepo = new TodoRepository(db) - const commentRepo = new CommentRepository(db) - const reviewRepo = new ReviewRepository(db) - - if (values.review) { - const review = reviewRepo.findById(values.review) - if (!review) { - console.error(`Error: Review not found: ${values.review}`) + let ask: AskSession + try { + ask = await createAskSession(baseUrl, runtime.config.authToken, { + message: buildRequestMessage(positionals.join(' ')), + reviewId: values.review, + }) + } catch (err) { + if (err instanceof AskCommandApiError && err.status < 500) { + console.error(`Error: ${err.message}`) process.exitCode = 1 return } + throw err } - const sessionStartedAt = Date.now() + const askUrl = getAppUrl(runtime.config, `/ask/${ask.id}`) - const requestTodo = todoRepo.create({ - id: randomUUID(), - content: buildRequestContent(positionals.join(' ')), - completed: false, - reviewId: values.review ?? null, - }) + await notifyServer(baseUrl, runtime.config.authToken, 'asks') - await notifyServer(url, runtime.config.authToken) + if (runtime.openBrowser) { + await open(askUrl) + } if (!values.json) { - console.error(`GitHuman available at: ${url}`) - console.error(`Waiting for human review: ${requestTodo.content}`) - console.error('Mark the request todo as done in GitHuman when feedback is ready.') + console.error(`GitHuman ask page: ${askUrl}`) + console.error(`Waiting for human review: ${ask.message}`) + console.error('The reviewer should click "Continue assistant" in the ask UI when feedback is ready.') } while (true) { if (interrupted) { - console.error(`\nInterrupted. GitHuman is still available at: ${url}`) + console.error(`\nInterrupted. GitHuman ask page is still available at: ${askUrl}`) process.exitCode = 1 return } - const currentTodo = todoRepo.findById(requestTodo.id) - if (!currentTodo) { + let currentAsk: AskSessionDetails + try { + currentAsk = await getAskSession(baseUrl, runtime.config.authToken, ask.id) + } catch (err) { + if (err instanceof AskCommandApiError && err.status === 404) { + console.error('Ask session no longer exists') + process.exitCode = 1 + return + } + throw err + } + + if (currentAsk.status === 'cancelled') { console.error('Review request was cancelled') process.exitCode = 1 return } - if (currentTodo.completed) { - const allTodos = todoRepo.findAll() - const filteredTodos = allTodos - .filter(todo => todo.id !== requestTodo.id) - .filter(todo => !todo.completed) - .filter(todo => !values.review || todo.reviewId === values.review) - .filter(todo => wasTouchedDuringSession(todo, sessionStartedAt)) - - const reviews = reviewRepo.findAll({ - repositoryPath: runtime.config.repositoryPath, - pageSize: 1000, - }).data - - const comments = (values.review - ? commentRepo.findByReview(values.review) - : reviews.flatMap(review => commentRepo.findByReview(review.id))) - .filter(comment => wasTouchedDuringSession(comment, sessionStartedAt)) - .sort((a, b) => Date.parse(a.createdAt) - Date.parse(b.createdAt)) + if (currentAsk.status === 'ready_for_agent') { + const feedback = await getAskFeedback(baseUrl, runtime.config.authToken, currentAsk.id) const result: AskResult = { - url, - requestTodo: currentTodo, - reviewStatus: getLatestReviewStatus(reviews, sessionStartedAt, values.review), - todos: filteredTodos, - comments, + ...feedback, + url: askUrl, } if (values.json) { @@ -294,4 +318,4 @@ export async function askCommand (args: string[]) { } } -export { buildRequestContent } +export { buildRequestMessage } diff --git a/src/cli/server-runtime.ts b/src/cli/server-runtime.ts index 741e4fb..eb44cf9 100644 --- a/src/cli/server-runtime.ts +++ b/src/cli/server-runtime.ts @@ -31,16 +31,21 @@ function getConnectHost (host: string): string { return host } -export function getAccessUrl (config: ServerConfig): string { +export function getAppUrl (config: ServerConfig, pathname = '/'): string { const protocol = config.https ? 'https' : 'http' const host = getConnectHost(config.host) - const base = `${protocol}://${host}:${config.port}` + const url = new URL(`${protocol}://${host}:${config.port}`) + url.pathname = pathname - if (!config.authToken) { - return base + if (config.authToken) { + url.searchParams.set('token', config.authToken) } - return `${base}?token=${encodeURIComponent(config.authToken)}` + return url.toString() +} + +export function getAccessUrl (config: ServerConfig): string { + return getAppUrl(config, '/') } async function healthCheck (config: ServerConfig): Promise { diff --git a/src/server/app.ts b/src/server/app.ts index ea95a09..ed7d3b2 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -11,7 +11,7 @@ import cors from '@fastify/cors' import fastifyStatic from '@fastify/static' import fastifySSE from '@fastify/sse' import { existsSync } from 'node:fs' -import { join, dirname } from 'node:path' +import { join, dirname, extname } from 'node:path' import { fileURLToPath } from 'node:url' import authPlugin from './plugins/auth.ts' import diffRoutes, { imageRoute } from './routes/diff.ts' @@ -20,6 +20,7 @@ import commentRoutes from './routes/comments.ts' import todoRoutes from './routes/todos.ts' import gitRoutes from './routes/git.ts' import eventsRoutes from './routes/events.ts' +import askRoutes from './routes/asks.ts' import type { ServerConfig } from './config.ts' import type { HealthResponse } from '../shared/types.ts' import { HealthResponseSchema } from './schemas/common.ts' @@ -128,6 +129,7 @@ export async function buildApp ( { name: 'diff', description: 'Git diff operations' }, { name: 'git', description: 'Git repository information' }, { name: 'events', description: 'Server-sent events for real-time updates' }, + { name: 'asks', description: 'Assistant↔human handoff sessions' }, ], servers: [ { @@ -199,6 +201,7 @@ export async function buildApp ( await app.register(todoRoutes) await app.register(gitRoutes) await app.register(eventsRoutes) + await app.register(askRoutes) // Serve static files if enabled and dist/web exists if (options.serveStatic !== false) { @@ -207,14 +210,20 @@ export async function buildApp ( await app.register(fastifyStatic, { root: staticPath, prefix: '/', - wildcard: false, }) - // SPA fallback - serve index.html for non-API routes + // SPA fallback - serve index.html only for app routes, not missing assets app.setNotFoundHandler(async (request, reply) => { - if (!request.url.startsWith('/api/')) { + const pathname = request.url.split('?')[0] + const isAppRoute = request.method === 'GET' && + !pathname.startsWith('/api/') && + !pathname.startsWith('/docs') && + extname(pathname) === '' + + if (isAppRoute) { return reply.sendFile('index.html') } + return reply.code(404).send({ error: 'Not found' }) }) } diff --git a/src/server/db/migrations.ts b/src/server/db/migrations.ts index 81bde03..874e5a0 100644 --- a/src/server/db/migrations.ts +++ b/src/server/db/migrations.ts @@ -139,6 +139,29 @@ export const migrations: Migration[] = [ CREATE UNIQUE INDEX idx_review_files_review_path ON review_files(review_id, file_path); `, }, + { + version: 7, + name: 'create_ask_sessions_table', + up: ` + CREATE TABLE ask_sessions ( + id TEXT PRIMARY KEY, + repository_path TEXT NOT NULL, + review_id TEXT, + message TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'waiting_for_human', + assistant_context TEXT, + completed_at TEXT, + created_at TEXT DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT DEFAULT CURRENT_TIMESTAMP, + FOREIGN KEY (review_id) REFERENCES reviews(id) ON DELETE SET NULL + ) STRICT; + + CREATE INDEX idx_ask_sessions_status ON ask_sessions(status); + CREATE INDEX idx_ask_sessions_review ON ask_sessions(review_id); + CREATE INDEX idx_ask_sessions_repository ON ask_sessions(repository_path); + CREATE INDEX idx_ask_sessions_created ON ask_sessions(created_at DESC); + `, + }, ] function getCurrentVersion (db: DatabaseSync): number { diff --git a/src/server/repositories/ask-session.repo.ts b/src/server/repositories/ask-session.repo.ts new file mode 100644 index 0000000..a671134 --- /dev/null +++ b/src/server/repositories/ask-session.repo.ts @@ -0,0 +1,112 @@ +/** + * Ask session repository - data access layer for assistant↔human handoff sessions + */ +import type { DatabaseSync, StatementSync } from 'node:sqlite' +import type { AskSession, AskStatus } from '../../shared/types.ts' + +interface AskSessionRow { + id: string; + repository_path: string; + review_id: string | null; + message: string; + status: string; + assistant_context: string | null; + completed_at: string | null; + created_at: string; + updated_at: string; +} + +function rowToAskSession (row: AskSessionRow): AskSession { + return { + id: row.id, + repositoryPath: row.repository_path, + reviewId: row.review_id, + message: row.message, + status: row.status as AskStatus, + assistantContext: row.assistant_context, + completedAt: row.completed_at, + createdAt: row.created_at, + updatedAt: row.updated_at, + } +} + +export class AskSessionRepository { + private stmtFindById: StatementSync + private stmtFindLatestByReview: StatementSync + private stmtInsert: StatementSync + private stmtUpdate: StatementSync + + constructor (db: DatabaseSync) { + this.stmtFindById = db.prepare(` + SELECT * FROM ask_sessions WHERE id = ? + `) + + this.stmtFindLatestByReview = db.prepare(` + SELECT * FROM ask_sessions + WHERE review_id = ? + ORDER BY created_at DESC + LIMIT 1 + `) + + this.stmtInsert = db.prepare(` + INSERT INTO ask_sessions ( + id, repository_path, review_id, message, status, assistant_context, completed_at, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `) + + this.stmtUpdate = db.prepare(` + UPDATE ask_sessions + SET status = COALESCE(?, status), + completed_at = ?, + updated_at = ? + WHERE id = ? + `) + } + + findById (id: string): AskSession | null { + const row = this.stmtFindById.get(id) as AskSessionRow | undefined + return row ? rowToAskSession(row) : null + } + + findLatestByReview (reviewId: string): AskSession | null { + const row = this.stmtFindLatestByReview.get(reviewId) as AskSessionRow | undefined + return row ? rowToAskSession(row) : null + } + + create (ask: Omit): AskSession { + const now = new Date().toISOString() + + this.stmtInsert.run( + ask.id, + ask.repositoryPath, + ask.reviewId, + ask.message, + ask.status, + ask.assistantContext, + ask.completedAt, + now, + now + ) + + return this.findById(ask.id)! + } + + update (id: string, updates: { status?: AskStatus; completedAt?: string | null }): AskSession | null { + const existing = this.findById(id) + if (!existing) { + return null + } + + const now = new Date().toISOString() + const completedAt = updates.completedAt === undefined ? existing.completedAt : updates.completedAt + + this.stmtUpdate.run( + updates.status ?? null, + completedAt, + now, + id + ) + + return this.findById(id) + } +} diff --git a/src/server/routes/asks.ts b/src/server/routes/asks.ts new file mode 100644 index 0000000..82dbc83 --- /dev/null +++ b/src/server/routes/asks.ts @@ -0,0 +1,217 @@ +/** + * Ask session API routes + */ +import { Type, type FastifyPluginAsyncTypebox } from '@fastify/type-provider-typebox' +import { getDatabase } from '../db/index.ts' +import { AskSessionError, AskSessionService } from '../services/ask-session.service.ts' +import { ErrorSchema } from '../schemas/common.ts' + +const AskStatusSchema = Type.Union([ + Type.Literal('waiting_for_human'), + Type.Literal('ready_for_agent'), + Type.Literal('cancelled'), +]) + +const AskSessionSchema = Type.Object({ + id: Type.String(), + repositoryPath: Type.String(), + reviewId: Type.Union([Type.String(), Type.Null()]), + message: Type.String(), + status: AskStatusSchema, + assistantContext: Type.Union([Type.String(), Type.Null()]), + completedAt: Type.Union([Type.String({ format: 'date-time' }), Type.Null()]), + createdAt: Type.String({ format: 'date-time' }), + updatedAt: Type.String({ format: 'date-time' }), +}) + +const AskSessionReviewContextSchema = Type.Object({ + id: Type.String(), + sourceType: Type.Union([Type.Literal('staged'), Type.Literal('branch'), Type.Literal('commits')]), + sourceRef: Type.Union([Type.String(), Type.Null()]), + baseRef: Type.Union([Type.String(), Type.Null()]), + status: Type.Union([ + Type.Literal('in_progress'), + Type.Literal('approved'), + Type.Literal('changes_requested'), + ]), +}) + +const AskSessionDetailsSchema = Type.Intersect([ + AskSessionSchema, + Type.Object({ + review: Type.Union([AskSessionReviewContextSchema, Type.Null()]), + feedback: Type.Object({ + reviewStatus: Type.Union([ + Type.Literal('in_progress'), + Type.Literal('approved'), + Type.Literal('changes_requested'), + Type.Null(), + ]), + todoCount: Type.Integer(), + commentCount: Type.Integer(), + unresolvedCommentCount: Type.Integer(), + }), + }), +]) + +const TodoSchema = Type.Object({ + id: Type.String(), + content: Type.String(), + completed: Type.Boolean(), + reviewId: Type.Union([Type.String(), Type.Null()]), + position: Type.Integer(), + createdAt: Type.String({ format: 'date-time' }), + updatedAt: Type.String({ format: 'date-time' }), +}) + +const CommentSchema = Type.Object({ + id: Type.String(), + reviewId: Type.String(), + filePath: Type.String(), + lineNumber: Type.Union([Type.Integer(), Type.Null()]), + lineType: Type.Union([ + Type.Literal('added'), + Type.Literal('removed'), + Type.Literal('context'), + Type.Null(), + ]), + content: Type.String(), + suggestion: Type.Union([Type.String(), Type.Null()]), + resolved: Type.Boolean(), + createdAt: Type.String({ format: 'date-time' }), + updatedAt: Type.String({ format: 'date-time' }), +}) + +const AskFeedbackSchema = Type.Object({ + ask: AskSessionSchema, + reviewStatus: Type.Union([ + Type.Literal('in_progress'), + Type.Literal('approved'), + Type.Literal('changes_requested'), + Type.Null(), + ]), + todos: Type.Array(TodoSchema), + comments: Type.Array(CommentSchema), +}) + +const CreateAskSessionSchema = Type.Object({ + message: Type.String({ minLength: 1 }), + reviewId: Type.Optional(Type.String()), + assistantContext: Type.Optional(Type.String()), +}) + +const AskIdParamsSchema = Type.Object({ + id: Type.String(), +}) + +const askRoutes: FastifyPluginAsyncTypebox = async (fastify) => { + const getService = () => { + const db = getDatabase() + return new AskSessionService(db) + } + + fastify.post('/api/asks', { + schema: { + tags: ['asks'], + summary: 'Create a new ask session', + description: 'Create an assistant↔human handoff session', + body: CreateAskSessionSchema, + response: { + 201: AskSessionSchema, + 400: ErrorSchema, + }, + }, + }, async (request, reply) => { + try { + const ask = getService().create(fastify.config.repositoryPath, request.body) + reply.code(201) + return ask + } catch (err) { + if (err instanceof AskSessionError) { + return reply.code(400).send({ + error: err.message, + code: err.code, + }) + } + throw err + } + }) + + fastify.get('/api/asks/:id', { + schema: { + tags: ['asks'], + summary: 'Get an ask session', + description: 'Get ask session details and feedback counts', + params: AskIdParamsSchema, + response: { + 200: AskSessionDetailsSchema, + 404: ErrorSchema, + }, + }, + }, async (request, reply) => { + const ask = getService().getDetails(request.params.id) + if (!ask) { + return reply.code(404).send({ error: 'Ask session not found' }) + } + return ask + }) + + fastify.get('/api/asks/:id/feedback', { + schema: { + tags: ['asks'], + summary: 'Get ask session feedback', + description: 'Get the feedback collected during an ask session', + params: AskIdParamsSchema, + response: { + 200: AskFeedbackSchema, + 404: ErrorSchema, + }, + }, + }, async (request, reply) => { + const feedback = getService().collectFeedback(request.params.id) + if (!feedback) { + return reply.code(404).send({ error: 'Ask session not found' }) + } + return feedback + }) + + fastify.post('/api/asks/:id/continue', { + schema: { + tags: ['asks'], + summary: 'Continue the assistant', + description: 'Mark the ask session as ready for the assistant to resume', + params: AskIdParamsSchema, + response: { + 200: AskSessionSchema, + 404: ErrorSchema, + }, + }, + }, async (request, reply) => { + const ask = getService().markReadyForAgent(request.params.id) + if (!ask) { + return reply.code(404).send({ error: 'Ask session not found' }) + } + return ask + }) + + fastify.post('/api/asks/:id/cancel', { + schema: { + tags: ['asks'], + summary: 'Cancel an ask session', + description: 'Cancel an assistant↔human handoff session', + params: AskIdParamsSchema, + response: { + 200: AskSessionSchema, + 404: ErrorSchema, + }, + }, + }, async (request, reply) => { + const ask = getService().cancel(request.params.id) + if (!ask) { + return reply.code(404).send({ error: 'Ask session not found' }) + } + return ask + }) +} + +export default askRoutes diff --git a/src/server/routes/events.ts b/src/server/routes/events.ts index 6a72c63..07f61d7 100644 --- a/src/server/routes/events.ts +++ b/src/server/routes/events.ts @@ -11,7 +11,7 @@ import { SuccessSchema } from '../schemas/common.ts' import { loadGitignore } from '../utils/gitignore.ts' // Event types that can be broadcast -export type EventType = 'todos' | 'reviews' | 'comments' | 'files' +export type EventType = 'todos' | 'reviews' | 'comments' | 'files' | 'asks' // Extended message type for our events interface EventMessage extends Message { @@ -36,7 +36,7 @@ async function broadcast (emitter: MQEmitterType, eventType: EventType, data?: u const NotifyBodySchema = Type.Object( { - type: Type.Union([Type.Literal('todos'), Type.Literal('reviews'), Type.Literal('comments'), Type.Literal('files')], { + type: Type.Union([Type.Literal('todos'), Type.Literal('reviews'), Type.Literal('comments'), Type.Literal('files'), Type.Literal('asks')], { description: 'Type of resource that changed', }), action: Type.Optional( diff --git a/src/server/services/ask-session.service.ts b/src/server/services/ask-session.service.ts new file mode 100644 index 0000000..7cc8361 --- /dev/null +++ b/src/server/services/ask-session.service.ts @@ -0,0 +1,190 @@ +/** + * Ask session service - business logic for assistant↔human handoff sessions + */ +import { randomUUID } from 'node:crypto' +import type { DatabaseSync } from 'node:sqlite' +import { AskSessionRepository } from '../repositories/ask-session.repo.ts' +import { ReviewRepository } from '../repositories/review.repo.ts' +import { CommentRepository } from '../repositories/comment.repo.ts' +import { TodoRepository } from '../repositories/todo.repo.ts' +import type { + AskFeedback, + AskSession, + AskSessionDetails, + CreateAskSessionRequest, + Review, + ReviewStatus, +} from '../../shared/types.ts' + +function wasCreatedDuringSession (record: { createdAt: string }, sessionStartedAt: number): boolean { + return Date.parse(record.createdAt) >= sessionStartedAt +} + +function wasTouchedDuringSession (record: { createdAt: string; updatedAt: string }, sessionStartedAt: number): boolean { + return Math.max(Date.parse(record.createdAt), Date.parse(record.updatedAt)) >= sessionStartedAt +} + +export class AskSessionService { + private repo: AskSessionRepository + private reviewRepo: ReviewRepository + private commentRepo: CommentRepository + private todoRepo: TodoRepository + + constructor (db: DatabaseSync) { + this.repo = new AskSessionRepository(db) + this.reviewRepo = new ReviewRepository(db) + this.commentRepo = new CommentRepository(db) + this.todoRepo = new TodoRepository(db) + } + + create (repositoryPath: string, request: CreateAskSessionRequest): AskSession { + if (!request.message.trim()) { + throw new AskSessionError('Message is required', 'INVALID_MESSAGE') + } + + const reviewId: string | null = request.reviewId ?? null + if (reviewId) { + const review = this.reviewRepo.findById(reviewId) + if (!review) { + throw new AskSessionError('Review not found', 'REVIEW_NOT_FOUND') + } + if (review.repositoryPath !== repositoryPath) { + throw new AskSessionError('Review does not belong to this repository', 'REVIEW_NOT_FOUND') + } + } + + return this.repo.create({ + id: randomUUID(), + repositoryPath, + reviewId, + message: request.message.trim(), + status: 'waiting_for_human', + assistantContext: request.assistantContext?.trim() || null, + completedAt: null, + }) + } + + getById (id: string): AskSession | null { + return this.repo.findById(id) + } + + getDetails (id: string): AskSessionDetails | null { + const ask = this.repo.findById(id) + if (!ask) { + return null + } + + const feedback = this.collectFeedbackInternal(ask) + const review = ask.reviewId ? this.reviewRepo.findById(ask.reviewId) : null + + return { + ...ask, + review: review + ? { + id: review.id, + sourceType: review.sourceType, + sourceRef: review.sourceRef, + baseRef: review.baseRef, + status: review.status, + } + : null, + feedback: { + reviewStatus: feedback.reviewStatus, + todoCount: feedback.todos.length, + commentCount: feedback.comments.length, + unresolvedCommentCount: feedback.comments.filter(comment => !comment.resolved).length, + }, + } + } + + markReadyForAgent (id: string): AskSession | null { + return this.repo.update(id, { + status: 'ready_for_agent', + completedAt: new Date().toISOString(), + }) + } + + cancel (id: string): AskSession | null { + return this.repo.update(id, { + status: 'cancelled', + completedAt: new Date().toISOString(), + }) + } + + collectFeedback (id: string): AskFeedback | null { + const ask = this.repo.findById(id) + if (!ask) { + return null + } + + return this.collectFeedbackInternal(ask) + } + + private collectFeedbackInternal (ask: AskSession): AskFeedback { + const sessionStartedAt = Date.parse(ask.createdAt) + + const reviews = this.reviewRepo.findAll({ + repositoryPath: ask.repositoryPath, + pageSize: 1000, + }).data + + const reviewStatus = this.getLatestReviewStatus(reviews, sessionStartedAt, ask.reviewId ?? undefined) + + if (ask.reviewId) { + const todos = this.todoRepo.findByReview(ask.reviewId) + .filter(todo => wasCreatedDuringSession(todo, sessionStartedAt)) + const comments = this.commentRepo.findByReview(ask.reviewId) + .filter(comment => wasCreatedDuringSession(comment, sessionStartedAt)) + .sort((a, b) => Date.parse(a.createdAt) - Date.parse(b.createdAt)) + + return { + ask, + reviewStatus, + todos, + comments, + } + } + + const reviewIds = new Set(reviews.map(review => review.id)) + const comments = reviews + .flatMap(review => this.commentRepo.findByReview(review.id)) + .filter(comment => wasCreatedDuringSession(comment, sessionStartedAt)) + .sort((a, b) => Date.parse(a.createdAt) - Date.parse(b.createdAt)) + + const todos = this.todoRepo.findAll() + .filter(todo => { + if (!wasCreatedDuringSession(todo, sessionStartedAt)) return false + if (todo.reviewId == null) return true + return reviewIds.has(todo.reviewId) + }) + + return { + ask, + reviewStatus, + todos, + comments, + } + } + + private getLatestReviewStatus (reviews: Review[], sessionStartedAt: number, reviewId?: string): ReviewStatus | null { + if (reviewId) { + return reviews.find(review => review.id === reviewId)?.status ?? null + } + + const recent = reviews + .filter(review => wasTouchedDuringSession(review, sessionStartedAt)) + .sort((a, b) => Date.parse(b.updatedAt) - Date.parse(a.updatedAt))[0] + + return recent?.status ?? null + } +} + +export class AskSessionError extends Error { + code: string + + constructor (message: string, code: string) { + super(message) + this.name = 'AskSessionError' + this.code = code + } +} diff --git a/src/shared/types.ts b/src/shared/types.ts index 3a31164..6926978 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -16,6 +16,7 @@ export interface Review { export type ReviewStatus = 'in_progress' | 'approved' | 'changes_requested' export type ReviewSourceType = 'staged' | 'branch' | 'commits' +export type AskStatus = 'waiting_for_human' | 'ready_for_agent' | 'cancelled' export interface Comment { id: string; @@ -40,6 +41,43 @@ export interface Todo { updatedAt: string; } +export interface AskSession { + id: string; + repositoryPath: string; + reviewId: string | null; + message: string; + status: AskStatus; + assistantContext: string | null; + createdAt: string; + updatedAt: string; + completedAt: string | null; +} + +export interface AskSessionReviewContext { + id: string; + sourceType: ReviewSourceType; + sourceRef: string | null; + baseRef: string | null; + status: ReviewStatus; +} + +export interface AskSessionDetails extends AskSession { + review: AskSessionReviewContext | null; + feedback: { + reviewStatus: ReviewStatus | null; + todoCount: number; + commentCount: number; + unresolvedCommentCount: number; + }; +} + +export interface AskFeedback { + ask: AskSession; + reviewStatus: ReviewStatus | null; + todos: Todo[]; + comments: Comment[]; +} + export interface DiffFile { oldPath: string; newPath: string; @@ -114,6 +152,16 @@ export interface UpdateTodoRequest { completed?: boolean; } +export interface CreateAskSessionRequest { + message: string; + reviewId?: string; + assistantContext?: string; +} + +export interface UpdateAskSessionRequest { + status?: AskStatus; +} + export interface ApiError { error: string; message: string; diff --git a/src/web/App.tsx b/src/web/App.tsx index 40f698c..a42aab5 100644 --- a/src/web/App.tsx +++ b/src/web/App.tsx @@ -7,6 +7,7 @@ import { HomePage } from './pages/HomePage' import { ReviewPage } from './pages/ReviewPage' import { StagedChangesPage } from './pages/StagedChangesPage' import { NewReviewPage } from './pages/NewReviewPage' +import { AskPage } from './pages/AskPage' export function App () { return ( @@ -14,6 +15,7 @@ export function App () { + } /> }> } /> } /> diff --git a/src/web/api/asks.ts b/src/web/api/asks.ts new file mode 100644 index 0000000..7f7be39 --- /dev/null +++ b/src/web/api/asks.ts @@ -0,0 +1,27 @@ +/** + * Ask sessions API client + */ +import { api } from './client' +import type { + AskFeedback, + AskSession, + AskSessionDetails, + CreateAskSessionRequest, +} from '../../shared/types' + +export const asksApi = { + create: (data: CreateAskSessionRequest) => + api.post('/asks', data), + + get: (id: string) => + api.get(`/asks/${id}`), + + getFeedback: (id: string) => + api.get(`/asks/${id}/feedback`), + + continue: (id: string) => + api.post(`/asks/${id}/continue`, {}), + + cancel: (id: string) => + api.post(`/asks/${id}/cancel`, {}), +} diff --git a/src/web/components/layout/AskSessionDock.tsx b/src/web/components/layout/AskSessionDock.tsx new file mode 100644 index 0000000..556ab10 --- /dev/null +++ b/src/web/components/layout/AskSessionDock.tsx @@ -0,0 +1,47 @@ +import { Link, useSearchParams } from 'react-router-dom' +import { useAskSession, useContinueAsk } from '../../hooks/useAsks' + +export function AskSessionDock () { + const [searchParams] = useSearchParams() + const askId = searchParams.get('ask') + const { data, refetch } = useAskSession(askId ?? '') + const { mutate: continueAsk, loading: continuing } = useContinueAsk() + + if (!askId || !data) { + return null + } + + const isDone = data.status !== 'waiting_for_human' + + if (isDone) { + return null + } + + return ( +
+
+ Assistant handoff active +
+ +
+ + Handoff + Back to handoff + + +
+
+ ) +} diff --git a/src/web/components/layout/Layout.tsx b/src/web/components/layout/Layout.tsx index 9a703cd..3a7ded4 100644 --- a/src/web/components/layout/Layout.tsx +++ b/src/web/components/layout/Layout.tsx @@ -1,6 +1,7 @@ import { useState, useCallback } from 'react' import { Outlet } from 'react-router-dom' import { Header } from './Header' +import { AskSessionDock } from './AskSessionDock' import { TodoDrawer } from '../todos' import { LoginForm } from '../auth' import { useRepositoryInfo } from '../../hooks/useRepositoryInfo' @@ -38,6 +39,7 @@ export function Layout () {
+ ) diff --git a/src/web/hooks/useAsks.ts b/src/web/hooks/useAsks.ts new file mode 100644 index 0000000..14d18af --- /dev/null +++ b/src/web/hooks/useAsks.ts @@ -0,0 +1,110 @@ +import { useCallback, useEffect, useState } from 'react' +import { asksApi } from '../api/asks' +import { ApiClientError } from '../api/client' +import type { AskFeedback, AskSession, AskSessionDetails } from '../../shared/types' + +interface UseAskSessionResult { + data: AskSessionDetails | null; + loading: boolean; + error: ApiClientError | null; + refetch: () => Promise; +} + +export function useAskSession (id: string): UseAskSessionResult { + const [data, setData] = useState(null) + const [loading, setLoading] = useState(true) + const [error, setError] = useState(null) + + const fetchData = useCallback(async (showLoadingState: boolean) => { + if (!id) return + + if (showLoadingState) { + setLoading(true) + setError(null) + } + + try { + setData(await asksApi.get(id)) + setError(null) + } catch (err) { + setError(err instanceof ApiClientError ? err : new ApiClientError('Unknown error', 500)) + } finally { + if (showLoadingState) { + setLoading(false) + } + } + }, [id]) + + useEffect(() => { + setData(null) + setError(null) + setLoading(true) + fetchData(true) + }, [id, fetchData]) + + const refetch = useCallback(async () => { + await fetchData(false) + }, [fetchData]) + + return { data, loading, error, refetch } +} + +interface UseAskMutationResult { + mutate: (id: string) => Promise; + loading: boolean; + error: ApiClientError | null; +} + +function useAskMutation (fn: (id: string) => Promise): UseAskMutationResult { + const [loading, setLoading] = useState(false) + const [error, setError] = useState(null) + + const mutate = useCallback(async (id: string): Promise => { + setLoading(true) + setError(null) + try { + return await fn(id) + } catch (err) { + const apiError = err instanceof ApiClientError ? err : new ApiClientError('Unknown error', 500) + setError(apiError) + throw apiError + } finally { + setLoading(false) + } + }, [fn]) + + return { mutate, loading, error } +} + +export function useContinueAsk (): UseAskMutationResult { + return useAskMutation(asksApi.continue) +} + +export function useCancelAsk (): UseAskMutationResult { + return useAskMutation(asksApi.cancel) +} + +export function useAskFeedback (id: string) { + const [data, setData] = useState(null) + const [loading, setLoading] = useState(true) + const [error, setError] = useState(null) + + const fetchData = useCallback(async () => { + if (!id) return + setLoading(true) + setError(null) + try { + setData(await asksApi.getFeedback(id)) + } catch (err) { + setError(err instanceof ApiClientError ? err : new ApiClientError('Unknown error', 500)) + } finally { + setLoading(false) + } + }, [id]) + + useEffect(() => { + fetchData() + }, [fetchData]) + + return { data, loading, error, refetch: fetchData } +} diff --git a/src/web/hooks/useServerEvents.ts b/src/web/hooks/useServerEvents.ts index 8713dca..e90e1a7 100644 --- a/src/web/hooks/useServerEvents.ts +++ b/src/web/hooks/useServerEvents.ts @@ -5,7 +5,7 @@ import { useEffect, useRef } from 'react' import { getAuthToken } from '../api/client' -export type EventType = 'todos' | 'reviews' | 'comments' | 'files' | 'connected' +export type EventType = 'todos' | 'reviews' | 'comments' | 'files' | 'asks' | 'connected' interface ServerEvent { type: EventType; diff --git a/src/web/pages/AskPage.tsx b/src/web/pages/AskPage.tsx new file mode 100644 index 0000000..9efede9 --- /dev/null +++ b/src/web/pages/AskPage.tsx @@ -0,0 +1,204 @@ +import { useEffect } from 'react' +import { Link, useParams } from 'react-router-dom' +import { useAskSession, useCancelAsk, useContinueAsk } from '../hooks/useAsks' +import { useServerEvents } from '../hooks/useServerEvents' + +function sourceLabel (sourceType: 'staged' | 'branch' | 'commits', sourceRef: string | null) { + if (sourceType === 'staged') return 'Staged changes' + if (sourceType === 'branch') return sourceRef ? `Branch ${sourceRef}` : 'Branch review' + if (sourceType === 'commits') return sourceRef ? `Commits ${sourceRef}` : 'Commit review' + return 'Review' +} + +export function AskPage () { + const { id } = useParams<{ id: string }>() + const { data, loading, refetch } = useAskSession(id!) + const { mutate: continueAsk, loading: continuing } = useContinueAsk() + const { mutate: cancelAsk, loading: cancelling } = useCancelAsk() + + useEffect(() => { + if (!id) return + + const timer = setInterval(() => { + refetch().catch(() => {}) + }, 2000) + + return () => clearInterval(timer) + }, [id, refetch]) + + useServerEvents({ + eventTypes: ['asks', 'reviews', 'comments', 'todos'], + onEvent: () => { + refetch().catch(() => {}) + }, + enabled: !!id, + }) + + if (loading && !data) { + return ( +
+
+
+

Loading assistant handoff...

+
+
+ ) + } + + if (!data) { + return ( +
+
+

Ask session not found

+

+ This assistant handoff could not be loaded. +

+ + Back to GitHuman + +
+
+ ) + } + + const isDone = data.status !== 'waiting_for_human' + const askQuery = `?ask=${encodeURIComponent(data.id)}` + + return ( +
+
+
+
+
+ Assistant review request +

+ Review this work and hand it back to the assistant +

+

+ Leave your comments in GitHuman, add todos if helpful, then click Continue assistant when you are done. +

+
+ + {data.status === 'ready_for_agent' && ( +
+

Assistant can continue now.

+

+ This handoff has been marked ready for the assistant. +

+
+ )} + + {data.status === 'cancelled' && ( +
+

This request was cancelled.

+

+ The assistant will not resume from this handoff. +

+
+ )} + +
+

Assistant message

+

{data.message}

+ {data.assistantContext && ( +
+ {data.assistantContext} +
+ )} +
+ +
+
+

Where to leave feedback

+ {data.review + ? ( + <> +

{sourceLabel(data.review.sourceType, data.review.sourceRef)}

+

+ Open the linked review, add comments or todos there, then return here. +

+
+ + Open review + + + All reviews + +
+ + ) + : ( + <> +

No linked review

+

+ Use GitHuman to inspect the current changes, then come back here to continue the assistant. +

+
+ + Open changes + + + Browse reviews + +
+ + )} +
+ +
+

Feedback in this handoff

+
+
+

Comments

+

{data.feedback.commentCount}

+
+
+

Unresolved

+

{data.feedback.unresolvedCommentCount}

+
+
+

Todos

+

{data.feedback.todoCount}

+
+
+

+ Review status:{' '} + + {data.feedback.reviewStatus ?? 'unknown'} + +

+
+
+
+
+
+ +
+
+ + +
+
+
+ ) +} diff --git a/tests/cli/ask.test.ts b/tests/cli/ask.test.ts index 26200d7..a391018 100644 --- a/tests/cli/ask.test.ts +++ b/tests/cli/ask.test.ts @@ -9,6 +9,7 @@ import { initDatabase, closeDatabase, getDatabase } from '../../src/server/db/in import { buildApp } from '../../src/server/app.ts' import { createConfig } from '../../src/server/config.ts' import { ReviewRepository } from '../../src/server/repositories/review.repo.ts' +import { AskSessionRepository } from '../../src/server/repositories/ask-session.repo.ts' import { TodoRepository } from '../../src/server/repositories/todo.repo.ts' import { CommentRepository } from '../../src/server/repositories/comment.repo.ts' @@ -55,27 +56,28 @@ function createReview (repoDir: string): string { return reviewId } -async function waitForRequestTodo (repoDir: string, content: string): Promise<{ id: string; content: string }> { +async function waitForAskSession (repoDir: string, message: string): Promise<{ id: string; message: string }> { const deadline = Date.now() + 10000 while (Date.now() < deadline) { const db = openTestDatabase(repoDir) - const todoRepo = new TodoRepository(db) - const todo = todoRepo.findAll().find(item => item.content === content) + const stmt = db.prepare('SELECT * FROM ask_sessions WHERE message = ? ORDER BY created_at DESC LIMIT 1') + const row = stmt.get(message) as { id: string; message: string } | undefined db.close() - if (todo) { - return { id: todo.id, content: todo.content } + if (row) { + return row } await new Promise(resolve => setTimeout(resolve, 50)) } - throw new Error('Timed out waiting for ask request todo') + throw new Error('Timed out waiting for ask session') } -async function completeAskSession (repoDir: string, reviewId: string, requestTodoId: string) { +async function completeAskSession (repoDir: string, reviewId: string, askId: string) { const db = openTestDatabase(repoDir) + const askRepo = new AskSessionRepository(db) const todoRepo = new TodoRepository(db) const commentRepo = new CommentRepository(db) const reviewRepo = new ReviewRepository(db) @@ -102,7 +104,10 @@ async function completeAskSession (repoDir: string, reviewId: string, requestTod status: 'changes_requested', }) - todoRepo.update(requestTodoId, { completed: true }) + askRepo.update(askId, { + status: 'ready_for_agent', + completedAt: new Date().toISOString(), + }) db.close() } @@ -157,11 +162,11 @@ async function runAsk (args: string[], cwd: string): Promise { } describe('githuman ask', { concurrency: 1 }, () => { - it('waits for the human to finish and prints plain-text feedback', async (t) => { + it('waits for the human to continue the assistant and prints plain-text feedback', async (t) => { const repoDir = await createTestRepoWithDb(t) const reviewId = createReview(repoDir) const port = 45000 + Math.floor(Math.random() * 1000) - const requestContent = 'AI review request: Please review the parser refactor' + const askMessage = 'Please review the parser refactor' const server = await startAskServer(repoDir, port) try { @@ -170,17 +175,19 @@ describe('githuman ask', { concurrency: 1 }, () => { '--no-open', '--interval', '50', '--port', String(port), - 'Please review the parser refactor', + askMessage, ], repoDir) - const requestTodo = await waitForRequestTodo(repoDir, requestContent) - await completeAskSession(repoDir, reviewId, requestTodo.id) + const ask = await waitForAskSession(repoDir, askMessage) + await completeAskSession(repoDir, reviewId, ask.id) const result = await askPromise assert.strictEqual(result.exitCode, 0, `${result.stderr}\n${result.stdout}`) - assert.ok(result.stderr.includes('GitHuman available at: http://localhost:')) + assert.ok(result.stderr.includes('GitHuman ask page: http://localhost:')) + assert.ok(result.stderr.includes('Continue assistant')) assert.ok(result.stdout.includes('GitHuman feedback ready')) + assert.ok(result.stdout.includes('Ask status: ready_for_agent')) assert.ok(result.stdout.includes('Review status: changes_requested')) assert.ok(result.stdout.includes('Add a regression test for whitespace-only input')) assert.ok(result.stdout.includes('src/parser.ts:42')) @@ -194,7 +201,7 @@ describe('githuman ask', { concurrency: 1 }, () => { const repoDir = await createTestRepoWithDb(t) const reviewId = createReview(repoDir) const port = 46000 + Math.floor(Math.random() * 1000) - const requestContent = 'AI review request: Please review the current changes.' + const askMessage = 'Please review the current changes.' const server = await startAskServer(repoDir, port) try { @@ -206,16 +213,17 @@ describe('githuman ask', { concurrency: 1 }, () => { '--json', ], repoDir) - const requestTodo = await waitForRequestTodo(repoDir, requestContent) - await completeAskSession(repoDir, reviewId, requestTodo.id) + const ask = await waitForAskSession(repoDir, askMessage) + await completeAskSession(repoDir, reviewId, ask.id) const result = await askPromise assert.strictEqual(result.exitCode, 0, `${result.stderr}\n${result.stdout}`) const data = JSON.parse(result.stdout) assert.ok(data.url.startsWith('http://localhost:')) + assert.strictEqual(data.ask.status, 'ready_for_agent') + assert.strictEqual(data.ask.message, askMessage) assert.strictEqual(data.reviewStatus, 'changes_requested') - assert.strictEqual(data.requestTodo.content, requestContent) assert.strictEqual(data.todos.length, 1) assert.strictEqual(data.comments.length, 1) assert.strictEqual(data.todos[0].content, 'Add a regression test for whitespace-only input') diff --git a/tests/server/app.test.ts b/tests/server/app.test.ts index 3c080eb..b6b7115 100644 --- a/tests/server/app.test.ts +++ b/tests/server/app.test.ts @@ -1,5 +1,7 @@ import { describe, it, before, after } from 'node:test' import assert from 'node:assert' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' import { buildApp } from '../../src/server/app.ts' import { createConfig } from '../../src/server/config.ts' import type { FastifyInstance } from 'fastify' @@ -121,6 +123,47 @@ describe('app', () => { }) }) + describe('static app assets', () => { + let app: FastifyInstance + + before(async () => { + const config = createConfig() + app = await buildApp(config, { logger: false }) + }) + + after(async () => { + await app.close() + }) + + it('should serve built JavaScript assets instead of index.html', async () => { + const webDir = join(process.cwd(), 'dist', 'web') + const indexHtml = readFileSync(join(webDir, 'index.html'), 'utf-8') + const scriptMatch = indexHtml.match(/]+src="([^"]+)"/) + + assert.ok(scriptMatch, 'expected built index.html to reference a script asset') + const assetPath = scriptMatch[1] + + const response = await app.inject({ + method: 'GET', + url: assetPath, + }) + + assert.strictEqual(response.statusCode, 200) + assert.match(response.headers['content-type'] ?? '', /javascript|ecmascript|text\/plain/) + assert.doesNotMatch(response.body, //) + }) + + it('should return 404 for missing asset paths', async () => { + const response = await app.inject({ + method: 'GET', + url: '/assets/does-not-exist.js', + }) + + assert.strictEqual(response.statusCode, 404) + assert.doesNotMatch(response.body, //) + }) + }) + describe('HTTPS configuration', () => { it('should build app without HTTPS when https is false', async () => { const config = createConfig({ https: false }) diff --git a/tests/server/routes/asks.test.ts b/tests/server/routes/asks.test.ts new file mode 100644 index 0000000..80987a9 --- /dev/null +++ b/tests/server/routes/asks.test.ts @@ -0,0 +1,254 @@ +import { describe, it, before, after } from 'node:test' +import assert from 'node:assert' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { execSync } from 'node:child_process' +import type { FastifyInstance } from 'fastify' +import { buildApp } from '../../../src/server/app.ts' +import { createConfig } from '../../../src/server/config.ts' +import { closeDatabase, getDatabase, initDatabase } from '../../../src/server/db/index.ts' +import { ReviewRepository } from '../../../src/server/repositories/review.repo.ts' +import { TodoRepository } from '../../../src/server/repositories/todo.repo.ts' +import { CommentRepository } from '../../../src/server/repositories/comment.repo.ts' +import { TEST_TOKEN, authHeader } from '../helpers.ts' + +function createTempGitRepo (): string { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ask-route-test-')) + execSync('git init', { cwd: tempDir, stdio: 'ignore' }) + execSync('git config user.email "test@test.com"', { cwd: tempDir, stdio: 'ignore' }) + execSync('git config user.name "Test"', { cwd: tempDir, stdio: 'ignore' }) + fs.writeFileSync(path.join(tempDir, 'README.md'), '# Test') + execSync('git add .', { cwd: tempDir, stdio: 'ignore' }) + execSync('git commit -m "Initial commit"', { cwd: tempDir, stdio: 'ignore' }) + return tempDir +} + +describe('ask routes', () => { + let app: FastifyInstance + let testDbDir: string + let testRepoDir: string + let reviewId: string + + before(async () => { + testDbDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ask-db-test-')) + const dbPath = path.join(testDbDir, 'test.db') + testRepoDir = createTempGitRepo() + initDatabase(dbPath) + + const reviewRepo = new ReviewRepository(getDatabase()) + reviewId = 'review-1' + reviewRepo.create({ + id: reviewId, + repositoryPath: testRepoDir, + baseRef: 'main', + sourceType: 'branch', + sourceRef: 'feature/test', + snapshotData: JSON.stringify({ + repository: { + name: 'test', + branch: 'feature/test', + remote: null, + path: testRepoDir, + }, + version: 2, + }), + status: 'in_progress', + }) + + const config = createConfig({ + repositoryPath: testRepoDir, + dbPath, + authToken: TEST_TOKEN, + }) + app = await buildApp(config, { logger: false }) + }) + + after(async () => { + await app.close() + closeDatabase() + fs.rmSync(testDbDir, { recursive: true, force: true }) + fs.rmSync(testRepoDir, { recursive: true, force: true }) + }) + + it('creates an ask session', async () => { + const response = await app.inject({ + method: 'POST', + url: '/api/asks', + headers: authHeader(), + payload: { + message: 'Please review the parser refactor', + reviewId, + }, + }) + + assert.strictEqual(response.statusCode, 201) + const body = JSON.parse(response.body) + assert.strictEqual(body.message, 'Please review the parser refactor') + assert.strictEqual(body.reviewId, reviewId) + assert.strictEqual(body.status, 'waiting_for_human') + }) + + it('returns details with feedback counts', async () => { + const createResponse = await app.inject({ + method: 'POST', + url: '/api/asks', + headers: authHeader(), + payload: { + message: 'Please review the branch changes', + reviewId, + }, + }) + const ask = JSON.parse(createResponse.body) + + const detailsResponse = await app.inject({ + method: 'GET', + url: `/api/asks/${ask.id}`, + headers: authHeader(), + }) + + assert.strictEqual(detailsResponse.statusCode, 200) + const body = JSON.parse(detailsResponse.body) + assert.strictEqual(body.review.id, reviewId) + assert.strictEqual(body.feedback.todoCount, 0) + assert.strictEqual(body.feedback.commentCount, 0) + assert.strictEqual(body.feedback.reviewStatus, 'in_progress') + }) + + it('marks an ask session ready for agent and returns collected feedback', async () => { + const createResponse = await app.inject({ + method: 'POST', + url: '/api/asks', + headers: authHeader(), + payload: { + message: 'Please review and continue', + reviewId, + }, + }) + const ask = JSON.parse(createResponse.body) + + const db = getDatabase() + const todoRepo = new TodoRepository(db) + const commentRepo = new CommentRepository(db) + const reviewRepo = new ReviewRepository(db) + + todoRepo.create({ + id: 'todo-1', + content: 'Add a regression test', + completed: false, + reviewId, + }) + + commentRepo.create({ + id: 'comment-1', + reviewId, + filePath: 'src/parser.ts', + lineNumber: 42, + lineType: 'added', + content: 'Please reject undefined explicitly.', + suggestion: null, + resolved: false, + }) + + reviewRepo.update(reviewId, { status: 'changes_requested' }) + + const continueResponse = await app.inject({ + method: 'POST', + url: `/api/asks/${ask.id}/continue`, + headers: authHeader(), + }) + + assert.strictEqual(continueResponse.statusCode, 200) + const continued = JSON.parse(continueResponse.body) + assert.strictEqual(continued.status, 'ready_for_agent') + assert.ok(continued.completedAt) + + const feedbackResponse = await app.inject({ + method: 'GET', + url: `/api/asks/${ask.id}/feedback`, + headers: authHeader(), + }) + + assert.strictEqual(feedbackResponse.statusCode, 200) + const feedback = JSON.parse(feedbackResponse.body) + assert.strictEqual(feedback.ask.id, ask.id) + assert.strictEqual(feedback.reviewStatus, 'changes_requested') + assert.strictEqual(feedback.todos.length, 1) + assert.strictEqual(feedback.comments.length, 1) + assert.strictEqual(feedback.todos[0].content, 'Add a regression test') + assert.strictEqual(feedback.comments[0].content, 'Please reject undefined explicitly.') + }) + + it('ignores older todos that were only updated during an unscoped ask session', async () => { + const db = getDatabase() + const todoRepo = new TodoRepository(db) + const commentRepo = new CommentRepository(db) + const reviewRepo = new ReviewRepository(db) + + todoRepo.create({ + id: 'todo-before-ask', + content: 'Old global todo', + completed: false, + reviewId: null, + }) + + const createResponse = await app.inject({ + method: 'POST', + url: '/api/asks', + headers: authHeader(), + payload: { + message: 'Review the latest comments', + }, + }) + const ask = JSON.parse(createResponse.body) + + todoRepo.update('todo-before-ask', { completed: true }) + commentRepo.create({ + id: 'comment-after-unscoped-ask', + reviewId, + filePath: 'src/app.ts', + lineNumber: 10, + lineType: 'added', + content: 'Fresh comment for this ask session.', + suggestion: null, + resolved: false, + }) + reviewRepo.update(reviewId, { status: 'changes_requested' }) + + const feedbackResponse = await app.inject({ + method: 'GET', + url: `/api/asks/${ask.id}/feedback`, + headers: authHeader(), + }) + + assert.strictEqual(feedbackResponse.statusCode, 200) + const feedback = JSON.parse(feedbackResponse.body) + assert.strictEqual(feedback.reviewStatus, 'changes_requested') + assert.strictEqual(feedback.todos.length, 0) + assert.strictEqual(feedback.comments.length, 1) + assert.strictEqual(feedback.comments[0].content, 'Fresh comment for this ask session.') + }) + + it('cancels an ask session', async () => { + const createResponse = await app.inject({ + method: 'POST', + url: '/api/asks', + headers: authHeader(), + payload: { + message: 'Cancel this request', + }, + }) + const ask = JSON.parse(createResponse.body) + + const cancelResponse = await app.inject({ + method: 'POST', + url: `/api/asks/${ask.id}/cancel`, + headers: authHeader(), + }) + + assert.strictEqual(cancelResponse.statusCode, 200) + const body = JSON.parse(cancelResponse.body) + assert.strictEqual(body.status, 'cancelled') + assert.ok(body.completedAt) + }) +}) diff --git a/tests/server/routes/events.test.ts b/tests/server/routes/events.test.ts index 11e9a2c..03f9b14 100644 --- a/tests/server/routes/events.test.ts +++ b/tests/server/routes/events.test.ts @@ -75,6 +75,19 @@ describe('events routes', () => { assert.strictEqual(body.success, true) }) + it('should accept asks notification', async () => { + const response = await app.inject({ + method: 'POST', + url: '/api/events/notify', + headers: authHeader(), + payload: { type: 'asks', action: 'updated' }, + }) + + assert.strictEqual(response.statusCode, 200) + const body = JSON.parse(response.body) + assert.strictEqual(body.success, true) + }) + it('should accept notification without action', async () => { const response = await app.inject({ method: 'POST', diff --git a/tests/web/components/AskSessionDock.test.tsx b/tests/web/components/AskSessionDock.test.tsx new file mode 100644 index 0000000..a092a48 --- /dev/null +++ b/tests/web/components/AskSessionDock.test.tsx @@ -0,0 +1,64 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen } from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' +import { AskSessionDock } from '../../../src/web/components/layout/AskSessionDock' + +const refetch = vi.fn() +const continueAsk = vi.fn() + +vi.mock('../../../src/web/hooks/useAsks', () => ({ + useAskSession: vi.fn(() => ({ + data: { + id: 'ask-1', + repositoryPath: '/repo', + reviewId: null, + message: 'Please review the latest changes.', + status: 'waiting_for_human', + assistantContext: null, + createdAt: '2026-04-04T10:00:00.000Z', + updatedAt: '2026-04-04T10:00:00.000Z', + completedAt: null, + review: null, + feedback: { + reviewStatus: null, + todoCount: 0, + commentCount: 0, + unresolvedCommentCount: 0, + }, + }, + refetch, + })), + useContinueAsk: vi.fn(() => ({ + mutate: continueAsk, + loading: false, + error: null, + })), +})) + +describe('AskSessionDock', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('renders handoff actions when ask query parameter is present', () => { + render( + + + + ) + + expect(screen.getByText('Assistant handoff active')).toBeDefined() + expect(screen.getByText('Continue assistant')).toBeDefined() + expect(screen.getByRole('link').getAttribute('href')).toBe('/ask/ask-1') + }) + + it('does not render without ask query parameter', () => { + const { container } = render( + + + + ) + + expect(container.firstChild).toBeNull() + }) +}) diff --git a/tests/web/hooks/useAsks.test.ts b/tests/web/hooks/useAsks.test.ts new file mode 100644 index 0000000..bd05c76 --- /dev/null +++ b/tests/web/hooks/useAsks.test.ts @@ -0,0 +1,117 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { renderHook, act, waitFor } from '@testing-library/react' +import { useAskSession } from '../../../src/web/hooks/useAsks' +import { ApiClientError } from '../../../src/web/api/client' +import type { AskSessionDetails } from '../../../src/shared/types' +import { asksApi } from '../../../src/web/api/asks' + +vi.mock('../../../src/web/api/asks', () => ({ + asksApi: { + get: vi.fn(), + getFeedback: vi.fn(), + continue: vi.fn(), + cancel: vi.fn(), + create: vi.fn(), + }, +})) + +const mockedAsksApi = vi.mocked(asksApi) + +function createAskDetails (overrides: Partial = {}): AskSessionDetails { + return { + id: 'ask-1', + repositoryPath: '/repo', + reviewId: 'review-1', + message: 'Please review the ask flow.', + status: 'waiting_for_human', + assistantContext: null, + createdAt: '2026-04-04T10:00:00.000Z', + updatedAt: '2026-04-04T10:00:00.000Z', + completedAt: null, + review: { + id: 'review-1', + sourceType: 'branch', + sourceRef: 'feature', + baseRef: 'main', + status: 'in_progress', + }, + feedback: { + reviewStatus: 'in_progress', + todoCount: 0, + commentCount: 0, + unresolvedCommentCount: 0, + }, + ...overrides, + } +} + +describe('useAskSession', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('keeps existing data visible during background refetches', async () => { + const initialData = createAskDetails() + const refreshedData = createAskDetails({ + feedback: { + reviewStatus: 'changes_requested', + todoCount: 2, + commentCount: 3, + unresolvedCommentCount: 1, + }, + }) + + let resolveRefetch: ((value: AskSessionDetails) => void) | undefined + + mockedAsksApi.get + .mockResolvedValueOnce(initialData) + .mockImplementationOnce(async () => await new Promise((resolve) => { + resolveRefetch = resolve + })) + + const { result } = renderHook(() => useAskSession('ask-1')) + + await waitFor(() => { + expect(result.current.loading).toBe(false) + }) + + expect(result.current.data).toEqual(initialData) + + await act(async () => { + const refetchPromise = result.current.refetch() + + expect(result.current.loading).toBe(false) + expect(result.current.data).toEqual(initialData) + + resolveRefetch?.(refreshedData) + await refetchPromise + }) + + expect(result.current.loading).toBe(false) + expect(result.current.data).toEqual(refreshedData) + }) + + it('preserves existing data when a background refetch fails', async () => { + const initialData = createAskDetails() + + mockedAsksApi.get + .mockResolvedValueOnce(initialData) + .mockRejectedValueOnce(new ApiClientError('Request failed', 500)) + + const { result } = renderHook(() => useAskSession('ask-1')) + + await waitFor(() => { + expect(result.current.loading).toBe(false) + }) + + expect(result.current.data).toEqual(initialData) + + await act(async () => { + await result.current.refetch() + }) + + expect(result.current.loading).toBe(false) + expect(result.current.data).toEqual(initialData) + expect(result.current.error).toBeInstanceOf(ApiClientError) + }) +}) diff --git a/website/index.html b/website/index.html index c846a85..3fd7ad8 100644 --- a/website/index.html +++ b/website/index.html @@ -540,7 +540,7 @@

Human reviews the changes

4
-

Mark the request todo as done

+

Click Continue assistant

That explicit action tells GitHuman the human's turn is complete

From b96e1906a13cdeb3d3f4e4c8d81be3b485046a31 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 5 Apr 2026 23:14:56 +0200 Subject: [PATCH 4/4] Remove ask redesign plan --- ASK.md | 468 --------------------------------------------------------- 1 file changed, 468 deletions(-) delete mode 100644 ASK.md diff --git a/ASK.md b/ASK.md deleted file mode 100644 index 5057192..0000000 --- a/ASK.md +++ /dev/null @@ -1,468 +0,0 @@ -# `githuman ask` redesign plan - -## Status - -The current implementation proved that a synchronous agent ↔ human handoff is valuable, but it also exposed a major UX problem: - -- using todos as the control mechanism is wrong -- the human has to understand internal workflow details -- the UI is too generic for a handoff flow -- there is no obvious single action that says “give this back to the assistant” - -This document replaces the earlier todo-driven plan. - ---- - -## Product goal - -`githuman ask` should feel like a first-class handoff between an assistant and a human reviewer. - -Desired flow: - -1. the assistant runs `githuman ask` -2. GitHuman opens a dedicated review/handoff page -3. the human reviews the changes and leaves feedback -4. the human clicks a clear primary action: **Continue assistant** -5. `githuman ask` exits and prints the feedback for the assistant - -The completion signal must be explicit and built for this feature, not borrowed from todos. - ---- - -## Core product decisions - -## 1. `ask` must be a first-class object - -Do **not** model ask sessions as todos. - -Introduce a dedicated persisted entity, for example `ask_sessions`. - -Suggested fields: - -- `id` -- `repository_path` -- `review_id` nullable -- `message` -- `status` - - `waiting_for_human` - - `ready_for_agent` - - `cancelled` -- `assistant_context` nullable -- `created_at` -- `updated_at` -- `completed_at` nullable - -Optional later fields: - -- `feedback_summary` -- `url_token` or access metadata if needed -- `agent_name` / `agent_id` - -## 2. Completion must be explicit - -The human should finish the handoff by clicking a primary UI button: - -- **Continue assistant** - -Optional secondary button: - -- `Cancel request` - -This button should update the ask session status to `ready_for_agent`. - -That status change is the only completion signal `githuman ask` should wait on. - -## 3. Todos remain useful, but not for control flow - -Todos can still exist as review artifacts. - -They may be part of the feedback returned to the assistant, but they must not be used to: - -- represent the ask session itself -- determine whether the human is done -- force the human to understand internal mechanics - ---- - -## UX requirements - -## Dedicated ask page - -`githuman ask` should send the human to a focused page, not a generic dashboard. - -Example routes: - -- `/ask/:id` -- or `/handoff/:id` - -This page should be massively simpler than the normal review UI. - -### Page goals - -The page should answer three questions immediately: - -1. What is the assistant asking for? -2. Where should I leave feedback? -3. How do I hand this back to the assistant? - -### Page content - -Recommended layout: - -- title: `Assistant review request` -- assistant message -- review context - - linked review, if available - - repository / branch context -- concise instructions -- current feedback state - - comments count - - unresolved comments count - - todos count -- large sticky primary button: - - **Continue assistant** -- smaller secondary button: - - `Cancel request` - -### Interaction model - -The human should be able to: - -1. inspect the review -2. leave comments -3. add todos if useful -4. click **Continue assistant** - -No hidden rules. -No “mark this special todo as done.” -No generic workflow leakage. - ---- - -## CLI behavior - -## Command flow - -```bash -githuman ask "Please review the parser refactor" -``` - -Behavior: - -1. resolve config like `serve` -2. reuse a running GitHuman server if available, otherwise start one -3. create an ask session -4. print the dedicated ask URL -5. optionally open the browser -6. wait for ask session status to become `ready_for_agent` or `cancelled` -7. collect feedback from the session -8. print plain text or JSON output for the assistant - -## Suggested output - -Plain text: - -```text -GitHuman feedback ready -URL: https://host:port/ask/123 -Ask status: ready_for_agent -Review status: changes_requested - -Todos: -- Add a regression test for whitespace-only input - -Comments: -- src/parser.ts:42 - "Please reject undefined explicitly." -``` - -JSON: - -```json -{ - "url": "https://host:port/ask/123", - "askStatus": "ready_for_agent", - "reviewStatus": "changes_requested", - "todos": [], - "comments": [] -} -``` - ---- - -## Feedback semantics - -When `githuman ask` completes, it should return feedback created during that ask session. - -Recommended scope: - -- comments created or updated after ask session start -- todos created or updated after ask session start -- review status at completion time -- optionally unresolved comments for the targeted review - -If the ask session is tied to a review, filter primarily by that review. - ---- - -## Config behavior - -Keep the new config work. - -Use repo-local config: - -- `.githuman/config.json` - -Supported defaults: - -- `host` -- `port` -- `https` -- `open` -- `authToken` - -Precedence: - -1. CLI flags -2. `.githuman/config.json` -3. built-in defaults - -This part of the implementation is still valid and should remain. - ---- - -## Backend/API changes - -## New persistence - -Add an `ask_sessions` table. - -Suggested migration: - -- create table -- add indexes for `status`, `review_id`, `repository_path`, `created_at` - -## Repository/service layer - -Add: - -- `AskSessionRepository` -- `AskSessionService` - -Capabilities: - -- create ask session -- fetch ask session by ID -- list ask sessions if needed -- mark as ready for agent -- cancel ask session -- compute session feedback summary - -## API routes - -Add endpoints such as: - -- `POST /api/asks` -- `GET /api/asks/:id` -- `PATCH /api/asks/:id` -- `POST /api/asks/:id/continue` -- `POST /api/asks/:id/cancel` -- optional `GET /api/asks/:id/feedback` - -This makes the flow explicit and testable. - ---- - -## Frontend changes - -## New ask page - -Add a dedicated page and route for the ask handoff. - -Likely additions: - -- `src/web/pages/AskPage.tsx` -- route in `src/web/App.tsx` -- API client methods in `src/web/api/...` -- hooks for ask session loading and completion - -## Simplified UI requirements - -The ask page should: - -- prioritize the assistant message -- make the next action obvious -- reduce navigation noise -- prominently show the **Continue assistant** button - -The page may link into the full review UI, but the handoff page itself must stay simple. - -## Continue button behavior - -Clicking **Continue assistant** should: - -- update ask session status to `ready_for_agent` -- optionally confirm the action -- show immediate success state -- allow `githuman ask` to exit - ---- - -## Documentation updates - -This is still a flagship feature and needs docs alongside implementation. - -## README - -Update README to describe the new first-class handoff flow: - -- `githuman ask` creates a dedicated assistant review request -- the human completes the handoff with **Continue assistant** -- the feature is distinct from todos -- config file usage and precedence - -## CLI docs - -Document: - -- `githuman ask` options -- server reuse/start behavior -- JSON output -- cancellation behavior - -## Explanation docs - -Add a short explanation of why this feature exists: - -- AI agents need a clean human checkpoint -- GitHuman provides a true human-in-the-loop handoff -- the assistant resumes only when the human explicitly returns control - ---- - -## Website updates - -The website should highlight this redesigned flow, not the old todo workaround. - -Messaging should emphasize: - -- assistant asks for review -- human reviews in a focused UI -- human clicks **Continue assistant** -- assistant resumes with structured feedback - -Recommended updates: - -- hero/feature callout for `githuman ask` -- simple visual 3- or 4-step flow -- screenshot of the simplified ask page -- terminal example showing `githuman ask` - ---- - -## Testing plan - -## Backend tests - -Add tests for: - -- ask session creation -- status transitions -- continue/cancel actions -- feedback collection semantics - -## CLI tests - -Add tests for: - -- creating an ask session -- waiting for `ready_for_agent` -- handling `cancelled` -- JSON output -- config precedence -- server reuse vs startup - -## Frontend tests - -Add tests for: - -- ask page rendering -- Continue button visibility -- Continue button action -- cancellation flow -- simplified page state - -## E2E tests - -Add Playwright coverage for: - -1. run `githuman ask` -2. open ask page -3. leave comment(s) -4. click **Continue assistant** -5. assert CLI exits with collected feedback - ---- - -## Delivery plan - -## Phase 1 — Preserve useful infrastructure - -Keep and refine: - -- shared config loading -- serve/ask runtime sharing -- CLI output formatting -- docs/website momentum - -## Phase 2 — Introduce first-class ask sessions - -- add DB migration -- add repository/service/routes -- update CLI to use ask sessions instead of todos - -## Phase 3 — Build the dedicated ask page - -- add new route and page -- simplify UI aggressively -- add **Continue assistant** button - -## Phase 4 — Wire completion and feedback - -- CLI waits for ask session status -- collect scoped feedback -- finalize text and JSON output - -## Phase 5 — Docs and website - -- update README -- update CLI docs -- update landing page -- add screenshot/demo assets - ---- - -## Non-goals for v1 of the redesign - -Avoid these until the core flow feels right: - -- overly smart automation for detecting completion -- making todos mandatory -- embedding too much general review UI into the ask page -- multi-agent orchestration - -The first priority is a simple, obvious, trustworthy handoff. - ---- - -## Acceptance criteria - -This redesign is successful when: - -- `githuman ask` no longer depends on todos for completion -- the human sees a dedicated, simplified handoff page -- there is a clear **Continue assistant** button -- the assistant reliably resumes with structured feedback -- the flow is understandable without explanation - -If a first-time user can complete the handoff without being told about implementation details, the redesign worked.