feat(push): pad push — user-authored instruction dispatch to agent sessions (IDEA-2544 Phase 1) (#1090)

* feat(push): add pad push <ref> -m vertical (IDEA-2544 Phase 1)

Self-addressed, human-to-harness dispatch over the existing watch-events
bus/stream: CLI -> POST .../items/{itemSlug}/push -> a new KindPush
Notification (carrying the generalized TargetUserID addressed-to field
KindAsk will later share) -> watchNotificationVisible delivers it back
to the pushing user's own connected monitor sessions. Transient,
fire-and-forget by design (no migration, no durable inbox) since
assignment already covers the durable-notification case and this is
meant to be the explicit, no-inference dispatch verb instead.

* docs(plugin): document the push notification contract (IDEA-2544 Phase 1)

Push is the one notification kind that IS an instruction rather than a
passive fact, so it gets its own lead bullet in the plugin skill's
notification-etiquette section (ahead of the read-only/park default,
which it explicitly lifts) and a mention in the monitor's description.
The embed-source skills/pad/SKILL.md has no notification section to
mirror this into (the two files diverge by design) and is left
untouched.

* fix(push): reject over-long push messages instead of unbounded Summary

Comments truncate their notification Summary to a preview (the full
body is still fetchable), but a push message IS the payload — silently
truncating it would corrupt the instruction with nothing to recover it
from. Add maxPushMessageLen (4096, measured post-collapse) and reject
anything over it with a 400 rather than truncating; state the same
bound in `pad push --help` so it's discoverable before a 400, not only
from one.

* fix(push): close the watch-fallthrough leak, disambiguate SKILL.md exceptions

Codex round 1 P1: watchNotificationVisible's push branch only returned
early on a MATCH — a non-target caller fell through to the watch-map
check below it, so anyone holding an unconditional (or predicated)
watch on the item received every push addressed to every OTHER user,
instruction text included. Push is addressed private dispatch, not an
item-level fact watchers have a legitimate claim on (unlike assignment,
which watchers are expected to see per `pad watch --help`) — the branch
now returns unconditionally for KindPush, gating strictly on
TargetUserID and never reaching the watch-map fallback either way.
Pinned explicitly since Phase 4's session targeting is expected to
inherit this same exclusivity.

Also (codex P2): reworded the SKILL.md notification-etiquette bullets —
the new push exception and the pre-existing assignment/ask exception
literally contradicted each other ("the ONE narrow exception" claimed
singularity after push had already claimed exception status). Now
explicitly enumerated as the first and second exceptions to the
never-write rule.

* test(cli): pin that PushItem inherits X-Pad-Agent (BUG-2542 rebase)

Verified, not assumed: PushItem builds its request via c.post ->
c.newRequest like every other mutating client method (CreateWatch
included), so the attribution fix's client.agentName wiring covers it
for free with zero code changes needed on this branch. Adds a live
httptest assertion rather than trusting the code-path read alone —
the same shape as TestClientSendsResolvedAgentHeader, scoped to
PushItem specifically since that's the one method this PR added.

* fix(push): disambiguate workspace in the monitor line and skill contract

Codex round 2 P1: the watch-events stream is user-scoped ACROSS every
workspace a caller has watches in, but formatMonitorLine printed only
ItemRef/Kind/Actor/Summary and dropped the Workspace field the wire
payload already carried — a session linked to workspace A receiving a
notification for workspace B would resolve the wrong item (or 404) with
no signal in the line that anything was off.

Fixed universally, not push-only: grepped plugin/ and skills/ for
anything parsing "PAD ..." lines and found none — the Claude Code
plugin host ingests the stdout line as free-text notification prose,
formatMonitorLine's only real consumer is its own fmt.Println, so there
is no wire-format consumer a workspace prefix could break. The
ambiguity predates push (any watched item across workspaces already had
it); push just makes the consequence sharper because it carries an
instruction rather than a passive fact.

SKILL.md's push bullet now tells the agent to resolve with
`pad --workspace <workspace> item show <ref>` using the slug read off
the notification line, not a bare `pad item show <ref>`.

* fix(push): respect --format json instead of hardcoding plain text

Codex round 2 P2: pushCmd's RunE ignored the global format flag and
always printed "Pushed <ref>", silently discarding --format json.

- server.pushResponse replaces the bare map the handler wrote before —
  a typed {ref, workspace, pushed, message} shape, with workspace
  resolved to the CANONICAL slug via s.getWorkspace (not merely echoed
  from whatever the URL contained), matching the same disambiguation
  need the round-2 P1 fix addressed for the monitor line.
- cli.PushItem now returns (*PushResult, error) instead of discarding
  the response body.
- pushCmd checks formatFlag == "json" and calls cli.PrintJSON, mirroring
  runCreateWatch's existing pattern.

internal/cli/agent_identity_test.go's TestPushItemSendsResolvedAgentHeader
needed a one-line update for PushItem's new two-value return — caught by
`go vet ./...`, not `go build ./...` (which doesn't compile test files);
folding vet into my own pre-flight going forward.
This commit is contained in:
xarmian
2026-08-13 18:41:46 -04:00
committed by GitHub
parent f9195c5b09
commit da6ce642da
16 changed files with 832 additions and 13 deletions
+76
View File
@@ -0,0 +1,76 @@
package main
import (
"fmt"
"strings"
"github.com/spf13/cobra"
"github.com/PerpetualSoftware/pad/internal/cli"
)
// maxPushMessageLenForHelp mirrors server.maxPushMessageLen
// (handlers_push.go) — KEEP IN SYNC. Duplicated rather than shared
// because the two live in different packages with no existing shared
// constants package for a single value; it exists here purely so
// `pad push --help` states the bound instead of a user only learning it
// from a 400. The server is still the enforcing source of truth — this
// value is advisory (help text), not validated client-side.
const maxPushMessageLenForHelp = 4096
// pushCmd is `pad push <ref> -m "message"` (IDEA-2544 Phase 1) — the
// "push this to my agent" verb: an explicit, user-authored instruction
// bound to an item, delivered to the caller's own connected monitor
// sessions over the same watch-events stream `pad watch --stream
// --for-session` consumes. Distinct from `pad watch` (a durable, item-
// scoped subscription): a push is one-shot, transient, and IS an
// instruction rather than a passive fact — see the plugin skill's
// notification-etiquette section for the receiving-agent behavior
// contract.
func pushCmd() *cobra.Command {
var message string
cmd := &cobra.Command{
Use: "push <ref>",
Short: "Push an item + instruction to your own connected agent session(s)",
Long: fmt.Sprintf(`pad push <ref> -m "message"
Publish a self-addressed push notification on an item. Every one of
your OWN connected plugin-monitor sessions (pad watch --stream
--for-session) receives it this is fire-and-forget over the
in-memory watch-events bus, with no durable inbox: a push with no
connected session listening is simply not seen (Phase 1 scope).
-m/--message is required and must not be blank; it is the
instruction text the receiving agent acts on (load the item first,
then do what the message says see the plugin skill's notification
section for the full contract). Newlines are collapsed to spaces:
the monitor stream is a one-line-per-event wire contract. Limited to
%d characters after that collapse the server rejects anything
longer rather than truncating it.`, maxPushMessageLenForHelp),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
trimmed := strings.Join(strings.Fields(message), " ")
if trimmed == "" {
return fmt.Errorf("-m/--message is required and must not be blank")
}
client, _ := getClient()
ws := getWorkspace()
result, err := client.PushItem(ws, args[0], trimmed)
if err != nil {
return err
}
if formatFlag == "json" {
return cli.PrintJSON(result)
}
fmt.Printf("Pushed %s\n", args[0])
return nil
},
}
cmd.Flags().StringVarP(&message, "message", "m", "",
fmt.Sprintf("instruction text to push (required, max %d characters after whitespace collapse)", maxPushMessageLenForHelp))
return cmd
}
+138
View File
@@ -0,0 +1,138 @@
package main
import (
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
)
// setupPushTest mirrors setupItemOpenTest (item_open_test.go): points
// getClient()/getWorkspace() at a fake httptest server via the same
// urlFlag/workspaceFlag override the real CLI entry points read.
func setupPushTest(t *testing.T, handler http.Handler) *httptest.Server {
t.Helper()
server := httptest.NewServer(handler)
t.Cleanup(server.Close)
t.Setenv("HOME", t.TempDir())
previousWorkspace := workspaceFlag
previousURL := urlFlag
workspaceFlag = "demo"
urlFlag = server.URL + "/"
t.Cleanup(func() {
workspaceFlag = previousWorkspace
urlFlag = previousURL
})
return server
}
// TestPushCmd_SendsCollapsedMessage covers the client round trip: the
// CLI posts the trimmed, newline-collapsed message to the item's /push
// endpoint and reports success.
func TestPushCmd_SendsCollapsedMessage(t *testing.T) {
var gotPath string
var gotBody map[string]string
setupPushTest(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotPath = r.URL.Path
_ = json.NewDecoder(r.Body).Decode(&gotBody)
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(map[string]any{"ref": "TASK-5", "pushed": true})
}))
var output bytes.Buffer
cmd := pushCmd()
cmd.SetOut(&output)
cmd.SetArgs([]string{"TASK-5", "-m", "triage this\nwith the triage playbook"})
if err := cmd.Execute(); err != nil {
t.Fatalf("execute push: %v", err)
}
wantPath := "/api/v1/workspaces/demo/items/TASK-5/push"
if gotPath != wantPath {
t.Fatalf("path = %q, want %q", gotPath, wantPath)
}
wantMessage := "triage this with the triage playbook"
if gotBody["message"] != wantMessage {
t.Fatalf("message = %q, want %q", gotBody["message"], wantMessage)
}
}
// TestPushCmd_FormatJSON covers the P2 finding (dispatcher review round
// 2, codex): --format json was silently ignored — the RunE hardcoded
// plain text regardless of formatFlag. Mirrors runCreateWatch's
// formatFlag == "json" branch (cmd_watch.go) and asserts the full
// PushResult shape (ref, workspace, pushed, message), not just that
// SOME JSON came out.
func TestPushCmd_FormatJSON(t *testing.T) {
setupPushTest(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(map[string]any{
"ref": "TASK-5", "workspace": "demo", "pushed": true, "message": "triage this",
})
}))
prevFormat := formatFlag
formatFlag = "json"
defer func() { formatFlag = prevFormat }()
cmd := pushCmd()
cmd.SetArgs([]string{"TASK-5", "-m", "triage this"})
var execErr error
out := captureStdout(t, func() { execErr = cmd.Execute() })
if execErr != nil {
t.Fatalf("execute push: %v", execErr)
}
var result PushResultForTest
if err := json.Unmarshal([]byte(out), &result); err != nil {
t.Fatalf("output is not valid JSON: %v\noutput: %s", err, out)
}
if result.Ref != "TASK-5" || result.Workspace != "demo" || !result.Pushed || result.Message != "triage this" {
t.Fatalf("unexpected JSON result: %+v", result)
}
}
// PushResultForTest mirrors cli.PushResult's wire shape — cmd/pad
// doesn't import internal/cli's type back out for a test-local decode,
// so this pins the same JSON tags independently (a divergence here
// would fail this test, which is the point).
type PushResultForTest struct {
Ref string `json:"ref"`
Workspace string `json:"workspace"`
Pushed bool `json:"pushed"`
Message string `json:"message"`
}
// TestPushCmd_RequiresNonBlankMessage covers the client-side guard: an
// absent or whitespace-only -m never reaches the server at all.
func TestPushCmd_RequiresNonBlankMessage(t *testing.T) {
called := false
setupPushTest(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
called = true
w.WriteHeader(http.StatusOK)
}))
for _, args := range [][]string{
{"TASK-5"},
{"TASK-5", "-m", ""},
{"TASK-5", "-m", " "},
{"TASK-5", "-m", "\n\t "},
} {
called = false
cmd := pushCmd()
cmd.SetArgs(args)
err := cmd.Execute()
if err == nil {
t.Fatalf("args %v: expected an error for a blank message, got nil", args)
}
if called {
t.Fatalf("args %v: expected no HTTP call for a blank message", args)
}
}
}
+21 -3
View File
@@ -69,8 +69,8 @@ type watchStreamPayload struct {
// formatMonitorLine renders one notification as the exact stdout-line
// contract `pad watch --stream --for-session` promises (DOC-2479):
// "PAD TASK-214 → done (Dave): fix verified" — one line, facts only, no
// etiquette prose (that lives in the plugin skill per DR-4).
// "PAD demo/TASK-214 → done (Dave): fix verified" — one line, facts
// only, no etiquette prose (that lives in the plugin skill per DR-4).
//
// TASK-2533 plan interpretation: DOC-2479's example names a target
// STATUS VALUE and reads as though it combines two facts (a status
@@ -83,8 +83,26 @@ type watchStreamPayload struct {
// (not a status value) as the arrow's target — it's the one thing every
// kind (status-change / assignment / comment / the reserved ask) can
// supply uniformly from the actual payload fields.
//
// The workspace slug prefix (IDEA-2544 Phase 1, dispatcher review round
// 2, codex P1) is UNIVERSAL — every kind, not push-specific — because
// the ambiguity it closes predates push: GET /api/v1/events/stream is
// user-scoped ACROSS every workspace the caller belongs to (a watch is
// personal, not workspace-scoped — see Store.ListWatchesForUser), so
// ANY kind can arrive for a workspace other than the one linked in the
// caller's cwd, not just push. The payload already carried Workspace
// (DOC-2479's wire contract); it was simply never rendered. Dropping it
// silently means a session linked to workspace A that receives a
// notification for workspace B resolves the wrong item (or 404s) with
// no signal in the line that anything was off. Safe to change
// universally: formatMonitorLine's only consumer is this file's own
// fmt.Println (grepped plugin/ and skills/ for anything else parsing
// "PAD ..." lines — there is none; the Claude Code plugin host ingests
// the stdout line as free-text session-notification prose, not a
// structured format any code parses), so there is no wire-format
// consumer to break by adding a field.
func formatMonitorLine(p watchStreamPayload) string {
return fmt.Sprintf("PAD %s → %s (%s): %s", p.ItemRef, p.Kind, p.Actor, p.Summary)
return fmt.Sprintf("PAD %s/%s → %s (%s): %s", p.Workspace, p.ItemRef, p.Kind, p.Actor, p.Summary)
}
// sleepOrDone waits for d or ctx cancellation, whichever comes first.
+16 -6
View File
@@ -67,18 +67,28 @@ func TestFormatMonitorLine(t *testing.T) {
}{
{
name: "status change",
in: watchStreamPayload{ItemRef: "TASK-214", Kind: "status-change", Actor: "Dave", Summary: "open → done"},
want: "PAD TASK-214 → status-change (Dave): open → done",
in: watchStreamPayload{Workspace: "demo", ItemRef: "TASK-214", Kind: "status-change", Actor: "Dave", Summary: "open → done"},
want: "PAD demo/TASK-214 → status-change (Dave): open → done",
},
{
name: "assignment",
in: watchStreamPayload{ItemRef: "BUG-5", Kind: "assignment", Actor: "Alice", Summary: "assigned to Alice"},
want: "PAD BUG-5 → assignment (Alice): assigned to Alice",
in: watchStreamPayload{Workspace: "demo", ItemRef: "BUG-5", Kind: "assignment", Actor: "Alice", Summary: "assigned to Alice"},
want: "PAD demo/BUG-5 → assignment (Alice): assigned to Alice",
},
{
name: "comment",
in: watchStreamPayload{ItemRef: "TASK-1", Kind: "comment", Actor: "Bob", Summary: "fix verified"},
want: "PAD TASK-1 → comment (Bob): fix verified",
in: watchStreamPayload{Workspace: "demo", ItemRef: "TASK-1", Kind: "comment", Actor: "Bob", Summary: "fix verified"},
want: "PAD demo/TASK-1 → comment (Bob): fix verified",
},
{
// IDEA-2544 Phase 1, dispatcher review round 2 (codex P1): the
// workspace prefix matters most here — push carries an
// instruction, so a caller resolving it against the wrong
// linked workspace is a worse failure mode than for a passive
// fact.
name: "push, different workspace than the item ref alone would suggest",
in: watchStreamPayload{Workspace: "other-workspace", ItemRef: "TASK-9", Kind: "push", Actor: "Dave", Summary: "triage this with the triage playbook"},
want: "PAD other-workspace/TASK-9 → push (Dave): triage this with the triage playbook",
},
}
for _, c := range cases {
+1
View File
@@ -148,6 +148,7 @@ func newRootCmd() *cobra.Command {
bootstrapCmd(),
playbookCmd(),
watchCmdGroup(),
pushCmd(),
sessionCmd(),
)
+38
View File
@@ -135,6 +135,44 @@ func TestClientSendsResolvedAgentHeader(t *testing.T) {
}
}
// TestPushItemSendsResolvedAgentHeader (IDEA-2544 Phase 1, dispatcher
// review) pins that PushItem inherits X-Pad-Agent the same as every
// other mutating client method, rather than assuming: PushItem doesn't
// build its own request, it goes through c.post -> c.newRequest like
// CreateWatch and everything else, so the BUG-2542 fix upgraded it for
// free — but "the code path implies it" and "verified against a live
// request" are different claims, and only the second belongs in a
// report. Also confirms the server side of the loop: handlePushToItem's
// actor/actorName come from actorFromRequest/actorNameFromRequest,
// which read this exact header.
func TestPushItemSendsResolvedAgentHeader(t *testing.T) {
chdir(t, t.TempDir())
for _, k := range []string{"PAD_AGENT", "CLAUDECODE"} {
t.Setenv(k, "")
os.Unsetenv(k)
}
t.Setenv("CLAUDECODE", "1")
var got string
var seen bool
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got, seen = r.Header.Get("X-Pad-Agent"), true
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"ref":"TASK-1","pushed":true}`))
}))
defer srv.Close()
if _, err := NewClientFromURL(srv.URL).PushItem("demo", "TASK-1", "triage this"); err != nil {
t.Fatalf("PushItem: %v", err)
}
if !seen {
t.Fatal("server never saw the request")
}
if got != "claude-code" {
t.Errorf("X-Pad-Agent = %q, want %q", got, "claude-code")
}
}
// ActorKind backs the one place the client must self-describe: structured
// entries inside an item's fields JSON, which the server never parses. An
// earlier revision of the BUG-2542 fix removed the CLI's hardcoded "user"
+23
View File
@@ -334,6 +334,29 @@ func (c *Client) DeleteWatch(wsSlug, itemSlug string) error {
return c.delete("/workspaces/" + wsSlug + "/items/" + itemSlug + "/watch")
}
// PushResult is the response body of a successful pad push, mirroring
// server.pushResponse's wire shape. Workspace is the CANONICAL slug the
// server resolved the push against (dispatcher review round 2, codex
// P1/P2) — a JSON consumer needs it because the notification stream is
// user-scoped across every workspace the caller belongs to, not just
// the one this call happened to target.
type PushResult struct {
Ref string `json:"ref"`
Workspace string `json:"workspace"`
Pushed bool `json:"pushed"`
Message string `json:"message"`
}
// PushItem publishes a self-addressed push notification (IDEA-2544
// Phase 1) on an item, over the same watch-events bus/stream `pad watch
// --stream --for-session` consumes. Transient, fire-and-forget — see
// server.handlePushToItem's doc comment for the no-durability rationale.
func (c *Client) PushItem(wsSlug, itemSlug, message string) (*PushResult, error) {
body := map[string]string{"message": message}
var result PushResult
return &result, c.post("/workspaces/"+wsSlug+"/items/"+itemSlug+"/push", body, &result)
}
// ListWatches returns every watch the current user holds, across all
// workspaces they belong to (TASK-2533 — a watch is personal, not
// workspace-scoped; see Store.ListWatchesForUser's doc comment).
+151
View File
@@ -0,0 +1,151 @@
package server
import (
"fmt"
"net/http"
"strings"
"github.com/go-chi/chi/v5"
"github.com/PerpetualSoftware/pad/internal/watchevents"
)
// pushRequest is the body of POST .../items/{itemSlug}/push.
type pushRequest struct {
Message string `json:"message"`
}
// pushResponse is the body of a successful push (dispatcher review round
// 2, codex P2: `pad push --format json` needs a real shape, not a
// discarded response). Workspace is included per the round-2 P1 fix's
// same rationale — the stream is user-scoped across every workspace the
// caller belongs to, so a JSON consumer needs it disambiguated same as
// the monitor line does.
type pushResponse struct {
Ref string `json:"ref"`
Workspace string `json:"workspace"`
Pushed bool `json:"pushed"`
Message string `json:"message"`
}
// maxPushMessageLen bounds a push's instruction text, measured in runes
// AFTER whitespace collapse (dispatcher review round 1). Two
// constraints in tension set this: a push message is a free-form
// instruction, not a short label — truncating one the way
// truncateForSummary shortens a comment preview would silently corrupt
// what the user actually asked for, and unlike a comment (whose full
// body is still fetchable via `pad item show`), a push has no
// persistence to recover the untruncated text from (see
// handlePushToItem's doc comment). But Notification.Summary rides a
// single stdout line into a plugin monitor / terminal session (`pad
// watch --stream --for-session`'s one-line-per-event wire contract), so
// it can't be unbounded either. 4096 runes gives several paragraphs of
// headroom — comfortably more than any reasonable single instruction —
// while keeping that one line a sane size; a message over the cap is
// rejected with a 400 rather than silently truncated.
const maxPushMessageLen = 4096
// handlePushToItem publishes a self-addressed watchevents.KindPush
// notification (IDEA-2544 Phase 1) — the "push this to my agent" verb:
// an explicit, user-authored instruction bound to an item, delivered to
// every one of the pushing user's OWN connected monitor sessions via
// GET /api/v1/events/stream. Unlike watch/assignment notifications,
// this has no durable backing (Dave's product call: fire-and-forget is
// acceptable for v1 — no inbox, no "no session connected" warning; the
// bus's replay buffer is the only resilience a push gets).
//
// Self-addressed only: pushing into someone else's session is a consent
// question, not a code question (IDEA-2544 plan), so TargetUserID is
// always set to the CALLER's own ID, never a request-supplied target —
// there is no cross-user push in Phase 1.
//
// POST /api/v1/workspaces/{slug}/items/{itemSlug}/push
func (s *Server) handlePushToItem(w http.ResponseWriter, r *http.Request) {
if s.watchEvents == nil {
// Unlike handleCreateWatch (a durable store write that works fine
// without the bus), a push has NO persistence to fall back on —
// if there's no bus, the message is unrecoverably lost. Fail
// loudly here rather than returning 200 for an instruction that
// silently went nowhere.
writeError(w, http.StatusServiceUnavailable, "unavailable", "Push is not available")
return
}
// Full workspace object, not just the ID (getWorkspaceID), so the
// response can echo the CANONICAL slug — the caller may have passed
// an ID in the URL, and pushResponse.Workspace exists specifically
// to disambiguate which workspace a JSON consumer should resolve the
// ref against (same rationale as the monitor line's workspace
// prefix), so it needs to be the real slug, not an echo of whatever
// the URL happened to contain.
ws, ok := s.getWorkspace(w, r)
if !ok {
return
}
workspaceID := ws.ID
itemSlug := chi.URLParam(r, "itemSlug")
item, err := s.store.ResolveItem(workspaceID, itemSlug)
if err != nil {
writeInternalError(w, err)
return
}
if item == nil {
s.writeItemResolveError(w, r, workspaceID, itemSlug)
return
}
if !s.requireItemVisible(w, r, workspaceID, item) {
return
}
userID := currentUserID(r)
if userID == "" {
writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required")
return
}
var input pushRequest
if err := decodeJSON(r, &input); err != nil {
writeError(w, http.StatusBadRequest, "bad_request", err.Error())
return
}
// Collapse newlines to spaces, matching truncateForSummary's
// rationale in handlers_comments.go: Notification.Summary is a
// single-line wire contract (`pad watch --stream --for-session`
// prints exactly one stdout line per event). Trimmed BEFORE the
// empty check so a whitespace-only -m (" ") is rejected the same
// as a genuinely empty one, rather than publishing a blank
// instruction.
message := strings.Join(strings.Fields(input.Message), " ")
if message == "" {
writeError(w, http.StatusBadRequest, "bad_request", "message must not be empty")
return
}
if length := len([]rune(message)); length > maxPushMessageLen {
writeError(w, http.StatusBadRequest, "bad_request",
fmt.Sprintf("message must be %d characters or fewer after whitespace collapse (got %d)", maxPushMessageLen, length))
return
}
actor, _ := actorFromRequest(r)
actorName := actorNameFromRequest(r)
s.watchEvents.Publish(watchevents.Notification{
WorkspaceID: workspaceID,
ItemID: item.ID,
CollectionID: item.CollectionID,
ItemRef: item.Ref,
Kind: watchevents.KindPush,
Actor: actor,
ActorName: actorName,
Summary: message,
TargetUserID: userID,
})
writeJSON(w, http.StatusOK, pushResponse{
Ref: item.Ref,
Workspace: ws.Slug,
Pushed: true,
Message: message,
})
}
+193
View File
@@ -0,0 +1,193 @@
package server
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"github.com/PerpetualSoftware/pad/internal/watchevents"
)
// TestPushToItem_RequiresMessage covers the 400-on-empty(-after-trim)
// validation: an absent message and a whitespace-only one are both
// rejected, so a blank push instruction can never reach the bus.
func TestPushToItem_RequiresMessage(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, item, tok, _ := setupWatchTestUser(t, srv)
for _, body := range [][]byte{
nil,
[]byte(`{}`),
[]byte(`{"message":""}`),
[]byte(`{"message":" "}`),
[]byte(`{"message":"\n\t "}`),
} {
rr := bearerCall(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token, body)
if rr.Code != http.StatusBadRequest {
t.Fatalf("body %q: expected 400, got %d (body: %s)", body, rr.Code, rr.Body.String())
}
}
}
// TestPushToItem_ResponseCarriesWorkspaceSlug covers the P2 finding
// (dispatcher review round 2, codex): a successful push's JSON response
// must carry ref/workspace/pushed/message so `pad push --format json`
// has something real to print, and workspace must be the CANONICAL
// slug (not merely whatever the URL happened to contain) — same
// disambiguation rationale as the monitor line's workspace prefix.
func TestPushToItem_ResponseCarriesWorkspaceSlug(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, item, tok, _ := setupWatchTestUser(t, srv)
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token,
map[string]interface{}{"message": "triage this"})
if rr.Code != http.StatusOK {
t.Fatalf("push: %d %s", rr.Code, rr.Body.String())
}
var resp pushResponse
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatalf("parse response: %v", err)
}
if resp.Ref != item.Ref {
t.Fatalf("expected ref %q, got %q", item.Ref, resp.Ref)
}
if resp.Workspace != slug {
t.Fatalf("expected workspace %q, got %q", slug, resp.Workspace)
}
if !resp.Pushed {
t.Fatal("expected pushed: true")
}
if resp.Message != "triage this" {
t.Fatalf("expected message %q, got %q", "triage this", resp.Message)
}
}
// TestPushToItem_RejectsOverLongMessage covers the 400-over-cap
// validation (dispatcher review round 1): a message whose COLLAPSED
// length exceeds maxPushMessageLen is rejected rather than silently
// truncated — the message is the payload, not a preview, so truncation
// would corrupt what the user asked for. Built with extra internal
// whitespace so this also pins that the cap is measured AFTER
// whitespace collapse, not before (a message that's only over-length
// due to now-collapsed whitespace must NOT be rejected).
func TestPushToItem_RejectsOverLongMessage(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, item, tok, _ := setupWatchTestUser(t, srv)
overLong := strings.Repeat("a \n ", maxPushMessageLen) // collapses to > maxPushMessageLen chars
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token,
map[string]interface{}{"message": overLong})
if rr.Code != http.StatusBadRequest {
t.Fatalf("expected 400 for an over-cap message, got %d (body: %s)", rr.Code, rr.Body.String())
}
}
// TestPushToItem_MessageAtCapSurvivesIntact proves a message whose
// COLLAPSED length is exactly at maxPushMessageLen is accepted and
// delivered byte-for-byte (not silently truncated at the boundary) —
// the counterpart to the over-cap rejection test above.
func TestPushToItem_MessageAtCapSurvivesIntact(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, item, tok, _ := setupWatchTestUser(t, srv)
ts := httptest.NewServer(srv)
defer ts.Close()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ch := connectWatchStream(ctx, t, ts.URL, tok.Token)
waitForWatchEvent(t, ch, 3*time.Second) // connected
atCap := strings.Repeat("a", maxPushMessageLen) // already whitespace-free: collapse is a no-op
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token,
map[string]interface{}{"message": atCap})
if rr.Code != http.StatusOK {
t.Fatalf("expected 200 for a message exactly at the cap, got %d (body: %s)", rr.Code, rr.Body.String())
}
ev := waitForWatchEvent(t, ch, 3*time.Second)
var payload watchEventPayload
if err := json.Unmarshal([]byte(ev.Data), &payload); err != nil {
t.Fatalf("parse payload: %v", err)
}
if payload.Summary != atCap {
t.Fatalf("expected the at-cap message to survive collapse+publish intact (len %d), got len %d", len(atCap), len(payload.Summary))
}
}
// TestPushToItem_UnavailableWithoutBus covers the 503-on-nil-bus guard:
// unlike a durable watch, a push has no persistence to fall back on, so
// a missing bus must fail loudly rather than silently losing the
// message.
func TestPushToItem_UnavailableWithoutBus(t *testing.T) {
t.Parallel()
srv := testServer(t) // NOT testServerWithWatchEvents — bus is nil
slug, item, tok, _ := setupWatchTestUser(t, srv)
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token,
map[string]interface{}{"message": "triage this"})
if rr.Code != http.StatusServiceUnavailable {
t.Fatalf("expected 503, got %d (body: %s)", rr.Code, rr.Body.String())
}
}
// TestPushToItem_DeliversSelfAddressed mirrors
// TestWatchEventsStream_AddressedToYou_Assignment: a push must reach the
// SAME user's own connected stream with no explicit watch required, and
// carry the collapsed message as Summary.
func TestPushToItem_DeliversSelfAddressed(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, item, tok, _ := setupWatchTestUser(t, srv)
ts := httptest.NewServer(srv)
defer ts.Close()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ch := connectWatchStream(ctx, t, ts.URL, tok.Token)
waitForWatchEvent(t, ch, 3*time.Second) // connected
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/"+item.Slug+"/push", tok.Token,
map[string]interface{}{"message": "triage this\nwith the triage playbook"})
if rr.Code != http.StatusOK {
t.Fatalf("push: %d %s", rr.Code, rr.Body.String())
}
ev := waitForWatchEvent(t, ch, 3*time.Second)
var payload watchEventPayload
if err := json.Unmarshal([]byte(ev.Data), &payload); err != nil {
t.Fatalf("parse payload: %v", err)
}
if payload.Kind != watchevents.KindPush {
t.Fatalf("expected kind %q, got %q", watchevents.KindPush, payload.Kind)
}
if payload.ItemRef != item.Ref {
t.Fatalf("expected item_ref %q, got %q", item.Ref, payload.ItemRef)
}
if payload.Summary != "triage this with the triage playbook" {
t.Fatalf("expected newline-collapsed summary, got %q", payload.Summary)
}
}
// TestPushToItem_InvisibleItemDenied covers the same resolve/visibility
// gate every other item-scoped handler enforces: pushing a nonexistent
// ref 404s.
func TestPushToItem_InvisibleItemDenied(t *testing.T) {
t.Parallel()
srv := testServerWithWatchEvents(t)
slug, _, tok, _ := setupWatchTestUser(t, srv)
rr := bearerJSON(t, srv, "POST", "/api/v1/workspaces/"+slug+"/items/NOPE-999/push", tok.Token,
map[string]interface{}{"message": "triage this"})
if rr.Code != http.StatusNotFound {
t.Fatalf("expected 404, got %d (body: %s)", rr.Code, rr.Body.String())
}
}
+24
View File
@@ -405,6 +405,30 @@ func watchNotificationVisible(watches map[string]string, vis watchAccessVisibili
return true
}
// Push (IDEA-2544 Phase 1) is semantically DIFFERENT from assignment,
// not just a same-shaped variant, and that difference is why this
// branch RETURNS here — for either outcome — instead of falling
// through like the assignment branch above does. Assignment is an
// item-level event: it's ordinary, expected behavior for someone who
// already holds an unconditional watch on the item to also see who
// it got assigned to (`pad watch --help` promises exactly that — any
// notification on a watched item fires). A push is addressed PRIVATE
// dispatch: one specific user putting an item + instruction in front
// of their OWN session, not a fact about the item that anyone
// watching it has a claim on. Watching an item must not leak a push
// someone else addressed to themselves — that's the whole point of
// TargetUserID gating it (dispatcher review round 2, codex P1: the
// original code fell through to the watch-map check on a
// non-matching TargetUserID, so any unconditional watcher on the item
// received every push addressed to every other user, instruction
// text included). Phase 4's session targeting is expected to inherit
// this same "addressed traffic is exclusive of watch-matched
// delivery" semantics, so it's pinned explicitly here rather than
// left to fall out of the shared TargetUserID field's shape.
if n.Kind == watchevents.KindPush {
return n.TargetUserID != "" && n.TargetUserID == userID
}
predicate, watched := watches[n.ItemID]
if !watched {
return false
@@ -571,6 +571,88 @@ func TestWatchNotificationVisible_AssignmentToSomeoneElseDenied(t *testing.T) {
}
}
// TestWatchNotificationVisible_PushToYou mirrors
// TestWatchNotificationVisible_AddressedToYouIgnoresWatchList for
// KindPush (IDEA-2544 Phase 1): a push fires purely off TargetUserID,
// independent of any watch.
func TestWatchNotificationVisible_PushToYou(t *testing.T) {
t.Parallel()
watches := map[string]string{} // no watches at all
n := watchevents.Notification{ItemID: "item-1", Kind: watchevents.KindPush, TargetUserID: "user-1"}
if !watchNotificationVisible(watches, watchAccessVisibility{fullAccess: true}, "user-1", n) {
t.Fatal("expected a push-to-you notification to be visible with no watch")
}
}
// TestWatchNotificationVisible_PushToSomeoneElseDenied mirrors
// TestWatchNotificationVisible_AssignmentToSomeoneElseDenied for
// KindPush: Phase 1 only ever publishes self-addressed pushes, but the
// delivery rule itself must still deny a push addressed to a different
// user, exactly like the assignment branch.
func TestWatchNotificationVisible_PushToSomeoneElseDenied(t *testing.T) {
t.Parallel()
watches := map[string]string{}
n := watchevents.Notification{ItemID: "item-1", Kind: watchevents.KindPush, TargetUserID: "user-2"}
if watchNotificationVisible(watches, watchAccessVisibility{fullAccess: true}, "user-1", n) {
t.Fatal("expected a push addressed to someone else to be denied")
}
}
// TestWatchNotificationVisible_PushToSomeoneElseDeniedEvenWithUnconditionalWatch
// covers codex round 1's P1 finding: the original push branch returned
// true only on a match and otherwise FELL THROUGH to the watch-map
// check below it — so a non-target caller holding an unconditional
// watch on the item still received the push (instruction text
// included), because an unconditional watch matches "any notification
// on this item". TestWatchNotificationVisible_PushToSomeoneElseDenied
// used an EMPTY watch map, which never exercised that fall-through path
// at all. Push must be exclusive of watch-matched delivery: watching an
// item grants no claim on private dispatch someone else addressed to
// their own session — see the doc comment on the push branch in
// watchNotificationVisible.
func TestWatchNotificationVisible_PushToSomeoneElseDeniedEvenWithUnconditionalWatch(t *testing.T) {
t.Parallel()
watches := map[string]string{"item-1": ""} // unconditional watch on the item
n := watchevents.Notification{ItemID: "item-1", Kind: watchevents.KindPush, TargetUserID: "user-2"}
if watchNotificationVisible(watches, watchAccessVisibility{fullAccess: true}, "user-1", n) {
t.Fatal("expected a push addressed to someone else to be denied even when the caller holds an unconditional watch on the item")
}
}
// TestWatchNotificationVisible_PushToSomeoneElseDeniedEvenWithPredicateWatch
// is the predicated-watch variant of the same fall-through blind spot —
// same shape, a non-empty predicate instead of an unconditional watch.
func TestWatchNotificationVisible_PushToSomeoneElseDeniedEvenWithPredicateWatch(t *testing.T) {
t.Parallel()
watches := map[string]string{"item-1": "status=done"}
n := watchevents.Notification{ItemID: "item-1", Kind: watchevents.KindPush, TargetUserID: "user-2"}
if watchNotificationVisible(watches, watchAccessVisibility{fullAccess: true}, "user-1", n) {
t.Fatal("expected a push addressed to someone else to be denied even when the caller holds a predicated watch on the item")
}
}
// TestWatchNotificationVisible_PushStillGatedByAccess mirrors
// TestWatchNotificationVisible_AddressedToYouStillGatedByAccess: the
// SAME current-access check that gates every other kind must also gate
// a push-to-you notification, not just the assignment branch.
func TestWatchNotificationVisible_PushStillGatedByAccess(t *testing.T) {
t.Parallel()
watches := map[string]string{}
deny := watchAccessVisibility{} // zero value: nothing visible
n := watchevents.Notification{
ItemID: "item-1", CollectionID: "coll-1",
Kind: watchevents.KindPush, TargetUserID: "user-1",
}
if watchNotificationVisible(watches, deny, "user-1", n) {
t.Fatal("expected a push-to-you notification to be denied when the caller has no current access to the item's collection")
}
allow := watchAccessVisibility{visibleCollIDs: map[string]bool{"coll-1": true}}
if !watchNotificationVisible(watches, allow, "user-1", n) {
t.Fatal("expected a push-to-you notification to be visible once the caller has current access to the item's collection")
}
}
// TestWatchNotificationVisible_AddressedToYouStillGatedByAccess covers
// TASK-2533 codex round 2 finding 2: the addressed-to-you branch used to
// return true unconditionally, with NO access check — an item assigned
+5
View File
@@ -1561,6 +1561,11 @@ func (s *Server) setupRouter() {
// pipeline. `pad watch <ref>` / `pad watch remove <ref>`.
r.Post("/watch", s.handleCreateWatch)
r.Delete("/watch", s.handleDeleteWatch)
// Push (IDEA-2544 Phase 1): transient, self-addressed
// human→harness dispatch over the SAME watch-events
// bus/stream — no durable row, see handlePushToItem's
// doc comment. `pad push <ref> -m "message"`.
r.Post("/push", s.handlePushToItem)
})
// Links (v2)
+24
View File
@@ -42,6 +42,15 @@ const (
// enum so the wire contract doesn't have to change when a producer
// eventually exists.
KindAsk = "ask"
// KindPush is IDEA-2544 Phase 1's human→harness addressed-dispatch
// event: POST .../items/{itemSlug}/push publishes exactly one of
// these, self-addressed (TargetUserID == the pushing user), any time
// a user wants to put an item + instruction in front of their own
// harness right now rather than waiting on assignment/watch
// semantics. KindAsk is push's reserved sibling in the other
// direction (harness→human) — see TargetUserID's doc comment for the
// shared envelope shape the two are meant to converge on.
KindPush = "push"
)
// Notification is one watch-worthy fact: an item's status changed, it was
@@ -74,6 +83,21 @@ type Notification struct {
// addressed-to-you check compares this directly against the
// connected caller's user ID — see server.sseWatchVisible.
AssignedUserID string
// TargetUserID is IDEA-2544's generalized addressed-to field:
// populated on Kind == KindPush with the user the push is addressed
// to (Phase 1 always sets this to the pushing user's own ID —
// self-addressed only, per Dave's product call that pushing into
// someone else's session is a consent question, not a code
// question). watchNotificationVisible compares this directly against
// the connected caller's user ID, exactly like AssignedUserID's role
// for KindAssignment. Reserved KindAsk is expected to share this same
// field (harness→human addressed traffic) rather than growing its
// own, once it has a producer. Deliberately NOT part of
// watchEventPayload's wire shape (server.go) — it exists purely to
// gate delivery server-side; a client never needs to know who else a
// notification could have been addressed to, and echoing it back
// would leak a user ID for no consumer that needs it.
TargetUserID string
// StatusFieldKey / ToStatus are populated on Kind == KindStatusChange,
// mirroring models.ItemMutationSignal, so the `--until field=value`
// watch predicate can be evaluated against a Notification directly
+35
View File
@@ -30,6 +30,41 @@ func TestMemoryBus_PublishDeliversToSubscriber(t *testing.T) {
}
}
// TestMemoryBus_PublishDeliversPushWithTargetUserID covers IDEA-2544
// Phase 1: KindPush and TargetUserID must round-trip through Publish
// like any other kind/field — the bus itself is kind-agnostic, but this
// pins that a new kind + a new addressed-to field don't need any bus
// changes to work, matching the plan's "zero formatter/bus changes"
// claim for a new kind.
func TestMemoryBus_PublishDeliversPushWithTargetUserID(t *testing.T) {
t.Parallel()
b := New()
defer b.Close()
ch := b.Subscribe()
defer b.Unsubscribe(ch)
b.Publish(Notification{
WorkspaceID: "ws1",
ItemID: "item1",
Kind: KindPush,
Summary: "triage this",
TargetUserID: "user-1",
})
select {
case n := <-ch:
if n.Kind != KindPush {
t.Fatalf("expected kind %q, got %q", KindPush, n.Kind)
}
if n.TargetUserID != "user-1" {
t.Fatalf("expected TargetUserID %q, got %q", "user-1", n.TargetUserID)
}
case <-time.After(time.Second):
t.Fatal("timed out waiting for notification")
}
}
func TestMemoryBus_UnsubscribeClosesChannel(t *testing.T) {
t.Parallel()
b := New()
+1 -1
View File
@@ -2,6 +2,6 @@
{
"name": "pad-events",
"command": "while ! pad watch --help >/dev/null 2>&1; do sleep 3600; done; exec pad watch --stream --for-session",
"description": "Pad item events: explicit subscriptions (pad watch <ref>) and items addressed to you (currently: assignment — ask-events are reserved in the wire contract but not yet emitted). Silent when the pad CLI isn't installed, no .pad.toml is present, or padd is unreachable."
"description": "Pad item events: explicit subscriptions (pad watch <ref>), items addressed to you (assignment, push — see pad push <ref> -m; ask-events are reserved in the wire contract but not yet emitted). Silent when the pad CLI isn't installed, no .pad.toml is present, or padd is unreachable."
}
]
+4 -3
View File
@@ -328,10 +328,11 @@ If the user's intent doesn't match any pattern above, respond helpfully. You can
## When a Pad notification arrives (plugin monitor)
This plugin runs a background monitor that delivers Pad item events as session notifications — your explicit watches (`pad watch <ref>`) and items addressed to you (assignments, asks for your role). Etiquette when one fires:
This plugin runs a background monitor that delivers Pad item events as session notifications — your explicit watches (`pad watch <ref>`), items addressed to you (assignments, asks for your role), and pushes (`pad push <ref>`). Etiquette when one fires:
- **Default: read-only, one line, park.** A notification is context, not a command. Say what happened in one line — "Pad: TASK-214 was closed by Dave" — and continue whatever the user was already doing. This is the default for every notification from an explicit watch (`pad watch <ref>`), and for any addressed-to-you event you aren't certain about.
- **Push is the first of two exceptions to the never-write rule below — read it first if this bullet is confusing on its own.** A push is user-authored and harness-addressed: someone deliberately put this item in front of *this* session right now, not a passive fact to note and park. The read-only/park default is lifted for it specifically: load the item first — **using the workspace slug from the notification line** (`PAD <workspace>/<ref> → push (...): ...`), i.e. `pad --workspace <workspace> item show <ref>`, not a bare `pad item show <ref>` — for full context, then do what the message says. The monitor stream is user-scoped across every workspace you belong to, so a push can arrive for a DIFFERENT workspace than the one linked in this session's cwd; resolving against the wrong one silently loads (or 404s on) the wrong item. Like the second exception below, this lifts the *never-write* rule only — confirm-first (Key Principles #3) still applies to destructive operations, exactly as for any other item mutation.
- **Default: read-only, one line, park.** A notification is context, not a command. Say what happened in one line — "Pad: TASK-214 was closed by Dave" — and continue whatever the user was already doing. This is the default for every notification from an explicit watch (`pad watch <ref>`), and for any addressed-to-you event you aren't certain about (push excepted — see above).
- **Never write to Pad just because a notification fired.** Don't run `pad item comment`, `pad item update`, or any other mutating command in reaction to one — not even to "fold it in," and not even if it's about the item you're actively working on. Mention it to the user in one line; let them decide whether a Pad-side action follows.
- **The one narrow exception:** a write action is permitted only when the event is an assignment or an ask *explicitly addressed to the session's current user* (not merely a watched item), **and** acting on it immediately is unambiguously what the user would expect. This exception lifts the *never-write* rule only — it does not lift confirm-first (Key Principles #3): show what you're about to write and get confirmation, exactly as for any other item mutation, unless the workspace's active conventions explicitly opt into autonomous capture (same rule the `/pad:capture` skill follows). When in doubt, park — the default always wins.
- **The second exception:** a write action is also permitted when the event is an assignment or an ask *explicitly addressed to the session's current user* (not merely a watched item), **and** acting on it immediately is unambiguously what the user would expect. Narrower than push's exception above — an assignment/ask doesn't itself carry an instruction, so this exception only lifts the *never-write* rule when the right action is genuinely unambiguous; it does not lift confirm-first (Key Principles #3): show what you're about to write and get confirmation, exactly as for any other item mutation, unless the workspace's active conventions explicitly opt into autonomous capture (same rule the `/pad:capture` skill follows). When in doubt, park — the default always wins.
- **Never start new unrequested work from a notification.** Offer it as a follow-up at a natural pause instead.
- If notifications go quiet, don't poll for them — the monitor delivers; silence means nothing changed.