Fix reply recipients and verify thread delivery

[code-monger-givenall]

What changed

hey reply used the recipient list from the topic page. On incoming mail, that list describes the original message and can point a reply back at the current user instead of the sender.

The command now reads the latest entry from the typed topic API, then loads the reply form for that entry and uses the exact To, CC, and BCC envelope HEY prepared. The shared compose --thread-id path uses the same envelope.

This also adds:

• --preview with From, To, CC, BCC, subject, the complete body, attachment metadata, and the target entry_id.
• Read-only attachment validation during preview. Files are not uploaded until send.
• --expect-entry <entry_id> on send. The command checks that entry after message input and again after any attachment upload, immediately before delivery. If the thread changed, it stops and asks for a fresh preview.
• Post-send verification that matches a typed thread entry by sender ID and normalized message content. An unrelated incoming message cannot be mistaken for the sent reply.
• A clear safety error when HEY accepts a reply but the matching entry cannot be verified. The error tells callers not to retry automatically, which avoids duplicate replies when thread updates are delayed.

The verification schedule is created per call, so tests do not mutate package-level timing state.

Fixes #113

Related to #66

Tests

• env GOWORK=off mise x go@1.26.6 golangci-lint@2.12.2 -- make check
• env GOWORK=off mise x go@1.26.6 -- make check-surface
• Smoke suite compiles with the preview and send binding.
• Full suite passed, including reply attachments, attachment-only replies, preview without writes, stale-entry rejection before delivery, unrelated-entry rejection, sender and content matching, recipient parsing, delayed propagation, and unverified-send handling.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a safer “reply” workflow by introducing a preview mode that shows the full outbound envelope/body before sending, and by verifying that a sent reply actually appears in the thread.

Changes:

• Add hey reply --preview (no send) and update reply docs/examples to promote preview-before-send.
• Introduce HTML parsers to extract topic subject and reply-form recipients (To/CC/BCC) from HEY pages.
• After sending, poll the thread entries to verify the reply was created before reporting success.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
skills/hey/SKILL.md	Updates skill guidance and quick references to require preview-before-send for replies.
internal/htmlutil/topic.go	New HTML parser to extract topic subject from multiple possible DOM locations.
internal/htmlutil/topic_test.go	Tests for topic subject parsing (DOM subject, title fallback, metadata).
internal/htmlutil/reply.go	New HTML parser to locate the reply form and extract recipients.
internal/htmlutil/reply_test.go	Tests for reply form detection and recipient extraction (select + hidden inputs, errors).
internal/cmd/reply.go	Adds --preview, uses reply form envelope, and verifies created entry via polling.
internal/cmd/reply_test.go	Coverage for preview behavior and send verification (including propagation delays).
README.md	Updates usage examples to show preview vs send.
API-COVERAGE.md	Documents the new HTML gap endpoint used to resolve the live reply envelope.
.surface	Adds hey reply --preview to the command surface list.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/htmlutil/topic.go:97

• strings.SplitSeq is relatively new in the standard library; if this repo’s Go version in go.mod is older than the version that introduced it, this will fail to compile. To keep compatibility stable, prefer iterating over strings.Split(text, "\n"), or use a small manual scan for newline boundaries (which also avoids allocating a slice).

func firstTextLine(text string) string {
	for line := range strings.SplitSeq(text, "\n") {
		if line = strings.TrimSpace(line); line != "" {
			return line
		}
	}
	return ""
}

internal/cmd/reply.go:141

• replyVerificationDelays is a package-level mutable variable and the tests mutate it to control timing. This can create data races/flaky behavior if tests are ever run in parallel within the cmd package. Consider making delays immutable and injectable (e.g., pass a delay strategy into verifyReplyCreated, or store delays in the replyCommand struct), or protect mutation behind a mutex/atomic and ensure callers always work on a copied slice.

var replyVerificationDelays = []time.Duration{
	0,
	250 * time.Millisecond,
	500 * time.Millisecond,
	1 * time.Second,
	2 * time.Second,
	4 * time.Second,
	8 * time.Second,
}

internal/cmd/reply_test.go:35

• The test helper ignores json.Unmarshal errors, which can mask unexpected non-JSON output (or partial output) and make failures harder to diagnose. It would be more robust to check the unmarshal error and t.Fatalf with the raw buffer contents when JSON parsing fails.

	if buf.Len() > 0 {
		_ = json.Unmarshal(buf.Bytes(), &resp)
	}

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

internal/cmd/reply.go:266

• Any concurrent thread update satisfies this condition, even when it is another participant's entry. The command can therefore return success immediately with that unrelated entry's ID before the accepted reply appears, defeating the promised delivery verification. Verify the discovered entry is the reply created by this invocation rather than only checking that the latest ID changed.

		if topic != nil && topic.LatestEntry.Id > 0 && topic.LatestEntry.Id != previousEntryID {
			return topic.LatestEntry.Id, nil

API-COVERAGE.md:39

• These coverage rows omit the shared compose --thread-id path: it now calls both Entries().CreateReply and Topics().Get through resolveThreadReply. Include that command in both rows so the endpoint coverage table reflects the behavior introduced by this PR.

| `/entries/{id}/replies.json` | POST | SDK `Entries().CreateReply` | `hey reply <topic-id>` | covered |
| `/topics/{id}.json` | GET | SDK `Topics().Get` | `hey forward <topic-id>`, `hey reply <topic-id>` | covered |

internal/cmd/thread_reply_test.go:60

• t.Fatalf calls FailNow, which must run in the test goroutine; this handler runs in the HTTP server's goroutine. On malformed input it exits only the handler and may leave the client with an unexplained EOF. Record the failure with t.Errorf, write an HTTP error, and return from the handler instead.

			if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
				t.Fatalf("decode reply: %v", err)
			}

