From f22abec18caa371a85f27ff3cb42d508cec63723 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Tue, 3 Nov 2026 12:55:09 -0500 Subject: [PATCH 1/5] Fix unreadable profile approval limits in stages and task children Factory stage allow bypassed the profile read, and task constructors replaced parent policies with fresh allowances that dropped ProfileReadError. Check the profile before stage allow and retain the existing failed-read signal when building worker, repair and auditor policies. Readable profiles keep their existing allowance behavior. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/factory_stage.go | 7 ++ cmd/codeaf/factory_stage_unreadable_test.go | 40 +++++++++ .../PRNUMBER-unreadable-config-limits.md | 13 +++ internal/session/approvalgate.go | 10 +++ internal/session/task_audit.go | 3 +- .../session/task_profile_approval_test.go | 90 +++++++++++++++++++ internal/session/task_run.go | 11 +-- 7 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 cmd/codeaf/factory_stage_unreadable_test.go create mode 100644 docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md create mode 100644 internal/session/task_profile_approval_test.go diff --git a/cmd/codeaf/factory_stage.go b/cmd/codeaf/factory_stage.go index 6c09104095..8c902a6b22 100644 --- a/cmd/codeaf/factory_stage.go +++ b/cmd/codeaf/factory_stage.go @@ -12,6 +12,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/env" "github.com/Agent-Field/codeaf/internal/factory" factoryrun "github.com/Agent-Field/codeaf/internal/factory/run" @@ -295,6 +296,7 @@ func factoryStagePosture() string { // name) still hold, because they sit under every policy and they are not a // person's preference. A person who wants a stage narrower says so with // [factoryPostureEnv]. +// An unreadable profile still requires a person: its saved denials are unknown. type stageApprovalGate struct { rows *v3ApprovalGate posture string @@ -305,6 +307,11 @@ func (g stageApprovalGate) Build(posture string) (*approval.Policy, bool, error) if posture != session.PostureAllow { return g.rows.Build(posture) } + if g.rows != nil { + if _, err := config.ReadProfileApprovalConfig(g.rows.profileDir); err != nil { + return g.rows.Build(posture) + } + } policy, err := stageAllowPolicy() return policy, false, err } diff --git a/cmd/codeaf/factory_stage_unreadable_test.go b/cmd/codeaf/factory_stage_unreadable_test.go new file mode 100644 index 0000000000..4712ce6659 --- /dev/null +++ b/cmd/codeaf/factory_stage_unreadable_test.go @@ -0,0 +1,40 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/session" +) + +func TestStageUnreadableProfileRequiresPerson(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + profile := v3Profile(t, map[string]any{config.KeyToolApprovalMode: "prompt", config.KeyToolApprovals: "write:deny"}) + file := filepath.Join(profile, "config.json") + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(file, 0o600) }) + rows := &v3ApprovalGate{workspace: t.TempDir(), profileDir: profile, headless: true} + for _, posture := range []string{session.PostureAllow, session.PostureAsk, session.PostureDeny} { + t.Run(posture, func(t *testing.T) { + var gate session.ApprovalGate = stageApprovalGate{rows: rows, posture: posture} + policy, guardian, err := gate.Build(posture) + if err != nil { + t.Fatal(err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"x","content":"x"}`)) + t.Logf("stage write: %s", decision) + if decision.Action != approval.ActionPrompt || guardian || !strings.Contains(decision.Rule, file) || !strings.Contains(decision.Rule, "could not be read") { + t.Fatalf("unreadable saved denial must require a person with a clear reason: decision=%+v guardian=%v", decision, guardian) + } + }) + } +} diff --git a/docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md b/docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md new file mode 100644 index 0000000000..b55ef4195a --- /dev/null +++ b/docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md @@ -0,0 +1,13 @@ +--- +kind: fixed +title: codeaf keeps unreadable profile approval limits in factory stages and task children +pr: PRNUMBER +surface: [engine, factory] +invalidates: + - "A factory stage's allow posture bypassed an unreadable profile's saved approval rules. It now requires a person when those rules cannot be read." + - "Task workers, repair rounds and auditors dropped their parent's profile read failure. They now retain that signal and refuse calls that need a person when nobody can answer." +--- + +A readable profile keeps its existing behavior, including a factory stage's own +allow posture. An unreadable profile cannot silently discard a saved denial: +the existing approval gate reports the failed file read and requires a person. diff --git a/internal/session/approvalgate.go b/internal/session/approvalgate.go index 0220fa6ff1..8452fde780 100644 --- a/internal/session/approvalgate.go +++ b/internal/session/approvalgate.go @@ -25,6 +25,16 @@ package session import "github.com/Agent-Field/codeaf/internal/approval" +// taskApprovalPolicy preserves the failed-read signal while keeping the task's +// own allowance. Unknown saved denials still require a person in every child. +func (a *Agent) taskApprovalPolicy() *approval.Policy { + policy := &approval.Policy{Default: approval.ActionAllow} + if parent := a.approvalGate(); parent != nil { + policy.ProfileReadError = parent.ProfileReadError + } + return policy +} + // SetApprovalPolicy replaces the standing gate for the rest of this session. // // A NIL NEVER REPLACES ANYTHING. Nil is the configured-nothing case and it diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index e91d95bead..b8b93bb69a 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -3211,7 +3211,8 @@ func (a *Agent) newAuditAgent(dir string, node *TaskNode, door auditDoor, on str // belt is what actually constrains this agent: there is no hand here // that writes. AskConsent is off and InTask is on for the node's own // reason — there is nobody in a worktree to ask. - ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, + // An unreadable profile still requires a person, just as it does for a worker. + ApprovalPolicy: a.taskApprovalPolicy(), AskConsent: false, InTask: true, // AND ITS CALLS ARE A JUDGE'S, NOT A WORKER'S. InTask above is about diff --git a/internal/session/task_profile_approval_test.go b/internal/session/task_profile_approval_test.go new file mode 100644 index 0000000000..459a9bc556 --- /dev/null +++ b/internal/session/task_profile_approval_test.go @@ -0,0 +1,90 @@ +package session + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" +) + +func TestTaskChildrenKeepUnreadableProfileApproval(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + profile := t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.WriteFile(file, []byte(`{"tools.approvalMode":"prompt","tools.approvals":"write:deny"}`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(file, 0o600) }) + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("mode 000 profile unexpectedly readable") + } + for _, kind := range []string{"worker", "repair", "auditor"} { + for _, pushed := range []bool{false, true} { + name := kind + "/configured" + if pushed { + name = kind + "/pushed" + } + t.Run(name, func(t *testing.T) { + tool, args := "write", `{"path":"denied.txt","content":"must not be written"}` + if kind == "auditor" { + tool, args = "bash", `{"command":"pwd"}` + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", tool, args), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + policy := &approval.Policy{ProfileReadError: readErr.Error()} + parent, _ := newTestAgent(t, script, func(c *Config) { + c.OneModel = true + if !pushed { + c.ApprovalPolicy = policy + } + }) + if pushed { + parent.SetApprovalPolicy(policy) + } + g := parent.graph() + g.run = func(*TaskNode) {} + id := g.reserve() + g.admit(id, taskSpec{title: "work", brief: "answer", acceptance: "done"}) + workspace := t.TempDir() + var child *Agent + var err error + switch kind { + case "auditor": + child, err = parent.newAuditAgent(workspace, g.node(id), plainDoor(auditReadCommands), "") + case "repair": + child, err = parent.newTaskAgentOn(context.Background(), workspace, g.node(id), "-repair1", "", true) + default: + child, err = parent.newTaskAgent(context.Background(), workspace, g.node(id), "") + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + events := collect(t, mustSubmit(t, child, "attempt the call")) + failed, ok := firstOfKind(events, EventToolFailed) + t.Logf("child %s: failed=%v output=%q", tool, ok, failed.Output) + if !ok || !strings.Contains(failed.Output, "saved approval rules could not be read") || !strings.Contains(failed.Output, readErr.Error()) { + t.Errorf("child lost failed-read reason: events=%v refusal=%q", kinds(events), failed.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "denied.txt")); !os.IsNotExist(err) { + t.Errorf("refused write created file: %v", err) + } + }) + } + } +} diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 5e6d809134..b2bc4f2d3a 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -65,10 +65,11 @@ package session // ── WHY A NODE NEVER ASKS ── // // There is nobody to ask. Its approval posture is composed from the same -// [approval.Policy] the conversation uses — allow everything, with the critical +// approval machinery the conversation uses — allow everything, with the critical // table (rm -rf /, mkfs, a redirect onto a raw disk, shutdown) still a floor -// under it — and where a conversation would raise a question, the node reads a -// refusal it can act on. That is exactly consent.go's law for a headless run, +// under it and any profile read failure carried forward. Where a conversation +// would raise a question, the node reads a refusal it can act on. +// That is exactly consent.go's law for a headless run, // said in the node's own words. import ( @@ -89,7 +90,6 @@ import ( "time" "github.com/Agent-Field/agentfield/sdk/go/ai" - "github.com/Agent-Field/codeaf/internal/approval" "github.com/Agent-Field/codeaf/internal/cell" "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/effort" @@ -7898,7 +7898,8 @@ func (a *Agent) newTaskAgentOn(ctx context.Context, dir string, node *TaskNode, // turns an allow into a "prompt" for the handful of shapes that destroy // a disk or drop the machine, and a prompt in a node is a refusal it can // read (consent.go, InTask) — never a question and never a hang. - ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, + // An unreadable profile also requires a person because saved denials are unknown. + ApprovalPolicy: a.taskApprovalPolicy(), AskConsent: false, InTask: true, // InTask is also what makes this agent's calls PATIENT with a provider From aa5e28be9ebffc4acb254e4a23b6762fe7db5bd5 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Tue, 3 Nov 2026 13:06:28 -0500 Subject: [PATCH 2/5] Manual and changelog for the unreadable profile limits Co-Authored-By: Claude Opus 5.5 --- ...-config-limits.md => 2325-unreadable-config-limits.md} | 2 +- internal/manual/chat/permissions.md | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) rename docs/changes/unreleased/{PRNUMBER-unreadable-config-limits.md => 2325-unreadable-config-limits.md} (98%) diff --git a/docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md b/docs/changes/unreleased/2325-unreadable-config-limits.md similarity index 98% rename from docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md rename to docs/changes/unreleased/2325-unreadable-config-limits.md index b55ef4195a..ab229afc54 100644 --- a/docs/changes/unreleased/PRNUMBER-unreadable-config-limits.md +++ b/docs/changes/unreleased/2325-unreadable-config-limits.md @@ -1,7 +1,7 @@ --- kind: fixed title: codeaf keeps unreadable profile approval limits in factory stages and task children -pr: PRNUMBER +pr: 2325 surface: [engine, factory] invalidates: - "A factory stage's allow posture bypassed an unreadable profile's saved approval rules. It now requires a person when those rules cannot be read." diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index 6ef8e1a50a..ad23c91d47 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -613,10 +613,10 @@ cannot answer these questions for you. A headless call with nobody to ask return an error naming the file. Once the file is readable, its rules apply again. Malformed JSON still fails policy construction. -Two kinds of work do not get this protection yet. A default Factory stage runs on -its own allow posture and does not read your saved rules, whether the file is -readable or not. A task child started by a conversation does not carry the failed -read over from its parent, so it can run a call your saved rules would deny. +Factory stages, task workers, repair rounds and auditors get the same protection: +every call needs a person, and with nobody to ask it is refused. One limit remains. +With a readable file, a default Factory stage runs on its own allow posture and +does not apply your saved rules. ## Default YOLO in interactive conversations and headless runs From a4f1b4238d17d01e22246cba8ff42b4bb745e6c9 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Tue, 3 Nov 2026 13:07:04 -0500 Subject: [PATCH 3/5] Manual uses the person's words for task work Co-Authored-By: Claude Opus 5.5 --- internal/manual/chat/permissions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index ad23c91d47..abb3aa0548 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -613,7 +613,7 @@ cannot answer these questions for you. A headless call with nobody to ask return an error naming the file. Once the file is readable, its rules apply again. Malformed JSON still fails policy construction. -Factory stages, task workers, repair rounds and auditors get the same protection: +Factory stages and every kind of work a task starts get the same protection: every call needs a person, and with nobody to ask it is refused. One limit remains. With a readable file, a default Factory stage runs on its own allow posture and does not apply your saved rules. From 33c7e845e5335e4c2d5a2e5124c3127d4deddd20 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Tue, 3 Nov 2026 13:51:38 -0500 Subject: [PATCH 4/5] fix: retain live unreadable profile limits in child work Keep filesystem failures distinct from readable parse errors, carry the current failed-read signal through child gates and standalone run workers, and retain terminal config-file refusals in job results. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/factory_stage.go | 25 ++- cmd/codeaf/factory_stage_profile_job_test.go | 125 ++++++++++++++ cmd/codeaf/factory_stage_unreadable_test.go | 55 +++++++ .../2325-unreadable-config-limits.md | 11 +- internal/approval/approval.go | 6 + internal/factory/run/chatstage.go | 6 + internal/manual/chat/permissions.md | 18 ++- internal/run/bashworker.go | 8 +- internal/run/bashworker_profile_test.go | 66 ++++++++ internal/run/crew.go | 1 + internal/session/agent.go | 1 + internal/session/approvalgate.go | 16 +- internal/session/bashbelt_profile_test.go | 52 ++++++ internal/session/bashbelt_worker.go | 37 ++++- internal/session/child_profile_live_test.go | 152 ++++++++++++++++++ internal/session/consent.go | 12 +- internal/session/loop.go | 2 + internal/session/session.go | 6 +- internal/session/task_audit.go | 2 +- internal/session/task_run.go | 3 +- 20 files changed, 571 insertions(+), 33 deletions(-) create mode 100644 cmd/codeaf/factory_stage_profile_job_test.go create mode 100644 internal/run/bashworker_profile_test.go create mode 100644 internal/session/bashbelt_profile_test.go create mode 100644 internal/session/child_profile_live_test.go diff --git a/cmd/codeaf/factory_stage.go b/cmd/codeaf/factory_stage.go index 8c902a6b22..93dbe370d0 100644 --- a/cmd/codeaf/factory_stage.go +++ b/cmd/codeaf/factory_stage.go @@ -304,14 +304,16 @@ type stageApprovalGate struct { // Build is the gate for one posture. func (g stageApprovalGate) Build(posture string) (*approval.Policy, bool, error) { - if posture != session.PostureAllow { - return g.rows.Build(posture) - } if g.rows != nil { - if _, err := config.ReadProfileApprovalConfig(g.rows.profileDir); err != nil { - return g.rows.Build(posture) + _, err := config.ReadProfileApprovalConfig(g.rows.profileDir) + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return &approval.Policy{ProfileReadError: err.Error()}, false, nil } } + if posture != session.PostureAllow { + return g.rows.Build(posture) + } policy, err := stageAllowPolicy() return policy, false, err } @@ -492,6 +494,10 @@ func (c *stageConversation) drain(events <-chan session.Event) { } c.say(factory.ToolCaptionPrefix + factory.ToolLogText(hint, c.dir)) case session.EventToolFailed: + var refused *approval.ProfileReadFailure + if errors.As(event.Err, &refused) { + c.fail(refused) + } reason := event.Hint if reason == "" && event.Err != nil { reason = event.Err.Error() @@ -525,7 +531,10 @@ func (c *stageConversation) fail(err error) { } c.mu.Lock() defer c.mu.Unlock() - c.failure = err + var refused *approval.ProfileReadFailure + if !errors.As(c.failure, &refused) { + c.failure = err + } } // say puts one caption on the account, dropping it rather than holding the @@ -637,6 +646,10 @@ func (c *stageConversation) Wait(ctx context.Context) error { case <-done: c.mu.Lock() defer c.mu.Unlock() + var refused *approval.ProfileReadFailure + if errors.As(c.failure, &refused) { + return refused + } if c.budget != nil { if why := c.budget.Refusal(); why != "" { // A daily refusal already recorded was not caused by memory, so diff --git a/cmd/codeaf/factory_stage_profile_job_test.go b/cmd/codeaf/factory_stage_profile_job_test.go new file mode 100644 index 0000000000..3017120d94 --- /dev/null +++ b/cmd/codeaf/factory_stage_profile_job_test.go @@ -0,0 +1,125 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/factory" + factoryrun "github.com/Agent-Field/codeaf/internal/factory/run" + "github.com/Agent-Field/codeaf/internal/factory/store" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/remote" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +func TestUnreadableStageJobKeepsConfigReason(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + root := resolvedTempDir(t) + t.Setenv("HOME", filepath.Join(root, "home")) + t.Setenv(home.EnvVar, filepath.Join(root, "state")) + t.Setenv(config.ProfileDirEnv, "") + t.Setenv("OPENROUTER_API_KEY", "test-key") + t.Setenv(factoryPostureEnv, "") + workspace := filepath.Join(root, "work") + if err := os.MkdirAll(workspace, 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(workspace, "NOTES.md") + writeArgs, _ := json.Marshal(map[string]string{"path": target, "content": "written by the stage\n"}) + model := newScriptedStageModel(t, stageCall{"write", string(writeArgs)}, stageCall{"stage_result", stageModelReport}) + t.Setenv("CODEAF_BASE_URL", model.server.URL) + t.Chdir(workspace) + freshEngineProcess(t) + stageSettle = 20 * time.Millisecond + t.Cleanup(func() { stageSettle = unattendedRunSettleTick }) + + proc, err := openEngineProcess() + if err != nil { + t.Fatal(err) + } + launch, err := openV3Launch(proc, engineLaunchOptions(remote.Hello{Version: remote.Version, Workspace: workspace}, workspace, "")) + if err != nil { + t.Fatal(err) + } + st, err := store.Open(filepath.Join(root, "factory")) + if err != nil { + t.Fatal(err) + } + id, err := st.Add(context.Background(), factory.Item{ + Title: "write the notes", Repo: "work", Tier: factory.TierOwner, + Stages: []factory.Stage{{Name: "write", Kind: factory.StageChat, Ask: "write the notes", Until: "done", On: true}}, + }) + if err != nil { + t.Fatal(err) + } + if err := st.Update(id, func(it *factory.Item) error { it.Stream = &factory.Stream{}; return nil }); err != nil { + t.Fatal(err) + } + it, _ := st.Get(id) + + profile := "" + file := config.BudgetConfigPath(profile) + if err := os.MkdirAll(filepath.Dir(file), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(file, []byte(`{"tools.approvals":"write:deny"}`), 0600); err != nil { + t.Fatal(err) + } + oldHandler := model.server.Config.Handler + var once sync.Once + model.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + r.Body = io.NopCloser(bytes.NewReader(body)) + if bytes.Contains(body, []byte(`"stage_result"`)) { + once.Do(func() { + if err := os.Chmod(file, 0); err != nil { + t.Error(err) + } + }) + } + oldHandler.ServeHTTP(w, r) + }) + t.Cleanup(func() { _ = os.Chmod(file, 0600) }) + maker := factoryStageMaker(st, workspace, profile, launch.Config) + exec := factoryChatStage(st, profile, factoryrun.NewChatExecutor(maker)) + ctx, cancel := context.WithTimeout(context.Background(), 12*time.Second) + defer cancel() + res, err := exec.Run(ctx, factoryrun.Job{Item: it, Stage: it.Stages[0], Index: 0, Round: 1, Dir: workspace, Log: func(line string) { + t.Logf("JOB LOG: %s", line) + if strings.Contains(line, "reminded") { + t.Error("terminal profile refusal must not retry reporting") + } + }}) + if err != nil { + t.Fatalf("ROUND ERROR: %v", err) + } + t.Logf("ROUND RESULT: %+v", res) + if res.Done || res.Exit != 1 || !strings.Contains(res.Output, file) || !strings.Contains(res.Output, "saved approval rules could not be read") { + t.Errorf("job lost the terminal profile-read reason: %+v", res) + } + model.mu.Lock() + defer model.mu.Unlock() + for _, brief := range model.briefs { + if i := strings.Index(brief, "no resolver is attached"); i >= 0 { + t.Logf("MODEL RECEIVED REFUSAL: %.650s", brief[i:]) + } + } + got, readErr := os.ReadFile(target) + t.Logf("TARGET: content=%q err=%v", got, readErr) + if !os.IsNotExist(readErr) { + t.Fatalf("unreadable stage wrote file") + } + if ctx.Err() != nil { + t.Fatalf("stage failed to stop before timeout: %v", ctx.Err()) + } +} diff --git a/cmd/codeaf/factory_stage_unreadable_test.go b/cmd/codeaf/factory_stage_unreadable_test.go index 4712ce6659..2faa70c1d9 100644 --- a/cmd/codeaf/factory_stage_unreadable_test.go +++ b/cmd/codeaf/factory_stage_unreadable_test.go @@ -38,3 +38,58 @@ func TestStageUnreadableProfileRequiresPerson(t *testing.T) { }) } } + +func TestStageProfileReadFailureWinsOverProjectParseError(t *testing.T) { + profile, workspace := t.TempDir(), t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.Mkdir(file, 0o700); err != nil { + t.Fatal(err) + } + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("expected a filesystem read failure") + } + if err := os.Mkdir(filepath.Join(workspace, ".codeaf"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, ".codeaf", "config.json"), []byte("{"), 0o600); err != nil { + t.Fatal(err) + } + for _, posture := range []string{session.PostureAllow, session.PostureAsk, session.PostureDeny, session.PostureGuardian, session.PostureAuto} { + t.Run(posture, func(t *testing.T) { + var gate session.ApprovalGate = stageApprovalGate{rows: &v3ApprovalGate{workspace: workspace, profileDir: profile}, posture: posture} + policy, guardian, err := gate.Build(posture) + if err != nil { + t.Fatalf("known profile failure lost to project error: %v", err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"notes.txt","content":"no"}`)) + if guardian || decision.Action != approval.ActionPrompt || !strings.Contains(decision.ProfileReadError, file) { + t.Fatalf("lost profile failure: guardian=%v decision=%+v", guardian, decision) + } + }) + } +} + +func TestStageReadableMalformedProfileKeepsDefaultAllowance(t *testing.T) { + for _, contents := range []string{"{", "null", "[]"} { + t.Run(contents, func(t *testing.T) { + profile := t.TempDir() + if err := os.WriteFile(filepath.Join(profile, "config.json"), []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + rows := &v3ApprovalGate{workspace: t.TempDir(), profileDir: profile} + var gate session.ApprovalGate = stageApprovalGate{rows: rows, posture: session.PostureAllow} + policy, guardian, err := gate.Build(session.PostureAllow) + if err != nil { + t.Fatal(err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"notes.txt","content":"ok"}`)) + if guardian || decision.Action != approval.ActionAllow || decision.ProfileReadError != "" { + t.Fatalf("readable default-stage bypass changed: guardian=%v decision=%+v", guardian, decision) + } + if _, _, err := gate.Build(session.PostureAsk); err == nil { + t.Fatal("readable malformed JSON must still fail ordinary row construction") + } + }) + } +} diff --git a/docs/changes/unreleased/2325-unreadable-config-limits.md b/docs/changes/unreleased/2325-unreadable-config-limits.md index ab229afc54..61342502c6 100644 --- a/docs/changes/unreleased/2325-unreadable-config-limits.md +++ b/docs/changes/unreleased/2325-unreadable-config-limits.md @@ -5,9 +5,18 @@ pr: 2325 surface: [engine, factory] invalidates: - "A factory stage's allow posture bypassed an unreadable profile's saved approval rules. It now requires a person when those rules cannot be read." - - "Task workers, repair rounds and auditors dropped their parent's profile read failure. They now retain that signal and refuse calls that need a person when nobody can answer." + - "Task workers, repair rounds and auditors dropped their parent's profile read failure. They now consult that signal at each decision and refuse calls that need a person when nobody can answer." + - "A project parse error could hide a known profile read failure. The stage now returns the failure already in hand before reading project rules." + - "Standalone run workers discarded profile read failures. They now check their run's profile before each call, including after work starts." + - "A refused job could lose the config-file reason during report recovery. Its result now keeps that reason without futile report retries." --- A readable profile keeps its existing behavior, including a factory stage's own allow posture. An unreadable profile cannot silently discard a saved denial: the existing approval gate reports the failed file read and requires a person. + +The named readable default-stage bypass includes readable malformed JSON. +Other postures retain their existing parse-error behavior. Filesystem read +failures remain distinct from JSON parse failures. Task, repair, checker and +fleet children keep their own normal allowance while following the parent's +current failed-read signal; recovery clears that inherited signal too. diff --git a/internal/approval/approval.go b/internal/approval/approval.go index f6e20169fc..48f07369b0 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -450,3 +450,9 @@ func toList(value any) ([]any, bool) { return nil, false } } + +// ProfileReadFailure is an unattended call refused because the saved rules +// could not be read. Runners retain this reason ahead of report recovery. +type ProfileReadFailure struct{ Reason string } + +func (e *ProfileReadFailure) Error() string { return e.Reason } diff --git a/internal/factory/run/chatstage.go b/internal/factory/run/chatstage.go index 8acdb7b139..8ab28a6ba1 100644 --- a/internal/factory/run/chatstage.go +++ b/internal/factory/run/chatstage.go @@ -45,6 +45,7 @@ import ( "sync" "time" + "github.com/Agent-Field/codeaf/internal/approval" "github.com/Agent-Field/codeaf/internal/factory" "github.com/Agent-Field/codeaf/internal/factory/verification" ) @@ -402,6 +403,11 @@ loop: // one that is idle with nothing reported is asked again (stagereport.go) // until there is nothing left to ask. A spending limit is never asked // past: the turn it would start is refused before it begins. + var refused *approval.ProfileReadFailure + if errors.As(waitErr, &refused) { + idleFailure = &factory.StageResult{Exit: 1, Output: refused.Error()} + break loop + } if _, stopped := spendStopOf(waitErr); stopped { break loop } diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index abb3aa0548..31fefcd235 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -611,12 +611,18 @@ whatever posture or `--yolo` you selected. The prompt names the file and explain that saved approval rules could not be read. Remembered approvals and the guardian cannot answer these questions for you. A headless call with nobody to ask returns an error naming the file. Once the file is readable, its rules apply again. -Malformed JSON still fails policy construction. - -Factory stages and every kind of work a task starts get the same protection: -every call needs a person, and with nobody to ask it is refused. One limit remains. -With a readable file, a default Factory stage runs on its own allow posture and -does not apply your saved rules. +Malformed JSON still fails ordinary policy construction; the readable default-stage +bypass below also applies to malformed JSON. + +Factory stages and work started by tasks get the same protection: every call +needs a person, and with nobody to ask it is refused. Workers, repair rounds and +checkers see a parent's current read failure even after they start. Standalone +run workers check the profile before each call. A refused Factory job keeps the +failed-read reason and config filename in its result, without report retries. + +**Readable default-stage bypass:** with a readable file, a default Factory stage +runs on its own allow posture and does not apply your saved rules. This named +limit also covers readable malformed JSON. ## Default YOLO in interactive conversations and headless runs diff --git a/internal/run/bashworker.go b/internal/run/bashworker.go index dcae2cf52e..771ec22bf9 100644 --- a/internal/run/bashworker.go +++ b/internal/run/bashworker.go @@ -58,9 +58,10 @@ import ( // CODEAF_TASK_BELT names the older belt, so on that road not one byte of any // prompt, belt or landing changes — nothing constructs this worker. type BashWorker struct { - store *plandb.Store - workspace string - model string + profileDir string + store *plandb.Store + workspace string + model string // always is the memory section over this place — every line kept for the // person, this machine and this project — rendered by the door // ([session.MemoryWorld]) and closed on by the worker's brief. It is the @@ -177,6 +178,7 @@ func (w *BashWorker) Run(ctx context.Context, task plandb.Task) (rep Report, run } agent, err := session.NewBeltWorker(session.Config{ Effort: w.taskEffort, + ApprovalGate: session.BeltWorkerApprovalGate(w.profileDir), Workspace: w.workspace, WorkspaceURI: w.workspaceURI, WorkspaceSet: w.workspaceSet.Clone(), diff --git a/internal/run/bashworker_profile_test.go b/internal/run/bashworker_profile_test.go new file mode 100644 index 0000000000..16b32a1ae6 --- /dev/null +++ b/internal/run/bashworker_profile_test.go @@ -0,0 +1,66 @@ +package run_test + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/run" + "github.com/Agent-Field/codeaf/internal/session" +) + +func TestCrewWorkerReadsCurrentProfileBeforeEachCall(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_PLANDB_BIN", stubCLI(t)) + t.Setenv("CODEAF_HOME", t.TempDir()) + profile, workspace := t.TempDir(), t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.WriteFile(file, []byte(`{}`), 0o600); err != nil { + t.Fatal(err) + } + var refused bool + s := &seat{script: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolReply(`{"command":"printf BEFORE > before.txt"}`), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + if err := os.Remove(file); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(file, 0o700); err != nil { + t.Fatal(err) + } + return toolReply(`{"command":"printf AFTER > after.txt"}`), nil + }, + func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + for _, m := range messages { + text := messageContent(m) + if m.Role == "tool" && strings.Contains(text, file) && strings.Contains(text, "saved approval rules could not be read") { + refused = true + } + } + return nil, errors.New("API error (401): fixture complete") + }, + }} + store := runOpenStore(t) + factory := run.CrewFactory(store, workspace, profile, run.Seats{One: "test/model"}, "", false, func(string) session.Completer { return s }) + worker := factory(*store.Task(store.RootID())) + _, err := worker.Run(runContext(t), *store.Task(store.RootID())) + if err == nil { + t.Fatal("scripted provider should end with its authentication refusal") + } + t.Logf("scripted provider ending: %v", err) + if _, err := os.Stat(filepath.Join(workspace, "before.txt")); err != nil { + t.Fatalf("readable worker failed: %v", err) + } + if _, err := os.Stat(filepath.Join(workspace, "after.txt")); !os.IsNotExist(err) { + t.Errorf("worker wrote after failed read: %v", err) + } + if !refused { + t.Fatal("worker lost config filename and failed-read reason") + } +} diff --git a/internal/run/crew.go b/internal/run/crew.go index 05cc02339c..0b72ea8b72 100644 --- a/internal/run/crew.go +++ b/internal/run/crew.go @@ -151,6 +151,7 @@ func crewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, dailyBudgetUSD, _ := config.DailyBudgetUSDAt(profileDir) workerFor := func(model string, completer session.Completer) *BashWorker { worker := NewBashWorker(store, workspace, model, completer, workerOpts...) + worker.profileDir = profileDir worker.dailyBudgetUSD = dailyBudgetUSD worker.spendPreauthorized = preauthorized // The memory section rides on every worker this run seats, read once by diff --git a/internal/session/agent.go b/internal/session/agent.go index 121574241d..3f5b6a101e 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -116,6 +116,7 @@ func (a *Agent) newChildAgent(config Config) (*Agent, error) { if err != nil { return nil, err } + child.approvalReadParent = a child.managedClient = managed child.clientAccount = account child.clientPool = pool diff --git a/internal/session/approvalgate.go b/internal/session/approvalgate.go index 8452fde780..34c0b230dc 100644 --- a/internal/session/approvalgate.go +++ b/internal/session/approvalgate.go @@ -25,16 +25,6 @@ package session import "github.com/Agent-Field/codeaf/internal/approval" -// taskApprovalPolicy preserves the failed-read signal while keeping the task's -// own allowance. Unknown saved denials still require a person in every child. -func (a *Agent) taskApprovalPolicy() *approval.Policy { - policy := &approval.Policy{Default: approval.ActionAllow} - if parent := a.approvalGate(); parent != nil { - policy.ProfileReadError = parent.ProfileReadError - } - return policy -} - // SetApprovalPolicy replaces the standing gate for the rest of this session. // // A NIL NEVER REPLACES ANYTHING. Nil is the configured-nothing case and it @@ -72,6 +62,12 @@ func (a *Agent) SetApprovalPolicy(policy *approval.Policy) { // [Agent.SetApprovalPolicy] existed, a single unguarded read became a race the // detector finds in a second and a person would never reproduce. func (a *Agent) approvalGate() *approval.Policy { + if a.approvalReadParent != nil { + if parent := a.approvalReadParent.approvalGate(); parent != nil && parent.ProfileReadError != "" { + return &approval.Policy{ProfileReadError: parent.ProfileReadError} + } + } + if source, ok := a.jobBudgetOf().(interface{ FactoryAuto() bool }); ok && source.FactoryAuto() { return automaticJobPolicy(a.baseApprovalGate()) } diff --git a/internal/session/bashbelt_profile_test.go b/internal/session/bashbelt_profile_test.go new file mode 100644 index 0000000000..4002af4faf --- /dev/null +++ b/internal/session/bashbelt_profile_test.go @@ -0,0 +1,52 @@ +package session + +import ( + "context" + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/plandb" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBeltWorkerKeepsProfileReadFailure(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv(planCLIBinEnv, filepath.Join(t.TempDir(), "stub-codeaf")) + profile := t.TempDir() + if err := os.Mkdir(filepath.Join(profile, "config.json"), 0o700); err != nil { + t.Fatal(err) + } + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("expected read failure") + } + dir := t.TempDir() + store, err := plandb.Open(filepath.Join(dir, planStoreFilename), "the work", planRootID, "", "") + if err != nil { + t.Fatal(err) + } + workspace := t.TempDir() + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", "bash", `{"command":"printf WRITTEN > denied.txt"}`), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + cfg := Config{Workspace: workspace, Model: "test/model", System: "SYSTEM", ApprovalPolicy: &approval.Policy{ProfileReadError: readErr.Error()}} + child, err := NewBeltWorker(cfg, script, store.Task(planRootID), store.Path(), store.RootID()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + events := collect(t, mustSubmit(t, child, "write")) + failed, ok := firstOfKind(events, EventToolFailed) + _, statErr := os.Stat(filepath.Join(workspace, "denied.txt")) + t.Logf("belt refusal=%v reason=%q file_exists=%v", ok, failed.Output, statErr == nil) + if !ok || !strings.Contains(failed.Output, readErr.Error()) || !os.IsNotExist(statErr) { + t.Fatal("belt worker lost unreadable signal") + } +} diff --git a/internal/session/bashbelt_worker.go b/internal/session/bashbelt_worker.go index e10c177e40..e443c20bf2 100644 --- a/internal/session/bashbelt_worker.go +++ b/internal/session/bashbelt_worker.go @@ -32,6 +32,7 @@ import ( "strings" "github.com/Agent-Field/codeaf/internal/approval" + profileconfig "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/effort" "github.com/Agent-Field/codeaf/internal/plandb" "github.com/Agent-Field/codeaf/internal/router" @@ -90,7 +91,14 @@ func NewBeltWorker(config Config, completer Completer, task *plandb.Task, storeP // allow into a prompt for the shapes that destroy a disk or drop the // machine, and a prompt in a worker is a refusal it can read — never a // question and never a hang. - config.ApprovalPolicy = &approval.Policy{Default: approval.ActionAllow} + policy := &approval.Policy{Default: approval.ActionAllow} + if config.ApprovalPolicy != nil { + policy.ProfileReadError = config.ApprovalPolicy.ProfileReadError + } + config.ApprovalPolicy = policy + if config.ApprovalGate == nil { + config.ApprovalGate = beltWorkerApprovalGate{policy: policy, profileDir: config.ProfileDir} + } config.AskConsent = false // THE WORKER'S OWN RECORD FOLDER. The transcript this agent writes and the // whole outputs the belt's cut files land in the task's own folder beside @@ -365,3 +373,30 @@ func (a *Agent) sendBeltStep(ctx context.Context, hub *eventHub, event Event) { case <-ctx.Done(): } } + +// BeltWorkerApprovalGate supplies live failed-read protection to a standalone +// run worker without giving unrelated session settings a different profile. +func BeltWorkerApprovalGate(profileDir string) ApprovalGate { + return beltWorkerApprovalGate{profileDir: profileDir, policy: &approval.Policy{Default: approval.ActionAllow}} +} + +// beltWorkerApprovalGate checks the run's profile at each decision because a +// standalone run worker has no parent agent whose current gate it can consult. +// Readable files retain the worker's own allowance, including parse failures. +type beltWorkerApprovalGate struct { + policy *approval.Policy + profileDir string +} + +func (g beltWorkerApprovalGate) Standing() string { + return PostureAllow +} + +func (g beltWorkerApprovalGate) Build(_ string) (*approval.Policy, bool, error) { + _, err := profileconfig.ReadProfileApprovalConfig(g.profileDir) + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return &approval.Policy{ProfileReadError: err.Error()}, false, nil + } + return g.policy, false, nil +} diff --git a/internal/session/child_profile_live_test.go b/internal/session/child_profile_live_test.go new file mode 100644 index 0000000000..babe7a99f2 --- /dev/null +++ b/internal/session/child_profile_live_test.go @@ -0,0 +1,152 @@ +package session + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/orchestrate" +) + +func newProfileTestChild(t *testing.T, parent *Agent, kind, workspace string) *Agent { + t.Helper() + g := parent.graph() + g.run = func(*TaskNode) {} + id := g.reserve() + g.admit(id, taskSpec{title: "work", brief: "answer", acceptance: "done"}) + var child *Agent + var err error + switch kind { + case "worker": + child, err = parent.newTaskAgent(context.Background(), workspace, g.node(id), "") + case "repair": + child, err = parent.newTaskAgentOn(context.Background(), workspace, g.node(id), "-repair1", "", true) + case "auditor": + child, err = parent.newAuditAgent(workspace, g.node(id), plainDoor(auditReadCommands), "") + case "fleet": + executor := &orchestrateExec{agent: parent, id: "work"} + child, err = executor.newChild(workspace, orchestrate.Node{ID: "n1", Goal: "answer"}) + default: + t.Fatalf("unknown child kind %q", kind) + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + return child +} + +func profileTestReadFailure(t *testing.T) string { + t.Helper() + profile := t.TempDir() + if err := os.Mkdir(filepath.Join(profile, "config.json"), 0o700); err != nil { + t.Fatal(err) + } + _, err := config.ReadProfileApprovalConfig(profile) + if err == nil { + t.Fatal("expected filesystem read failure") + } + return err.Error() +} + +func TestChildrenObserveCurrentProfileReadFailure(t *testing.T) { + for _, kind := range []string{"worker", "repair", "auditor", "fleet"} { + for _, after := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/failed_after_creation=%v", kind, after), func(t *testing.T) { + tool, args := "write", `{"path":"denied.txt","content":"must not be written"}` + if kind == "auditor" { + tool, args = "bash", `{"command":"pwd"}` + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", tool, args), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + parent, _ := newTestAgent(t, script, func(c *Config) { c.OneModel = true; c.ApprovalPolicy = &approval.Policy{Default: approval.ActionAllow} }) + readError := profileTestReadFailure(t) + failed := &approval.Policy{ProfileReadError: readError} + if !after { + parent.SetApprovalPolicy(failed) + } + workspace := t.TempDir() + child := newProfileTestChild(t, parent, kind, workspace) + if after { + parent.SetApprovalPolicy(failed) + } + events := collect(t, mustSubmit(t, child, "attempt the call")) + refusal, ok := firstOfKind(events, EventToolFailed) + if !ok || !strings.Contains(refusal.Output, readError) { + t.Fatalf("lost current profile failure: events=%v refusal=%q", kinds(events), refusal.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "denied.txt")); !os.IsNotExist(err) { + t.Fatalf("refused write created file: %v", err) + } + }) + } + } +} + +func TestChildObservesProfileFailureDuringTurn(t *testing.T) { + for _, kind := range []string{"worker", "repair", "auditor", "fleet"} { + for _, stage := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/stage=%v", kind, stage), func(t *testing.T) { + readError := profileTestReadFailure(t) + // The child must keep its own allowance even when the parent's rules deny calls. + normal := &approval.Policy{Default: approval.ActionDeny} + var parent *Agent + call := func(id string) *ai.Response { + if kind == "auditor" { + return toolResponse(id, "bash", `{"command":"pwd"}`) + } + return toolResponse(id, "write", fmt.Sprintf(`{"path":%q,"content":"written"}`, id+".txt")) + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { return call("before"), nil }, + func(context.Context, []ai.Message) (*ai.Response, error) { + parent.SetApprovalPolicy(&approval.Policy{ProfileReadError: readError}) + return call("after"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + parent.SetApprovalPolicy(normal) + return call("recovered"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + parent, _ = newTestAgent(t, script, func(c *Config) { + c.OneModel = true + c.ApprovalPolicy = normal + if stage { + c.Stage = &fakeStageDoor{} + c.Unattended = true + } + }) + workspace := t.TempDir() + child := newProfileTestChild(t, parent, kind, workspace) + events := collect(t, mustSubmit(t, child, "attempt three calls")) + succeeded := make(map[string]bool) + var refusal Event + for _, event := range events { + if event.Kind == EventToolEnd { + succeeded[event.CallID] = true + } + if event.Kind == EventToolFailed && event.CallID == "after" { + refusal = event + } + } + if !succeeded["before"] || !succeeded["recovered"] || succeeded["after"] || !strings.Contains(refusal.Output, readError) { + t.Fatalf("child must allow, refuse, then allow on its own rules: succeeded=%v refusal=%q", succeeded, refusal.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "after.txt")); !os.IsNotExist(err) { + t.Fatalf("refused write created file: %v", err) + } + }) + } + } +} diff --git a/internal/session/consent.go b/internal/session/consent.go index 012963f2e1..bdc47278aa 100644 --- a/internal/session/consent.go +++ b/internal/session/consent.go @@ -281,14 +281,14 @@ func (a *Agent) approve(ctx context.Context, hub *eventHub, call ai.ToolCall) (t // worker with no colleague in the room is that this one needed a person and // there is not one. if a.config.InTask { - return refusal("refused in a task: " + decision.Rule + " — nobody to ask"), false + return approvalRefusal("refused in a task: "+decision.Rule+" — nobody to ask", decision), false } // Nobody is watching. Denying is the only honest answer: blocking would // hang a headless run forever on a question with no reader, and allowing // would make "prompt" mean "allow" wherever the surface is not a terminal. if !a.config.AskConsent || hub == nil { - return refusal("needs approval but no resolver is attached: " + decision.Rule), false + return approvalRefusal("needs approval but no resolver is attached: "+decision.Rule, decision), false } allowed, err := a.ask(ctx, hub, call, decision) @@ -538,6 +538,14 @@ func (a *Agent) PendingConsent() []uint64 { return ids } +func approvalRefusal(reason string, decision approval.Decision) toolResult { + result := refusal(reason) + if decision.ProfileReadError != "" { + result.err = &approval.ProfileReadFailure{Reason: reason} + } + return result +} + // refusal is one refused call as the model sees it: an error result, with the // reason in the words the person would read. func refusal(reason string) toolResult { diff --git a/internal/session/loop.go b/internal/session/loop.go index d00a0b988c..64146e39a4 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -3206,6 +3206,7 @@ func backoffWait(ctx context.Context, delay time.Duration) error { // ── tools ─────────────────────────────────────────────────────────────────── type toolResult struct { + err error text string isError bool interruption string @@ -3670,6 +3671,7 @@ func (a *Agent) runToolsWarm(ctx context.Context, ep *episode, calls []ai.ToolCa if results[index].isError { a.sendBeltStep(ctx, hub, Event{ Kind: EventToolFailed, + Err: results[index].err, Tool: call.Function.Name, Hint: clip(firstLine(results[index].text), hintLimit), Args: rendered[index], diff --git a/internal/session/session.go b/internal/session/session.go index 1e5df8f920..b78ea9ff8d 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2799,8 +2799,10 @@ type Agent struct { crewDayHeld *SpendDay // Clarification streams and their deferred history share the agent lock. // questionParent is installed before a child becomes reachable. - questionParent func(Event) - approvalParent *Agent + questionParent func(Event) + approvalParent *Agent + // approvalReadParent shares only failed reads; the child keeps its own rules. + approvalReadParent *Agent discussionEvents []Event discussionEventSeq uint64 discussionRecorded map[string]bool diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index b8b93bb69a..ffd71de6cb 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -3212,7 +3212,7 @@ func (a *Agent) newAuditAgent(dir string, node *TaskNode, door auditDoor, on str // that writes. AskConsent is off and InTask is on for the node's own // reason — there is nobody in a worktree to ask. // An unreadable profile still requires a person, just as it does for a worker. - ApprovalPolicy: a.taskApprovalPolicy(), + ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, AskConsent: false, InTask: true, // AND ITS CALLS ARE A JUDGE'S, NOT A WORKER'S. InTask above is about diff --git a/internal/session/task_run.go b/internal/session/task_run.go index b2bc4f2d3a..71d60a06a1 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -90,6 +90,7 @@ import ( "time" "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" "github.com/Agent-Field/codeaf/internal/cell" "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/effort" @@ -7899,7 +7900,7 @@ func (a *Agent) newTaskAgentOn(ctx context.Context, dir string, node *TaskNode, // a disk or drop the machine, and a prompt in a node is a refusal it can // read (consent.go, InTask) — never a question and never a hang. // An unreadable profile also requires a person because saved denials are unknown. - ApprovalPolicy: a.taskApprovalPolicy(), + ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, AskConsent: false, InTask: true, // InTask is also what makes this agent's calls PATIENT with a provider From 8c908c5645e473d44f745b1632a1a550cbd1f466 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Tue, 3 Nov 2026 14:21:04 -0500 Subject: [PATCH 5/5] fix: retain config read reason in budget admission The daily-limit refusal discarded the config read error before a stage could start. Keep that error in the existing refusal without changing admission or accounting. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/factory_stage_profile_job_test.go | 51 ++++++++++++++----- .../2325-unreadable-config-limits.md | 5 +- internal/manual/chat/permissions.md | 2 + internal/session/rail.go | 2 +- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/cmd/codeaf/factory_stage_profile_job_test.go b/cmd/codeaf/factory_stage_profile_job_test.go index 3017120d94..d2a43521cc 100644 --- a/cmd/codeaf/factory_stage_profile_job_test.go +++ b/cmd/codeaf/factory_stage_profile_job_test.go @@ -21,6 +21,15 @@ import ( ) func TestUnreadableStageJobKeepsConfigReason(t *testing.T) { + testUnreadableStageJobKeepsConfigReason(t, false) +} + +func TestInitiallyUnreadableStageJobKeepsConfigReason(t *testing.T) { + testUnreadableStageJobKeepsConfigReason(t, true) +} + +func testUnreadableStageJobKeepsConfigReason(t *testing.T, initiallyUnreadable bool) { + t.Helper() if os.Geteuid() == 0 { t.Skip("mode 000 requires nonroot") } @@ -75,20 +84,28 @@ func TestUnreadableStageJobKeepsConfigReason(t *testing.T) { if err := os.WriteFile(file, []byte(`{"tools.approvals":"write:deny"}`), 0600); err != nil { t.Fatal(err) } - oldHandler := model.server.Config.Handler - var once sync.Once - model.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, _ := io.ReadAll(r.Body) - r.Body = io.NopCloser(bytes.NewReader(body)) - if bytes.Contains(body, []byte(`"stage_result"`)) { - once.Do(func() { - if err := os.Chmod(file, 0); err != nil { - t.Error(err) - } - }) + reason := "saved approval rules could not be read" + if initiallyUnreadable { + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) } - oldHandler.ServeHTTP(w, r) - }) + reason = "permission denied" + } else { + oldHandler := model.server.Config.Handler + var once sync.Once + model.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + r.Body = io.NopCloser(bytes.NewReader(body)) + if bytes.Contains(body, []byte(`"stage_result"`)) { + once.Do(func() { + if err := os.Chmod(file, 0); err != nil { + t.Error(err) + } + }) + } + oldHandler.ServeHTTP(w, r) + }) + } t.Cleanup(func() { _ = os.Chmod(file, 0600) }) maker := factoryStageMaker(st, workspace, profile, launch.Config) exec := factoryChatStage(st, profile, factoryrun.NewChatExecutor(maker)) @@ -104,11 +121,17 @@ func TestUnreadableStageJobKeepsConfigReason(t *testing.T) { t.Fatalf("ROUND ERROR: %v", err) } t.Logf("ROUND RESULT: %+v", res) - if res.Done || res.Exit != 1 || !strings.Contains(res.Output, file) || !strings.Contains(res.Output, "saved approval rules could not be read") { + if res.Done || res.Exit != 1 || !strings.Contains(res.Output, file) || !strings.Contains(res.Output, reason) { t.Errorf("job lost the terminal profile-read reason: %+v", res) } model.mu.Lock() defer model.mu.Unlock() + if initiallyUnreadable { + t.Logf("STAGE MODEL REQUESTS: %d", len(model.briefs)) + if len(model.briefs) != 0 || res.Spent != 0 || res.Reports != 0 { + t.Fatalf("refused admission did work: requests=%d spent=%v reports=%d", len(model.briefs), res.Spent, res.Reports) + } + } for _, brief := range model.briefs { if i := strings.Index(brief, "no resolver is attached"); i >= 0 { t.Logf("MODEL RECEIVED REFUSAL: %.650s", brief[i:]) diff --git a/docs/changes/unreleased/2325-unreadable-config-limits.md b/docs/changes/unreleased/2325-unreadable-config-limits.md index 61342502c6..a6095dd0f2 100644 --- a/docs/changes/unreleased/2325-unreadable-config-limits.md +++ b/docs/changes/unreleased/2325-unreadable-config-limits.md @@ -8,7 +8,7 @@ invalidates: - "Task workers, repair rounds and auditors dropped their parent's profile read failure. They now consult that signal at each decision and refuse calls that need a person when nobody can answer." - "A project parse error could hide a known profile read failure. The stage now returns the failure already in hand before reading project rules." - "Standalone run workers discarded profile read failures. They now check their run's profile before each call, including after work starts." - - "A refused job could lose the config-file reason during report recovery. Its result now keeps that reason without futile report retries." + - "A refused job could lose the config-file reason during report recovery or first-turn budget admission. Its result now keeps that reason without futile report retries." --- A readable profile keeps its existing behavior, including a factory stage's own @@ -20,3 +20,6 @@ Other postures retain their existing parse-error behavior. Filesystem read failures remain distinct from JSON parse failures. Task, repair, checker and fleet children keep their own normal allowance while following the parent's current failed-read signal; recovery clears that inherited signal too. + +First-turn daily-limit refusals retain the config filename and read error. +Admission decisions, spending counts and charges are unchanged. diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index 31fefcd235..9d2673a9af 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -619,6 +619,8 @@ needs a person, and with nobody to ask it is refused. Workers, repair rounds and checkers see a parent's current read failure even after they start. Standalone run workers check the profile before each call. A refused Factory job keeps the failed-read reason and config filename in its result, without report retries. +This includes a daily-limit check that refuses the first turn because the +profile cannot be read; no model request is made. **Readable default-stage bypass:** with a readable file, a default Factory stage runs on its own allow posture and does not apply your saved rules. This named diff --git a/internal/session/rail.go b/internal/session/rail.go index 4f22797a8d..604a1c5c8e 100644 --- a/internal/session/rail.go +++ b/internal/session/rail.go @@ -116,7 +116,7 @@ func (a *Agent) railBlockLocked() error { // on a guess. daily, err := a.dailyCeiling() if err != nil { - return spendUnverified{said: "daily limit could not be read · /budget day sets it"} + return spendUnverified{said: fmt.Sprintf("daily limit could not be read: %v · /budget day sets it", err)} } if daily > 0 { spentToday, err := spentTodayOnLedgerChecked()