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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Review a page running on localhost:
/human-review (localhost URL)
```

Human Review opens the file in your browser. Make direct edits, leave comments, and click Send. Your agent receives all your feedback in one batch, updates the source, and refreshes the page for another review.
Human Review opens the file in your browser. Make direct edits, leave comments, and click Send. If it's already fine, click **No change** — or just close the tab. Either one tells the waiting agent to stop polling instead of hanging until timeout. Your agent receives feedback in one batch, updates the source, and refreshes the page for another review.

Note: For HTML files, direct edits and resizes save automatically. For Markdown and localhost pages, click Send so your agent can apply them to the source.

Expand All @@ -63,6 +63,7 @@ Note: For HTML files, direct edits and resizes save automatically. For Markdown
- **Remove elements** without explaining the deletion in chat.
- **Command-click links** to review multiple pages without losing your feedback.
- **Send every edit and comment at once** instead of writing a long chat message.
- **No change** (or closing the tab) ends the review so the agent stops polling.

I use Human Review to edit AI-generated plans, update landing pages, review localhost apps, and remove the extra copy AI likes to add to UX.

Expand Down
41 changes: 32 additions & 9 deletions src/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,37 @@ keeping its formatting syntax.
npx -y human-review http://localhost:3000/wiki
```

3. Wait for feedback. This blocks until they hit Send, or the timeout passes:
3. Wait for feedback. This blocks until they hit **Send**, hit **No change**,
close the review tab, or the timeout passes:

```sh
npx -y human-review poll path/to/file.html --timeout 600
```

Keep this command in the foreground. Do not end your turn while it is waiting.
If your shell returns a process or session handle, keep waiting on that handle
until the command exits. If it prints `{"status":"timeout"}`, no feedback has
arrived yet — run the same poll command again to keep waiting. Feedback is
saved even if a poll dies, so nothing is ever lost.
until the command exits.

If it prints `{"status":"closed"}`, the user ended the review from the
browser — stop polling and do not run the poll command again. Unsent
feedback is kept and ships the next time this target is reviewed.
**Stop polling. Do not run poll again when:**
- it prints `{"status":"closed"}` — they hit No change or closed the tab
- they say they are done in chat (kill any still-running poll)

4. Apply what comes back, then wait again. `--ack` clears the batch you just handled:
**Timeout:** if it prints `{"status":"timeout"}`, run `npx -y human-review status <file>`
first. If status is `closed`, stop. If the user has moved on in chat, stop and
kill the poll. Only re-poll if you are still waiting on this review.

Never leave a poll running in the background after the review is over.
Closing the browser is a decision: it ends the review.

4. Apply what comes back, then wait again. `--ack` clears a **feedback** batch:

```sh
npx -y human-review poll path/to/file.html --ack --timeout 600
```

Repeat 3–4 until the user says they are done.
Do **not** `--ack` a `closed` batch. That review is finished.

Repeat 3–4 until the user is done (`closed`, or they say so).

Not sure whether feedback is already waiting — say, at the start of a new turn?
This answers instantly without blocking:
Expand All @@ -60,6 +68,7 @@ npx -y human-review status path/to/file.html
```

It prints `{"status": "feedback-waiting"}` when a batch is ready for a poll,
`{"status": "closed"}` when they finished with no further changes,
plus counts of unsent comments and edits still in the browser.

## What you get
Expand Down Expand Up @@ -89,6 +98,20 @@ One batch covers every page the user visited, grouped by file or localhost URL.
}
```

A finished review with nothing to apply looks like:

```json
{
"status": "closed",
"reason": "no_change",
"next_step": "The user ended this review session. Stop polling — do not run the poll command again."
}
```

`reason` is `no_change` (they clicked No change) or `window_closed` (they
closed the tab). Either way: stop. Unsent comments from a closed tab are kept
and ship the next time this target is reviewed.

## Rules