skills/hey/SKILL.md:101

• This new rule applies to every outbound email, but the quick-reference rows below still send forwards and new compositions directly, and only reply exposes --preview. As written, an agent following those documented workflows violates the safety rule. Either scope the rule to replies or document/implement preview-and-confirm workflows for forward and compose as well.

4. **Preview every outbound email.** Show From, To, CC, BCC, Subject, the complete body, and attachments, then obtain explicit confirmation immediately before sending.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

internal/cmd/reply.go:111

• --expect-entry is checked at line 73, before the user may spend time in $EDITOR and before attachment uploads. If a newer entry arrives during either operation, this still posts to the old entry with the old envelope, despite the flag promising to send only while that entry remains latest. Revalidate immediately before uploads and again before delivery, or enforce the expected entry atomically in the server/SDK reply operation.

	message, err = attachFiles(ctx, message, c.attachments)
	if err != nil {
		return err
	}
	if err = sdk.Entries().CreateReply(ctx, target.EntryID, message, target.Addressed.To, target.Addressed.CC, target.Addressed.BCC); err != nil {

internal/cmd/thread_reply.go:31

• AGENTS.md:54-58 explicitly prohibits extending HTML scraping and requires missing API support to be added as a typed SDK operation. This introduces a new /replies/new HTML dependency plus ParseReplyFormHTML, so a markup change can break all reply sends. Add a typed reply-envelope operation to the SDK and consume it here instead of parsing the web form.

	replyResp, err := sdk.GetHTML(ctx, fmt.Sprintf("/entries/%d/replies/new", topic.LatestEntry.Id))

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/cmd/thread_reply.go:47

• This introduces a new HTML scraper for a HEY data read. The repository's API boundary requires new reads to be exposed as typed HEY SDK operations rather than extending markup parsing; otherwise a reply-form markup change can silently break recipient selection. Please add a typed reply-envelope operation to the SDK and consume it here instead of GetHTML/ParseReplyFormHTML.

	replyResp, err := sdk.GetHTML(ctx, fmt.Sprintf("/entries/%d/replies/new", topic.LatestEntry.Id))
	if err != nil {
		return nil, convertSDKError(err)
	}
	addressed, err := htmlutil.ParseReplyFormHTML(string(replyResp.Data))

internal/cmd/reply.go:351

• The arbitrary prefix match can verify the wrong entry. For example, after sending Thanks, another entry from the same sender containing Thanks for the update passes this condition even if the sent reply never appears. Require equality after normalization, or strip only specific server-added content before comparing, so sender/content verification cannot accept a merely longer message.

	return expected != "" && (actual == expected || strings.HasPrefix(actual, expected+" "))

skills/hey/SKILL.md:101

• This new rule covers every outbound email, but the same skill still directs agents to run hey forward and hey compose immediately (for example, lines 123-125 and 283-290), and those commands have no --preview option. That makes the safety workflow internally inconsistent. Either scope this rule to replies or document a concrete preview-and-confirm flow for compose and forward before their send commands.

4. **Preview every outbound email.** Show From, To, CC, BCC, Subject, the complete body, and attachments, then obtain explicit confirmation immediately before sending.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/cmd/reply.go:122

• The stale-preview check is still subject to a request-sized race: resolveExpectedThreadReply checks LatestEntry, then performs a separate reply-form GET before this POST. If a new entry arrives during that GET/parsing window, the command sends even though the thread changed after preview, contrary to the safety contract. Recheck the latest entry after loading the envelope and immediately before delivery (or use an atomic server-side precondition if available), and reject when it no longer equals --expect-entry.

	if err = sdk.Entries().CreateReply(ctx, target.EntryID, message, target.Addressed.To, target.Addressed.CC, target.Addressed.BCC); err != nil {

[code-monger-givenall]

One useful follow-up from the latest review: HEY serves the same reply envelope at /entries/{id}/replies/new.json. I opened basecamp/hey-sdk#87 to model it as Entries().NewReply. The local combined build is already using that typed path, and a live read-only preview passed. I am leaving this PR on the released SDK for now rather than adding a fork-only module pin. Once #87 is merged and available here, I will replace the form parser and remove it.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/cmd/thread_reply.go:47

• This introduces a new HTML-scraping dependency for reply behavior. The project’s SDK boundary requires unsupported HEY operations to be added as typed SDK operations rather than parsing web pages, because form markup can change independently of the CLI/API contract. Please expose the prepared reply envelope through the HEY SDK (and server API if needed) and consume that typed response here instead of GetHTML/ParseReplyFormHTML.

	replyResp, err := sdk.GetHTML(ctx, fmt.Sprintf("/entries/%d/replies/new", topic.LatestEntry.Id))
	if err != nil {
		return nil, convertSDKError(err)
	}
	addressed, err := htmlutil.ParseReplyFormHTML(string(replyResp.Data))

internal/cmd/reply.go:371

• Replies whose normalized content is empty can never pass verification because this condition rejects them unconditionally. The command currently accepts non-empty inputs such as whitespace or <br>, so after HEY accepts one, it waits through every retry and reports an unverified send. Reject content that cannot be verified before CreateReply (while continuing to allow attachment-only replies), or use another unambiguous verification signal.

	expected := normalizedReplyContent(sentContent)
	actual := normalizedReplyContent(message.Content)
	return expected != "" && actual == expected