diff --git a/cmd/pad/notes.go b/cmd/pad/notes.go index 879dd5df..9305cdb3 100644 --- a/cmd/pad/notes.go +++ b/cmd/pad/notes.go @@ -49,7 +49,13 @@ func noteCmd() *cobra.Command { Summary: strings.TrimSpace(args[1]), Details: body, CreatedAt: time.Now().UTC().Format(time.RFC3339), - CreatedBy: "user", + // The server never parses this entry — it lives inside the + // item's fields JSON — so the client is the last party that + // could know who wrote it. Hardcoding "user" here made every + // agent note claim a human wrote it (BUG-2542); leaving it + // empty would have left it authorless, since nothing + // downstream fills it in. Self-declared, like the header. + CreatedBy: cli.ActorKind(), } fields, err := models.AppendImplementationNote(item.Fields, entry) if err != nil { @@ -57,9 +63,9 @@ func noteCmd() *cobra.Command { } updated, err := client.UpdateItem(ws, item.Slug, models.ItemUpdate{ - Fields: &fields, - LastModifiedBy: "user", - Source: "cli", + Fields: &fields, + // LastModifiedBy left empty — server-stamped (BUG-2542). + Source: "cli", }) if err != nil { return err @@ -115,7 +121,13 @@ func decideCmd() *cobra.Command { Decision: strings.TrimSpace(args[1]), Rationale: body, CreatedAt: time.Now().UTC().Format(time.RFC3339), - CreatedBy: "user", + // The server never parses this entry — it lives inside the + // item's fields JSON — so the client is the last party that + // could know who wrote it. Hardcoding "user" here made every + // agent note claim a human wrote it (BUG-2542); leaving it + // empty would have left it authorless, since nothing + // downstream fills it in. Self-declared, like the header. + CreatedBy: cli.ActorKind(), } fields, err := models.AppendDecisionLogEntry(item.Fields, entry) if err != nil { @@ -123,9 +135,9 @@ func decideCmd() *cobra.Command { } updated, err := client.UpdateItem(ws, item.Slug, models.ItemUpdate{ - Fields: &fields, - LastModifiedBy: "user", - Source: "cli", + Fields: &fields, + // LastModifiedBy left empty — server-stamped (BUG-2542). + Source: "cli", }) if err != nil { return err diff --git a/internal/cli/agent_identity.go b/internal/cli/agent_identity.go new file mode 100644 index 00000000..acb613a4 --- /dev/null +++ b/internal/cli/agent_identity.go @@ -0,0 +1,86 @@ +package cli + +import "os" + +// ResolveAgentName decides what goes in the X-Pad-Agent header, which is the +// ONLY signal the server has for attributing a write to an agent rather than a +// human (server.actorFromRequest). Before BUG-2542 the header was populated +// from exactly one place — `agent_name` in .pad.toml — and nothing else, so any +// workspace that had not opted in recorded every agent write as a human one. +// The embedded skill meanwhile promised that `created_by: agent` was automatic. +// The contract was the thing that was wrong; this makes it true instead. +// +// Precedence, most explicit first: +// +// 1. `agent_name` in .pad.toml — a deliberate per-workspace choice. +// 2. $PAD_AGENT — the runtime-agnostic override. Any harness we have not +// taught this function about can set it and be attributed correctly. +// 3. A detected agent runtime, below. +// +// WHAT THIS IS NOT. The header is client-supplied and self-declared, so it +// records honesty, not identity: +// +// - an agent that omits it is indistinguishable from the human whose +// credentials it is using; +// - a human running `! pad ...` inside an agent's terminal inherits that +// terminal's environment and will be attributed to the agent. +// +// So this is not a basis for machine-verifiable human-approval provenance. A +// grant that has to be provable needs a channel the agent cannot author at all. +// BUG-2542's trail has the incident that made the distinction concrete: an +// agent's relay of a human's words was recorded indistinguishably from the +// human having typed them. +func ResolveAgentName() string { + if pt, _ := LoadPadToml(); pt != nil && pt.AgentName != "" { + return pt.AgentName + } + if name := os.Getenv("PAD_AGENT"); name != "" { + return name + } + return detectAgentRuntime() +} + +// agentRuntimeEnv maps an environment variable to the agent name recorded when +// it is set to a non-empty value. +// +// Only entries VERIFIED against a live session of that runtime belong here. +// Claude Code exports CLAUDECODE=1 to child processes (confirmed by reading the +// environment of a `pad` subprocess inside one). Other harnesses — Cursor, +// Windsurf, Aider, Codex — very likely have their own markers, but guessing at +// variable names would put unverified claims in a shipped binary and produce +// silent misattribution when wrong in either direction. They should set +// $PAD_AGENT until someone confirms a signature and adds it here with the same +// standard of evidence. +var agentRuntimeEnv = []struct { + env string + name string +}{ + {env: "CLAUDECODE", name: "claude-code"}, +} + +func detectAgentRuntime() string { + for _, r := range agentRuntimeEnv { + if os.Getenv(r.env) != "" { + return r.name + } + } + return "" +} + +// ActorKind is the writer role — "agent" or "user" — as this process can best +// describe itself. Same self-declared signal as ResolveAgentName, just reduced +// to the enum the created_by / last_modified_by fields hold. +// +// Use it ONLY where the client is the last party that could know: structured +// entries embedded in an item's fields JSON (implementation notes, decision +// log) are written whole by the CLI and the server never parses them for +// attribution, so nobody downstream can fill this in. Everywhere else, leave +// attribution empty and let the server stamp it from the request — an explicit +// value suppresses that stamp, which is how the CLI used to record every agent +// note as a human one (BUG-2542). +func ActorKind() string { + if ResolveAgentName() != "" { + return "agent" + } + return "user" +} diff --git a/internal/cli/agent_identity_test.go b/internal/cli/agent_identity_test.go new file mode 100644 index 00000000..93fb2c40 --- /dev/null +++ b/internal/cli/agent_identity_test.go @@ -0,0 +1,167 @@ +package cli + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" +) + +// BUG-2542. X-Pad-Agent is the only signal the server has for attributing a +// write to an agent, and it used to be populated from .pad.toml's agent_name +// and nothing else — so an agent session in a workspace that had not opted in +// was recorded as the human whose credentials it used. +// +// Each case pins one rung of the precedence AND the negative that makes it +// meaningful: a plain human shell with no markers must still resolve to "", +// because a resolver that returned a name unconditionally would satisfy every +// positive case here while attributing Dave's own commands to an agent. +func TestResolveAgentName(t *testing.T) { + for _, tc := range []struct { + name string + padToml string + env map[string]string + want string + }{ + { + name: "plain human shell resolves to nothing", + want: "", + }, + { + name: "detected runtime is used when nothing more explicit exists", + env: map[string]string{"CLAUDECODE": "1"}, + want: "claude-code", + }, + { + name: "PAD_AGENT overrides a detected runtime", + env: map[string]string{"CLAUDECODE": "1", "PAD_AGENT": "some-other-harness"}, + want: "some-other-harness", + }, + { + name: "pad.toml wins over both", + padToml: "workspace = \"w\"\nagent_name = \"wren\"\n", + env: map[string]string{"CLAUDECODE": "1", "PAD_AGENT": "some-other-harness"}, + want: "wren", + }, + { + name: "an empty marker is not a marker", + env: map[string]string{"CLAUDECODE": "", "PAD_AGENT": ""}, + want: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + // Run from a scratch cwd so a .pad.toml higher up the real tree + // (this repo has one) cannot leak into the no-toml cases. + dir := t.TempDir() + if tc.padToml != "" { + if err := os.WriteFile(filepath.Join(dir, ".pad.toml"), []byte(tc.padToml), 0o600); err != nil { + t.Fatalf("write .pad.toml: %v", err) + } + } + chdir(t, dir) + + // Clear every variable the resolver consults, then set this case's. + for _, k := range []string{"PAD_AGENT", "CLAUDECODE"} { + t.Setenv(k, "") + os.Unsetenv(k) + } + for k, v := range tc.env { + t.Setenv(k, v) + } + + if got := ResolveAgentName(); got != tc.want { + t.Errorf("ResolveAgentName() = %q, want %q", got, tc.want) + } + }) + } +} + +func chdir(t *testing.T, dir string) { + t.Helper() + prev, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + if err := os.Chdir(dir); err != nil { + t.Fatalf("chdir: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(prev) }) +} + +// The resolver is only half the path: its value has to reach the wire as +// X-Pad-Agent, which is the header the server keys attribution on. The +// server-side tests inject that header directly, so without this the +// resolver could be correct and the client still send nothing (which is +// exactly the pre-BUG-2542 state). +func TestClientSendsResolvedAgentHeader(t *testing.T) { + for _, tc := range []struct { + name string + env map[string]string + want string + }{ + {name: "agent session sends the header", env: map[string]string{"CLAUDECODE": "1"}, want: "claude-code"}, + {name: "human shell sends none", env: nil, want: ""}, + } { + t.Run(tc.name, func(t *testing.T) { + chdir(t, t.TempDir()) + for _, k := range []string{"PAD_AGENT", "CLAUDECODE"} { + t.Setenv(k, "") + os.Unsetenv(k) + } + for k, v := range tc.env { + t.Setenv(k, v) + } + + 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(`{"status":"ok"}`)) + })) + defer srv.Close() + + if err := NewClientFromURL(srv.URL).Health(); err != nil { + t.Fatalf("health: %v", err) + } + if !seen { + t.Fatal("server never saw the request") + } + if got != tc.want { + t.Errorf("X-Pad-Agent = %q, want %q", got, tc.want) + } + }) + } +} + +// 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" +// there on the theory that the server would stamp it — it does not, and the +// entries would have been written authorless. +func TestActorKind(t *testing.T) { + for _, tc := range []struct { + name string + env map[string]string + want string + }{ + {name: "agent session", env: map[string]string{"CLAUDECODE": "1"}, want: "agent"}, + {name: "explicit PAD_AGENT", env: map[string]string{"PAD_AGENT": "some-harness"}, want: "agent"}, + {name: "human shell", env: nil, want: "user"}, + } { + t.Run(tc.name, func(t *testing.T) { + chdir(t, t.TempDir()) + for _, k := range []string{"PAD_AGENT", "CLAUDECODE"} { + t.Setenv(k, "") + os.Unsetenv(k) + } + for k, v := range tc.env { + t.Setenv(k, v) + } + if got := ActorKind(); got != tc.want { + t.Errorf("ActorKind() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/cli/client.go b/internal/cli/client.go index 90ac7cb6..da0983e2 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -69,10 +69,11 @@ func NewClientFromURL(baseURL string) *Client { } } - // Auto-load agent name from .pad.toml if available - if pt, _ := LoadPadToml(); pt != nil && pt.AgentName != "" { - c.agentName = pt.AgentName - } + // Resolve the agent identity that becomes X-Pad-Agent. Used to read + // .pad.toml's agent_name and nothing else, which meant any workspace that + // had not opted in recorded agent writes as human ones (BUG-2542). See + // ResolveAgentName for the precedence and for what this signal cannot do. + c.agentName = ResolveAgentName() return c } diff --git a/internal/server/handlers_artifact_import.go b/internal/server/handlers_artifact_import.go index 8b137b4b..bf816378 100644 --- a/internal/server/handlers_artifact_import.go +++ b/internal/server/handlers_artifact_import.go @@ -140,12 +140,19 @@ func (s *Server) handleImportArtifact(w http.ResponseWriter, r *http.Request) { Content: body, Fields: string(fieldsJSON), } - // Attribution: a normal agent/api create. Source is left blank here so - // createItemChecked stamps it from the request auth context (cli/web), - // matching every other create path. - if u := currentUser(r); u != nil { - input.CreatedBy = u.ID - } + // Attribution: a normal agent/api create. CreatedBy and Source are both + // left blank so createItemChecked stamps them from the request auth + // context, matching every other create path. + // + // This used to set CreatedBy to the user's UUID, which is the wrong + // DOMAIN for the field, not just the wrong value: created_by holds the + // role — "user" or "agent" — and consumers compare it to those literals + // (CommentThread.svelte, TimelineVersionCard.svelte). An imported item + // therefore matched neither and rendered as neither. Found while fixing + // BUG-2542; it also would have defeated that fix here, since a non-empty + // CreatedBy suppresses the actor stamp. The user's identity is already + // carried by the items.created_by_user_id column, which no create path + // currently populates — separate gap, not widened into this change. // Enforce the workspace item-count limit (workspace-scoped), identical to // handleCreateItem. Writes the 403 plan_limit_exceeded response itself when diff --git a/internal/server/handlers_item_links.go b/internal/server/handlers_item_links.go index e633ea52..cc7f4d5b 100644 --- a/internal/server/handlers_item_links.go +++ b/internal/server/handlers_item_links.go @@ -158,6 +158,16 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { return } + // Non-parent links (blocks / blocked-by / relates / implements) fall + // through to here. The CLI doesn't send created_by, so the store defaulted + // it to "user" and an agent's `pad item block` recorded a human. The parent + // branch above already passes the actor to SetParentLink; this mirrors it. + // An explicit body value still wins, same as everywhere else (BUG-2542). + if input.CreatedBy == "" { + linkActor, _ := actorFromRequest(r) + input.CreatedBy = linkActor + } + link, err := s.store.CreateItemLink(workspaceID, input, item.ID) if err != nil { if strings.Contains(err.Error(), "UNIQUE constraint") { diff --git a/internal/server/handlers_item_versions.go b/internal/server/handlers_item_versions.go index 7d147ee6..42eaa61c 100644 --- a/internal/server/handlers_item_versions.go +++ b/internal/server/handlers_item_versions.go @@ -152,13 +152,18 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request // client lazy-seeds from the restored content; the boundary rejects any // in-flight pre-restore snapshot. This replaces the earlier // applier/epoch/watermark routing: prune+reseed needs none of it. + // Stamp the writer from the request rather than hardcoding user/web: a + // restore driven by an agent used to record itself as a human web edit + // (BUG-2542). Source falls back to "web" for cookie-authenticated callers, + // which is what actorFromRequest already returns for them. + restoreActor, restoreSource := actorFromRequest(r) content := targetVersion.Content summary := "Restored from version " + targetVersion.CreatedAt.Format("Jan 2, 2006 3:04 PM") input := models.ItemUpdate{ Content: &content, ChangeSummary: summary, - LastModifiedBy: "user", - Source: "web", + LastModifiedBy: restoreActor, + Source: restoreSource, // A restore must always leave an undo point + a version bracketing the // content it moves items.content back to, even on a repeat restore within // the version-throttle window (VersionThrottleInterval = 1h). diff --git a/internal/server/handlers_items.go b/internal/server/handlers_items.go index e4b918b0..0f0ffc0d 100644 --- a/internal/server/handlers_items.go +++ b/internal/server/handlers_items.go @@ -708,9 +708,21 @@ func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *mo // on. // If a client explicitly sent a source in the body (e.g. an agent // marking itself as 'skill'), respect it. - if input.Source == "" { - _, src := actorFromRequest(r) - input.Source = src + if input.Source == "" || input.CreatedBy == "" { + actor, src := actorFromRequest(r) + if input.Source == "" { + input.Source = src + } + // BUG-2542: the actor half used to be discarded here, so every item + // created through this path fell through to store.CreateItem's + // `created_by = "user"` default — even for an agent that DID send + // X-Pad-Agent. Comments have always stamped it (handlers_comments.go); + // item creation silently did not, which made the skill's "items you + // create will have created_by: agent" contract false on its own terms. + // Same shape as Source: an explicit body value wins. + if input.CreatedBy == "" { + input.CreatedBy = actor + } } item, err := s.store.CreateItem(workspaceID, coll.ID, input) @@ -1286,6 +1298,16 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { input.VersionSource = "collab-snapshot" } + // BUG-2542: stamp the writer on single-item updates. Bulk ops already do + // this (handlers_items_bulk.go), but this path did not, so an agent's + // PATCH left last_modified_by at store.UpdateItem's "user" default and an + // item edited only by agents read as human-edited. An explicit body value + // wins, matching Source/CreatedBy on the create path. + if input.LastModifiedBy == "" { + updateActor, _ := actorFromRequest(r) + input.LastModifiedBy = updateActor + } + // Server-side gate: reject collab-snapshot PATCHes whose // op_log_cursor is below MIN(item_yjs_updates.id). Such a // cursor proves the flushing tab's Y.Doc was built on op-log diff --git a/internal/server/handlers_items_attribution_test.go b/internal/server/handlers_items_attribution_test.go new file mode 100644 index 00000000..c0c6504c --- /dev/null +++ b/internal/server/handlers_items_attribution_test.go @@ -0,0 +1,206 @@ +package server + +import ( + "bytes" + "encoding/json" + "io" + "net/http/httptest" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// BUG-2542. Two write paths took the actor from actorFromRequest and dropped +// it: item CREATE discarded it entirely (keeping only source), and the +// single-item PATCH never set LastModifiedBy at all. Both then fell through to +// the store's `"user"` defaults, so an item created and edited exclusively by +// an agent — one that DID send X-Pad-Agent — read back as human-authored. +// +// Every case runs twice, agent and human, because the bug's signature is that +// the two are indistinguishable. A test that only asserted the agent leg would +// pass against a server that hardcoded "agent", which is the same defect +// pointing the other way. +func TestItemAttribution_AgentVsHuman(t *testing.T) { + for _, tc := range []struct { + name string + agent string // X-Pad-Agent value; empty = human + want string + }{ + {name: "agent write", agent: "claude-code", want: "agent"}, + {name: "human write (control)", agent: "", want: "user"}, + } { + t.Run(tc.name, func(t *testing.T) { + srv := testServer(t) + ws := createTestWorkspaceViaAPI(t, srv) + + created := doAttributionRequest(t, srv, tc.agent, "POST", + "/api/v1/workspaces/"+ws+"/collections/tasks/items", + map[string]any{"title": "attribution probe"}) + var item models.Item + decodeAttributionBody(t, created, &item) + + if item.CreatedBy != tc.want { + t.Errorf("created_by = %q, want %q (X-Pad-Agent=%q)", item.CreatedBy, tc.want, tc.agent) + } + // Source is pre-existing behaviour and must not regress. Both legs + // authenticate the same way here (no Authorization header, no API + // token), so actorFromRequest returns "web" for both — assert the + // value, not merely that something was written. + if item.Source != "web" { + t.Errorf("source = %q, want %q", item.Source, "web") + } + + // The updater is deliberately the OTHER kind of writer. Patching + // with the same one proves nothing: insertItemTx seeds + // last_modified_by FROM created_by, so a same-writer update leaves + // the expected value in place whether or not the PATCH stamps + // anything — verified by reverting the update stamp alone, which + // that version of this test passed. The cross leg is the only + // shape where the update stamp is the sole mechanism that can + // produce the result. + crossAgent, crossWant := "", "user" + if tc.agent == "" { + crossAgent, crossWant = "claude-code", "agent" + } + updated := doAttributionRequest(t, srv, crossAgent, "PATCH", + "/api/v1/workspaces/"+ws+"/items/"+item.Slug, + map[string]any{"title": "attribution probe (edited)"}) + var after models.Item + decodeAttributionBody(t, updated, &after) + + if after.LastModifiedBy != crossWant { + t.Errorf("last_modified_by after a %s edit = %q, want %q (creator was %q)", + crossWant, after.LastModifiedBy, crossWant, tc.want) + } + // created_by must NOT be rewritten by whoever edited it. + if after.CreatedBy != tc.want { + t.Errorf("created_by after update = %q, want %q — an edit must not restamp the creator", after.CreatedBy, tc.want) + } + }) + } +} + +// An explicit body value still wins, so a caller that knows better than the +// header (an agent recording a write it made on a human's behalf, say) is not +// overridden by it. This is the contract the Source field already had. +func TestItemAttribution_ExplicitBodyValueWins(t *testing.T) { + srv := testServer(t) + ws := createTestWorkspaceViaAPI(t, srv) + + rr := doAttributionRequest(t, srv, "claude-code", "POST", + "/api/v1/workspaces/"+ws+"/collections/tasks/items", + map[string]any{"title": "explicit", "created_by": "user"}) + var item models.Item + decodeAttributionBody(t, rr, &item) + + if item.CreatedBy != "user" { + t.Errorf("created_by = %q, want %q — an explicit body value must beat the header", item.CreatedBy, "user") + } +} + +func doAttributionRequest(t *testing.T, srv *Server, agent, method, path string, body any) *httptest.ResponseRecorder { + t.Helper() + var r io.Reader + if body != nil { + data, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal body: %v", err) + } + r = bytes.NewReader(data) + } + req := httptest.NewRequest(method, path, r) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + if agent != "" { + req.Header.Set("X-Pad-Agent", agent) + } + req.RemoteAddr = "192.0.2.1:1234" + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + if rr.Code != 200 && rr.Code != 201 { + t.Fatalf("%s %s = %d: %s", method, path, rr.Code, rr.Body.String()) + } + return rr +} + +func decodeAttributionBody(t *testing.T, rr *httptest.ResponseRecorder, out any) { + t.Helper() + if err := json.Unmarshal(rr.Body.Bytes(), out); err != nil { + t.Fatalf("decode response: %v (body: %s)", err, rr.Body.String()) + } +} + +func createTestWorkspaceViaAPI(t *testing.T, srv *Server) string { + t.Helper() + rr := doRequest(srv, "POST", "/api/v1/workspaces", map[string]any{ + "name": "Attribution", "slug": "attribution", "template": "startup", + }) + if rr.Code != 200 && rr.Code != 201 { + t.Fatalf("create workspace = %d: %s", rr.Code, rr.Body.String()) + } + var ws struct { + Slug string `json:"slug"` + } + decodeAttributionBody(t, rr, &ws) + if ws.Slug == "" { + t.Fatal("workspace create returned no slug") + } + return ws.Slug +} + +// Non-parent links (blocks / blocked-by / relates) went through a different +// store call than parent links and never carried the actor, so an agent's +// `pad item block` recorded a human. Parent links were already correct, so the +// human leg here is the control that proves the assertion isn't just reading a +// hardcoded default. +func TestItemLinkAttribution_AgentVsHuman(t *testing.T) { + for _, tc := range []struct { + name string + agent string + want string + }{ + {name: "agent link", agent: "claude-code", want: "agent"}, + {name: "human link (control)", agent: "", want: "user"}, + } { + t.Run(tc.name, func(t *testing.T) { + srv := testServer(t) + ws := createTestWorkspaceViaAPI(t, srv) + + mk := func(title string) models.Item { + rr := doAttributionRequest(t, srv, tc.agent, "POST", + "/api/v1/workspaces/"+ws+"/collections/tasks/items", + map[string]any{"title": title}) + var it models.Item + decodeAttributionBody(t, rr, &it) + return it + } + src := mk("link source") + + // Every non-parent type in models.ItemLinkType*, not just + // `blocks`. They share routing, so one case would arguably do — + // but "shared routing makes the rest fine" is exactly the + // assumption a table costs nothing to stop assuming. It earned + // that immediately: the first version of this list included + // `blocked-by`, which is CLI surface sugar that inverts + // source/target into a `blocks` row, not a stored link type. The + // API rejects it with 400, on BOTH writer legs — which is also + // how you can tell that failure apart from an attribution one. + for _, linkType := range []string{"blocks", "related", "implements", "supersedes", "split_from"} { + t.Run(linkType, func(t *testing.T) { + target := mk("link target " + linkType) + rr := doAttributionRequest(t, srv, tc.agent, "POST", + "/api/v1/workspaces/"+ws+"/items/"+src.Slug+"/links", + map[string]any{"target_id": target.ID, "link_type": linkType}) + var link models.ItemLink + decodeAttributionBody(t, rr, &link) + + if link.CreatedBy != tc.want { + t.Errorf("%s link created_by = %q, want %q (X-Pad-Agent=%q)", + linkType, link.CreatedBy, tc.want, tc.agent) + } + }) + } + }) + } +} diff --git a/plugin/skills/pad/SKILL.md b/plugin/skills/pad/SKILL.md index e2d4c492..641af38a 100644 --- a/plugin/skills/pad/SKILL.md +++ b/plugin/skills/pad/SKILL.md @@ -314,7 +314,7 @@ See the **Onboarding** entry under Natural Language Routing above — it branche 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items you create will have `created_by: agent` and `source: cli` automatically. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally. diff --git a/skills/pad/SKILL.md b/skills/pad/SKILL.md index b41de4f1..f2fa8b5e 100644 --- a/skills/pad/SKILL.md +++ b/skills/pad/SKILL.md @@ -308,7 +308,7 @@ Run the **onboard** invokable playbook — see the **Onboarding** entry under Na 5. **Be conversational.** You're not a command executor. You're a project partner. 6. **Reference existing items.** Use `[[Item Title]]` links in content to connect items. 7. **Keep it practical.** Size each item so it's a single meaningful unit of work — what "meaningful" means depends on the workspace (one branch/PR for code, one interview round for hiring, one research question for research). Ideas should be actionable. Docs should be concise. Check the workspace's conventions for domain-specific sizing rules. -8. **Attribution matters.** Items you create will have `created_by: agent` and `source: cli` automatically. +8. **Attribution matters.** Items and comments you create are stamped `created_by: agent` and `source: cli` automatically — but the agent half only works if the CLI can tell it is being run by an agent. It detects Claude Code on its own; under any other harness, set `PAD_AGENT=` in the environment (or `agent_name` in `.pad.toml`) or your writes will be recorded as the human whose credentials you are using. Note this is self-declared, not proof: it makes the trail honest, it does not make it verifiable, so never treat `created_by` on a comment as evidence that a human said something. 9. **Follow project conventions.** Always load and follow active conventions before performing work. They are project-specific rules that override your defaults. When a role is active, load both role-specific and global conventions. 10. **Learn and teach.** When the user corrects your behavior or teaches you a project-specific rule, offer to save it as a convention: "Should I save this as a project convention so future agents follow it too?" Use `pad item create convention "Title" --field trigger= --field scope= --field priority=should --stdin` with an appropriate trigger inferred from the context. If the correction is role-specific, add `--field role=`. 11. **Role context is per-conversation.** If roles exist, ask which role the user is working as on first invocation. Remember it for the session. Auto-filter queries and suggest assignments accordingly. Never block on role — if the user says "no role" or the workspace has no roles, work normally.