Skip to content

feat(agent-ledger): assign update for allow-list extension#24

Merged
albertgwo merged 2 commits into
mainfrom
feat/agent-ledger-assign-update
May 12, 2026
Merged

feat(agent-ledger): assign update for allow-list extension#24
albertgwo merged 2 commits into
mainfrom
feat/agent-ledger-assign-update

Conversation

@albertgwo
Copy link
Copy Markdown
Contributor

Purpose

Close the kernel gap where an active assignment's allow-list was fixed for the lifetime of the task. Operators that wanted to extend an in-flight assignment had to choose between living with warn policy drift, editing SQLite by hand, or rotating to a new task_id and breaking the worker's AGENT_LEDGER_TASK_ID env.

Summary of changes

  • CLI: new agent-ledger assign update --task <id> --agent <agent-id> --add-allow <glob>... --reason "<why>" subcommand under the existing assign parent. Allow-list extension only; idempotent (rerunning the same flags returns changed=false reused=true).
  • Domain: new domain.SupersedeAndInsertAssignment does lookup, merge, supersede, and insert in one BEGIN IMMEDIATE transaction. Generates ids and timestamp inside the callback so a writer that loses the lock race cannot commit timestamps that pre-date the winner. Strips reserved lineage keys (superseded_by, superseded_assignment_id, updated_from) from caller-supplied --metadata so callers cannot inject false lineage onto the new active row.
  • Events: new MVP event type assignment.superseded (replacement event, not closure). The new active row also emits task.assigned with metadata.superseded_assignment_id pointing back at the prior row.
  • SPEC: §11.3.1 documents the assignment-identity rule (an assignment_id is one immutable scope-contract instance; the human concept is a chain linked via the lineage metadata keys). §12.1 adds assignment.superseded. §18.3.1 documents the update surface, atomicity, idempotency, and exit codes.
  • Docs and changelog: walkthrough §5.1 demonstrates the round-trip; CHANGELOG Unreleased entry names the feature, the scope-out, and the reserved-key filter.
  • Tests: six domain unit tests (happy, idempotent, no-active, unsafe-reason, intent-lineage preservation, reserved-metadata-key rejection, stale-update rotation) and six CLI integration tests.

Changes to review

  • internal/domain/domain.go: confirm the WriteDomainEventImmediate callback in SupersedeAndInsertAssignment is the only writer that touches the row (no out-of-callback mutations) and that isReservedLineageKey covers every key the helper sets later.
  • internal/commands/assign_update.go: confirm ErrStaleUpdate and ErrAssignmentExists collapse to one user-facing assignment_stale_update code with identical retry guidance.
  • SPEC.md §11.3.1, §12.1, §18.3.1: confirm the identity rule and event-type promotion match the implementation, and that the documented exit-code list (missing_flag, reason_unsafe, no_active_assignment, assignment_stale_update) is complete.
  • Scope-out: --add-forbid, --remove-*, full-replacement, --policy change, and standalone assignment.closed are intentionally deferred. Adding a forbid glob that overlaps an already-claimed path narrows what an in-flight intent may write, and record validates against intent path hashes (not current assignment scope), so the failure surfaces only at verify time. The deferral is named explicitly in SPEC, walkthrough, and CHANGELOG.

Comment thread agent-ledger/internal/commands/assign_update.go
Comment thread agent-ledger/internal/domain/domain_test.go Outdated
Comment thread agent-ledger/internal/domain/domain.go Outdated
Comment thread agent-ledger/internal/commands/assign_update.go Outdated
Comment thread agent-ledger/SPEC.md
Comment thread agent-ledger/internal/domain/domain.go Outdated
Comment thread agent-ledger/internal/domain/domain_test.go Outdated
Comment thread agent-ledger/internal/commands/assign_update_test.go Outdated
… docs

Address PR #24 review findings:

- Reject blank or whitespace-only --add-allow values at the CLI
  boundary and add a domain-level ErrEmptyAddAllowedPaths guard so a
  direct caller cannot get a Reused=true no-op from an empty addition
  set.
- Drop the metadata.updated_from key. The chain is already represented
  by superseded_by and superseded_assignment_id; the second key had no
  consumer and no documented meaning.
- Include --orchestrator overrides and non-reserved --metadata keys in
  idempotent change detection so those inputs no longer drop silently
  on a request whose --add-allow values are all already present.
- Reword --agent help text: omitting or passing empty intentionally
  targets the unassigned assignment chain. The prior 'required' claim
  contradicted the actual behavior.
- Rename the external stale-update test to describe what it really
  covers (rotation after an out-of-band supersede). Add an internal
  test that exercises supersedeAssignmentTx's zero-row branch directly,
  so the ErrStaleUpdate sentinel has real coverage.
- Fail loudly on Scan errors in the idempotent-path event-count queries
  and on the second json.Unmarshal in the CLI happy-path test so a
  silent schema or decode failure cannot mask a regression.
- Add SPEC entries for the new invalid_flag/invalid_metadata exit
  codes, the dropped reserved key, and the broader idempotency contract.
@albertgwo albertgwo merged commit 1ea7bb3 into main May 12, 2026
7 checks passed
@albertgwo albertgwo deleted the feat/agent-ledger-assign-update branch May 12, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant