feat(subagents): artifact_read + workspace staging, artifact counts, docs (M2+M3)

cmd/odek/artifact_read_test.go

@@ -0,0 +1,194 @@
+package main
+
+// TDD RED phase — M2 artifact_read (SUBAGENT_RESULT_ARTIFACTS_PLAN.md):
+// validated artifact refs registered at collation become readable content
+// via a parent-only built-in tool. The model supplies ONLY the id — path
+// resolution is internal to the registry, so no model input ever reaches
+// the filesystem as a path.
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/BackendStack21/odek/internal/artifact"
+)
+
+func regRef(t *testing.T, id, content string) (artifact.Ref, string) {
+	t.Helper()
+	dir := t.TempDir()
+	path := filepath.Join(dir, id+".md")
+	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	size := int64(len(content))
+	ref := artifact.Ref{
+		Schema: artifact.SchemaArtifactRef, ID: id, MediaType: "text/markdown",
+		URI: "file://" + path, SHA256: expectedSHA(t, content), SizeBytes: &size,
+	}
+	return ref, path
+}
+
+func TestArtifactRegistry_RegisterLookupEvict(t *testing.T) {
+	resetArtifactRegistryForTest()
+	ref, path := regRef(t, "alpha", "alpha content")
+	registerSubagentArtifact(artifactEntry{Ref: ref, Path: path, TaskIdx: 0})
+
+	got, ok := lookupSubagentArtifact("alpha")
+	if !ok || got.Path != path {
+		t.Fatalf("lookup failed: %+v ok=%v", got, ok)
+	}
+
+	// Eviction: cap + 10 more pushes alpha out (oldest first).
+	for i := 0; i < artifactRegistryCap+10; i++ {
+		r, p := regRef(t, fmt.Sprintf("fill-%03d", i), "x")
+		registerSubagentArtifact(artifactEntry{Ref: r, Path: p, TaskIdx: i})
+	}
+	if _, ok := lookupSubagentArtifact("alpha"); ok {
+		t.Error("oldest entry must be evicted at cap")
+	}
+	if _, ok := lookupSubagentArtifact("fill-000"); ok {
+		t.Error("second-oldest must be evicted too")
+	}
+}
+
+func TestArtifactRegistry_DuplicateIDLastWins(t *testing.T) {
+	resetArtifactRegistryForTest()
+	r1, p1 := regRef(t, "dup", "first")
+	registerSubagentArtifact(artifactEntry{Ref: r1, Path: p1, TaskIdx: 0})
+	r2, p2 := regRef(t, "dup", "second")
+	dup := registerSubagentArtifact(artifactEntry{Ref: r2, Path: p2, TaskIdx: 1})
+
+	if !dup {
+		t.Error("duplicate registration must be reported")
+	}
+	got, _ := lookupSubagentArtifact("dup")
+	if got.Path != p2 || got.TaskIdx != 1 {
+		t.Errorf("last-wins broken: %+v", got)
+	}
+}
+
+func TestArtifactReadTool_HappyPath(t *testing.T) {
+	resetArtifactRegistryForTest()
+	content := "# Report\n" + strings.Repeat("detail ", 200)
+	ref, path := regRef(t, "report", content)
+	registerSubagentArtifact(artifactEntry{Ref: ref, Path: path, TaskIdx: 0})
+
+	tool := &artifactReadTool{}
+	tool.SetContext(t.Context())
+	got, err := tool.Call(`{"id":"report"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(got, "detail") {
+		t.Errorf("content missing:\n%s", got)
+	}
+	if !strings.Contains(got, "report") || !strings.Contains(got, "text/markdown") {
+		t.Errorf("metadata header missing:\n%s", got)
+	}
+	if strings.Contains(got, path) {
+		t.Errorf("raw path must never render:\n%s", got)
+	}
+	if !strings.Contains(got, "untrusted") {
+		t.Errorf("artifact content must be untrusted-wrapped:\n%s", got)
+	}
+}
+
+func TestArtifactReadTool_OffsetLimit(t *testing.T) {
+	resetArtifactRegistryForTest()
+	content := strings.Repeat("A", 1000)
+	ref, path := regRef(t, "blob", content)
+	registerSubagentArtifact(artifactEntry{Ref: ref, Path: path})
+
+	tool := &artifactReadTool{}
+	tool.SetContext(t.Context())
+
+	got, err := tool.Call(`{"id":"blob","offset":900,"limit":50}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(got, strings.Repeat("A", 50)) {
+		t.Errorf("offset slice missing:\n%s", got)
+	}
+	if !strings.Contains(got, "TRUNCATED") {
+		t.Errorf("must flag truncation when more remains:\n%s", got)
+	}
+
+	// Hard cap: limit above the max is clamped, not honored.
+	if _, err := tool.Call(`{"id":"blob","limit":99999999}`); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestArtifactReadTool_UnknownIDListsAvailable(t *testing.T) {
+	resetArtifactRegistryForTest()
+	for _, id := range []string{"one", "two"} {
+		ref, path := regRef(t, id, "x")
+		registerSubagentArtifact(artifactEntry{Ref: ref, Path: path})
+	}
+
+	tool := &artifactReadTool{}
+	tool.SetContext(t.Context())
+	got, err := tool.Call(`{"id":"nope"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(got, "one") || !strings.Contains(got, "two") {
+		t.Errorf("unknown id must list available:\n%s", got)
+	}
+	// Traversal-shaped ids are just unknown — never paths.
+	if _, err := tool.Call(`{"id":"../../etc/passwd"}`); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestArtifactReadTool_VanishedFile(t *testing.T) {
+	resetArtifactRegistryForTest()
+	ref, path := regRef(t, "ghost", "data")
+	registerSubagentArtifact(artifactEntry{Ref: ref, Path: path})
+	os.Remove(path)
+
+	tool := &artifactReadTool{}
+	tool.SetContext(t.Context())
+	got, _ := tool.Call(`{"id":"ghost"}`)
+	if !strings.Contains(got, "no longer available") {
+		t.Errorf("vanished artifact must fail friendly:\n%s", got)
+	}
+}
+
+func TestArtifactReadEnabled_Gate(t *testing.T) {
+	if !artifactReadEnabled(toolConfig{}) {
+		t.Error("top-level operator run must get artifact_read")
+	}
+	if artifactReadEnabled(toolConfig{SelfTrust: "trusted"}) {
+		t.Error("sub-agents must NOT get artifact_read (parent-only)")
+	}
+	if artifactReadEnabled(toolConfig{SelfTrust: "untrusted"}) {
+		t.Error("untrusted sub-agents must NOT get artifact_read")
+	}
+}
+
+func TestRegisterTaskArtifacts_DuplicateNote(t *testing.T) {
+	resetArtifactRegistryForTest()
+	dir := t.TempDir()
+	c1, c2 := "first body", "second body"
+	p1 := filepath.Join(dir, "dup.md")
+	p2 := filepath.Join(dir, "dup2.md")
+	os.WriteFile(p1, []byte(c1), 0o600)
+	os.WriteFile(p2, []byte(c2), 0o600)
+	size1, size2 := int64(len(c1)), int64(len(c2))
+	raw1 := fmt.Sprintf(`{"status":"success","summary":"ok","artifacts":[{"schema":%q,"id":"dup","uri":"file://%s","media_type":"text/markdown","sha256":%q,"size_bytes":%d}]}`,
+		artifact.SchemaArtifactRef, p1, expectedSHA(t, c1), size1)
+	raw2 := fmt.Sprintf(`{"status":"success","summary":"ok","artifacts":[{"schema":%q,"id":"dup","uri":"file://%s","media_type":"text/markdown","sha256":%q,"size_bytes":%d}]}`,
+		artifact.SchemaArtifactRef, p2, expectedSHA(t, c2), size2)
+
+	if notes := registerTaskArtifacts(raw1, dir, 0); len(notes) != 0 {
+		t.Errorf("first registration must not note: %v", notes)
+	}
+	notes := registerTaskArtifacts(raw2, dir, 1)
+	if len(notes) != 1 || !strings.Contains(notes[0], "duplicate") {
+		t.Errorf("duplicate must produce a note: %v", notes)
+	}
+}

cmd/odek/artifact_read_tool.go

@@ -0,0 +1,147 @@
+package main
+
+// artifact_read (M2) — parent-only built-in that resolves a registered
+// artifact id to validated content. The model supplies ONLY the id (plus an
+// optional byte offset/limit); path resolution happens exclusively through
+// the collation-time registry, so no model input ever reaches the
+// filesystem as a path. Content is returned inside the untrusted boundary
+// like every other child-derived tool result, and ingested into the audit
+// log by the standard per-call recorder.
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/BackendStack21/odek"
+)
+
+const (
+	// artifactReadDefaultLimit is the per-call byte budget.
+	artifactReadDefaultLimit = 64 << 10 // 64 KiB
+	// artifactReadMaxLimit is the hard per-call cap; larger requests clamp.
+	artifactReadMaxLimit = 256 << 10 // 256 KiB
+)
+
+// artifactReadTool reads registered sub-agent result artifacts by id.
+type artifactReadTool struct {
+	ctxTool
+}
+
+var _ odek.Tool = (*artifactReadTool)(nil)
+
+func (t *artifactReadTool) Name() string { return "artifact_read" }
+
+func (t *artifactReadTool) Description() string {
+	return `Read the full content of a sub-agent result artifact registered this session. Use it when a delegate_tasks result references artifacts whose inlined preview is missing or truncated.
+
+- id: the artifact id from the delegate_tasks result (required)
+- offset: byte offset to start reading from (default 0)
+- limit: max bytes to return per call (default 65536; hard cap 262144)
+
+Paths are resolved internally from the session registry — never pass file paths. Parent-side tool only.`
+}
+
+func (t *artifactReadTool) Schema() any {
+	return map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"id": map[string]any{
+				"type":        "string",
+				"description": "Artifact id from the delegate_tasks result metadata.",
+			},
+			"offset": map[string]any{
+				"type":        "integer",
+				"description": "Byte offset to start reading from (default 0).",
+			},
+			"limit": map[string]any{
+				"type":        "integer",
+				"description": "Max bytes per call (default 65536, hard cap 262144).",
+			},
+		},
+		"required": []string{"id"},
+	}
+}
+
+// artifactReadArgs is the tool input contract.
+type artifactReadArgs struct {
+	ID     string `json:"id"`
+	Offset int64  `json:"offset"`
+	Limit  int64  `json:"limit"`
+}
+
+func (t *artifactReadTool) Call(args string) (string, error) {
+	var in artifactReadArgs
+	if err := json.Unmarshal([]byte(args), &in); err != nil {
+		return fmt.Sprintf(`{"error":"parse failed: %v"}`, err), nil
+	}
+	if in.ID == "" {
+		return `{"error":"id is required"}`, nil
+	}
+	if in.Offset < 0 {
+		in.Offset = 0
+	}
+	if in.Limit <= 0 {
+		in.Limit = artifactReadDefaultLimit
+	}
+	if in.Limit > artifactReadMaxLimit {
+		in.Limit = artifactReadMaxLimit
+	}
+
+	entry, ok := lookupSubagentArtifact(in.ID)
+	if !ok {
+		return fmt.Sprintf(`{"error":"unknown artifact id %q — registered artifacts: %s"}`, in.ID, artifactIDList()), nil
+	}
+
+	// Re-verify at read time: the janitor backstop or a session delete may
+	// have removed the subtree since collation.
+	info, err := os.Stat(entry.Path)
+	if err != nil || !info.Mode().IsRegular() {
+		return fmt.Sprintf(`{"error":"artifact %q is no longer available (removed by cleanup)"}`, in.ID), nil
+	}
+	if in.Offset >= info.Size() {
+		return fmt.Sprintf(`{"error":"artifact %q is %d bytes; offset %d is past the end"}`, in.ID, info.Size(), in.Offset), nil
+	}
+
+	f, err := os.Open(entry.Path)
+	if err != nil {
+		return fmt.Sprintf(`{"error":"artifact %q unreadable: %v"}`, in.ID, err), nil
+	}
+	defer f.Close()
+	if _, err := f.Seek(in.Offset, io.SeekStart); err != nil {
+		return fmt.Sprintf(`{"error":"artifact %q seek failed: %v"}`, in.ID, err), nil
+	}
+	// Read one extra byte to detect truncation without a second stat.
+	data, err := io.ReadAll(io.LimitReader(f, in.Limit+1))
+	if err != nil {
+		return fmt.Sprintf(`{"error":"artifact %q read failed: %v"}`, in.ID, err), nil
+	}
+	truncated := int64(len(data)) > in.Limit
+	if truncated {
+		data = data[:in.Limit]
+	}
+
+	size := int64(0)
+	if entry.Ref.SizeBytes != nil {
+		size = *entry.Ref.SizeBytes
+	}
+	shaPrefix := entry.Ref.SHA256
+	if len(shaPrefix) > 12 {
+		shaPrefix = shaPrefix[:12]
+	}
+
+	var b strings.Builder
+	fmt.Fprintf(&b, "artifact %s (%s, %d bytes, sha256:%s) — bytes %d..%d of %d",
+		entry.Ref.ID, entry.Ref.MediaType, size, shaPrefix, in.Offset, in.Offset+int64(len(data)), info.Size())
+	if truncated {
+		b.WriteString(" — TRUNCATED, call again with offset to continue")
+	}
+	b.WriteString("\n\n")
+	b.Write(data)
+
+	// Child-derived content: inside the untrusted boundary, recorded by the
+	// per-call audit ingest like every other tool result.
+	return wrapUntrusted(t.toolCtx(), "artifact_read", b.String()), nil
+}

cmd/odek/main.go

@@ -2299,6 +2299,13 @@ func builtinTools(dc danger.DangerousConfig, sm *skills.SkillManager, approver d
 		newBrowserTool(dc),
 	}
 
+	// artifact_read is registered only for top-level runs (SelfTrust empty):
+	// sub-agents run in their own process whose artifact registry is always
+	// empty — parent-only by design (SUBAGENT_RESULT_ARTIFACTS_PLAN.md M2).
+	if artifactReadEnabled(tcfg) {
+		tools = append(tools, &artifactReadTool{})
+	}
+
 	// web_search is registered only when a SearXNG backend is configured —
 	// without a base_url there is no instance to query, so the tool would just
 	// confuse the agent. The Docker compose setup sets this automatically.