diff --git a/cmd/org/AGENTS.md b/cmd/org/AGENTS.md index ed5ecb1b..4f0b5949 100644 --- a/cmd/org/AGENTS.md +++ b/cmd/org/AGENTS.md @@ -66,6 +66,31 @@ system name there is Baton; this binary is its first runtime slice. record; a crash mid-composite leaves ordinary recoverable state. Plain `release` now warns on stderr when unfinished items remain held — finished work exits via `complete`/`done`, `yield` means pausing. +- **`adopt`** puts work that is already in flight onto an UNHELD lane's plate, + without the working session's cooperation: `attach` → `note` → `assign` → + `release`, four records, **no claim**. It exists because a session that never + attached still produced a branch, a PR and a head SHA — all observable from + outside — and none of it could become ownership. Two properties are the + kernel's rather than the verb's, and both are what make it safe to hand to a + watcher: it cannot displace a live session (`attach` on a held lane is refused + `already_held`, so adopt can never leak into takeover), and it never claims, + because a lane holds many items and acts on exactly one — putting work on a + plate is not starting it. `-by` is required: an unattributed assign is + indistinguishable from the lane's own coverage sweep, and since `assign` takes + no body the adopter's identity has nowhere to live but the note — which is why + this is a verb rather than a three-command recipe, since in a recipe the note + is the optional step. Refuses when another lane already holds the work + (`work_already_held`) rather than manufacturing the `assign_conflict` sweep + reports — a preflight over the peer chains, not an admission law: appends + lock one role chain at a time and there is no tenant-wide lock or + cross-chain transaction (FOLLOWUPS), so two adoptions of the same work into + two idle lanes that both scan before either assigns will both land, and + `sweep` reports the result as `assign_conflict` — detected, not prevented, + the same posture as `transfer`. Warns on scope drift, like `transfer`. Whoever picks the work up + resumes it with `org begin -work `, no pin needed. It manufactures no + authority — those four appends were always available to any process that can + read the state directory — so WHO may invoke it is a charter question for the + invoking role, not a property of this verb. - **`intake`** is the routing reflex before assign: given `-work ` it reports which chartered lanes' scopes cover it (the `contracts/org.InScope` predicate — prefix-at-a-boundary, never across schemes), which lanes diff --git a/cmd/org/CLAUDE.md b/cmd/org/CLAUDE.md index ed5ecb1b..4f0b5949 100644 --- a/cmd/org/CLAUDE.md +++ b/cmd/org/CLAUDE.md @@ -66,6 +66,31 @@ system name there is Baton; this binary is its first runtime slice. record; a crash mid-composite leaves ordinary recoverable state. Plain `release` now warns on stderr when unfinished items remain held — finished work exits via `complete`/`done`, `yield` means pausing. +- **`adopt`** puts work that is already in flight onto an UNHELD lane's plate, + without the working session's cooperation: `attach` → `note` → `assign` → + `release`, four records, **no claim**. It exists because a session that never + attached still produced a branch, a PR and a head SHA — all observable from + outside — and none of it could become ownership. Two properties are the + kernel's rather than the verb's, and both are what make it safe to hand to a + watcher: it cannot displace a live session (`attach` on a held lane is refused + `already_held`, so adopt can never leak into takeover), and it never claims, + because a lane holds many items and acts on exactly one — putting work on a + plate is not starting it. `-by` is required: an unattributed assign is + indistinguishable from the lane's own coverage sweep, and since `assign` takes + no body the adopter's identity has nowhere to live but the note — which is why + this is a verb rather than a three-command recipe, since in a recipe the note + is the optional step. Refuses when another lane already holds the work + (`work_already_held`) rather than manufacturing the `assign_conflict` sweep + reports — a preflight over the peer chains, not an admission law: appends + lock one role chain at a time and there is no tenant-wide lock or + cross-chain transaction (FOLLOWUPS), so two adoptions of the same work into + two idle lanes that both scan before either assigns will both land, and + `sweep` reports the result as `assign_conflict` — detected, not prevented, + the same posture as `transfer`. Warns on scope drift, like `transfer`. Whoever picks the work up + resumes it with `org begin -work `, no pin needed. It manufactures no + authority — those four appends were always available to any process that can + read the state directory — so WHO may invoke it is a charter question for the + invoking role, not a property of this verb. - **`intake`** is the routing reflex before assign: given `-work ` it reports which chartered lanes' scopes cover it (the `contracts/org.InScope` predicate — prefix-at-a-boundary, never across schemes), which lanes diff --git a/cmd/org/README.md b/cmd/org/README.md index ddd5a3fc..6eb95e8d 100644 --- a/cmd/org/README.md +++ b/cmd/org/README.md @@ -24,6 +24,11 @@ org charter -role lead:agentic-development \ # new work arrives: ask where it belongs before anything is written org intake -work github:itsHabib/workbench#88 +# work already in flight that never attached: put it on a plate, do not start it +org adopt -role steward:workbench -work github:itsHabib/workbench#88 \ + -pin "head 9f2c1ab · branch fix/thing" -by supervisor:workbench \ + -evidence "gh pr view 88 --json headRefOid" + # move work between two attached lanes (assign-first, both tips fenced) org transfer -role steward:a -work github:itsHabib/workbench#88 \ -to steward:b -to-incarnation "$B_INC" -incarnation "$A_INC" diff --git a/cmd/org/adopt_test.go b/cmd/org/adopt_test.go new file mode 100644 index 00000000..79cd7ac0 --- /dev/null +++ b/cmd/org/adopt_test.go @@ -0,0 +1,232 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/itsHabib/workbench/contracts/org" +) + +// adoptFixture is one tenant with an owner lane, so each test states only what +// it is actually about. +type adoptFixture struct { + t *testing.T + state string + role string + work string +} + +func newAdoptFixture(t *testing.T) *adoptFixture { + t.Helper() + f := &adoptFixture{t: t, state: t.TempDir(), role: "steward:api", work: "github:acme/api#4612"} + f.must("charter", "-scope", "github:acme/api", "-supervisor", "human:op") + return f +} + +func (f *adoptFixture) run(verb ...string) (int, string, string) { + f.t.Helper() + return exec(f.t, f.state, append(verb, "-tenant", "acme", "-role", f.role)...) +} + +func (f *adoptFixture) must(verb ...string) (string, string) { + f.t.Helper() + code, out, errOut := f.run(verb...) + if code != 0 { + f.t.Fatalf("%v: exit %d: %s", verb, code, errOut) + } + return out, errOut +} + +// adopt performs the canonical adoption of f.work onto f.role. +func (f *adoptFixture) adopt(extra ...string) (string, string) { + f.t.Helper() + return f.must(append([]string{ + "adopt", "-work", f.work, + "-pin", "head 9f2c1ab · branch camauto-1102 · wt .claude/worktrees/camauto", + "-by", "supervisor:api", + }, extra...)...) +} + +// records reads a role's chain without folding it. +func (f *adoptFixture) records(role string) []org.Record { + f.t.Helper() + path := filepath.Join(f.state, "acme", strings.ReplaceAll(role, ":", "--"), "chain.jsonl") + raw, err := os.ReadFile(path) + if err != nil { + f.t.Fatal(err) + } + var out []org.Record + for _, line := range strings.Split(strings.TrimSpace(string(raw)), "\n") { + var r org.Record + if err := json.Unmarshal([]byte(line), &r); err != nil { + f.t.Fatalf("chain line %q: %v", line, err) + } + out = append(out, r) + } + return out +} + +// blob reads a body straight off disk. Deliberately not `org blob `: +// that verb takes its digest positionally, and Go's flag package stops parsing +// at the first non-flag argument, so a trailing -state is silently ignored and +// the read lands in the default state root. +func (f *adoptFixture) blob(digest string) string { + f.t.Helper() + raw, err := os.ReadFile(filepath.Join(f.state, "blobs", strings.ReplaceAll(digest, ":", "-"))) + if err != nil { + f.t.Fatal(err) + } + return string(raw) +} + +// TestAdoptPutsWorkOnAPlateWithoutStartingIt is the whole verb in one pass: +// four records, the lane released at the end, the work HELD and not active, +// an attributed note, and a successor able to pick it up with begin. +// +// The last assertion is the one that matters most. Adoption is only worth +// anything if the work it records is work somebody can resume without knowing +// an adoption happened — so the test proves the handoff, not just the writes. +func TestAdoptPutsWorkOnAPlateWithoutStartingIt(t *testing.T) { + f := newAdoptFixture(t) + + out, _ := f.adopt("-evidence", "gh pr view 4612 --json headRefOid") + for _, kind := range []string{"attach", "note", "assign", "release"} { + if !strings.Contains(out, kind) { + t.Fatalf("adopt lacks %s:\n%s", kind, out) + } + } + if !strings.Contains(out, "phase chartered") { + t.Fatalf("adopt did not leave the lane released:\n%s", out) + } + + // Held, and deliberately not active: putting work on a plate is not + // starting it, and a lane acts on exactly one item. + boot, _ := f.must("boot") + if !strings.Contains(boot, "held (1): "+f.work) { + t.Fatalf("adopted work is not held:\n%s", boot) + } + if strings.Contains(boot, "phase: active") { + t.Fatalf("adoption started the work:\n%s", boot) + } + + // The note is the attribution the assign cannot carry: without it an + // adoption is indistinguishable from the lane's own coverage sweep. + rs := f.records(f.role) + if len(rs) != 5 { // charter + the four + t.Fatalf("chain has %d records, want 5", len(rs)) + } + if rs[2].Kind != org.KindNote || rs[2].BodyDigest == "" { + t.Fatalf("record 3 is not a note with a body: %+v", rs[2]) + } + body := f.blob(rs[2].BodyDigest) + for _, want := range []string{"adopted by supervisor:api", f.work, "9f2c1ab", "gh pr view 4612", "held, not claimed"} { + if !strings.Contains(body, want) { + t.Fatalf("note body missing %q:\n%s", want, body) + } + } + // Every adopted record carries the incarnation the adopt's own attach + // minted, so the four are one tenure rather than four anonymous writes. + inc := rs[2].Incarnation + if inc == "" || rs[3].Incarnation != inc || rs[4].Incarnation != inc { + t.Fatalf("adopted records do not share one incarnation: %q %q %q", + rs[2].Incarnation, rs[3].Incarnation, rs[4].Incarnation) + } + + // The handoff: a successor resumes adopted work with no pin, because the + // pin is already on the chain. + out, _ = f.must("begin", "-work", f.work) + if !strings.Contains(out, "claim") || !strings.Contains(out, "phase active") { + t.Fatalf("begin did not pick up adopted work:\n%s", out) + } +} + +// TestAdoptRefusesRatherThanDisplaceOrDoubleHold pins the two refusals that +// make adoption safe to hand to a watcher, plus the no-op that makes it safe +// to re-run. Both refusals must land on the refusal exit code: they are the +// substrate declining, not the command failing. +func TestAdoptRefusesRatherThanDisplaceOrDoubleHold(t *testing.T) { + f := newAdoptFixture(t) + + // A live session holds the lane. Adoption never displaces one — that is a + // takeover, which the kernel gates on the charter's supervisors. + f.must("attach") + code, _, errOut := f.run("adopt", "-work", f.work, "-pin", "x", "-by", "supervisor:api") + if code != codeRefused || !strings.Contains(errOut, org.ReasonAlreadyHeld) { + t.Fatalf("adopt onto a held lane: exit %d: %s", code, errOut) + } + if !strings.Contains(errOut, "takeover") { + t.Fatalf("refusal did not name what it is not: %s", errOut) + } + f.must("release") + + // Idempotent by state: re-adopting work the lane already holds says so and + // writes nothing. + f.adopt() + before := len(f.records(f.role)) + out, _ := f.adopt() + if !strings.Contains(out, "already adopted") { + t.Fatalf("re-adopt was not a no-op:\n%s", out) + } + if after := len(f.records(f.role)); after != before { + t.Fatalf("re-adopt wrote %d records", after-before) + } + + // A second lane already holds it. Adopting here would manufacture the + // assign_conflict sweep reports, and which lane should own it is a routing + // question no mechanical verb gets to answer. + if code, _, errOut := exec(t, f.state, "charter", "-tenant", "acme", "-role", "steward:other", + "-scope", "github:acme/api", "-supervisor", "human:op"); code != 0 { + t.Fatal(errOut) + } + code, _, errOut = exec(t, f.state, "adopt", "-tenant", "acme", "-role", "steward:other", + "-work", f.work, "-pin", "x", "-by", "supervisor:api") + if code != codeRefused || !strings.Contains(errOut, org.ReasonWorkAlreadyHeld) { + t.Fatalf("double-hold adopt: exit %d: %s", code, errOut) + } + if !strings.Contains(errOut, f.role) { + t.Fatalf("refusal did not name the existing holder: %s", errOut) + } +} + +// TestAdoptDemandsAnAdopterAndAPin pins the two flags without which an +// adoption record is worse than no record: unsigned, or undetectable as drift. +func TestAdoptDemandsAnAdopterAndAPin(t *testing.T) { + f := newAdoptFixture(t) + + code, _, errOut := f.run("adopt", "-work", f.work, "-pin", "x") + if code != codeError || !strings.Contains(errOut, "-work and -by are required") { + t.Fatalf("unsigned adopt: exit %d: %s", code, errOut) + } + code, _, errOut = f.run("adopt", "-work", f.work, "-by", "supervisor:api") + if code != codeError || !strings.Contains(errOut, "-digest or -pin is required") { + t.Fatalf("unpinned adopt: exit %d: %s", code, errOut) + } + // A lane with no chain at all: adoption puts work on an EXISTING plate. + code, _, errOut = exec(t, f.state, "adopt", "-tenant", "acme", "-role", "steward:ghost", + "-work", f.work, "-pin", "x", "-by", "supervisor:api") + if code != codeError || !strings.Contains(errOut, "has no chain") { + t.Fatalf("adopt onto an unchartered lane: exit %d: %s", code, errOut) + } + if n := len(f.records(f.role)); n != 1 { + t.Fatalf("a refused adopt wrote %d records to the target lane", n-1) + } +} + +// TestAdoptWarnsOnScopeDrift mirrors transfer: adoption reports the drift it +// is about to create and proceeds, because the operator may be adopting work +// deliberately ahead of a charter — but sweep will say so, so the command +// says so first. +func TestAdoptWarnsOnScopeDrift(t *testing.T) { + f := newAdoptFixture(t) + _, errOut := f.must("adopt", "-work", "jira:CAMAUTO-1102", "-pin", "ticket", "-by", "supervisor:api") + if !strings.Contains(errOut, "scope_drift") { + t.Fatalf("out-of-scope adopt did not warn: %s", errOut) + } + code, out, _ := exec(t, f.state, "sweep", "-tenant", "acme") + if code != 0 || !strings.Contains(out, "scope_drift") { + t.Fatalf("sweep did not report the drift the warning promised:\n%s", out) + } +} diff --git a/cmd/org/main.go b/cmd/org/main.go index 6f75beef..9f8bdeb0 100644 --- a/cmd/org/main.go +++ b/cmd/org/main.go @@ -55,6 +55,7 @@ var verbs = map[string]func(*env, []string) error{ "annul": cmdAnnul, "attach": cmdAttach, "assign": cmdAssign, + "adopt": cmdAdopt, "transfer": cmdTransfer, "unassign": cmdWork(org.KindUnassign), "claim": cmdWork(org.KindClaim), @@ -119,6 +120,7 @@ lifecycle charter · attach · release · retire · takeover · revoke · dele correction annul (repudiate the tip; corrects forward, does not revert) work assign · transfer · unassign · claim · yield · complete · abandon composite begin (attach+assign+claim) · done (claim?+complete+release) + adopt (attach+note+assign+release; puts in-flight work on a plate) obligations intent · resolve · escalate · seal narrative note · mark · checkpoint · report · message (-body "…" | -body -) read boot · intake · status · sweep · log · verify · blob @@ -450,6 +452,168 @@ func claimable(state org.RoleState, work string) error { return nil } +// cmdAdopt puts work that is already in flight onto a lane's plate, without the +// working session's cooperation and without touching what it is doing. +// +// The gap it closes: a session that never attached still produced a branch, a +// PR and a head SHA, and every one of those is observable from outside. Nothing +// in the substrate could turn that into ownership, so unowned work stayed +// unowned however plainly it existed. +// +// Four records on the ADOPTED lane: attach → note → assign → release. The note +// is the reason this is a verb rather than a documented recipe. `assign` takes +// no body and `attach` rejects one, so an adopter's identity has nowhere to +// live except a record of its own — and in a recipe that record is the optional +// step, which means it is the step missing from the tick where it mattered. An +// unattributed assign is indistinguishable from the lane's own coverage sweep, +// and that indistinguishability is the whole cost of adopting at all. +// +// It manufactures no authority. `attach` carries no authorization check, so any +// process that can read the state directory can already write these four +// appends by hand; what the composite adds is that the trace is not optional +// and the pin is not hand-assembled. WHO may invoke it is a question for the +// invoking skill's charter, not for this verb. +// +// Two limits come from the kernel rather than from this verb, and both are what +// make adoption safe to hand to a watcher: +// +// - It never claims. Adoption writes Held, never Active. A lane holds many +// items and acts on exactly one, so putting work on a plate and starting it +// are different acts — and adoption is only ever the first. +// - It cannot displace. attach on a held lane is refused already_held, which +// means a live session is untouchable by construction. Adopt is not +// takeover, and the difference is enforced rather than promised. +func cmdAdopt(e *env, args []string) error { + s := newScope("adopt") + work := s.fs.String("work", "", "work URI already in flight, e.g. github:owner/repo#88") + digest := s.fs.String("digest", "", "content digest pinning the work item (sha256:…)") + pin := s.fs.String("pin", "", "observable identity to pin instead of -digest: head SHA, branch, worktree") + by := s.fs.String("by", "", "role id performing the adoption, recorded on the adopted lane") + evidence := s.fs.String("evidence", "", "how the work was observed, ideally re-runnable commands") + h, err := s.open(args, true) + if err != nil { + return err + } + if *work == "" || *by == "" { + return fmt.Errorf("-work and -by are required: an adoption nobody signed is indistinguishable from the lane's own sweep") + } + if *digest == "" && *pin == "" { + return fmt.Errorf("-digest or -pin is required: an unpinned assignment cannot detect drift") + } + _, state, err := h.Load(s.tenant, s.role) + if err != nil { + return err + } + if held(state, *work) >= 0 { + return reportNoOp(e, s, fmt.Sprintf("already adopted: %s holds %s", s.role, *work)) + } + if err := adoptable(h, s.tenant, s.role, state, *work); err != nil { + return err + } + if *digest == "" { + *digest = org.DigestBytes([]byte(*pin)) + } + r, st, err := step(h, s, home.Draft{Kind: org.KindAttach}) + if err != nil { + return err + } + steps, inc := []receipt{r}, st.Holder + r, _, err = step(h, s, home.Draft{ + Kind: org.KindNote, Body: adoptionNote(*by, *work, *pin, *evidence), + BodyClass: "narrative", Incarnation: inc, + }) + if err != nil { + return err + } + steps = append(steps, r) + r, _, err = step(h, s, home.Draft{ + Kind: org.KindAssign, + Subject: org.Subject{Work: *work, Digest: *digest}, Incarnation: inc, + }) + if err != nil { + return err + } + steps = append(steps, r) + r, _, err = step(h, s, home.Draft{Kind: org.KindRelease, Incarnation: inc}) + if err != nil { + return err + } + if _, ok := org.MatchScope(state.Terms.Scope, *work); !ok { + fmt.Fprintf(e.stderr, "warning: %s is outside %s's charter scope %v; org sweep will report it as scope_drift\n", + *work, s.role, state.Terms.Scope) + } + return reportSteps(e, s, append(steps, r)) +} + +// adoptable reports why an adoption cannot proceed, before any record is +// written. Each refusal names a frozen reason, so the exit-code seam holds: +// these are the substrate declining, not the command failing. +func adoptable(h *home.Home, tenant, role string, state org.RoleState, work string) error { + if state.Phase == org.PhaseVoid { + return fmt.Errorf("%s has no chain; adoption puts work on an existing lane's plate, it does not charter one", role) + } + if state.Holder != "" { + return &org.Refusal{Reason: org.ReasonAlreadyHeld, Detail: fmt.Sprintf( + "%s is held by %s — somebody is working it. Adoption never displaces a live session; that would be a takeover", + role, shortDigest(state.Holder))} + } + other, err := otherHolder(h, tenant, role, work) + if err != nil { + return err + } + if other == "" { + return nil + } + return &org.Refusal{Reason: org.ReasonWorkAlreadyHeld, Detail: fmt.Sprintf( + "%s is already held by %s; adopting it here would manufacture the assign_conflict org sweep reports. Move it with org transfer, or leave it", + work, other)} +} + +// otherHolder reports which OTHER role in the tenant holds work, or "". A +// second holder is the one thing adoption must not create: it is the state the +// substrate can see but not resolve, and the routing question behind it — +// which lane should own this — is not one a mechanical verb gets to answer. +// +// This is a preflight, not an admission law. Appends lock one role chain at a +// time and the substrate has no tenant-wide lock or cross-chain transaction +// (FOLLOWUPS), so two adoptions of the same work into two idle lanes can both +// scan clean and both assign; sweep then reports the assign_conflict. The +// window is the same one transfer documents — detected, not prevented — and +// closing it is the cross-chain transaction FOLLOWUPS already names. +func otherHolder(h *home.Home, tenant, role, work string) (string, error) { + pairs, err := h.RolesForTenant(tenant) + if err != nil { + return "", err + } + for _, p := range pairs { + if p[1] == role { + continue + } + // An unreadable peer chain is not evidence that it does not hold the + // work, and adopting past it could double-hold. Refuse with the cause. + _, st, err := h.Load(p[0], p[1]) + if err != nil { + return "", fmt.Errorf("cannot tell whether %s holds %s: %w", p[1], work, err) + } + if held(st, work) >= 0 { + return p[1], nil + } + } + return "", nil +} + +// adoptionNote is the record that makes an adoption legible six weeks later: +// who did it, what was observed, and the word "adopted" so a reader never has +// to infer it from an assign that looks exactly like a coverage sweep. +func adoptionNote(by, work, pin, evidence string) []byte { + var sb strings.Builder + fmt.Fprintf(&sb, "adopted by %s: %s was in flight and on no chain.\n", by, work) + fmt.Fprintf(&sb, "pin: %s\n", orDashText(pin)) + fmt.Fprintf(&sb, "evidence: %s\n", orDashText(evidence)) + sb.WriteString("the work is held, not claimed: whoever picks it up starts it with org begin.\n") + return []byte(sb.String()) +} + // orDashText renders an empty string as a dash, so an absent value reads as // absent rather than as a gap in the line. func orDashText(s string) string {