- **`edits` are changes the user already made.** `after` is their exact wording —
Expand Down
39 changes: 30 additions & 9 deletions src/chrome-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const state = {
save: "idle",
savedAt: "",
sent: false,
closed: false,
orphans: new Set(),
pollCommand: "",
editsExpanded: false,
Expand Down Expand Up @@ -303,10 +304,14 @@ function render() {
// note-only batches; the button must not stay dead while one is typed.
const hasNote = $("note").value.trim().length > 0;
const send = $("send");
const end = $("endReview");
const delivered = state.agent === "working";
const stranded = state.agent === "stranded";
const busy = delivered || stranded || state.sent;
send.disabled = (total === 0 && !hasNote) || busy;
const finished = state.agent === "closed" || state.closed;
const busy = delivered || stranded || state.sent || finished;
const empty = total === 0 && !hasNote;
send.disabled = empty || busy;
send.hidden = empty && !busy && !delivered && !stranded && !state.sent;
send.textContent = delivered
? "Feedback delivered"
: stranded
Expand All @@ -318,13 +323,20 @@ function render() {
: hasNote
? "Send note to agent"
: "Nothing to send yet";
if (!send.disabled) {
if (!send.disabled && !send.hidden) {
const key = document.createElement("span");
key.className = "key";
key.textContent = "⌘⏎";
send.append(" ", key);
}

end.disabled = busy;
end.classList.toggle("primary", empty && !busy);
end.textContent = finished ? "Review ended" : empty ? "No change" : "End review";
end.title = empty
? "Looks good — end the review without comments"
: "Stop this review and release the waiting agent";

// After sending, say what happens next. If nothing is polling, the loop would
// otherwise dead-end silently, so hand over the exact command to run.
$("agentLine").hidden = !delivered;
Expand Down Expand Up @@ -701,14 +713,22 @@ $("endReview").addEventListener("click", async () => {
const page = state.page;
const otherTotal = (state.others || []).reduce((sum, o) => sum + o.count, 0);
const unsent = page ? (page.comments || []).length + (page.edits || []).length + otherTotal : 0;
const message = unsent
? `End this review? ${unsent} unsent ${unsent === 1 ? "item" : "items"} will be kept for next time.`
: "End this review? The waiting agent will be told to stop polling.";
if (!window.confirm(message)) return;
const hasNote = $("note").value.trim().length > 0;
if (unsent || hasNote) {
const message = unsent
? `End this review? ${unsent} unsent ${unsent === 1 ? "item" : "items"} will be kept for next time.`
: "End this review? The waiting agent will be told to stop polling.";
if (!window.confirm(message)) return;
}
// Ship anything still sitting in the SDK's debounce windows first.
await flushFrame();
try {
await api(`/api/session/${state.sessionId}/end`, { method: "POST" });
await api(`/api/session/${state.sessionId}/end`, {
method: "POST",
body: JSON.stringify({ reason: "no_change" }),
});
state.closed = true;
state.agent = "closed";
showEnded();
} catch (err) {
toast(err.message);
Expand Down Expand Up @@ -766,7 +786,8 @@ document.addEventListener("keydown", (event) => {
const meta = event.metaKey || event.ctrlKey;
if (meta && event.key === "Enter") {
event.preventDefault();
if (!$("send").disabled) $("send").click();
if (!$("send").disabled && !$("send").hidden) $("send").click();
else if (!$("endReview").disabled) $("endReview").click();
return;
}
// ⌘S is reassurance only: flush pending keystrokes, never a state change.
Expand Down
12 changes: 12 additions & 0 deletions src/chrome.css
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,18 @@ body.collapsed .handle { right: 0; }
border-radius: 6px;
}
.end-review:hover { color: var(--danger); background: var(--danger-soft); }
.end-review.primary {
margin-top: 0;
margin-bottom: 8px;
padding: 10px 12px;
border: 1px solid var(--btn-bg);
background: var(--btn-bg);
color: var(--btn-fg);
font-size: 12.5px;
font-weight: 500;
}
.end-review.primary:hover { filter: brightness(1.08); color: var(--btn-fg); background: var(--btn-bg); }
.end-review:disabled { color: var(--faint); cursor: default; background: none; }

.ended {
position: fixed;
Expand Down
4 changes: 2 additions & 2 deletions src/chrome.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@

<div id="cards" class="cards"></div>

<div id="empty" class="empty">Select text or click an element to comment on it</div>
<div id="empty" class="empty">Select text or click a block to comment. If this is fine, hit No change — closing the tab also ends the review.</div>

<section id="othersBox" hidden>
<div class="rail-head tight">
Expand Down Expand Up @@ -73,7 +73,7 @@
<div class="handoff-cmd"><code id="handoffCmd"></code></div>
<button type="button" id="handoffCopy" class="btn-ghost">Copy prompt</button>
</div>
<button type="button" id="endReview" class="end-review" title="Stop this review and release the waiting agent">End review</button>
<button type="button" id="endReview" class="end-review" title="Looks good — end the review without comments">No change</button>
</div>
</aside>
</div>
Expand Down
4 changes: 3 additions & 1 deletion src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ async function pollCommand(input, { ack = false, timeoutSecs = 0 } = {}) {
const server = await ensureServer();

const label = /^https?:\/\//i.test(target) ? target : path.basename(target);
process.stderr.write(`Waiting for feedback on ${label} — comment in the browser, then hit Send.\n`);
process.stderr.write(
`Waiting for feedback on ${label} — comment and hit Send, or hit No change if it's fine.\n`
);

const deadline = timeoutSecs ? Date.now() + timeoutSecs * 1000 : null;
for (let attempt = 0; attempt < 3; attempt += 1) {
Expand Down
95 changes: 73 additions & 22 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ const WATCH_INTERVAL_MS = 400;
const IDLE_SHUTDOWN_MS = Number(process.env.HUMAN_REVIEW_IDLE_MS || 45 * 60 * 1000);
/** A window with no live connection this long is treated as closed for good. */
const SESSION_TTL_MS = 30 * 60 * 1000;
/** Last SSE client gone this long → the human closed the tab. Short enough that
* a close actually ends the poll; long enough that a refresh can reconnect. */
const windowClosedMs = () => Number(process.env.HUMAN_REVIEW_WINDOW_CLOSED_MS || 4000);
const MAX_LOCAL_REDIRECTS = 5;
/** Generous enough for a dev server's cold compile, but a wedged one can't hang us forever. */
const LOCAL_FETCH_TIMEOUT_MS = 30000;
Expand Down Expand Up @@ -109,6 +112,8 @@ export function createServer() {
const sessions = new Map(); // sessionId -> { id, entryKey, activeKey, visited, clients:Set<res>, lastSeen }
/** Agent long-polls, keyed by the entry page they were started on. */
const pollers = new Map(); // entryKey -> Set<{ res, timer }>
/** Human finished this review (No change, End review, or they closed the tab). */
const closed = new Map(); // entryKey -> { reason, at, batch }
/** Pending batches awaiting --ack; mirrored to the store so they survive restarts. */
const batches = new Map(
Object.entries(store.allBatches()).map(([key, record]) => [key, { batch: record.batch, cleanup: record.cleanup, delivered: false }])
Expand Down Expand Up @@ -145,6 +150,7 @@ export function createServer() {
* Feedback sent with nothing listening is "stranded", and the browser says so.
*/
function agentState(entryKey) {
if (closed.has(entryKey)) return "closed";
const pending = batches.get(entryKey);
if (pending && pending.delivered) return "working";
const set = pollers.get(entryKey);
Expand Down Expand Up @@ -329,12 +335,41 @@ export function createServer() {
return true;
}

function closedBatch(reason) {
return {
status: "closed",
reason,
next_step:
"The user ended this review session. Stop polling — do not run the poll command again. " +
"Any unsent feedback is kept and will ship the next time this target is reviewed.",
};
}

function releasePollers(entryKey, payload) {
const set = pollers.get(entryKey);
if (!set) return;
for (const poller of [...set]) {
clearInterval(poller.timer);
set.delete(poller);
poller.res.end(JSON.stringify(payload));
}
}

function closeEntry(entryKey, reason) {
const batch = closedBatch(reason);
closed.set(entryKey, { reason, at: Date.now(), batch });
releasePollers(entryKey, batch);
broadcastAgent(entryKey);
}

/**
* A deliberate stop, not a tab close: the browser forgets the session and
* any waiting agent is released with a clear "stop polling" answer instead
* of being left to burn its timeout. Unsent feedback stays in the store.
* A deliberate stop: the browser forgets the session and any waiting agent
* is released with a clear "stop polling" answer instead of being left to
* burn its timeout. Unsent feedback stays in the store. Closing the last
* tab uses the same path after a short grace, so a refresh can reconnect.
*/
function endSession(session) {
function endSession(session, reason = "no_change") {
clearTimeout(session.closeTimer);
sessions.delete(session.id);
for (const res of session.clients) {
res.write(`event: ended\ndata: {}\n\n`);
Expand All @@ -343,20 +378,18 @@ export function createServer() {
session.clients.clear();
// Another window on the same target keeps its agent connection alive.
if (sessionsForEntry(session.entryKey).length > 0) return;
const set = pollers.get(session.entryKey);
if (!set) return;
for (const poller of [...set]) {
clearInterval(poller.timer);
set.delete(poller);
poller.res.end(
JSON.stringify({
status: "closed",
next_step:
"The user ended this review session. Stop polling — do not run the poll command again. " +
"Any unsent feedback is kept and will ship the next time this target is reviewed.",
})
);
}
closeEntry(session.entryKey, reason);
}

function scheduleWindowClosed(session) {
clearTimeout(session.closeTimer);
session.closeTimer = setTimeout(() => {
if (!sessions.has(session.id)) return;
if (session.clients.size > 0) return;
const live = sessionsForEntry(session.entryKey).some((s) => s.clients.size > 0);
if (live) return;
endSession(session, "window_closed");
}, windowClosedMs());
}

// ----------------------------------------------------------------- routes
Expand Down Expand Up @@ -511,7 +544,16 @@ export function createServer() {
}
watchPage(page.key);
const id = uid("s");
sessions.set(id, { id, entryKey: page.key, activeKey: page.key, visited: new Set([page.key]), clients: new Set(), lastSeen: Date.now() });
closed.delete(page.key);
sessions.set(id, {
id,
entryKey: page.key,
activeKey: page.key,
visited: new Set([page.key]),
clients: new Set(),
lastSeen: Date.now(),
closeTimer: null,
});
return json(res, 200, { sessionId: id, key: page.key, path: `/s/${id}` });
}

Expand Down Expand Up @@ -607,8 +649,10 @@ export function createServer() {
comments += page.comments.length;
edits += page.edits.length;
}
const finished = closed.get(entryKey);
return json(res, 200, {
status: pending ? "feedback-waiting" : "idle",
status: pending ? "feedback-waiting" : finished ? "closed" : "idle",
...(finished ? { reason: finished.reason } : {}),
feedback_waiting: !!pending,
agent_listening: listening,
server_running: true,
Expand Down Expand Up @@ -794,8 +838,10 @@ export function createServer() {
if (endMatch && req.method === "POST") {
const session = sessions.get(endMatch[1]);
if (!session) return json(res, 404, { error: "unknown session" });
endSession(session);
return json(res, 200, { ok: true });
const body = await readBody(req).catch(() => ({}));
const reason = body.reason === "window_closed" ? "window_closed" : "no_change";
endSession(session, reason);
return json(res, 200, { ok: true, status: "closed", reason });
}

// --- which page a window is currently showing
Expand Down Expand Up @@ -871,12 +917,14 @@ export function createServer() {
res.write(": open\n\n");
session.clients.add(res);
seen(session);
clearTimeout(session.closeTimer);
emit(session, "agent", { state: agentState(session.entryKey) });
const beat = setInterval(() => res.write(": beat\n\n"), POLL_HEARTBEAT_MS);
req.on("close", () => {
clearInterval(beat);
session.clients.delete(res);
seen(session);
if (session.clients.size === 0 && sessions.has(session.id)) scheduleWindowClosed(session);
});
return undefined;
}
Expand All @@ -887,6 +935,9 @@ export function createServer() {
const entryKey = targetKey(target);
if (url.searchParams.get("ack") === "1") ack(entryKey);

const finished = closed.get(entryKey);
if (finished) return json(res, 200, finished.batch);

const pending = batches.get(entryKey);
if (pending) {
pending.delivered = true;
Expand Down
Loading