Consolidated addition, removal, and listing of locks under a single narratio locks command

This commit is contained in:
2026-05-21 19:36:14 -05:00
parent 135407ba7c
commit 870c2d69d5
7 changed files with 282 additions and 72 deletions

View File

@@ -7,7 +7,7 @@ import (
"strings"
)
var supportedCommands = []string{"run", "plan", "status", "resume", "run-stage", "restore", "session", "artifacts", "locks", "lock", "unlock"}
var supportedCommands = []string{"run", "plan", "status", "resume", "run-stage", "restore", "session", "artifacts", "locks"}
// Execute dispatches CLI commands and returns a process exit code.
func Execute(args []string, stdout, stderr io.Writer) int {
@@ -40,10 +40,6 @@ func Execute(args []string, stdout, stderr io.Writer) int {
err = Artifacts(ctx, cmdArgs, stdout)
case "locks":
err = Locks(ctx, cmdArgs, stdout)
case "lock":
err = Lock(ctx, cmdArgs, stdout)
case "unlock":
err = Unlock(ctx, cmdArgs, stdout)
default:
fmt.Fprintf(stderr, "unknown command: %q\n\n", cmd)
printUsage(stderr)

View File

@@ -379,8 +379,23 @@ func ArtifactsList(ctx context.Context, args []string, out io.Writer) error {
return nil
}
// Locks lists effective archive locks.
// Locks dispatches archive lock list and mutation helpers.
func Locks(ctx context.Context, args []string, out io.Writer) error {
if len(args) > 0 && !strings.HasPrefix(args[0], "-") {
switch args[0] {
case "add":
return LocksAdd(ctx, args[1:], out)
case "remove":
return LocksRemove(ctx, args[1:], out)
default:
return fmt.Errorf("locks: unknown subcommand %q", args[0])
}
}
return LocksList(ctx, args, out)
}
// LocksList lists effective archive locks.
func LocksList(ctx context.Context, args []string, out io.Writer) error {
fs := flag.NewFlagSet("locks", flag.ContinueOnError)
fs.SetOutput(io.Discard)
var flags commonConfigFlags
@@ -391,6 +406,9 @@ func Locks(ctx context.Context, args []string, out io.Writer) error {
if fs.NArg() != 0 {
return fmt.Errorf("locks: unexpected positional arguments")
}
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks: --session-id is required")
}
cfg, _, locks, _, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("locks: %w", err)
@@ -399,9 +417,9 @@ func Locks(ctx context.Context, args []string, out io.Writer) error {
return nil
}
// Lock adds or updates one remote lock.
func Lock(ctx context.Context, args []string, out io.Writer) error {
fs := flag.NewFlagSet("lock", flag.ContinueOnError)
// LocksAdd adds or updates one remote lock.
func LocksAdd(ctx context.Context, args []string, out io.Writer) error {
fs := flag.NewFlagSet("locks add", flag.ContinueOnError)
fs.SetOutput(io.Discard)
var flags commonConfigFlags
var reason string
@@ -410,71 +428,77 @@ func Lock(ctx context.Context, args []string, out io.Writer) error {
fs.StringVar(&reason, "reason", "", "lock reason")
fs.BoolVar(&force, "force", false, "update existing remote lock")
if err := fs.Parse(args); err != nil {
return fmt.Errorf("lock: invalid flags: %w", err)
return fmt.Errorf("locks add: invalid flags: %w", err)
}
if fs.NArg() != 1 {
return fmt.Errorf("lock: expected exactly one source id")
return fmt.Errorf("locks add: expected exactly one source id")
}
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks add: --session-id is required")
}
source := strings.TrimSpace(fs.Arg(0))
cfg, store, locks, _, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("lock: %w", err)
return fmt.Errorf("locks add: %w", err)
}
if _, err := config.ValidateArchiveLockRules([]config.ArchiveLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, "lock"); err != nil {
return fmt.Errorf("lock: %w", err)
if _, err := config.ValidateArchiveLockRules([]config.ArchiveLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, "locks add"); err != nil {
return fmt.Errorf("locks add: %w", err)
}
if _, ok := lockSourceSet(locks.Static)[source]; ok {
return fmt.Errorf("lock: source %q is locked by pipeline config and cannot be modified remotely", source)
return fmt.Errorf("locks add: source %q is locked by pipeline config and cannot be modified remotely", source)
}
remoteSet := lockSourceSet(locks.Remote)
if _, exists := remoteSet[source]; exists && !force {
return fmt.Errorf("lock: remote lock for %q already exists; pass --force to update", source)
return fmt.Errorf("locks add: remote lock for %q already exists; pass --force to update", source)
}
remoteSet[source] = config.ArchiveLockRule{Source: source, Reason: strings.TrimSpace(reason)}
remoteLocks := lockMapValues(remoteSet)
if _, err := config.ValidateArchiveLockRules(remoteLocks, cfg.Pipeline.Scriptorium, "locks"); err != nil {
return fmt.Errorf("lock: %w", err)
return fmt.Errorf("locks add: %w", err)
}
if err := uploadRemoteLockStore(ctx, store, locks.Key, &config.ArchiveLockStore{Locks: remoteLocks}); err != nil {
return fmt.Errorf("lock: %w", err)
return fmt.Errorf("locks add: %w", err)
}
_, err = fmt.Fprintf(out, "narratio lock: locked %s\n", source)
_, err = fmt.Fprintf(out, "narratio locks add: locked %s\n", source)
return err
}
// Unlock removes one remote lock.
func Unlock(ctx context.Context, args []string, out io.Writer) error {
fs := flag.NewFlagSet("unlock", flag.ContinueOnError)
// LocksRemove removes one remote lock.
func LocksRemove(ctx context.Context, args []string, out io.Writer) error {
fs := flag.NewFlagSet("locks remove", flag.ContinueOnError)
fs.SetOutput(io.Discard)
var flags commonConfigFlags
addCommonConfigFlags(fs, &flags)
if err := fs.Parse(args); err != nil {
return fmt.Errorf("unlock: invalid flags: %w", err)
return fmt.Errorf("locks remove: invalid flags: %w", err)
}
if fs.NArg() != 1 {
return fmt.Errorf("unlock: expected exactly one source id")
return fmt.Errorf("locks remove: expected exactly one source id")
}
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks remove: --session-id is required")
}
source := strings.TrimSpace(fs.Arg(0))
cfg, store, locks, _, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("unlock: %w", err)
return fmt.Errorf("locks remove: %w", err)
}
if _, err := config.ValidateArchiveLockRules([]config.ArchiveLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, "unlock"); err != nil {
return fmt.Errorf("unlock: %w", err)
if _, err := config.ValidateArchiveLockRules([]config.ArchiveLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, "locks remove"); err != nil {
return fmt.Errorf("locks remove: %w", err)
}
remoteSet := lockSourceSet(locks.Remote)
if _, ok := remoteSet[source]; !ok {
if _, static := lockSourceSet(locks.Static)[source]; static {
return fmt.Errorf("unlock: source %q is locked by pipeline config and cannot be unlocked remotely", source)
return fmt.Errorf("locks remove: source %q is locked by pipeline config and cannot be unlocked remotely", source)
}
return fmt.Errorf("unlock: remote lock for %q does not exist", source)
return fmt.Errorf("locks remove: remote lock for %q does not exist", source)
}
delete(remoteSet, source)
remoteLocks := lockMapValues(remoteSet)
if err := uploadRemoteLockStore(ctx, store, locks.Key, &config.ArchiveLockStore{Locks: remoteLocks}); err != nil {
return fmt.Errorf("unlock: %w", err)
return fmt.Errorf("locks remove: %w", err)
}
_, err = fmt.Fprintf(out, "narratio unlock: unlocked %s\n", source)
_, err = fmt.Fprintf(out, "narratio locks remove: unlocked %s\n", source)
return err
}

View File

@@ -91,7 +91,7 @@ inputs:
}
}
func TestExecuteLockAndUnlockUseRemoteLockStore(t *testing.T) {
func TestExecuteLocksAddListAndRemoveUseRemoteLockStore(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
fake := &storage.FakeBackend{}
@@ -101,15 +101,16 @@ func TestExecuteLockAndUnlockUseRemoteLockStore(t *testing.T) {
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute([]string{
"lock",
"locks", "add",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"--reason", "manual edit",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code != 0 {
t.Fatalf("lock exit code = %d, want 0; stderr=%q", code, stderr.String())
t.Fatalf("locks add exit code = %d, want 0; stderr=%q", code, stderr.String())
}
key := artifacts.S3SessionLocksKey(artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03"))
obj, ok := fake.Objects[key]
@@ -123,24 +124,187 @@ func TestExecuteLockAndUnlockUseRemoteLockStore(t *testing.T) {
stdout.Reset()
stderr.Reset()
code = Execute([]string{
"unlock",
"locks",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
}, &stdout, &stderr)
if code != 0 {
t.Fatalf("locks list exit code = %d, want 0; stderr=%q", code, stderr.String())
}
if !strings.Contains(stdout.String(), "- narratio.transcript.trimmed origin=remote") {
t.Fatalf("stdout = %q, want remote lock", stdout.String())
}
stdout.Reset()
stderr.Reset()
code = Execute([]string{
"locks", "remove",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code != 0 {
t.Fatalf("unlock exit code = %d, want 0; stderr=%q", code, stderr.String())
t.Fatalf("locks remove exit code = %d, want 0; stderr=%q", code, stderr.String())
}
store, err := config.LoadArchiveLockStoreBytes("locks.yml", fake.Objects[key].Data, nil)
if err != nil {
t.Fatalf("LoadArchiveLockStoreBytes() error = %v", err)
}
if len(store.Locks) != 0 {
t.Fatalf("locks after unlock = %#v, want empty", store.Locks)
t.Fatalf("locks after remove = %#v, want empty", store.Locks)
}
if storeInitCalls != 2 {
t.Fatalf("object store init calls = %d, want 2", storeInitCalls)
if storeInitCalls != 3 {
t.Fatalf("object store init calls = %d, want 3", storeInitCalls)
}
}
func TestExecuteLocksAddDuplicateRequiresForce(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
fake := &storage.FakeBackend{}
var storeInitCalls int
restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath})
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute([]string{
"locks", "add",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"--reason", "first",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code != 0 {
t.Fatalf("initial locks add exit code = %d, want 0; stderr=%q", code, stderr.String())
}
stdout.Reset()
stderr.Reset()
code = Execute([]string{
"locks", "add",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"--reason", "second",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code == 0 {
t.Fatal("duplicate locks add exit code = 0, want non-zero")
}
if !strings.Contains(stderr.String(), "pass --force to update") {
t.Fatalf("stderr = %q, want force guidance", stderr.String())
}
stdout.Reset()
stderr.Reset()
code = Execute([]string{
"locks", "add",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"--reason", "second",
"--force",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code != 0 {
t.Fatalf("forced locks add exit code = %d, want 0; stderr=%q", code, stderr.String())
}
key := artifacts.S3SessionLocksKey(artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03"))
if !strings.Contains(string(fake.Objects[key].Data), "reason: second") {
t.Fatalf("lock store data = %q, want updated reason", string(fake.Objects[key].Data))
}
}
func TestExecuteLocksRequireSessionID(t *testing.T) {
tests := []struct {
name string
args []string
want string
}{
{"list", []string{"locks"}, "locks: --session-id is required"},
{"add", []string{"locks", "add", "narratio.transcript.trimmed"}, "locks add: --session-id is required"},
{"remove", []string{"locks", "remove", "narratio.transcript.trimmed"}, "locks remove: --session-id is required"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute(tt.args, &stdout, &stderr)
if code == 0 {
t.Fatal("exit code = 0, want non-zero")
}
if !strings.Contains(stderr.String(), tt.want) {
t.Fatalf("stderr = %q, want %q", stderr.String(), tt.want)
}
})
}
}
func TestExecuteLocksCannotModifyStaticLocks(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
addStaticArchiveLockToPipelineConfig(t, pipelinePath, "narratio.transcript.trimmed")
fake := &storage.FakeBackend{}
var storeInitCalls int
restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath})
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute([]string{
"locks", "add",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code == 0 {
t.Fatal("locks add static lock exit code = 0, want non-zero")
}
if !strings.Contains(stderr.String(), "locked by pipeline config") {
t.Fatalf("stderr = %q, want static lock error", stderr.String())
}
stdout.Reset()
stderr.Reset()
code = Execute([]string{
"locks", "remove",
"--config", pipelinePath,
"--campaign", campaignPath,
"--session", sessionPath,
"--session-id", "2026-05-03",
"narratio.transcript.trimmed",
}, &stdout, &stderr)
if code == 0 {
t.Fatal("locks remove static lock exit code = 0, want non-zero")
}
if !strings.Contains(stderr.String(), "locked by pipeline config") {
t.Fatalf("stderr = %q, want static lock error", stderr.String())
}
}
func TestExecuteTopLevelLockAndUnlockAreRemoved(t *testing.T) {
tests := []string{"lock", "unlock"}
for _, cmd := range tests {
t.Run(cmd, func(t *testing.T) {
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute([]string{cmd, "narratio.transcript.trimmed"}, &stdout, &stderr)
if code == 0 {
t.Fatal("exit code = 0, want non-zero")
}
if !strings.Contains(stderr.String(), `unknown command: "`+cmd+`"`) {
t.Fatalf("stderr = %q, want unknown command", stderr.String())
}
})
}
}
@@ -237,3 +401,23 @@ func writeValidArchiveConfigFiles(t *testing.T, workspaceRoot string) (string, s
}
return pipelinePath, campaignPath, sessionPath
}
func addStaticArchiveLockToPipelineConfig(t *testing.T, pipelinePath, source string) {
t.Helper()
data, err := os.ReadFile(pipelinePath)
if err != nil {
t.Fatalf("read pipeline: %v", err)
}
updated := strings.Replace(
string(data),
"archive:\n enabled: true\n upload_run: false\n",
"archive:\n enabled: true\n upload_run: false\n locks:\n - source: "+source+"\n reason: static review\n",
1,
)
if updated == string(data) {
t.Fatalf("archive section not found in pipeline config")
}
if err := os.WriteFile(pipelinePath, []byte(updated), 0o644); err != nil {
t.Fatalf("write pipeline: %v", err)
}
}