diff --git a/.surface b/.surface index 95c60dd98..514cca239 100644 --- a/.surface +++ b/.surface @@ -11042,6 +11042,8 @@ FLAG basecamp logout --verbose type=count FLAG basecamp mcp --account type=string FLAG basecamp mcp --agent type=bool FLAG basecamp mcp --cache-dir type=string +FLAG basecamp mcp --connect-state type=string +FLAG basecamp mcp --connect-token-fd type=string FLAG basecamp mcp --count type=bool FLAG basecamp mcp --domains type=stringSlice FLAG basecamp mcp --help type=bool diff --git a/internal/cli/inherited_fds_linux.go b/internal/cli/inherited_fds_linux.go new file mode 100644 index 000000000..4f0011c55 --- /dev/null +++ b/internal/cli/inherited_fds_linux.go @@ -0,0 +1,104 @@ +//go:build linux + +package cli + +import ( + "errors" + "fmt" + "math" + "os" + + "golang.org/x/sys/unix" + + "github.com/basecamp/basecamp-cli/internal/sysfd" +) + +// sealInheritedDescriptors keeps every descriptor this process inherited out +// of the processes it starts, by marking each one close-on-exec. +// +// It decides nothing. No flag, no argument and no environment variable says +// which descriptor is which, so there is nothing here that can disagree with +// the command cobra goes on to run: a connector-started worker is handed its +// task token on an inherited pipe, and by the time the command reads it, the +// descriptor has been out of reach of any child since the first line of the +// program — including the children the root command's persistent hooks may +// start while loading configuration or checking for an update. +// +// Marking a descriptor close-on-exec neither closes it nor disturbs what is +// buffered in it, so nothing is consumed for an invocation that never serves. +// Standard input, output and error are left alone: a child is meant to share +// those. +// +// It fails closed. This is the credential handover boundary, and a process +// that cannot say every inherited descriptor is sealed cannot say the token +// descriptor is; the error it returns stops startup rather than letting the +// program run on with a promise it did not keep. +func sealInheritedDescriptors() error { + if err := unix.CloseRange(uint(sysfd.FirstNonStandard), math.MaxUint32, unix.CLOSE_RANGE_CLOEXEC); err == nil { + return nil + } + // Kernels before 5.11 do not know CLOSE_RANGE_CLOEXEC. Ask the process + // which descriptors it actually has and mark those. + return sealListedDescriptors(procSelfFD) +} + +// procSelfFD is where Linux lists the descriptors a process holds. It is a +// parameter of sealListedDescriptors only so a test can watch the walk refuse +// a listing it cannot trust. +const procSelfFD = "/proc/self/fd" + +func sealListedDescriptors(listingDir string) error { + dir, err := os.Open(listingDir) + if err != nil { + return fmt.Errorf("could not open %s to seal the descriptors this process inherited: %w", listingDir, err) + } + defer func() { _ = dir.Close() }() + names, err := dir.Readdirnames(-1) + if err != nil { + return fmt.Errorf("could not read %s to seal the descriptors this process inherited: %w", listingDir, err) + } + listing, err := sysfd.Of(dir.Fd()) + if err != nil { + return fmt.Errorf("could not seal the descriptors this process inherited: %w", err) + } + for _, name := range names { + fd, err := sysfd.Parse(name) + if err != nil { + return fmt.Errorf("could not seal the descriptors this process inherited: %s holds %q, which is not a descriptor", listingDir, name) + } + if fd < sysfd.FirstNonStandard || fd == listing { + continue + } + if err := sealDescriptor(fd); err != nil { + return err + } + } + return nil +} + +// sealDescriptor marks one descriptor close-on-exec. +// +// A descriptor that is no longer open is the one failure that is not a +// failure: the listing is a snapshot, and a number that has been closed since +// it was taken is already out of reach of every child. Anything else means +// the descriptor is open and still inheritable, which is the thing this +// refuses to let happen quietly. +func sealDescriptor(fd sysfd.Descriptor) error { + flags, err := unix.FcntlInt(fd.Uintptr(), unix.F_GETFD, 0) + if err != nil { + if errors.Is(err, unix.EBADF) { + return nil + } + return fmt.Errorf("could not read the flags of inherited descriptor %s: %w", fd, err) + } + if flags&unix.FD_CLOEXEC != 0 { + return nil + } + if _, err := unix.FcntlInt(fd.Uintptr(), unix.F_SETFD, flags|unix.FD_CLOEXEC); err != nil { + if errors.Is(err, unix.EBADF) { + return nil + } + return fmt.Errorf("could not keep inherited descriptor %s from the processes this one starts: %w", fd, err) + } + return nil +} diff --git a/internal/cli/inherited_fds_linux_test.go b/internal/cli/inherited_fds_linux_test.go new file mode 100644 index 000000000..f4d14512e --- /dev/null +++ b/internal/cli/inherited_fds_linux_test.go @@ -0,0 +1,178 @@ +//go:build linux + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +// standardDescriptors are stdin, stdout and stderr by number, written out +// here so these tests do not agree with the code they check about which +// descriptors those are. +var standardDescriptors = []int{0, 1, 2} + +// The descriptor number is high on purpose: a child picks the lowest free one +// for its own files, so a low number could be in a listing for a reason that +// has nothing to do with inheritance. +const inheritedFD = 200 + +// An inherited descriptor does not reach the processes this one starts. +func TestSealedDescriptorsDoNotReachChildren(t *testing.T) { + shell, err := exec.LookPath("sh") + if err != nil { + t.Skip("no shell to start a child with") + } + inherit(t, inheritedFD) + + require.NoError(t, sealInheritedDescriptors()) + + out, err := exec.CommandContext(t.Context(), shell, "-c", "ls /proc/self/fd").Output() //nolint:gosec // G204: a listing of the child's own descriptors + require.NoError(t, err) + assert.NotContains(t, strings.Fields(string(out)), "200", "the child inherited the descriptor") + assert.True(t, fdIsOpen(inheritedFD), "and this process still has it") +} + +// It is marked, not closed, and nothing buffered in it is consumed: the +// command that the descriptor is for still reads what was written to it. +func TestSealingLeavesTheDescriptorReadable(t *testing.T) { + inherit(t, inheritedFD) + + require.NoError(t, sealInheritedDescriptors()) + + flags, err := unix.FcntlInt(uintptr(inheritedFD), unix.F_GETFD, 0) + require.NoError(t, err) + assert.NotZero(t, flags&unix.FD_CLOEXEC) + buffer := make([]byte, len("a-task-token\n")) + read, err := unix.Read(inheritedFD, buffer) + require.NoError(t, err) + assert.Equal(t, "a-task-token\n", string(buffer[:read])) +} + +// Kernels before 5.11 have no CLOSE_RANGE_CLOEXEC, and the walk that stands in +// for it there is the same promise. +func TestSealingWithoutCloseRange(t *testing.T) { + inherit(t, inheritedFD) + + require.NoError(t, sealListedDescriptors(procSelfFD)) + + flags, err := unix.FcntlInt(uintptr(inheritedFD), unix.F_GETFD, 0) + require.NoError(t, err) + assert.NotZero(t, flags&unix.FD_CLOEXEC) +} + +// Standard input, output and error are a child's to share. +func TestSealingLeavesTheStandardDescriptors(t *testing.T) { + shareStandardDescriptors(t) + before := standardFlags(t) + + require.NoError(t, sealInheritedDescriptors()) + + assert.Equal(t, before, standardFlags(t)) +} + +// inherit puts a readable pipe at fd, the way an exec'd child receives one: +// open, with no close-on-exec flag of its own. +func inherit(t *testing.T, fd int) { + t.Helper() + reader, writer, err := os.Pipe() + require.NoError(t, err) + _, err = writer.WriteString("a-task-token\n") + require.NoError(t, err) + require.NoError(t, writer.Close()) + t.Cleanup(func() { _ = reader.Close() }) + + require.NoError(t, unix.Dup3(int(reader.Fd()), fd, 0)) + t.Cleanup(func() { _ = unix.Close(fd) }) + flags, err := unix.FcntlInt(uintptr(fd), unix.F_GETFD, 0) + require.NoError(t, err) + require.Zero(t, flags&unix.FD_CLOEXEC, "an inherited descriptor arrives without it") +} + +func fdIsOpen(fd int) bool { + _, err := unix.FcntlInt(uintptr(fd), unix.F_GETFD, 0) + return err == nil +} + +// shareStandardDescriptors puts stdin, stdout and stderr in the state a child +// inherits them in, so the test is about what sealing leaves alone rather than +// about how the test binary happened to be started. +func shareStandardDescriptors(t *testing.T) { + t.Helper() + for _, fd := range standardDescriptors { + flags, err := unix.FcntlInt(uintptr(fd), unix.F_GETFD, 0) + require.NoError(t, err) + _, err = unix.FcntlInt(uintptr(fd), unix.F_SETFD, flags&^unix.FD_CLOEXEC) + require.NoError(t, err) + t.Cleanup(func() { _, _ = unix.FcntlInt(uintptr(fd), unix.F_SETFD, flags) }) + } +} + +func standardFlags(t *testing.T) []int { + t.Helper() + flags := make([]int, 0, len(standardDescriptors)) + for _, fd := range standardDescriptors { + got, err := unix.FcntlInt(uintptr(fd), unix.F_GETFD, 0) + require.NoError(t, err) + flags = append(flags, got) + } + return flags +} + +// The walk that stands in for CLOSE_RANGE_CLOEXEC says so when it cannot read +// the listing it works from. Returning quietly would leave the caller +// believing every inherited descriptor was sealed when none of them were. +func TestSealingRefusesWhenItCannotListTheDescriptors(t *testing.T) { + err := sealListedDescriptors(filepath.Join(t.TempDir(), "no-such-listing")) + + require.Error(t, err) + assert.Contains(t, err.Error(), "seal the descriptors this process inherited") +} + +// A listing this process cannot make sense of is not a listing it may skip: +// an entry that is not a descriptor number means the walk cannot say which +// descriptors it covered. +func TestSealingRefusesAListingItCannotRead(t *testing.T) { + listing := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(listing, "not-a-descriptor"), nil, 0o600)) + + err := sealListedDescriptors(listing) + + require.Error(t, err) + assert.Contains(t, err.Error(), `"not-a-descriptor"`) +} + +// A descriptor named by the listing but no longer open is the one failure +// that is not one: it is already out of reach of every child. The listing is +// a snapshot, so this happens whenever anything closes a descriptor while the +// walk runs. +func TestSealingAcceptsADescriptorThatHasSinceBeenClosed(t *testing.T) { + listing := t.TempDir() + closedFD := inheritedFD + 1 + require.False(t, fdIsOpen(closedFD), "the test needs a descriptor number nothing holds") + require.NoError(t, os.WriteFile(filepath.Join(listing, strconv.Itoa(closedFD)), nil, 0o600)) + + assert.NoError(t, sealListedDescriptors(listing)) +} + +// The walk seals what the listing names, so a listing standing in for +// /proc/self/fd reaches the same descriptor the real one would. +func TestSealingSealsWhatTheListingNames(t *testing.T) { + inherit(t, inheritedFD) + listing := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(listing, strconv.Itoa(inheritedFD)), nil, 0o600)) + + require.NoError(t, sealListedDescriptors(listing)) + + flags, err := unix.FcntlInt(uintptr(inheritedFD), unix.F_GETFD, 0) + require.NoError(t, err) + assert.NotZero(t, flags&unix.FD_CLOEXEC) +} diff --git a/internal/cli/inherited_fds_other.go b/internal/cli/inherited_fds_other.go new file mode 100644 index 000000000..b488ba929 --- /dev/null +++ b/internal/cli/inherited_fds_other.go @@ -0,0 +1,10 @@ +//go:build !linux + +package cli + +// sealInheritedDescriptors has nothing to seal where the handover it protects +// cannot happen: a task token arrives on an inherited descriptor only on +// Linux, which is what internal/commands/mcp_token_linux.go accepts and +// internal/commands/mcp_token_other.go refuses. Elsewhere this keeps the +// program building and starting the same way. +func sealInheritedDescriptors() error { return nil } diff --git a/internal/cli/root.go b/internal/cli/root.go index a68c83b4a..b6cb34309 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -303,6 +303,19 @@ func postRunNoticesEnabled(app *appctx.App) bool { // Execute runs the root command. func Execute() { + // Before anything else: a descriptor this process inherited belongs to + // this process, not to the children it starts. The root command's + // persistent hooks load configuration, tighten directories and may check + // for an update before any command's own RunE runs, and a connector- + // started worker arrives holding its task token on an inherited pipe. + // + // A process that cannot seal them cannot make that promise, so it does + // not start: continuing would run every hook with the token descriptor + // still there for a child to inherit, and say nothing about it. + if err := sealInheritedDescriptors(); err != nil { + os.Exit(reportStartupFailure(os.Stderr, err)) + } + cmd := NewRootCmd() // Add subcommands @@ -541,6 +554,16 @@ func reportWireError(w io.Writer, err error) int { return output.ExitCodeFor(apiErr.Code) } +// reportStartupFailure says on stderr why the program will not run at all. +// It comes before cobra has parsed anything, so there is no command to ask +// for an output format and no envelope to fill; stderr also keeps it clear of +// a command whose stdout speaks a wire protocol. +func reportStartupFailure(w io.Writer, err error) int { + apiErr := output.AsError(err) + fmt.Fprintln(w, "Error: "+richtext.SanitizeSingleLine(apiErr.Message)) + return output.ExitCodeFor(apiErr.Code) +} + // jqUsable reports whether a filter parses and compiles. Only these failures // are knowable before any output is produced, which is what makes clearing the // filter safe: a filter that fails at runtime may already have written. diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 48304f577..88ce7c965 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "errors" "os" "runtime" "testing" @@ -409,6 +410,20 @@ func TestReportWireError(t *testing.T) { assert.Equal(t, "Error: badgateway injected\n", buf.String()) } +// TestReportStartupFailure pins what a refused startup looks like: the +// program has not parsed a command yet, so there is no output format to +// honor and no envelope to fill — one sanitized line on stderr and a +// non-zero exit. sealInheritedDescriptors is the only caller: a process that +// cannot keep an inherited credential from its children does not run. +func TestReportStartupFailure(t *testing.T) { + var buf bytes.Buffer + + code := reportStartupFailure(&buf, errors.New("could not open /proc/self/fd to seal the descriptors this process inherited")) + + assert.Equal(t, "Error: could not open /proc/self/fd to seal the descriptors this process inherited\n", buf.String()) + assert.NotEqual(t, output.ExitOK, code) +} + // TestResolveProfileMayCreatePassesUnknownName: a command that registers the // named profile itself (auth login --with-token) receives the name; every // other command is still refused an unknown --profile / BASECAMP_PROFILE. diff --git a/internal/cli/token_handover_platforms_test.go b/internal/cli/token_handover_platforms_test.go new file mode 100644 index 000000000..554d8cb22 --- /dev/null +++ b/internal/cli/token_handover_platforms_test.go @@ -0,0 +1,69 @@ +package cli + +import ( + "go/build" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// sealingFile is the only implementation that keeps an inherited descriptor +// from the processes this one starts; refusalFile is the readTaskToken that +// refuses a task token outright. Named rather than inferred, so this test +// asks about the code rather than about a build tag it repeats. +const ( + sealingDir = "." + sealingFile = "inherited_fds_linux.go" + handoverDir = "../commands" + refusalFile = "mcp_token_other.go" +) + +// releasePlatforms are the targets `make build-all` produces, plus the other +// Unix ones a `go build` is expected to work on. +var releasePlatforms = []struct{ goos, goarch string }{ + {"linux", "amd64"}, + {"linux", "arm64"}, + {"darwin", "arm64"}, + {"darwin", "amd64"}, + {"freebsd", "amd64"}, + {"openbsd", "amd64"}, + {"netbsd", "amd64"}, + {"windows", "amd64"}, +} + +// A platform reads a task token off an inherited descriptor exactly where it +// seals inherited descriptors. +// +// These are two build constraints in two packages answering one question, and +// they are only ever right together: a platform that accepts the handover +// without the seal leaves the token descriptor inheritable through every hook +// that runs before the command reads it, which is the guarantee the handover +// exists for. The pair drifted once already — the reader was built for every +// Unix while the seal was Linux-only — so it is checked rather than +// remembered. +func TestTheTokenIsOnlyReadWhereItIsSealed(t *testing.T) { + for _, platform := range releasePlatforms { + t.Run(platform.goos+"/"+platform.goarch, func(t *testing.T) { + seals := compilesFile(t, sealingDir, platform.goos, platform.goarch, sealingFile) + refuses := compilesFile(t, handoverDir, platform.goos, platform.goarch, refusalFile) + + assert.Equal(t, seals, !refuses, + "%s/%s compiles %s (seals inherited descriptors) = %v, and compiles %s (refuses the token handover) = %v; it must do exactly one", + platform.goos, platform.goarch, sealingFile, seals, refusalFile, refuses) + }) + } +} + +// compilesFile reports whether a build for this platform includes the file. +func compilesFile(t *testing.T, dir, goos, goarch, file string) bool { + t.Helper() + context := build.Default + context.GOOS = goos + context.GOARCH = goarch + context.CgoEnabled = false + pkg, err := context.ImportDir(dir, 0) + require.NoError(t, err) + return slices.Contains(pkg.GoFiles, file) +} diff --git a/internal/commands/fcntl_unix_test.go b/internal/commands/fcntl_unix_test.go new file mode 100644 index 000000000..20512a9f3 --- /dev/null +++ b/internal/commands/fcntl_unix_test.go @@ -0,0 +1,7 @@ +//go:build unix + +package commands + +import "golang.org/x/sys/unix" + +func fcntlGetFD(fd int) (int, error) { return unix.FcntlInt(uintptr(fd), unix.F_GETFD, 0) } diff --git a/internal/commands/mcp.go b/internal/commands/mcp.go index 2fa1d22ed..5b4fc75cb 100644 --- a/internal/commands/mcp.go +++ b/internal/commands/mcp.go @@ -1,27 +1,69 @@ package commands import ( + "context" + "errors" + "fmt" "log/slog" "os" "os/signal" + "path/filepath" + "strings" "syscall" + "time" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/spf13/cobra" "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/connector" "github.com/basecamp/basecamp-cli/internal/mcpserver" "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/sysfd" ) // mcpTransport is a seam so tests can drive the server over in-memory // transports instead of the process's stdin/stdout. var mcpTransport = func() mcp.Transport { return &mcp.StdioTransport{} } +// connectTaskTokenEnv is where an earlier draft of the connector put a +// worker's task token. It is not a way in: the command refuses to serve when +// a token is found there, so nothing is led to hand it over that way. +const connectTaskTokenEnv = "BASECAMP_CONNECT_TASK_TOKEN" + +// connectTokenDescriptor reads the descriptor the task token arrives on. +// Which descriptors are acceptable for a token is readTaskToken's business; +// this answers only whether one was named at all, and whether the value is a +// descriptor this process could act on. +func connectTokenDescriptor(value string) (sysfd.Descriptor, error) { + if strings.TrimSpace(value) == "" { + return 0, output.ErrUsage("--connect-state needs the task token on an inherited descriptor: pass --connect-token-fd") + } + descriptor, err := sysfd.Parse(value) + if err != nil { + return 0, output.ErrUsage(fmt.Sprintf("--connect-token-fd: %v", err)) + } + return descriptor, nil +} + +// connectStateGiven is the one rule for whether a state directory was given, +// so the startup step and the command never disagree about an invocation. +func connectStateGiven(state string) bool { return strings.TrimSpace(state) != "" } + +// maxTaskTokenBytes bounds what is read from the token descriptor. A token is +// 43 characters; anything near this is not one. +const maxTaskTokenBytes = 4096 + +// taskTokenReadTimeout bounds the wait for the token. A write end left open +// somewhere — leaked into another process — must not hang startup silently. +var taskTokenReadTimeout = 5 * time.Second + // NewMCPCmd creates the mcp command serving Basecamp over MCP on stdio. func NewMCPCmd() *cobra.Command { var readOnly bool var domains []string + var connectState string + var connectTokenFD string cmd := &cobra.Command{ Use: "mcp", @@ -30,10 +72,17 @@ func NewMCPCmd() *cobra.Command { "projects, todos, cards, messages, and more as tools backed by your signed-in\n" + "account.\n\n" + "Register it with an MCP client as a stdio server, e.g.:\n\n" + - " claude mcp add basecamp -- basecamp mcp", + " claude mcp add basecamp -- basecamp mcp\n\n" + + "A worker started by the agent connector also gets the basecamp_connect domain,\n" + + "for its one task: --connect-state names the connector's state directory, and\n" + + "the task token arrives on an inherited pipe or socket named by\n" + + "--connect-token-fd, which is read and closed at startup. That descriptor is the\n" + + "only way in: the token is never taken from a flag value, a file or the\n" + + "environment, and a token found in $BASECAMP_CONNECT_TASK_TOKEN is refused.", Example: ` basecamp mcp basecamp mcp --read-only - basecamp mcp --domains projects,todos,cards`, + basecamp mcp --domains projects,todos,cards + basecamp mcp --connect-state ~/.local/state/basecamp/connect/- --connect-token-fd 3`, Args: cobra.NoArgs, Annotations: map[string]string{ "agent_notes": "Long-running server; stdout speaks the MCP wire protocol. Not for interactive use.", @@ -45,6 +94,44 @@ func NewMCPCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) + // The task token is read, and its descriptor closed, before + // anything else in this command runs: authentication can start + // helper processes, and this leaves them nothing to inherit even + // if a descriptor arrived without the close-on-exec flag startup + // put on it. + var taskToken string + // The environment is not a way in for any server: refused, and + // taken out of this process's environment as well, so nothing it + // might still start could read it there. + if _, set := os.LookupEnv(connectTaskTokenEnv); set { + _ = os.Unsetenv(connectTaskTokenEnv) + return output.ErrUsageHint("$"+connectTaskTokenEnv+" is not read", + "Hand the task token over on an inherited descriptor with --connect-token-fd, so it never sits in an environment.") + } + switch { + case !connectStateGiven(connectState) && cmd.Flags().Changed("connect-token-fd"): + return output.ErrUsage("--connect-token-fd is only for a server started with --connect-state") + case connectStateGiven(connectState): + if readOnly { + // Every connect action records something; refused before + // the token or the ledger is touched. + return output.ErrUsage("--connect-state cannot be combined with --read-only: every basecamp_connect action records what the worker did") + } + // Read here, once cobra has accepted the invocation, so a + // one-shot pipe is not drained for a run that never serves. + // The descriptor has been close-on-exec since the program's + // first line, so nothing started in between inherited it. + descriptor, err := connectTokenDescriptor(connectTokenFD) + if err != nil { + return err + } + token, err := readTaskToken(descriptor) + if err != nil { + return err + } + taskToken = token + } + // CheckAuthenticated, not IsAuthenticated: this refuses to // start the server, and a store it merely could not read — // another process mid-write, a keyring that would not open — @@ -63,7 +150,17 @@ func NewMCPCmd() *cobra.Command { return err } - srv, err := mcpserver.New(app.Account(), mcpserver.Config{ReadOnly: readOnly, Domains: domains}) + cfg := mcpserver.Config{ReadOnly: readOnly, Domains: domains} + if connectStateGiven(connectState) { + dispatch, closeLedger, err := openConnectDispatch(cmd.Context(), connectState, app.Config.AccountID, taskToken) + if err != nil { + return err + } + defer closeLedger() + cfg.Connect = dispatch + } + + srv, err := mcpserver.New(app.Account(), cfg) if err != nil { return err } @@ -85,6 +182,60 @@ func NewMCPCmd() *cobra.Command { cmd.Flags().BoolVar(&readOnly, "read-only", false, "Serve only read-only actions") cmd.Flags().StringSliceVar(&domains, "domains", nil, "Narrow to specific domains (comma-separated; default all)") + cmd.Flags().StringVar(&connectState, "connect-state", "", "Serve the basecamp_connect domain from this connector state directory, for the task whose token arrives on --connect-token-fd") + cmd.Flags().StringVar(&connectTokenFD, "connect-token-fd", "", "Read the task token from this inherited file descriptor (3 or above), then close it") return cmd } + +// stateDirHint says what the refused directory should have been. +func stateDirHint(refusal *connector.StateDirError) string { + switch refusal.Why { + case connector.StateDirOtherAccount: + return fmt.Sprintf("It belongs to account %s; this server serves account %s.", refusal.Account, refusal.Want) + default: + return fmt.Sprintf("The connector's state directories live in %s, named -.", refusal.Root) + } +} + +// openConnectDispatch opens the connector's ledger in stateDir and binds it to +// the task token read from the inherited descriptor (readTaskToken). +// +// The directory is the connector's own, named "-", +// and it must belong to the account this server serves: that is where the +// agent's id comes from, and a ledger for another account is refused rather +// than served. The ledger must already exist — a worker's server reads the +// connector's ledger, it never starts one. + +func openConnectDispatch(ctx context.Context, stateDir, accountID, token string) (*connector.TaskDispatch, func(), error) { + + dir, agentID, err := connector.ResolveStateDir(stateDir, accountID) + if err != nil { + var refusal *connector.StateDirError + if errors.As(err, &refusal) { + return nil, nil, output.ErrUsageHint( + fmt.Sprintf("--connect-state %s is %s", refusal.Dir, refusal.Why), + stateDirHint(refusal)) + } + return nil, nil, err + } + + // The connector owns the ledger: a worker's server opens it as it is, and + // never creates or migrates it. + ledger, err := connector.OpenExistingLedger(ctx, filepath.Join(dir, connector.LedgerFile)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil, output.ErrUsage(fmt.Sprintf("no connector ledger in %s", dir)) + } + return nil, nil, err + } + dispatch, err := ledger.Dispatch(ctx, token, agentID) + if err != nil { + _ = ledger.Close() + if errors.Is(err, connector.ErrTaskTokenRefused) { + return nil, nil, output.ErrUsage("the task token names no current task in " + dir) + } + return nil, nil, err + } + return dispatch, func() { _ = ledger.Close() }, nil +} diff --git a/internal/commands/mcp_connect_test.go b/internal/commands/mcp_connect_test.go new file mode 100644 index 000000000..fbdca71a2 --- /dev/null +++ b/internal/commands/mcp_connect_test.go @@ -0,0 +1,212 @@ +//go:build linux + +package commands + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "testing" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp/eventfeed" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/connector" + "github.com/basecamp/basecamp-cli/internal/connector/admission" +) + +const connectTestAgentID int64 = 52007412 + +// connectStateWithTask builds the connector's state directory for account 999 +// and the agent under a private state home, with one admitted mention on a +// task, and returns the directory and the task's grant. XDG_STATE_HOME is set +// to that home, so run it after setupMCPTestApp, which sets its own. +func connectStateWithTask(t *testing.T) (string, connector.TaskGrant, *connector.Ledger) { + t.Helper() + home := t.TempDir() + t.Setenv("XDG_STATE_HOME", home) + root, err := connector.StateRoot() + require.NoError(t, err) + require.NoError(t, os.MkdirAll(root, 0o700)) + dir := filepath.Join(root, connector.StateDirName("999", connectTestAgentID)) + require.NoError(t, os.Mkdir(dir, 0o700)) + ledger, err := connector.OpenLedger(filepath.Join(dir, connector.LedgerFile)) + require.NoError(t, err) + t.Cleanup(func() { _ = ledger.Close() }) + + ctx := context.Background() + _, err = ledger.RecordSeen(ctx, eventfeed.Event{ + ID: 1, EventType: "todo.created", Kind: "todo_created", Action: "created", + BucketID: 48699913, CreatorID: 26909558, RecordingID: 501, CreatedAt: time.Now(), + }, connector.LanePoll) + require.NoError(t, err) + _, err = ledger.Admission().Commit(ctx, admission.Verdict{ + EventID: 1, EventType: "todo.created", BucketID: 48699913, RecordingID: 501, + RequesterID: 26909558, State: admission.StateAdmitted, Trigger: admission.TriggerMentioned, + Acknowledge: true, ConversationKey: "recording:501", + Reply: &admission.ReplyDestination{Kind: admission.ReplyComment, RecordingID: 501}, + Routed: true, Route: "/work/secret-route", Class: "internal", + Snapshot: &admission.Snapshot{Type: "Todo", Title: "A to-do", Content: "please do it"}, + }) + require.NoError(t, err) + grant, err := ledger.CreateTask(ctx, []int64{1}) + require.NoError(t, err) + return dir, grant, ledger +} + +func unusedUpstream(t *testing.T) *httptest.Server { + t.Helper() + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + t.Cleanup(upstream.Close) + return upstream +} + +func toolNames(t *testing.T, session *mcp.ClientSession) []string { + t.Helper() + var names []string + for tool, err := range session.Tools(context.Background(), nil) { + require.NoError(t, err) + names = append(names, tool.Name) + } + return names +} + +// connectMCPApp builds the app, then the connector state under the state home +// the command will read. +func connectMCPApp(t *testing.T, accountID, baseURL string) (*appctx.App, string, connector.TaskGrant, *connector.Ledger) { + t.Helper() + t.Setenv("BASECAMP_TOKEN", "test-token") + app := setupMCPTestApp(t, accountID, baseURL) + dir, grant, ledger := connectStateWithTask(t) + return app, dir, grant, ledger +} + +// Done when: the domain is served from the ledger with the task token, and a +// server started without the token does not expose it. +func TestMCPCommandServesTheConnectDomainFromTheLedger(t *testing.T) { + app, dir, grant, ledger := connectMCPApp(t, "999", unusedUpstream(t).URL) + + session := runMCPCommandWithApp(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(tokenPipe(t, grant.Token))) + assert.Contains(t, toolNames(t, session), "basecamp_connect") + + res, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "basecamp_connect", Arguments: map[string]any{"action": "get_dispatch"}, + }) + require.NoError(t, err) + require.False(t, res.IsError) + text := res.Content[0].(*mcp.TextContent).Text + var body struct { + Instruction connector.Instruction `json:"instruction"` + } + require.NoError(t, json.Unmarshal([]byte(text), &body)) + assert.Equal(t, int64(1), body.Instruction.EventID) + assert.Equal(t, "please do it", body.Instruction.Content) + assert.NotContains(t, text, "secret-route") + assert.NotContains(t, text, grant.Token) + + // Read back through the connector's own handle: a repeat writes nothing, + // and reports the delivery the server wrote. + d, err := ledger.Dispatch(context.Background(), grant.Token, connectTestAgentID) + require.NoError(t, err) + again, ok, err := d.Get(context.Background(), 1) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, connector.DeliveryExposed, again.Delivery, "exposure was written to the connector's ledger") +} + +func TestMCPCommandMatchesTheAccountAsANumber(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "0999", unusedUpstream(t).URL) + + session := runMCPCommandWithApp(t, app, "--connect-state", dir+"/", "--connect-token-fd", strconv.Itoa(tokenPipe(t, grant.Token))) + assert.Contains(t, toolNames(t, session), "basecamp_connect") +} + +// Authentication can start helper processes, so the token is read and its +// descriptor closed before it runs — even when it then fails. +func TestMCPCommandTakesTheTokenBeforeAuthenticating(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + t.Setenv("BASECAMP_TOKEN", "") + fd := tokenPipe(t, grant.Token) + dev, ino, _ := fdIdentity(t, fd) + + err := executeMCPCommand(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(fd)) + require.Error(t, err) + assert.Contains(t, err.Error(), "Not authenticated") + if nowDev, nowIno, open := fdIdentity(t, fd); open { + assert.False(t, nowDev == dev && nowIno == ino, "the token descriptor was closed before authentication") + } +} + +// A read-only server serves no connect domain, so the startup read skips it +// entirely and the descriptor is left exactly as it was. +func TestMCPCommandRefusesToServeConnectReadOnly(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + fd := tokenPipe(t, grant.Token+"\n") + dev, ino, _ := fdIdentity(t, fd) + + err := executeMCPCommand(t, app, "--connect-state", dir, "--read-only", "--connect-token-fd", strconv.Itoa(fd)) + require.Error(t, err) + assert.Contains(t, err.Error(), "read-only") + nowDev, nowIno, open := fdIdentity(t, fd) + assert.True(t, open && nowDev == dev && nowIno == ino, "the descriptor was not touched") +} + +func TestMCPCommandWithoutConnectStateHasNoConnectDomain(t *testing.T) { + app, _, _, _ := connectMCPApp(t, "999", unusedUpstream(t).URL) + + session := runMCPCommandWithApp(t, app) + assert.NotContains(t, toolNames(t, session), "basecamp_connect") +} + +func TestMCPCommandRefusesABadConnectState(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + root, err := connector.StateRoot() + require.NoError(t, err) + mkdir := func(path string) string { + require.NoError(t, os.MkdirAll(path, 0o700)) + return path + } + otherAccount := mkdir(filepath.Join(root, connector.StateDirName("1000", connectTestAgentID))) + notAStateDir := mkdir(filepath.Join(root, "connect")) + empty := mkdir(filepath.Join(root, connector.StateDirName("999", 1))) + // The right name in the wrong place: a copy that renamed itself to match. + elsewhere := mkdir(filepath.Join(t.TempDir(), connector.StateDirName("999", connectTestAgentID))) + ledger, err := os.ReadFile(filepath.Join(dir, connector.LedgerFile)) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(elsewhere, connector.LedgerFile), ledger, 0o600)) + + for name, tc := range map[string]struct { + dir, token, want string + }{ + "no token": {dir, "", "--connect-token-fd"}, + "another account": {otherAccount, grant.Token, "belongs to account 1000"}, + "not a state dir": {notAStateDir, grant.Token, "not named -"}, + "named for an agent that is not a number": {mkdir(filepath.Join(root, "999-abc")), grant.Token, "not named"}, + "outside the root": {elsewhere, grant.Token, "outside the connector's state root"}, + "no ledger": {empty, grant.Token, "no connector ledger"}, + "a token for no task": {dir, "not-a-task-token", "names no current task"}, + } { + t.Run(name, func(t *testing.T) { + args := []string{"--connect-state", tc.dir} + if tc.token != "" { + args = append(args, "--connect-token-fd", strconv.Itoa(tokenPipe(t, tc.token))) + } + err := executeMCPCommand(t, app, args...) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.want) + }) + } + _, err = os.Stat(filepath.Join(empty, connector.LedgerFile)) + assert.True(t, os.IsNotExist(err), "a worker's server never creates the connector's ledger") +} diff --git a/internal/commands/mcp_connect_token_linux_test.go b/internal/commands/mcp_connect_token_linux_test.go new file mode 100644 index 000000000..bee398b8d --- /dev/null +++ b/internal/commands/mcp_connect_token_linux_test.go @@ -0,0 +1,216 @@ +//go:build linux + +package commands + +import ( + "bytes" + "io/fs" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// tokenPipe hands the token over the way the connector does: the read end of +// a pipe the child inherits, the write end written and closed. It returns the +// descriptor number to pass, which is the command's to close. +func tokenPipe(t *testing.T, token string) int { + t.Helper() + r, w, err := os.Pipe() + require.NoError(t, err) + _, err = w.WriteString(token) + require.NoError(t, err) + require.NoError(t, w.Close()) + // A descriptor of its own, so the test's *os.File never closes the one + // the command is handed. + fd, err := syscall.Dup(int(r.Fd())) + require.NoError(t, err) + require.NoError(t, r.Close()) + // The command closes it once it reads the token; a case that never gets + // that far leaves it to this. + t.Cleanup(func() { _ = syscall.Close(fd) }) + return fd +} + +func fdOpen(fd int) bool { + _, err := fcntlGetFD(fd) + return err == nil +} + +// fdIdentity is what a descriptor refers to. A closed number is reused by the +// next open, so "is fd N still the pipe" is asked of the file, not the number. +func fdIdentity(t *testing.T, fd int) (dev, ino uint64, open bool) { + t.Helper() + var st syscall.Stat_t + if err := syscall.Fstat(fd, &st); err != nil { + return 0, 0, false + } + return uint64(st.Dev), uint64(st.Ino), true //nolint:unconvert // Dev's width differs by platform +} + +// The token never exists where anything else can read it: not at a path, not +// in argv, not in the server's environment, and not on the descriptor it came +// in on once startup is over. +func TestMCPCommandTokenLeavesNoTrace(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", unusedUpstream(t).URL) + fd := tokenPipe(t, grant.Token+"\n") + pipeDev, pipeIno, _ := fdIdentity(t, fd) + args := []string{"--connect-state", dir, "--connect-token-fd", strconv.Itoa(fd)} + + session := runMCPCommandWithApp(t, app, args...) + assert.Contains(t, toolNames(t, session), "basecamp_connect", "the token came through the descriptor") + + for _, arg := range args { + assert.NotContains(t, arg, grant.Token, "argv") + } + for _, kv := range os.Environ() { + assert.NotContains(t, kv, grant.Token, "the server's environment") + } + if dev, ino, open := fdIdentity(t, fd); open { + assert.False(t, dev == pipeDev && ino == pipeIno, "the descriptor the token came in on is closed once it is read") + } + stateHome := os.Getenv("XDG_STATE_HOME") + require.NoError(t, filepath.WalkDir(stateHome, func(path string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || !d.Type().IsRegular() { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + assert.False(t, bytes.Contains(data, []byte(grant.Token)), "no file holds the token: %s", path) + return nil + })) +} + +// The environment is not a way in: a token left there is refused, and taken +// out, so no one is led to hand it over that way. +func TestMCPCommandRefusesATokenInTheEnvironment(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + t.Setenv("BASECAMP_CONNECT_TASK_TOKEN", grant.Token) + fd := tokenPipe(t, grant.Token) + + err := executeMCPCommand(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(fd)) + require.Error(t, err) + assert.Contains(t, err.Error(), "--connect-token-fd") + assert.Empty(t, os.Getenv("BASECAMP_CONNECT_TASK_TOKEN")) + assert.True(t, fdOpen(fd), "and the descriptor it never got to was left alone") +} + +// A descriptor that is not a pipe or a socket is refused and left alone: a +// file would be the token at a path, and a wrong number could be one the +// process already uses. +func TestMCPCommandLeavesADescriptorThatIsNotAPipeAlone(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + path := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(path, []byte(grant.Token), 0o600)) + file, err := os.Open(path) + require.NoError(t, err) + t.Cleanup(func() { _ = file.Close() }) + + err = executeMCPCommand(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(int(file.Fd()))) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a pipe or a socket") + assert.True(t, fdOpen(int(file.Fd())), "a descriptor that is not the token's is not closed") +} + +// Not only in connect mode: any server started with a token in the +// environment takes it out and refuses to start. +func TestMCPCommandRefusesATokenInTheEnvironmentWithoutConnectState(t *testing.T) { + app, _, grant, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + t.Setenv("BASECAMP_CONNECT_TASK_TOKEN", grant.Token) + + err := executeMCPCommand(t, app) + require.Error(t, err) + assert.Contains(t, err.Error(), "BASECAMP_CONNECT_TASK_TOKEN") + assert.Empty(t, os.Getenv("BASECAMP_CONNECT_TASK_TOKEN")) +} + +func TestMCPCommandRefusesABadTokenDescriptor(t *testing.T) { + app, dir, _, _ := connectMCPApp(t, "999", "https://3.basecampapi.com") + for name, tc := range map[string]struct { + args []string + want string + }{ + "no descriptor": {[]string{"--connect-state", dir}, "--connect-token-fd"}, + "a blank descriptor": {[]string{"--connect-state", dir, "--connect-token-fd", " "}, "--connect-token-fd"}, + "not a number": {[]string{"--connect-state", dir, "--connect-token-fd", "three"}, "not a file descriptor"}, + "past what int can hold": {[]string{"--connect-state", dir, "--connect-token-fd", "2147483648"}, "out of range"}, + "stdin is the MCP wire": {[]string{"--connect-state", dir, "--connect-token-fd", "0"}, "3 or above"}, + "stdout": {[]string{"--connect-state", dir, "--connect-token-fd", "1"}, "3 or above"}, + "not open": {[]string{"--connect-state", dir, "--connect-token-fd", "987"}, "it is not open"}, + "descriptor alone": {[]string{"--connect-token-fd", "5"}, "--connect-state"}, + "a negative descriptor alone": {[]string{"--connect-token-fd", "-1"}, "--connect-state"}, + "empty": {[]string{"--connect-state", dir, "--connect-token-fd", strconv.Itoa(tokenPipe(t, " \n"))}, "empty"}, + "too long": {[]string{"--connect-state", dir, "--connect-token-fd", strconv.Itoa(tokenPipe(t, strings.Repeat("x", maxTaskTokenBytes+1)))}, "not a task token"}, + } { + t.Run(name, func(t *testing.T) { + err := executeMCPCommand(t, app, tc.args...) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), tc.want), "%q does not say %q", err.Error(), tc.want) + }) + } +} + +// heldPipe is a token pipe whose write end the test keeps open, as a write +// end leaked into some other process would be. +func heldPipe(t *testing.T, written string) int { + t.Helper() + r, w, err := os.Pipe() + require.NoError(t, err) + t.Cleanup(func() { _ = w.Close() }) + _, err = w.WriteString(written) + require.NoError(t, err) + fd, err := syscall.Dup(int(r.Fd())) + require.NoError(t, err) + require.NoError(t, r.Close()) + t.Cleanup(func() { _ = syscall.Close(fd) }) + return fd +} + +// A write end left open somewhere does not hang startup: the token ends at its +// newline, and a token that never arrives is a refusal within the timeout. +func TestMCPCommandDoesNotWaitOnAWriteEndLeftOpen(t *testing.T) { + app, dir, grant, _ := connectMCPApp(t, "999", unusedUpstream(t).URL) + + session := runMCPCommandWithApp(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(heldPipe(t, grant.Token+"\n"))) + assert.Contains(t, toolNames(t, session), "basecamp_connect", "the newline ends the token") + + previous := taskTokenReadTimeout + taskTokenReadTimeout = 200 * time.Millisecond + t.Cleanup(func() { taskTokenReadTimeout = previous }) + started := time.Now() + err := executeMCPCommand(t, app, "--connect-state", dir, "--connect-token-fd", strconv.Itoa(heldPipe(t, grant.Token))) + require.Error(t, err) + assert.Contains(t, err.Error(), "no task token arrived") + assert.Less(t, time.Since(started), 5*time.Second) +} + +// A descriptor number no descriptor could have is refused where every other +// bad one is: at the read, by asking the operating system about it. +func TestABadDescriptorNumberIsRefusedAtTheRead(t *testing.T) { + _, err := readTaskToken(99999999) + require.Error(t, err) + assert.Contains(t, err.Error(), "not open") +} + +// And the command reads a whitespace state directory as absent as well, so it +// refuses the descriptor rather than reporting a token that was never read. +func TestABlankStateDirectoryIsNoStateDirectory(t *testing.T) { + t.Setenv("BASECAMP_TOKEN", "test-token") + app := setupMCPTestApp(t, "999", "https://3.basecampapi.com") + fd := tokenPipe(t, "token\n") + dev, ino, _ := fdIdentity(t, fd) + + err := executeMCPCommand(t, app, "--connect-state", " ", "--connect-token-fd", strconv.Itoa(fd)) + require.Error(t, err) + assert.Contains(t, err.Error(), "--connect-token-fd is only for a server started with --connect-state") + nowDev, nowIno, open := fdIdentity(t, fd) + assert.True(t, open && nowDev == dev && nowIno == ino, "and the descriptor was not touched") +} diff --git a/internal/commands/mcp_test.go b/internal/commands/mcp_test.go index 2caaf42c2..b95695799 100644 --- a/internal/commands/mcp_test.go +++ b/internal/commands/mcp_test.go @@ -102,20 +102,43 @@ func stubMCPTransport(t *testing.T) mcp.Transport { func runMCPCommand(t *testing.T, upstream *httptest.Server, args ...string) *mcp.ClientSession { t.Helper() t.Setenv("BASECAMP_TOKEN", "test-token") - app := setupMCPTestApp(t, "999", upstream.URL) + return runMCPCommandWithApp(t, setupMCPTestApp(t, "999", upstream.URL), args...) +} + +// runMCPCommandWithApp is runMCPCommand for an app the test has already +// built, so it can adjust the environment the command will read. +func runMCPCommandWithApp(t *testing.T, app *appctx.App, args ...string) *mcp.ClientSession { + t.Helper() clientTransport := stubMCPTransport(t) done := make(chan error, 1) go func() { done <- executeMCPCommand(t, app, args...) }() - t.Cleanup(func() { - require.NoError(t, <-done, "basecamp mcp exited with error") - }) - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.0.0"}, nil) - session, err := client.Connect(context.Background(), clientTransport, nil) - require.NoError(t, err, "MCP initialize failed") - t.Cleanup(func() { _ = session.Close() }) - return session + // Raced against the command: one that refuses to start exits without + // serving, and a client connect waiting on it would never return. + type connected struct { + session *mcp.ClientSession + err error + } + connecting := make(chan connected, 1) + go func() { + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.0.0"}, nil) + session, err := client.Connect(context.Background(), clientTransport, nil) + connecting <- connected{session, err} + }() + select { + case err := <-done: + require.NoError(t, err, "basecamp mcp refused to start") + t.Fatal("basecamp mcp exited before serving") + return nil + case c := <-connecting: + require.NoError(t, c.err, "MCP initialize failed") + t.Cleanup(func() { + require.NoError(t, <-done, "basecamp mcp exited with error") + }) + t.Cleanup(func() { _ = c.session.Close() }) + return c.session + } } func TestMCPCommandServesMCP(t *testing.T) { diff --git a/internal/commands/mcp_token_linux.go b/internal/commands/mcp_token_linux.go new file mode 100644 index 000000000..dbbbe30a3 --- /dev/null +++ b/internal/commands/mcp_token_linux.go @@ -0,0 +1,97 @@ +//go:build linux + +package commands + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "strings" + "time" + + "golang.org/x/sys/unix" + + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/sysfd" +) + +// readTaskToken reads the task token from an inherited descriptor and closes +// it. The connector hands the token over as the read end of a pipe, so it never +// exists at a path, in argv or in the environment; once read, the descriptor +// is gone too, and nothing this process starts can inherit it. +// +// This file is built for Linux alone, and shares that constraint with +// internal/cli's inherited_fds_linux.go on purpose: a credential may arrive +// on an inherited descriptor only where startup has already sealed every +// inherited descriptor against the children the pre-command hooks start. +// Everywhere else mcp_token_other.go refuses the handover, and +// TestTheTokenIsOnlyReadWhereItIsSealed holds the two constraints together. +// +// Descriptors below sysfd.FirstNonStandard are refused: stdin and stdout are +// the MCP wire, and stderr is the log. Only a pipe or a socket is taken, and +// anything else is left exactly as it was — not read, not closed: a regular +// file would be the token at a path, and a wrong number could name a +// descriptor this process already uses. +// +// The read ends at the first newline or at end of file, and is bounded in +// size and in time, so a write end left open somewhere cannot hang startup. A +// sender writes "token\n", or closes its end after the token. +func readTaskToken(descriptor sysfd.Descriptor) (string, error) { + fd := descriptor.Int() + if descriptor < sysfd.FirstNonStandard { + return "", output.ErrUsage(fmt.Sprintf("--connect-token-fd %d is standard I/O; the token descriptor must be %s or above", fd, sysfd.FirstNonStandard)) + } + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return "", output.ErrUsage(fmt.Sprintf("could not read the task token from descriptor %d: it is not open", fd)) + } + if kind := st.Mode & unix.S_IFMT; kind != unix.S_IFIFO && kind != unix.S_IFSOCK { + return "", output.ErrUsage(fmt.Sprintf("descriptor %d is not a pipe or a socket; the task token is handed over on one, never from a file", fd)) + } + // Non-blocking before it is wrapped, which is the order os.NewFile needs + // to hand back a pollable file, and a read deadline only applies to one. + // The flag is on the open file description, so anything else sharing it + // would see it too; the connector's bridge execs this server, so nothing + // does. From the moment the mode is changed the descriptor is ours, so + // this path closes it rather than leaving it open through the hooks that + // follow, where a child could inherit it. + if err := unix.SetNonblock(fd, true); err != nil { + _ = unix.Close(fd) + return "", output.ErrUsage(fmt.Sprintf("could not read the task token from descriptor %d: %v", fd, err)) + } + file := os.NewFile(descriptor.Uintptr(), "connect-token") + defer file.Close() + if err := file.SetReadDeadline(time.Now().Add(taskTokenReadTimeout)); err != nil { + return "", output.ErrUsage(fmt.Sprintf("could not read the task token from descriptor %d: %v", fd, err)) + } + + var data []byte + buf := make([]byte, 256) + for len(data) <= maxTaskTokenBytes && bytes.IndexByte(data, '\n') < 0 { + n, err := file.Read(buf) + data = append(data, buf[:n]...) + if err == nil { + continue + } + if errors.Is(err, os.ErrDeadlineExceeded) { + return "", output.ErrUsage(fmt.Sprintf("no task token arrived on descriptor %d within %s", fd, taskTokenReadTimeout)) + } + if errors.Is(err, io.EOF) { + break + } + return "", output.ErrUsage(fmt.Sprintf("could not read the task token from descriptor %d: %v", fd, err)) + } + if i := bytes.IndexByte(data, '\n'); i >= 0 { + data = data[:i] + } + if len(data) > maxTaskTokenBytes { + return "", output.ErrUsage(fmt.Sprintf("descriptor %d carries more than %d bytes; that is not a task token", fd, maxTaskTokenBytes)) + } + token := strings.TrimSpace(string(data)) + if token == "" { + return "", output.ErrUsage(fmt.Sprintf("descriptor %d carried an empty task token", fd)) + } + return token, nil +} diff --git a/internal/commands/mcp_token_other.go b/internal/commands/mcp_token_other.go new file mode 100644 index 000000000..6a11677df --- /dev/null +++ b/internal/commands/mcp_token_other.go @@ -0,0 +1,21 @@ +//go:build !linux + +package commands + +import ( + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/sysfd" +) + +// readTaskToken is refused wherever the descriptor carrying the token cannot +// be kept from the processes this one starts. Only Linux seals the +// descriptors this process inherited, in internal/cli's +// inherited_fds_linux.go, so only Linux may be handed a credential on one: +// accepting a token where nothing seals it would leave it inheritable through +// every hook that runs before this command, which is the whole point of +// handing it over on a descriptor instead of in a file or an environment. +// +// The connector runs on Linux, so this refuses nothing that works today. +func readTaskToken(sysfd.Descriptor) (string, error) { + return "", output.ErrUsage("--connect-state is only available on Linux, where an inherited descriptor can be kept from the processes this one starts") +} diff --git a/internal/connector/dispatch_lifecycle_test.go b/internal/connector/dispatch_lifecycle_test.go new file mode 100644 index 000000000..99b6810f3 --- /dev/null +++ b/internal/connector/dispatch_lifecycle_test.go @@ -0,0 +1,551 @@ +package connector + +import ( + "context" + "fmt" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/connector/admission" +) + +// TestDispatchLifecycleTable tries every transition of the dispatch lifecycle +// written at the top of ledger_dispatch.go — every pair, allowed and not — +// against the ledger, and checks the forbidden ones are refused. The tables +// here are that comment's, stated again independently of the code's own +// lifecycle map, so a change to either shows up as a disagreement. +func TestDispatchLifecycleTable(t *testing.T) { + t.Run("record", testRecordTransitions) + t.Run("delivery", testDeliveryTransitions) + t.Run("guard", testGuardTransitions) + t.Run("worker actions", testWorkerActions) + t.Run("task", testTaskTransitions) + t.Run("withdrawal", testWithdrawal) + t.Run("a pull comes first", testDeliveryNeedsAPull) +} + +// testWithdrawal: an exposure is withdrawn only on a superseded task, only +// while exposed, and only once — and then the record is work again. +func testWithdrawal(t *testing.T) { + for _, superseded := range []bool{false, true} { + for _, delivery := range deliveries { + want := superseded && delivery == DeliveryExposed + t.Run(fmt.Sprintf("at %s, superseded %v", delivery, superseded), func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + // Staged along the allowed steps, so the triggers are left in + // place. Past exposed, the steps are a worker's, so it pulled. + for _, step := range deliveries[1 : slices.Index(deliveries, delivery)+1] { + if step == DeliveryDelivered { + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET pulled_at = 'pulled' WHERE event_id = 1`) + require.NoError(t, err) + } + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = ? WHERE event_id = 1`, string(step)) + require.NoError(t, err) + } + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + if superseded { + require.NoError(t, f.ledger.supersedeTask(ctx, tx, f.grant.ID)) + } + + err = f.ledger.withdrawExposure(ctx, tx, f.grant.ID, 1, StateAdmitted, "") + + if !want { + require.Error(t, err) + return + } + require.NoError(t, err) + require.NoError(t, tx.Commit()) + assert.Equal(t, StateAdmitted, getRecord(t, f.ledger, 1).State, "withdrawn, it is work again") + + tx2, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx2.Rollback() }() + require.Error(t, f.ledger.withdrawExposure(ctx, tx2, f.grant.ID, 1, StateAdmitted, ""), "once") + require.Error(t, f.ledger.withdrawExposure(ctx, tx2, f.grant.ID, 2, StateAdmitted, ""), "a sibling never exposed has nothing to withdraw") + // The database refuses the same, whoever writes. + _, err = tx2.ExecContext(ctx, `UPDATE task_events SET withdrawn_at = 'raw' WHERE event_id = 2`) + require.Error(t, err) + _, err = tx2.ExecContext(ctx, `UPDATE task_events SET withdrawn_at = 'again' WHERE event_id = 1`) + require.Error(t, err, "once, whoever writes") + _, err = tx2.ExecContext(ctx, `UPDATE task_events SET delivery = 'delivered' WHERE event_id = 1`) + require.Error(t, err, "a withdrawn exposure moves no more") + }) + } + } +} + +var allRecordStates = []RecordState{StateSeen, StateAdmitted, StateQueued, StateBlocked, StateDispatched, StateCompleted, StateDiscarded} + +// recordTable is the record table as a plain state write sees it: from → the +// states SetState may reach, the state itself (a repeat) excluded. Into +// dispatched and out of it is the task's business — a record enters only +// when a live task carries it, and leaves (but to completed) only when none +// does — so a plain write finds no way in, and from a dispatched record on a +// live task only completed. refusedFor says which refusal each such pair gets. +// heldRecordTable is dispatched when a worker was handed the event. +var ( + recordTable = map[RecordState][]RecordState{ + StateSeen: {StateAdmitted, StateQueued, StateBlocked, StateDiscarded}, + StateAdmitted: {StateQueued, StateBlocked, StateDiscarded}, + StateQueued: {StateBlocked, StateDiscarded}, + StateBlocked: {StateAdmitted, StateQueued, StateDiscarded}, + StateDispatched: {StateCompleted}, + StateCompleted: nil, + StateDiscarded: nil, + } + heldRecordTable = []RecordState{StateCompleted} +) + +// refusedFor is the refusal a pair outside the table gets. +func refusedFor(from, to RecordState, held bool) error { + switch { + case held && (to == StateAdmitted || to == StateBlocked): + return ErrHeldByWorker + case to == StateDispatched && slices.Contains([]RecordState{StateAdmitted, StateQueued, StateBlocked}, from): + return ErrNotOnALiveTask + case from == StateDispatched && (to == StateAdmitted || to == StateBlocked): + return ErrOnALiveTask + default: + return ErrNotATransition + } +} + +// reachRecord puts event 1 in state, handed to a worker when held. +func reachRecord(t *testing.T, ledger *Ledger, state RecordState, held bool) { + t.Helper() + ctx := context.Background() + commit := func(v admission.Verdict) { + t.Helper() + _, err := ledger.Admission().Commit(ctx, v) + require.NoError(t, err) + } + seenRecord(t, ledger, 1) + switch state { + case StateSeen: + case StateAdmitted: + commit(admittedVerdict(1, 0, "recording:1")) + case StateQueued: + seenRecord(t, ledger, 9) + commit(admittedVerdict(9, 0, "recording:1")) + commit(admittedVerdict(1, 0, "recording:1")) + case StateBlocked: + commit(blockedVerdict(1, 0, admission.ReasonReadFailed)) + case StateDiscarded: + v := blockedVerdict(1, 0, admission.ReasonStale) + v.State = admission.StateDiscarded + commit(v) + case StateDispatched, StateCompleted: + commit(admittedVerdict(1, 0, "recording:1")) + grant, err := ledger.CreateTask(ctx, []int64{1}) + require.NoError(t, err) + if held { + d, err := ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + _, _, err = d.Get(ctx, 1) + require.NoError(t, err) + } + if state == StateCompleted { + require.NoError(t, ledger.SetState(ctx, 1, StateCompleted, "")) + } + } + require.Equal(t, state, getRecord(t, ledger, 1).State) +} + +func reasonFor(state RecordState) string { + if state == StateBlocked || state == StateDiscarded { + return "a_reason" + } + return "" +} + +func testRecordTransitions(t *testing.T) { + for _, held := range []bool{false, true} { + for _, from := range allRecordStates { + if held && from != StateDispatched { + continue + } + allowed := recordTable[from] + if held { + allowed = heldRecordTable + } + for _, to := range allRecordStates { + want := to == from || slices.Contains(allowed, to) + t.Run(fmt.Sprintf("%s to %s, held %v", from, to, held), func(t *testing.T) { + ledger := newTestLedger(t) + reachRecord(t, ledger, from, held) + + err := ledger.SetState(context.Background(), 1, to, reasonFor(to)) + + if want { + require.NoError(t, err) + assert.Equal(t, to, getRecord(t, ledger, 1).State) + return + } + require.Error(t, err) + assert.Equal(t, from, getRecord(t, ledger, 1).State, "a refused move moves nothing") + assert.ErrorIs(t, err, refusedFor(from, to, held)) + // The dispatch and terminal rules are the database's too, so + // they refuse whoever writes; the rest of the lifecycle map + // is the ledger's write to keep. + refusal := refusedFor(from, to, held) + if refusal != ErrNotATransition || from == StateCompleted || from == StateDiscarded { + _, rawErr := ledger.db.ExecContext(context.Background(), `UPDATE events SET state = ?, reason = ? WHERE id = 1`, string(to), reasonFor(to)) + assert.Error(t, rawErr, "a raw write of %s to %s", from, to) + } + }) + } + } + } +} + +var deliveries = []Delivery{DeliveryAdmitted, DeliveryExposed, DeliveryDelivered, DeliveryCompleted} + +// deliveryTable: from → the deliveries a row may move to, repeats excluded. +var deliveryTable = map[Delivery][]Delivery{ + DeliveryAdmitted: {DeliveryExposed}, + DeliveryExposed: {DeliveryDelivered, DeliveryCompleted}, + DeliveryDelivered: {DeliveryCompleted}, + DeliveryCompleted: nil, +} + +// The delivery and guard tables are enforced by the database itself, so they +// are tried with raw writes: whatever writes to the file meets them. +func testDeliveryTransitions(t *testing.T) { + for _, from := range deliveries { + for _, to := range deliveries { + want := to == from || slices.Contains(deliveryTable[from], to) + t.Run(fmt.Sprintf("%s to %s", from, to), func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `DROP TRIGGER task_events_delivery_moves_forward`) + require.NoError(t, err) + _, err = f.ledger.db.ExecContext(ctx, `DROP TRIGGER task_events_exposure_comes_first`) + require.NoError(t, err) + // A worker pulled it: acknowledging and completing are what a + // worker does with what it pulled. + if from != DeliveryAdmitted { + // Past admitted, a worker pulled it, which it does while + // the row is exposed: acknowledging and completing are + // what a worker does with what it pulled. + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET pulled_at = 'pulled' WHERE event_id = 1`) + require.NoError(t, err) + } + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = ? WHERE event_id = 1`, string(from)) + require.NoError(t, err) + reopened := f.ledger.restoreTriggers(t) + + _, err = reopened.db.ExecContext(ctx, `UPDATE task_events SET delivery = ? WHERE event_id = 1`, string(to)) + + if want { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Equal(t, string(from), f.row(t, 1).Delivery) + } + }) + } + } +} + +var guards = []string{"", "armed", "canceled", "fired"} + +var guardTable = map[string][]string{ + "": nil, + "armed": {"canceled", "fired"}, + "canceled": nil, + "fired": nil, +} + +func testGuardTransitions(t *testing.T) { + for _, from := range guards { + for _, to := range guards { + want := to == from || slices.Contains(guardTable[from], to) + t.Run(fmt.Sprintf("%q to %q", from, to), func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `DROP TRIGGER task_events_guard_settles_once`) + require.NoError(t, err) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET guard = ? WHERE event_id = 1`, from) + require.NoError(t, err) + reopened := f.ledger.restoreTriggers(t) + + _, err = reopened.db.ExecContext(ctx, `UPDATE task_events SET guard = ? WHERE event_id = 1`, to) + + if want { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Equal(t, from, f.row(t, 1).Guard) + } + }) + } + } +} + +// restoreTriggers puts back the triggers a test dropped to stage a row, by +// re-running migration 5's trigger statements. +func (l *Ledger) restoreTriggers(t *testing.T) *Ledger { + t.Helper() + ctx := context.Background() + for _, name := range []string{"task_events_delivery_moves_forward", "task_events_exposure_comes_first", "task_events_guard_settles_once"} { + var exists int + require.NoError(t, l.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM sqlite_master WHERE type = 'trigger' AND name = ?`, name).Scan(&exists)) + if exists == 1 { + continue + } + statement := triggerStatement(t, name) + _, err := l.db.ExecContext(ctx, statement) + require.NoError(t, err) + } + return l +} + +// triggerStatement is a trigger's CREATE statement as the migrations declare it. +func triggerStatement(t *testing.T, name string) string { + t.Helper() + for _, migration := range migrations { + start := strings.Index(migration, "CREATE TRIGGER "+name) + if start < 0 { + continue + } + end := strings.Index(migration[start:], "END;") + require.GreaterOrEqual(t, end, 0) + return migration[start : start+end+len("END;")] + } + t.Fatalf("no migration declares trigger %s", name) + return "" +} + +// testWorkerActions is the worker's side: each action against each delivery +// state, on a live task and on a superseded one. +func testWorkerActions(t *testing.T) { + type outcome struct { + err error + delivery Delivery + } + type action struct { + name string + do func(*TaskDispatch) error + } + actions := []action{ + {"get", func(d *TaskDispatch) error { _, _, err := d.Get(context.Background(), 1); return err }}, + {"ack", func(d *TaskDispatch) error { _, err := d.Ack(context.Background(), 1, nil); return err }}, + {"complete", func(d *TaskDispatch) error { + _, err := d.Complete(context.Background(), 1, Completion{Outcome: OutcomeSucceeded}) + return err + }}, + } + live := map[string]map[Delivery]outcome{ + "get": { + DeliveryAdmitted: {nil, DeliveryExposed}, DeliveryExposed: {nil, DeliveryExposed}, + DeliveryDelivered: {nil, DeliveryDelivered}, DeliveryCompleted: {nil, DeliveryCompleted}, + }, + "ack": { + DeliveryAdmitted: {ErrNotExposed, DeliveryAdmitted}, DeliveryExposed: {nil, DeliveryDelivered}, + DeliveryDelivered: {nil, DeliveryDelivered}, DeliveryCompleted: {nil, DeliveryCompleted}, + }, + "complete": { + DeliveryAdmitted: {ErrNotExposed, DeliveryAdmitted}, DeliveryExposed: {nil, DeliveryCompleted}, + DeliveryDelivered: {nil, DeliveryCompleted}, DeliveryCompleted: {nil, DeliveryCompleted}, + }, + } + reach := func(t *testing.T, f dispatchFixture, delivery Delivery) { + t.Helper() + ctx := context.Background() + if delivery == DeliveryAdmitted { + return + } + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + switch delivery { + case DeliveryAdmitted, DeliveryExposed: + case DeliveryDelivered: + _, err = f.d.Ack(ctx, 1, nil) + case DeliveryCompleted: + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + } + require.NoError(t, err) + } + for _, superseded := range []bool{false, true} { + for _, a := range actions { + for _, delivery := range deliveries { + t.Run(fmt.Sprintf("%s at %s, superseded %v", a.name, delivery, superseded), func(t *testing.T) { + f := newDispatchFixture(t) + reach(t, f, delivery) + if superseded { + require.NoError(t, f.ledger.SupersedeTask(context.Background(), f.grant.ID)) + } + + err := a.do(f.d) + + want := live[a.name][delivery] + if superseded { + want = outcome{ErrTaskTokenRefused, delivery} + } + if want.err == nil { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, want.err) + } + assert.Equal(t, string(want.delivery), f.row(t, 1).Delivery) + }) + } + } + } +} + +// A task's own columns are the database's too: supersession and retirement +// are final, and neither row is ever deleted. +func TestATaskIsSupersededNeverUnsupersededOrDeleted(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + + for name, statement := range map[string]string{ + "un-supersede the task": `UPDATE tasks SET superseded_at = NULL WHERE id = ?`, + "delete the task": `DELETE FROM tasks WHERE id = ?`, // its events reference it + "un-retire its events": `UPDATE task_events SET retired_at = NULL WHERE task_id = ?`, + "delete its events": `DELETE FROM task_events WHERE task_id = ?`, + "change the retired stamp": `UPDATE task_events SET retired_at = 'later' WHERE task_id = ?`, + } { + t.Run(name, func(t *testing.T) { + _, err := f.ledger.db.ExecContext(ctx, statement, f.grant.ID) + require.Error(t, err) + }) + } + _, err := f.ledger.Dispatch(ctx, f.grant.Token, adapterAgentID) + assert.ErrorIs(t, err, ErrTaskTokenRefused, "the token stays refused") + assert.ErrorIs(t, f.ledger.SupersedeTask(ctx, 404), ErrNoSuchTask, "an unknown task is not silently superseded") +} + +// testTaskTransitions: live to superseded, once, and a token valid only +// while its task is live. +func testTaskTransitions(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.Dispatch(ctx, f.grant.Token, adapterAgentID) + require.NoError(t, err, "live: the token binds") + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err = f.ledger.Dispatch(ctx, f.grant.Token, adapterAgentID) + require.ErrorIs(t, err, ErrTaskTokenRefused, "superseded: the token is refused") + + var first string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT superseded_at FROM tasks WHERE id = ?`, f.grant.ID).Scan(&first)) + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID), "superseded is terminal, and a repeat is harmless") + var again string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT superseded_at FROM tasks WHERE id = ?`, f.grant.ID).Scan(&again)) + assert.Equal(t, first, again) +} + +// Retirement follows supersession, and a pull is recorded only on a live +// exposure — in the database, so a raw writer meets the same rules. +func TestTheDatabaseTiesRetirementAndPullsToTheirTask(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET retired_at = 'now' WHERE event_id = 1`) + require.Error(t, err, "a live task's events are not retired") + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET pulled_at = 'now' WHERE event_id = 1`) + require.Error(t, err, "a retired exposure is not pulled") + + fresh := newDispatchFixture(t) + _, err = fresh.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + tx, err := fresh.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + require.NoError(t, fresh.ledger.supersedeTask(ctx, tx, fresh.grant.ID)) + require.NoError(t, fresh.ledger.withdrawExposure(ctx, tx, fresh.grant.ID, 1, StateAdmitted, "")) + require.NoError(t, tx.Commit()) + _, err = fresh.ledger.db.ExecContext(ctx, `UPDATE task_events SET pulled_at = 'now' WHERE event_id = 1`) + require.Error(t, err, "a withdrawn exposure is not pulled either") +} + +// Acknowledging and completing are what a worker does with what it pulled. An +// exposure written at launch that no worker pulled moves no further, except +// where the dispatcher settles the record itself. +func testDeliveryNeedsAPull(t *testing.T) { + for _, to := range []Delivery{DeliveryDelivered, DeliveryCompleted} { + t.Run(string(to), func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = ? WHERE event_id = 1`, string(to)) + require.Error(t, err, "nothing was pulled") + + // The dispatcher settling its record is the one other way to + // completed. + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = ? WHERE event_id = 1`, string(to)) + if to == DeliveryCompleted { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +} + +// A pull and a withdrawal are opposites: one says a worker has the +// instruction, the other that none ever did. No statement writes both, nor a +// pull together with retirement or a move — whichever of the delivery rules +// is the one that catches it. Which rule that is depends on the row's state; +// TestOneWriteCannotWithdrawAndComplete pins the case where the withdrawal +// rule's reading of the row being written is the only thing in the way. +func TestOneWriteCannotBothPullAndWithdraw(t *testing.T) { + for name, statement := range map[string]string{ + "pull and withdraw": `UPDATE task_events SET pulled_at = 'now', withdrawn_at = 'now' WHERE event_id = 1`, + "withdraw and pull": `UPDATE task_events SET withdrawn_at = 'now', pulled_at = 'now' WHERE event_id = 1`, + "pull and retire": `UPDATE task_events SET pulled_at = 'now', retired_at = 'now' WHERE event_id = 1`, + "pull and move on": `UPDATE task_events SET pulled_at = 'now', delivery = 'delivered' WHERE event_id = 1`, + } { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + f := newDispatchFixture(t) + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + + _, err = f.ledger.db.ExecContext(ctx, statement) + + require.Error(t, err) + assert.Equal(t, "exposed", f.rowContext(ctx, t, 1).Delivery) + }) + } +} + +// A withdrawal says no worker process ever existed; a completed delivery says +// one reported an outcome. One statement does not write both — and here the +// only thing that says so is the withdrawal rule reading the row as it is +// being written: the task is superseded with no live row, nothing was pulled, +// and the record was settled by the dispatcher, so every other delivery rule +// lets this statement through. +func TestOneWriteCannotWithdrawAndComplete(t *testing.T) { + ctx := context.Background() + f := newDispatchFixture(t) + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed', exposed_at = 'launch' WHERE event_id = 1`) + require.NoError(t, err) + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET withdrawn_at = 'now', delivery = 'completed' WHERE task_id = ? AND event_id = 1`, f.grant.ID) + + require.Error(t, err) + assert.Contains(t, err.Error(), "only a launch exposure no worker pulled") + var withdrawn *string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT withdrawn_at FROM task_events WHERE task_id = ? AND event_id = 1`, f.grant.ID).Scan(&withdrawn)) + assert.Nil(t, withdrawn) + assert.Equal(t, "exposed", f.rowContext(ctx, t, 1).Delivery) +} diff --git a/internal/connector/invariants_test.go b/internal/connector/invariants_test.go index 596262e02..e39605883 100644 --- a/internal/connector/invariants_test.go +++ b/internal/connector/invariants_test.go @@ -339,12 +339,7 @@ func reachTerminal(t *testing.T, ledger *Ledger, id int64, terminal RecordState) if terminal == StateDiscarded { return ledger.SetState(ctx, id, StateDiscarded, "untrusted_author") } - if err := ledger.SetState(ctx, id, StateAdmitted, ""); err != nil { - return err - } - if err := ledger.SetState(ctx, id, StateDispatched, ""); err != nil { - return err - } + dispatchForTest(t, ledger, id) return ledger.SetState(ctx, id, StateCompleted, "") } @@ -421,17 +416,16 @@ func TestInvariantE4TheLifecycleCarriesTheEdgesLaterCardsCommit(t *testing.T) { assert.Equal(t, StateQueued, record.State) }) - // A dispatched record whose worker never started has its exposure - // withdrawn and returns to admitted. + // A dispatched record never handed to a worker returns to admitted when + // its task is superseded. t.Run("dispatched back to admitted", func(t *testing.T) { ledger := newTestLedger(t) ctx := context.Background() _, err := ledger.RecordSeen(ctx, testEvent(1), LanePoll) require.NoError(t, err) - require.NoError(t, ledger.SetState(ctx, 1, StateAdmitted, "")) - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + grant := dispatchForTest(t, ledger, 1) - require.NoError(t, ledger.SetState(ctx, 1, StateAdmitted, "")) + require.NoError(t, ledger.SupersedeTask(ctx, grant.ID)) record, ok, err := ledger.Get(ctx, 1) require.NoError(t, err) diff --git a/internal/connector/ledger.go b/internal/connector/ledger.go index 90a4fdfe6..2d2821f61 100644 --- a/internal/connector/ledger.go +++ b/internal/connector/ledger.go @@ -8,6 +8,8 @@ import ( "os" "path/filepath" "strings" + "sync" + "sync/atomic" "time" "modernc.org/sqlite" // database/sql driver "sqlite", pure Go: no cgo on any of the five release targets. @@ -70,13 +72,55 @@ const ( // connector makes about a crash rests on the answer to "have I seen this id // before?" surviving the crash. type Ledger struct { - db *sql.DB - now func() time.Time + db *sql.DB + // file is this process's entry for the ledger file, shared with every + // other Ledger open on it; closed releases it once. + file *openLedgerFile + closed sync.Once + now func() time.Time } // OpenLedger opens (creating if absent) the ledger at path and brings its // schema up to date. func OpenLedger(path string) (*Ledger, error) { + return openLedger(context.Background(), path, true) +} + +// ErrLedgerSchema is a ledger whose schema is not the one this binary writes. +var ErrLedgerSchema = errors.New("the connector ledger's schema is not the version this basecamp writes") + +// OpenExistingLedger opens a ledger the connector already created, for a +// process that reads and reports into it rather than owns it — a worker's +// MCP server. It never creates the file and never migrates: a different +// basecamp binary started as a worker must not change the schema under the +// connector that holds it, so a ledger at any other schema version is +// refused. +// +// Neither the privacy check nor SQLite may create the file on this path: the +// check only inspects, and the database is opened with mode=rw, so a ledger +// removed at any moment is an error rather than a new empty one. +func OpenExistingLedger(ctx context.Context, path string) (*Ledger, error) { + return openLedger(ctx, path, false) +} + +// ledgerDSN is the SQLite URI for the ledger at path. owner opens it the way +// the connector does, creating it when absent; otherwise mode=rw makes SQLite +// refuse a file that is not there. +// +// _txlock=immediate takes the write lock when a transaction opens rather than +// on its first write. Without it two connectors racing on one file can both +// start, both read, and one is refused at COMMIT with the work already done. +func ledgerDSN(path string, owner bool) string { + dsn := "file:" + path + "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_pragma=foreign_keys(1)&_txlock=immediate" + if !owner { + dsn += "&mode=rw" + } + return dsn +} + +// openLedger opens the ledger; owner is the connector itself, which creates +// and migrates it. Any other opener does neither. +func openLedger(ctx context.Context, path string, owner bool) (*Ledger, error) { if path == "" { return nil, errors.New("connector: ledger path is required") } @@ -90,34 +134,59 @@ func OpenLedger(path string) (*Ledger, error) { // query, fragment or an escape, and open some other file. return nil, fmt.Errorf("connector: ledger path %q contains a character the SQLite URI cannot carry (?, # or %%)", path) } - if err := securePath(path); err != nil { + abs, err := filepath.Abs(path) + if err != nil { + return nil, fmt.Errorf("connector: ledger path %q: %w", path, err) + } + // The descriptor check runs for the first Ledger on this file and never + // while another one is open: its close would drop that one's locks. + file := claimLedger(abs) + if file.key != abs { + releaseLedger(file) + return nil, fmt.Errorf("connector: %s and %s are one file: %w", abs, file.key, ErrLedgerUnderAnotherName) + } + if err := checkLedgerFile(file, path, abs, owner); err != nil { + releaseLedger(file) return nil, err } - // _txlock=immediate takes the write lock when a transaction opens rather - // than on its first write. Without it two connectors racing on one file - // can both start, both read, and one is refused at COMMIT with the work - // already done. - dsn := "file:" + path + "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_pragma=foreign_keys(1)&_txlock=immediate" - db, err := sql.Open("sqlite", dsn) + db, err := sql.Open("sqlite", ledgerDSN(path, owner)) if err != nil { + releaseLedger(file) return nil, fmt.Errorf("connector: open ledger: %w", err) } // One writer. SQLite serializes writers anyway, and a pool merely turns // that serialization into SQLITE_BUSY under load. db.SetMaxOpenConns(1) - l := &Ledger{db: db, now: time.Now} - if err := retryBusy(func() error { return l.migrate(context.Background()) }); err != nil { - _ = db.Close() - return nil, err + l := &Ledger{db: db, file: file, now: time.Now} + if owner { + if err := retryBusy(func() error { return l.migrate(ctx) }); err != nil { + _ = l.Close() + return nil, err + } + } else { + var version int + err := retryBusy(func() error { + var err error + version, err = l.schemaVersion(ctx) + return err + }) + if err != nil { + _ = l.Close() + return nil, fmt.Errorf("connector: read ledger schema: %w", err) + } + if version != len(migrations) { + _ = l.Close() + return nil, fmt.Errorf("connector: ledger at schema %d, this basecamp writes %d: %w", version, len(migrations), ErrLedgerSchema) + } } // The WAL and shared-memory sidecars exist now and were created under the // process umask. The private directory already keeps other users out; // tightening them too costs nothing. for _, sidecar := range []string{path + "-wal", path + "-shm"} { if err := os.Chmod(sidecar, 0o600); err != nil && !os.IsNotExist(err) { - _ = db.Close() + _ = l.Close() return nil, fmt.Errorf("connector: secure ledger sidecar: %w", err) } } @@ -175,8 +244,12 @@ func isBusy(err error) bool { // a ledger whose privacy cannot be established is refused there rather than // opened — the same way setup refuses to write a trust file it cannot vouch // for. -func securePath(path string) error { - if err := setup.EnsurePrivateFile(path); err != nil { +func securePath(path string, create bool) error { + check := setup.CheckPrivateFile + if create { + check = setup.EnsurePrivateFile + } + if err := check(path); err != nil { return fmt.Errorf("connector: secure the ledger: %w", err) } // One rule of the ledger's own, beyond what a trust file needs: its @@ -195,8 +268,180 @@ func securePath(path string) error { return nil } -// Close releases the ledger's handle. -func (l *Ledger) Close() error { return l.db.Close() } +// Close releases the ledger's handle. A second Close is harmless: the file is +// released once, so a defensive extra call cannot take the entry away from +// another Ledger still holding the same file. +func (l *Ledger) Close() error { + // The database first, the entry after: between the two, an open racing + // this close must still find the entry, or its check would open the file + // while this connection still holds locks on it. + err := l.db.Close() + l.closed.Do(func() { releaseLedger(l.file) }) + return err +} + +// Opening one ledger file more than once in a process, safely. +// +// The privacy check opens the file and closes it, and POSIX drops every lock +// a process holds on a file when any descriptor for it is closed — including +// the locks SQLite is holding on another connection. So the check runs +// exactly once per file per process, while nothing else has it open. A later +// Ledger on the same file (a status read beside a running connector, a +// promote) is verified instead against what that check established: the same +// file, still this user's own, still owner-only, in a directory that is +// still 0700. Stat never opens anything, so it takes no locks away. +// +// ErrLedgerNotTheSameFile is a second open of a path that no longer names the +// file the check passed. +var ErrLedgerNotTheSameFile = errors.New("the ledger path no longer names the file this process checked") + +// ErrLedgerUnderAnotherName is an open of a file this process already has +// open under a different path — a hardlink, or a route through a symlink. +// SQLite names its write-ahead log and shared-memory files after the path it +// was given, so one file opened under two names is two different logs for one +// database. It is refused here, before the check that would open the file and +// drop the live handle's locks. +var ErrLedgerUnderAnotherName = errors.New("this process already has this ledger open under another name") + +var openLedgers struct { + sync.Mutex + files map[string]*openLedgerFile +} + +type openLedgerFile struct { + // key is this entry's key in the map, so it can be released by entry. + key string + refs int + // mu serializes the check itself, so opens that race each other on a + // fresh file do not verify against a check that has not run yet. + mu sync.Mutex + // info is the file this entry is for, recorded when it was claimed, so + // another name for the same file finds this entry even before the check + // has run. It is nil only when the file did not exist yet. + info os.FileInfo + // checked says the descriptor check has run for this file; info is then + // what that check saw. + checked bool +} + +// securePathRuns counts the checks that open the file. A test pins that a +// second Ledger on a live file runs none. +var securePathRuns atomic.Int64 + +// claimLedger records this process opening path and returns that file's +// entry, whose lock the caller takes to check it. +// +// The entry is found by what the path names, not by how it is spelled: a +// hardlink, a symlink or another route to the same file must meet the same +// entry, because the check this guards opens and closes the file itself. +func claimLedger(path string) *openLedgerFile { + openLedgers.Lock() + defer openLedgers.Unlock() + if openLedgers.files == nil { + openLedgers.files = map[string]*openLedgerFile{} + } + file := openLedgers.files[path] + if file == nil { + // Recorded at claim time, not at check time: an aliased open that + // arrives while the first one's check is still running must find this + // entry, since that check is the file-opening one. + info, err := os.Lstat(path) + if err == nil { + for _, open := range openLedgers.files { + if open.info != nil && os.SameFile(open.info, info) { + file = open + break + } + } + } + if file == nil { + file = &openLedgerFile{key: path, info: info} + openLedgers.files[path] = file + } + } + file.refs++ + return file +} + +func releaseLedger(file *openLedgerFile) { + openLedgers.Lock() + defer openLedgers.Unlock() + if file.refs--; file.refs <= 0 { + delete(openLedgers.files, file.key) + } +} + +// checkLedgerFile runs the descriptor check once per file, and holds every +// later open against what it established. +func checkLedgerFile(file *openLedgerFile, path, abs string, owner bool) error { + file.mu.Lock() + defer file.mu.Unlock() + openLedgers.Lock() + checked, info := file.checked, file.info + openLedgers.Unlock() + if checked { + return verifySameFile(abs, info) + } + securePathRuns.Add(1) + if err := securePath(path, owner); err != nil { + return err + } + // The check may have created the file, so what it saw is recorded now — + // under the map's own lock, because that is where the alias scan reads it. + info, err := os.Lstat(abs) + if err != nil { + return fmt.Errorf("connector: inspect the ledger: %w", err) + } + recordCheckedFile(file, info) + return nil +} + +// recordCheckedFile publishes what the descriptor check saw, under the lock +// the alias scan reads it with. +func recordCheckedFile(file *openLedgerFile, info os.FileInfo) { + openLedgers.Lock() + defer openLedgers.Unlock() + file.info, file.checked = info, true +} + +// ownedByThisUser and sameOwner read owners; see owner_unix.go. +// +// verifySameFile holds a second open to what the first one's check +// established, without opening anything. +func verifySameFile(path string, checked os.FileInfo) error { + info, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("connector: secure the ledger: %w", err) + } + // The first check established whose file it is. Ownership can change + // under an open handle, and a ledger that is no longer this user's own — + // or no longer the owner the check passed — is not one to read. + if !ownedByThisUser(info) || !sameOwner(info, checked) { + return fmt.Errorf("connector: secure the ledger: %s is no longer owned by the user the check passed", path) + } + if !info.Mode().IsRegular() || !os.SameFile(info, checked) { + return fmt.Errorf("connector: secure the ledger: %s: %w", path, ErrLedgerNotTheSameFile) + } + if perm := info.Mode().Perm(); perm&0o077 != 0 { + return fmt.Errorf("connector: secure the ledger: %s can be read by other users (mode %04o)", path, perm) + } + // The whole chain, not only the last directory: a path later redirected + // through a writable or foreign-owned ancestor is not the path the first + // check passed. Directories are vetted without opening the ledger, which + // is the one thing this path must not do. + dir := filepath.Dir(path) + if err := setup.CheckPrivateDir(dir); err != nil { + return fmt.Errorf("connector: secure the ledger: %w", err) + } + info, err = os.Lstat(dir) + if err != nil { + return fmt.Errorf("connector: inspect ledger directory: %w", err) + } + if perm := info.Mode().Perm(); perm&0o077 != 0 { + return fmt.Errorf("connector: ledger directory %s is readable by other users (mode %04o); it must be 0700", dir, perm) + } + return nil +} // migrations are applied in order, each exactly once. A migration is never // edited after it ships: the ledger outlives the binary that created it. @@ -322,6 +567,234 @@ ALTER TABLE events ADD COLUMN recording_url TEXT NOT NULL DEFAULT ''; ALTER TABLE events ADD COLUMN requester_id INTEGER NOT NULL DEFAULT 0; ALTER TABLE events ADD COLUMN snapshot BLOB; CREATE INDEX events_conversation ON events (conversation_key, state); +`, + // Migration 5. The worker's side of a dispatch: the task a worker's token + // names, and each event's delivery state on it. + // + // These are the columns the basecamp_connect domain reads and writes and + // nothing more. The dispatcher's attempts, deadlines and working + // directories extend these tables rather than replace them. + // + // A task keeps a hash of its token, never the token: the ledger is a + // file other processes open, and the token is what binds a worker to its + // task. superseded_at is set when a redispatch replaces the worker, and a + // superseded token is refused. + // + // delivery is admitted → exposed → delivered → completed and never goes + // back, held by the trigger as the events lifecycle is. guard is the + // thirty-second acknowledgement guard: '' where none applies, armed until + // get_dispatch cancels it or the connector fires it, and settled once. + // A record a worker was handed leaves dispatched only to completed. The + // whole lifecycle these enforce is written down at the top of + // ledger_dispatch.go. + // + // An event is on at most one live task. retired_at is set on every row of + // a task when it is superseded, and the unique index over the rows not + // retired is what refuses a second live task for the same event — in the + // database, so a dispatcher retrying a launch cannot hand one event to two + // workers whatever order its writes land in. + ` +CREATE TABLE tasks ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + token_sha256 TEXT NOT NULL UNIQUE, + created_at TEXT NOT NULL, + superseded_at TEXT +); + +CREATE TABLE task_events ( + task_id INTEGER NOT NULL REFERENCES tasks (id), + event_id INTEGER NOT NULL REFERENCES events (id), + -- The conversation the event is on, copied from the record when the row is + -- written: retention clears a terminal record's conversation_key, and a + -- task outlives its records' payloads, so the conversation a live task + -- holds has to be written where it stays readable. + conversation_key TEXT NOT NULL DEFAULT '', + delivery TEXT NOT NULL DEFAULT 'admitted' + CHECK (delivery IN ('admitted', 'exposed', 'delivered', 'completed')), + guard TEXT NOT NULL DEFAULT '' + CHECK (guard IN ('', 'armed', 'canceled', 'fired')), + exposed_at TEXT, + delivered_at TEXT, + completed_at TEXT, + ack_id INTEGER, + outcome TEXT NOT NULL DEFAULT '', + links TEXT NOT NULL DEFAULT '[]', + reply_id INTEGER, + retired_at TEXT, + pulled_at TEXT, + withdrawn_at TEXT, + PRIMARY KEY (task_id, event_id) +); + +CREATE UNIQUE INDEX task_events_one_live_task ON task_events (event_id) WHERE retired_at IS NULL; +CREATE INDEX task_events_event ON task_events (event_id, delivery); +CREATE INDEX task_events_conversation ON task_events (conversation_key) WHERE retired_at IS NULL; + +CREATE TRIGGER tasks_supersession_is_final +BEFORE UPDATE OF superseded_at ON tasks +WHEN OLD.superseded_at IS NOT NULL AND NEW.superseded_at IS NOT OLD.superseded_at +BEGIN + SELECT RAISE(ABORT, 'a superseded task stays superseded'); +END; + +-- Retirement is not a second step anyone can forget or skip: superseding a +-- task retires its events in the same write, so a task's token and its rows +-- stop being live together. +CREATE TRIGGER tasks_supersession_retires_its_events +AFTER UPDATE OF superseded_at ON tasks +WHEN NEW.superseded_at IS NOT NULL AND OLD.superseded_at IS NULL +BEGIN + UPDATE task_events SET retired_at = NEW.superseded_at + WHERE task_id = NEW.id AND retired_at IS NULL; +END; + +CREATE TRIGGER task_events_retirement_follows_supersession +BEFORE UPDATE OF retired_at ON task_events +WHEN NEW.retired_at IS NOT OLD.retired_at AND ( + OLD.retired_at IS NOT NULL + OR NEW.retired_at IS NULL + OR NOT EXISTS (SELECT 1 FROM tasks WHERE id = OLD.task_id AND superseded_at IS NOT NULL)) +BEGIN + SELECT RAISE(ABORT, 'a task event is retired when its task is superseded, once'); +END; + +CREATE TRIGGER task_events_are_not_deleted +BEFORE DELETE ON task_events +BEGIN + SELECT RAISE(ABORT, 'a task event is retired, never deleted'); +END; + +CREATE TRIGGER task_events_withdrawal_is_for_a_failed_spawn +BEFORE UPDATE OF withdrawn_at ON task_events +WHEN NEW.withdrawn_at IS NOT OLD.withdrawn_at AND ( + OLD.withdrawn_at IS NOT NULL + OR OLD.delivery <> 'exposed' + OR OLD.pulled_at IS NOT NULL + -- Nor can one statement withdraw and pull, or withdraw and move the row. + OR NEW.pulled_at IS NOT NULL + OR NEW.delivery <> 'exposed' + OR NOT EXISTS (SELECT 1 FROM tasks WHERE tasks.id = OLD.task_id AND tasks.superseded_at IS NOT NULL) + OR EXISTS (SELECT 1 FROM task_events live WHERE live.event_id = OLD.event_id AND live.retired_at IS NULL)) +BEGIN + SELECT RAISE(ABORT, 'only a launch exposure no worker pulled, on a superseded task and no live one, is withdrawn, and only once'); +END; + +CREATE TRIGGER task_events_pull_is_recorded_once +BEFORE UPDATE OF pulled_at ON task_events +WHEN NEW.pulled_at IS NOT OLD.pulled_at AND ( + OLD.pulled_at IS NOT NULL + OR OLD.delivery <> 'exposed' + OR OLD.retired_at IS NOT NULL + OR OLD.withdrawn_at IS NOT NULL + -- The same statement cannot both pull and retire or withdraw: these are a + -- BEFORE trigger's conditions, so the row being written is read as well. + OR NEW.retired_at IS NOT NULL + OR NEW.withdrawn_at IS NOT NULL + OR NEW.delivery <> 'exposed') +BEGIN + SELECT RAISE(ABORT, 'a pull is recorded once, and only on a live exposure'); +END; + +CREATE TRIGGER task_events_withdrawn_is_final +BEFORE UPDATE OF delivery ON task_events +WHEN OLD.withdrawn_at IS NOT NULL AND NEW.delivery <> OLD.delivery +BEGIN + SELECT RAISE(ABORT, 'a withdrawn exposure does not move'); +END; + +CREATE TRIGGER events_handed_work_settles_first +BEFORE UPDATE OF state ON events +WHEN OLD.state = 'dispatched' AND NEW.state NOT IN ('dispatched', 'completed') + AND EXISTS (SELECT 1 FROM task_events WHERE event_id = OLD.id AND delivery IN ('exposed', 'delivered') AND withdrawn_at IS NULL) +BEGIN + SELECT RAISE(ABORT, 'a worker was handed this event; it leaves dispatched only when completed'); +END; + +CREATE TRIGGER events_dispatched_while_on_a_live_task +BEFORE UPDATE OF state ON events +WHEN NEW.state <> OLD.state AND ( + (NEW.state = 'dispatched' + AND NOT EXISTS (SELECT 1 FROM task_events WHERE event_id = OLD.id AND retired_at IS NULL)) + OR (OLD.state = 'dispatched' AND NEW.state <> 'completed' + AND EXISTS (SELECT 1 FROM task_events WHERE event_id = OLD.id AND retired_at IS NULL))) +BEGIN + SELECT RAISE(ABORT, 'a record is dispatched exactly while a live task carries it'); +END; + +-- The acknowledgement settles with the delivery: the id a worker points at is +-- written when it acknowledges, or never. +CREATE TRIGGER task_events_acknowledgement_settles_once +BEFORE UPDATE OF ack_id ON task_events +WHEN NEW.ack_id IS NOT OLD.ack_id AND (OLD.ack_id IS NOT NULL OR OLD.delivery <> 'exposed') +BEGIN + SELECT RAISE(ABORT, 'an acknowledgement id is written with the acknowledgement, once'); +END; + +CREATE TRIGGER task_events_guard_settles_once +BEFORE UPDATE OF guard ON task_events +WHEN NEW.guard <> OLD.guard AND NOT (OLD.guard = 'armed' AND NEW.guard IN ('canceled', 'fired')) +BEGIN + SELECT RAISE(ABORT, 'a guard only goes from armed to canceled or fired'); +END; + +-- What these triggers are for: holding this package's own writes, and those +-- of a second connector on the same file, to the lifecycle. They are not a +-- defense against raw SQL. Anyone who can run that already has write access to +-- the operator's private ledger and could edit or replace the file; the trust +-- boundary there is file ownership and the private-path check, not a trigger. +CREATE TRIGGER task_events_join_live_tasks_only +BEFORE INSERT ON task_events +WHEN EXISTS (SELECT 1 FROM tasks WHERE id = NEW.task_id AND superseded_at IS NOT NULL) + OR NOT EXISTS (SELECT 1 FROM events WHERE id = NEW.event_id AND state IN ('admitted', 'queued', 'dispatched')) +BEGIN + SELECT RAISE(ABORT, 'only work waiting for a worker joins a task, and only a live one'); +END; + +-- One live task per conversation (invariant 2), as a rule about the rows +-- rather than about the records: two workers on one conversation would answer +-- each other's work, and a conversation whose records have since been +-- retained must still count. +CREATE TRIGGER task_events_one_live_task_per_conversation +BEFORE INSERT ON task_events +WHEN NEW.conversation_key <> '' AND NEW.retired_at IS NULL AND EXISTS ( + SELECT 1 FROM task_events live + WHERE live.conversation_key = NEW.conversation_key + AND live.retired_at IS NULL AND live.task_id <> NEW.task_id) +BEGIN + SELECT RAISE(ABORT, 'one live task per conversation'); +END; + +CREATE TRIGGER task_events_conversation_does_not_move +BEFORE UPDATE OF conversation_key ON task_events +WHEN NEW.conversation_key <> OLD.conversation_key +BEGIN + SELECT RAISE(ABORT, 'a task event stays on the conversation it was written for'); +END; + +CREATE TRIGGER task_events_do_not_move +BEFORE UPDATE OF task_id, event_id ON task_events +WHEN NEW.task_id <> OLD.task_id OR NEW.event_id <> OLD.event_id +BEGIN + SELECT RAISE(ABORT, 'a task event belongs to the task and event it was written for'); +END; + +CREATE TRIGGER task_events_delivery_moves_forward +BEFORE UPDATE OF delivery ON task_events +WHEN (CASE NEW.delivery WHEN 'admitted' THEN 0 WHEN 'exposed' THEN 1 WHEN 'delivered' THEN 2 ELSE 3 END) + < (CASE OLD.delivery WHEN 'admitted' THEN 0 WHEN 'exposed' THEN 1 WHEN 'delivered' THEN 2 ELSE 3 END) +BEGIN + SELECT RAISE(ABORT, 'a delivery state never goes back'); +END; + +CREATE TRIGGER task_events_exposure_comes_first +BEFORE UPDATE OF delivery ON task_events +WHEN (OLD.delivery = 'admitted' AND NEW.delivery IN ('delivered', 'completed')) + OR (NEW.delivery = 'delivered' AND OLD.delivery <> 'delivered' AND OLD.pulled_at IS NULL) + OR (NEW.delivery = 'completed' AND OLD.delivery <> 'completed' AND OLD.pulled_at IS NULL + AND NOT EXISTS (SELECT 1 FROM events WHERE id = OLD.event_id AND state = 'completed')) +BEGIN + SELECT RAISE(ABORT, 'a worker acknowledges and completes what it pulled; anything else is the dispatcher settling a completed record'); +END; `, } @@ -369,6 +842,19 @@ func (l *Ledger) migrate(ctx context.Context) error { // SchemaVersion reports the highest applied migration. func (l *Ledger) SchemaVersion(ctx context.Context) (int, error) { + return l.schemaVersion(ctx) +} + +// schemaVersion reads the version, and reads a ledger with no migration table +// as version 0 rather than failing. +func (l *Ledger) schemaVersion(ctx context.Context) (int, error) { + var tables int + if err := l.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'schema_migrations'`).Scan(&tables); err != nil { + return 0, err + } + if tables == 0 { + return 0, nil + } var version int err := l.db.QueryRowContext(ctx, `SELECT COALESCE(MAX(version), 0) FROM schema_migrations`).Scan(&version) return version, err diff --git a/internal/connector/ledger_admission_test.go b/internal/connector/ledger_admission_test.go index fb6ca95b9..a540a8562 100644 --- a/internal/connector/ledger_admission_test.go +++ b/internal/connector/ledger_admission_test.go @@ -122,12 +122,10 @@ func TestAdmissionSkipsARecordPastDeciding(t *testing.T) { require.NoError(t, ledger.SetState(ctx, 1, StateDiscarded, "untrusted_author")) case StateCompleted: require.NoError(t, reachTerminal(t, ledger, 7, StateCompleted)) - require.NoError(t, ledger.SetState(ctx, 1, StateAdmitted, "")) - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + dispatchForTest(t, ledger, 1) require.NoError(t, ledger.SetState(ctx, 1, StateCompleted, "")) case StateDispatched: - require.NoError(t, ledger.SetState(ctx, 1, StateAdmitted, "")) - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + dispatchForTest(t, ledger, 1) default: require.NoError(t, ledger.SetState(ctx, 1, state, "")) } @@ -245,8 +243,7 @@ func TestAdmissionNeverDecidesARecordPastDeciding(t *testing.T) { require.NoError(t, l.SetState(context.Background(), 1, StateAdmitted, "")) }, "dispatched": func(t *testing.T, l *Ledger) { - require.NoError(t, l.SetState(context.Background(), 1, StateAdmitted, "")) - require.NoError(t, l.SetState(context.Background(), 1, StateDispatched, "")) + dispatchForTest(t, l, 1) }, "discarded": func(t *testing.T, l *Ledger) { require.NoError(t, l.SetState(context.Background(), 1, StateDiscarded, "by_operator")) @@ -297,21 +294,21 @@ func TestAdmissionQueuesBehindALiveConversation(t *testing.T) { {"a dispatched record is a running task", func(t *testing.T, l *Ledger) { _, err := l.Admission().Commit(context.Background(), admittedVerdict(2, 0, key)) require.NoError(t, err) - require.NoError(t, l.SetState(context.Background(), 2, StateDispatched, "")) + dispatchForTest(t, l, 2) }, admission.StateQueued}, {"a queued record alone is not a task", func(t *testing.T, l *Ledger) { _, err := l.Admission().Commit(context.Background(), admittedVerdict(2, 0, key)) require.NoError(t, err) _, err = l.Admission().Commit(context.Background(), admittedVerdict(3, 0, key)) require.NoError(t, err) - require.NoError(t, l.SetState(context.Background(), 2, StateDispatched, "")) + dispatchForTest(t, l, 2) require.NoError(t, l.SetState(context.Background(), 2, StateCompleted, "")) require.Equal(t, StateQueued, getRecord(t, l, 3).State) }, admission.StateAdmitted}, {"a completed task is not live", func(t *testing.T, l *Ledger) { _, err := l.Admission().Commit(context.Background(), admittedVerdict(2, 0, key)) require.NoError(t, err) - require.NoError(t, l.SetState(context.Background(), 2, StateDispatched, "")) + dispatchForTest(t, l, 2) require.NoError(t, l.SetState(context.Background(), 2, StateCompleted, "")) }, admission.StateAdmitted}, {"a blocked record is not a task", func(t *testing.T, l *Ledger) { @@ -461,9 +458,12 @@ func TestEveryMoveKeepsTheBlockedScheduleInputs(t *testing.T) { assert.Nil(t, d.BlockedAt, "a record that left blocked is not blocked") assert.Nil(t, d.RetryAt) - // Back into blocked after a dispatch: a new window from now, and the - // verdict that follows keeps it. - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + // Blocked again after a dispatch that was superseded: a new window from + // now, and the verdict that follows keeps it. Superseding a task returns + // an event no worker was handed to admitted, and it is blocked from + // there. + grant := dispatchForTest(t, ledger, 1) + require.NoError(t, ledger.SupersedeTask(ctx, grant.ID)) require.NoError(t, ledger.SetState(ctx, 1, StateBlocked, "read_failed")) record := getRecord(t, ledger, 1) require.NotNil(t, record.Decision.BlockedAt) @@ -506,7 +506,7 @@ func TestAMoveToBlockedOrDiscardedDropsTheSnapshot(t *testing.T) { seenRecord(t, ledger, 1) _, err := ledger.Admission().Commit(ctx, admittedVerdict(1, 0, "recording:9")) require.NoError(t, err) - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + dispatchForTest(t, ledger, 1) require.NoError(t, ledger.SetState(ctx, 1, StateCompleted, "")) assert.NotEmpty(t, getRecord(t, ledger, 1).Decision.Snapshot) }) @@ -571,7 +571,7 @@ func TestDropContentTakesTheVerdictToo(t *testing.T) { seenRecord(t, ledger, 1) _, err := ledger.Admission().Commit(ctx, admittedVerdict(1, 0, "recording:9")) require.NoError(t, err) - require.NoError(t, ledger.SetState(ctx, 1, StateDispatched, "")) + dispatchForTest(t, ledger, 1) require.NoError(t, ledger.SetState(ctx, 1, StateCompleted, "")) dropped, err := ledger.DropContent(ctx, at.Add(time.Hour), at.Add(time.Hour)) @@ -602,7 +602,7 @@ func TestMigrationFourCarriesAnEarlierLedger(t *testing.T) { t.Cleanup(func() { _ = ledger.Close() }) version, err := ledger.SchemaVersion(context.Background()) require.NoError(t, err) - assert.Equal(t, 4, version) + assert.Equal(t, len(migrations), version) ev, ok, err := ledger.Admission().LoadUndecided(context.Background(), 1) require.NoError(t, err) diff --git a/internal/connector/ledger_dispatch.go b/internal/connector/ledger_dispatch.go new file mode 100644 index 000000000..59c8dbfbb --- /dev/null +++ b/internal/connector/ledger_dispatch.go @@ -0,0 +1,1393 @@ +package connector + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "database/sql" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "html" + "net/url" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "time" + + "modernc.org/sqlite" + sqlite3 "modernc.org/sqlite/lib" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" +) + +// The dispatch lifecycle, as one state machine. +// +// Three things have state here, and a fourth is the guard on one of them. +// This comment is the contract; lifecycle (ledger_events.go), move, the +// schema's triggers and index, and the functions below enforce exactly it, +// and TestDispatchLifecycleTable tries every transition against it. +// +// # Record (events.state) +// +// from to who rule +// seen admitted admission verdict, conversation not live +// seen queued admission verdict, conversation live +// seen blocked admission verdict +// seen discarded admission verdict +// blocked admitted admission re-decided +// blocked queued admission re-decided, conversation live +// blocked blocked admission re-decided, still blocked +// blocked discarded admission, operator verdict, or discard +// blocked dispatched — an edge the lifecycle map has and no +// caller can take: a task needs an +// instruction, and blocked has none +// admitted dispatched dispatcher (CreateTask) joins a task +// queued dispatched dispatcher (CreateTask) joins a task +// dispatched dispatched dispatcher (CreateTask) redispatch onto a new task +// dispatched admitted dispatcher (SupersedeTask) never handed to a worker, task retired +// dispatched admitted dispatcher (withdrawExposure) exposed at launch, spawn proven failed: retry +// dispatched blocked dispatcher (withdrawExposure) exposed at launch, spawn failed again +// dispatched completed worker (complete_dispatch), dispatcher the outcome, reported or settled +// admitted queued lifecycle bookkeeping — +// admitted blocked lifecycle bookkeeping — +// admitted discarded operator discard +// queued blocked lifecycle bookkeeping — +// queued discarded operator discard +// completed — nobody terminal +// discarded — nobody terminal +// +// Writing the state a record already has is a repeat and always allowed. Any +// pair not in the table is refused. Into dispatched and out of it, the task +// decides, as a rule about the moves rather than about the state: a record +// enters dispatched only when a live task already carries it, and leaves it — +// other than to completed — only when none does +// (events_dispatched_while_on_a_live_task, and move). A record can therefore +// be dispatched with no live task, and one case is expected: work a worker was +// handed, whose task was superseded, waiting for its outcome (invariant 7). +// +// # Task (tasks) +// +// live created by the dispatcher (CreateTask); its token is valid +// superseded by the dispatcher or an operator's redispatch (SupersedeTask); +// its token is refused; terminal, and the row is never deleted +// +// An operator's redispatch is the dispatcher's own two writes in one +// transaction: supersedeTask on the live task — which refuses its token from +// then on, retires its rows, and returns what no worker was handed to +// admitted — and createTask for the events being run again. It can therefore +// redispatch an admitted, queued or dispatched record, which covers held work +// waiting for its outcome and the spawn-failure retry. A completed or +// discarded record it cannot: those are terminal here, so redispatching one +// is a decision this ledger does not carry (plan step 21 owns it). +// +// # Delivery (task_events.delivery), per event on a task +// +// admitted → exposed worker (get_dispatch), dispatcher at launch +// exposed → delivered worker (ack_dispatch); the id the worker points at, +// when it has one, is written with this move or never +// (task_events_acknowledgement_settles_once), so an id +// arriving after the acknowledgement is a second +// report and refused +// exposed → completed worker (complete_dispatch) +// exposed → completed dispatcher settlement (worker gone before ack) +// delivered → completed worker (complete_dispatch), dispatcher settlement +// exposed → withdrawn dispatcher (withdrawExposure): the spawn failed +// before any worker process existed. The database +// holds what it can see: an exposure written at +// launch that no worker ever pulled (pulled_at, set by +// a worker's first get_dispatch), on a superseded task, +// with no live task carrying the event; once, and the +// row moves no more. That no process existed at all is +// the driver's report, which the dispatcher acts on. +// +// Forward only, and never skipping exposure: nothing a worker was never +// handed is acknowledged or completed. A row is retired (retired_at) +// when its task is superseded; a worker can touch only its live task's rows. +// +// # Guard (task_events.guard) +// +// armed → canceled worker (get_dispatch) +// armed → fired connector's thirty-second acknowledgement +// +// '' (none) and armed are only ever written when the row is created. +// +// # Invariants +// +// 1. One live task per event: task_events_one_live_task. +// 2. One task per conversation, and in the conversation's order: an event +// joins a task only if no live task holds its conversation, no record on +// that conversation is dispatched outside this task, and no admitted +// record older than it is waiting there — admission queues a record +// precisely because an earlier one is next, and a queued record does not +// go around it (conversationsAreFree, and +// task_events_one_live_task_per_conversation). The conversation a task +// holds is written on the task's own rows, because retention clears a +// terminal record's conversation_key while its task is still live. +// 3. The token is valid only while its task is live, checked inside every +// worker call's own transaction. +// 4. Nothing leaves dispatched while a worker may still act: a record with a +// delivery exposed or delivered, on any task and not withdrawn, leaves +// dispatched only to completed (move, and the +// events_handed_work_settles_first trigger). The one release is the +// spec's automatic retry: an exposure written at launch whose spawn +// failed before any worker process existed is withdrawn, and the record +// returns to admitted for its one retry, or goes to blocked after a +// second failure (withdrawExposure) — the only way from dispatched to +// blocked. +// 5. A worker acts only on its own task's rows, reports only what it +// pulled, and a reported outcome stands — the report path enforces that, +// and the delivery triggers hold the same shape for anything else writing +// to the file: forward only, never past a missing pull, with the +// dispatcher settling a completed record as the one exception. +// 6. A task is made only of instructions a worker can pull, and finished +// work is never handed out for the first time: a completed record is +// served, acknowledged and completed only by the worker that pulled it +// (pulled_at), never on the strength of an exposure written at launch. +// 7. Superseding retires the task's rows and returns only what it never +// exposed to admitted; what a worker was handed stays dispatched (4) and +// waits for its outcome or a redispatch, which supersedes and creates in +// one transaction (supersedeTask and createTask take the caller's). +// 8. Completed and discarded are terminal (events_terminal_is_terminal). +// +// Every transaction takes the write lock as it opens (_txlock=immediate), and +// each call ends it as soon as its ledger work is done. + +// Delivery is an event's delivery state on a task. It moves forward only. +type Delivery string + +const ( + // DeliveryAdmitted is on the task and not yet handed to a worker. + DeliveryAdmitted Delivery = "admitted" + // DeliveryExposed was handed to a worker, which may have acted on it. + DeliveryExposed Delivery = "exposed" + // DeliveryDelivered was acknowledged by the worker. + DeliveryDelivered Delivery = "delivered" + // DeliveryCompleted has the worker's outcome. + DeliveryCompleted Delivery = "completed" +) + +// Outcome is what a worker reports for an event. +type Outcome string + +const ( + OutcomeSucceeded Outcome = "succeeded" + OutcomeFailed Outcome = "failed" +) + +// Refusals a worker's dispatch call can meet. Each is an answer, not a fault. +var ( + // ErrTaskTokenRefused is a token that names no task, or one a + // redispatch superseded. The two are not told apart: a worker holding + // either has no task. + ErrTaskTokenRefused = errors.New("the task token names no current task") + // ErrNotOnTask is an event id the token's task does not carry. Whether + // the event exists on another task is not said. + ErrNotOnTask = errors.New("the event is not on this task") + // ErrNotExposed is an acknowledgement or completion for an event the + // worker was never handed. + ErrNotExposed = errors.New("the event was not handed to this worker; call get_dispatch for it first") + // ErrReportConflict is a second report that disagrees with the first. + // Reported outcomes stand. + ErrReportConflict = errors.New("the event already has a different report") + // ErrNotDispatchable is an event whose record left the path to a worker + // after it joined the task: blocked or withdrawn, or its content dropped. + ErrNotDispatchable = errors.New("the event can no longer be dispatched") + // ErrInvalidReport is a report the worker can correct: an outcome that + // is not one of the two, a link that is not a URL, too many links. + ErrInvalidReport = errors.New("the report is not valid") + // ErrConversationBusy is an event whose conversation already has a + // dispatched record outside the task being created: a running task, or + // work a worker was handed that is not settled yet. A conversation has + // one task at a time. + ErrConversationBusy = errors.New("the event's conversation already has a task") + // ErrNoSuchTask is a task id the ledger does not hold. + ErrNoSuchTask = errors.New("no such task") + // ErrEventOnLiveTask is an event a live task already carries. Handing it + // to a second task would give two workers one instruction. + ErrEventOnLiveTask = errors.New("the event is already on a live task") +) + +// TaskGrant is a new task and the token that binds a worker to it. The token +// is returned once and stored only as a hash. +type TaskGrant struct { + ID int64 + Token string +} + +// CreateTask puts records on a new task at delivery admitted, with the +// acknowledgement guard armed for the records whose verdict asks for one, and +// moves each record to dispatched in the same transaction: a record is +// dispatched exactly while a live task carries it. +// +// An event is on at most one live task. A second task for an event whose task +// was not superseded is refused with ErrEventOnLiveTask, and nothing is +// written. The refusal is the database's own (task_events_one_live_task), so a +// retried or concurrent launch cannot get past it. A redispatch supersedes the +// old task first. +func (l *Ledger) CreateTask(ctx context.Context, eventIDs []int64) (TaskGrant, error) { + var grant TaskGrant + err := retryBusy(func() error { + tx, err := l.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("connector: begin task: %w", err) + } + defer func() { _ = tx.Rollback() }() + if grant, err = l.createTask(ctx, tx, eventIDs); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("connector: commit task: %w", err) + } + return nil + }) + return grant, err +} + +// createTask writes a task inside the caller's transaction, so the dispatcher +// can write the task, its attempt and the originating event's exposure as one +// commit. Every guarantee CreateTask documents holds within tx; nothing is +// committed here, and a refusal leaves tx for the caller to roll back. +func (l *Ledger) createTask(ctx context.Context, tx *sql.Tx, eventIDs []int64) (TaskGrant, error) { + if len(eventIDs) == 0 { + return TaskGrant{}, errors.New("connector: a task needs at least one event") + } + seen := make(map[int64]bool, len(eventIDs)) + for _, id := range eventIDs { + if seen[id] { + return TaskGrant{}, fmt.Errorf("connector: event %d is named twice for one task", id) + } + seen[id] = true + } + raw := make([]byte, 32) + if _, err := rand.Read(raw); err != nil { + return TaskGrant{}, fmt.Errorf("connector: task token: %w", err) + } + token := base64.RawURLEncoding.EncodeToString(raw) + + // Read first, write after: a refusal leaves the caller's transaction as + // it found it, whatever the caller then does with it. + guards := make([]string, 0, len(eventIDs)) + conversations := make([]string, 0, len(eventIDs)) + for _, id := range eventIDs { + var ( + acknowledge, hasInstruction int + state, conversation string + ) + switch err := tx.QueryRowContext(ctx, `SELECT acknowledge, content_dropped = 0 AND snapshot IS NOT NULL AND length(snapshot) > 0, state, conversation_key FROM events WHERE id = ?`, id).Scan(&acknowledge, &hasInstruction, &state, &conversation); { + case errors.Is(err, sql.ErrNoRows): + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, ErrNoSuchRecord) + case err != nil: + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, err) + } + if s := RecordState(state); s != StateAdmitted && s != StateQueued && s != StateDispatched { + return TaskGrant{}, fmt.Errorf("connector: task event %d is %s; only admitted, queued or redispatched work joins a task", id, state) + } + var onLive bool + if err := tx.QueryRowContext(ctx, `SELECT EXISTS (SELECT 1 FROM task_events WHERE event_id = ? AND retired_at IS NULL)`, id).Scan(&onLive); err != nil { + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, err) + } + if onLive { + // The unique index refuses it too, whoever writes; this is the + // same refusal with the event named. + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, ErrEventOnLiveTask) + } + if hasInstruction == 0 { + // A task a worker could pull nothing from would read as a task + // with nothing left to do. A record without its instruction + // needs a new verdict first. + return TaskGrant{}, fmt.Errorf("connector: task event %d has no instruction: %w", id, ErrNotDispatchable) + } + guard := "" + if acknowledge != 0 { + guard = "armed" + } + guards = append(guards, guard) + conversations = append(conversations, conversation) + } + + // One task per conversation (invariant 2), asked as two questions, both + // before anything is written: a refusal leaves the caller's transaction + // untouched, and the records this call dispatches never count themselves. + if err := l.conversationsAreFree(ctx, tx, eventIDs, conversations); err != nil { + return TaskGrant{}, err + } + + res, err := tx.ExecContext(ctx, `INSERT INTO tasks (token_sha256, created_at) VALUES (?, ?)`, tokenHash(token), l.timestamp()) + if err != nil { + return TaskGrant{}, fmt.Errorf("connector: create task: %w", err) + } + taskID, err := res.LastInsertId() + if err != nil { + return TaskGrant{}, fmt.Errorf("connector: create task: %w", err) + } + for i, id := range eventIDs { + if _, err := tx.ExecContext(ctx, `INSERT INTO task_events (task_id, event_id, guard, conversation_key) VALUES (?, ?, ?, ?)`, taskID, id, guards[i], conversations[i]); err != nil { + if isConstraint(err) { + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, ErrEventOnLiveTask) + } + return TaskGrant{}, fmt.Errorf("connector: task event %d: %w", id, err) + } + } + for _, id := range eventIDs { + // Admitted or queued work joins a task; a dispatched record whose + // task was superseded joins its replacement. + // The pre-pass read this state; the move is what makes it so under a + // concurrent writer, and refuses if it changed underneath. + moved, err := l.move(ctx, tx, transition{id: id, state: StateDispatched, from: []RecordState{StateAdmitted, StateQueued, StateDispatched}}) + if err != nil { + return TaskGrant{}, err + } + if !moved { + return TaskGrant{}, fmt.Errorf("connector: task event %d changed state while its task was being written", id) + } + } + return TaskGrant{ID: taskID, Token: token}, nil +} + +// conversationsAreFree asks whether the conversations these events are on are +// free for a task of their own. +// +// Two questions, because the answer lives in two places. A record waiting for +// a worker or still dispatched is in events: admitted means admission decided +// it goes next and a queued sibling may not jump ahead of it, and dispatched +// means work a worker may still be holding, whether or not a live task +// carries it (invariant 7). A conversation a live task already holds is in +// task_events, which keeps the conversation of every row it was written for — +// the record's own conversation_key is cleared once retention reaches it, and +// a task whose records are all terminal is still a worker's task until it is +// superseded. +func (l *Ledger) conversationsAreFree(ctx context.Context, tx *sql.Tx, eventIDs []int64, conversations []string) error { + placeholders := strings.TrimSuffix(strings.Repeat("?, ", len(eventIDs)), ", ") + for i, conversation := range conversations { + if conversation == "" { + continue + } + args := make([]any, 0, len(eventIDs)+1) + args = append(args, conversation) + for _, id := range eventIDs { + args = append(args, id) + } + + var ( + other int64 + otherState string + ) + //nolint:gosec // G202: placeholders, not values + // + // An older admitted sibling is next on its conversation, and a queued + // one does not go around it: admission queues a record precisely + // because an earlier one holds the conversation. A dispatched sibling + // counts whatever its id, because a worker may still be holding it. + switch err := tx.QueryRowContext(ctx, ` +SELECT id, state FROM events +WHERE conversation_key = ? AND id NOT IN (`+placeholders+`) + AND (state = 'dispatched' OR (state = 'admitted' AND id < ?)) +ORDER BY id LIMIT 1`, append(args, eventIDs[i])...).Scan(&other, &otherState); { + case err == nil && RecordState(otherState) == StateDispatched: + return fmt.Errorf("connector: event %d is dispatched on the same conversation: %w", other, ErrConversationBusy) + case err == nil: + return fmt.Errorf("connector: event %d is admitted ahead of event %d on the same conversation: %w", other, eventIDs[i], ErrConversationBusy) + case !errors.Is(err, sql.ErrNoRows): + return fmt.Errorf("connector: read conversations: %w", err) + } + + var task int64 + //nolint:gosec // G202: placeholders, not values + switch err := tx.QueryRowContext(ctx, ` +SELECT task_id FROM task_events +WHERE conversation_key = ? AND retired_at IS NULL AND event_id NOT IN (`+placeholders+`) +ORDER BY task_id LIMIT 1`, args...).Scan(&task); { + case err == nil: + return fmt.Errorf("connector: task %d is live on the conversation of event %d: %w", task, eventIDs[i], ErrConversationBusy) + case !errors.Is(err, sql.ErrNoRows): + return fmt.Errorf("connector: read live tasks: %w", err) + } + } + return nil +} + +// SupersedeTask retires a task: its token is refused from then on, and its +// events are free to join a new task. An event this task never exposed +// returns to admitted, to be dispatched again once its conversation is free — +// unless something else still holds it dispatched, which invariant 4 decides +// and this call does not override: an earlier task's exposure that was never +// withdrawn keeps the record dispatched, waiting for its outcome, and a +// record a person or a later verdict has moved stays where it was put. An +// event a worker was handed stays dispatched for the same reason: that worker +// may have acted on it, and it waits for its settlement or a redispatch. +func (l *Ledger) SupersedeTask(ctx context.Context, taskID int64) error { + return retryBusy(func() error { + tx, err := l.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("connector: begin supersede: %w", err) + } + defer func() { _ = tx.Rollback() }() + if err := l.supersedeTask(ctx, tx, taskID); err != nil { + return err + } + return tx.Commit() + }) +} + +// supersedeTask is SupersedeTask inside the caller's transaction, so a +// redispatch can retire the old task and create the new one in one commit. +func (l *Ledger) supersedeTask(ctx context.Context, tx *sql.Tx, taskID int64) error { + var exists bool + if err := tx.QueryRowContext(ctx, `SELECT EXISTS (SELECT 1 FROM tasks WHERE id = ?)`, taskID).Scan(&exists); err != nil { + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + if !exists { + return fmt.Errorf("connector: supersede task %d: %w", taskID, ErrNoSuchTask) + } + rows, err := tx.QueryContext(ctx, `SELECT event_id FROM task_events WHERE task_id = ? AND retired_at IS NULL AND delivery = 'admitted'`, taskID) + if err != nil { + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + var unexposed []int64 + for rows.Next() { + var id int64 + if err := rows.Scan(&id); err != nil { + _ = rows.Close() + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + unexposed = append(unexposed, id) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + // Retiring the task on a partial list would leave the rest dispatched + // on no live task, never exposed and never returned. + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + if err := rows.Close(); err != nil { + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + + now := l.timestamp() + if _, err := tx.ExecContext(ctx, `UPDATE tasks SET superseded_at = COALESCE(superseded_at, ?) WHERE id = ?`, now, taskID); err != nil { + return fmt.Errorf("connector: supersede task %d: %w", taskID, err) + } + for _, id := range unexposed { + // Only a record still dispatched moves, and only if nothing else + // holds it: move's own refusals — a worker holding it on another + // task, a record a person or a later verdict moved — are the answer, + // not an error, so the result is read for what it is and not acted + // on. + if _, err := l.move(ctx, tx, transition{id: id, state: StateAdmitted, from: []RecordState{StateDispatched}}); err != nil { + return err + } + } + return nil +} + +// withdrawExposure releases an event whose worker never existed: the driver +// proved the spawn itself failed, after the dispatcher had written the +// originating event exposed at launch. It runs in the caller's transaction, +// after supersedeTask on the same task. The record goes to admitted, to be +// retried once, or — after a second failure — to blocked with reason. The +// database refuses the marker for anything but a launch exposure no worker +// pulled, on a superseded task, while no live task carries the event, and +// refuses it twice. So the order is supersede, withdraw, then create the +// retry task. +func (l *Ledger) withdrawExposure(ctx context.Context, tx *sql.Tx, taskID, eventID int64, to RecordState, reason string) error { + if to != StateAdmitted && to != StateBlocked { + return fmt.Errorf("connector: withdraw event %d: a withdrawn event is retried (admitted) or blocked, not %s", eventID, to) + } + res, err := tx.ExecContext(ctx, `UPDATE task_events SET withdrawn_at = ? WHERE task_id = ? AND event_id = ?`, l.timestamp(), taskID, eventID) + if err != nil { + return fmt.Errorf("connector: withdraw event %d: %w", eventID, err) + } + if n, err := res.RowsAffected(); err != nil || n == 0 { + return fmt.Errorf("connector: withdraw event %d: %w", eventID, ErrNotOnTask) + } + moved, err := l.move(ctx, tx, transition{id: eventID, state: to, reason: reason, from: []RecordState{StateDispatched}}) + if err != nil { + return err + } + if !moved { + return fmt.Errorf("connector: withdraw event %d: another worker still holds it: %w", eventID, ErrHeldByWorker) + } + return nil +} + +// isConstraint reports a SQLite constraint violation. +func isConstraint(err error) bool { + var sqliteErr *sqlite.Error + return errors.As(err, &sqliteErr) && sqliteErr.Code()&0xff == sqlite3.SQLITE_CONSTRAINT +} + +func tokenHash(token string) string { + sum := sha256.Sum256([]byte(token)) + return hex.EncodeToString(sum[:]) +} + +// TaskDispatch is the ledger as one worker sees it: the events on the task its +// token names, and nothing else. There is no listing; a worker never reads +// other tasks. +type TaskDispatch struct { + ledger *Ledger + hash string + agentID int64 + + // afterTx runs when a call has closed its transaction and is building its + // answer. A test seam: it is where the ledger must already be free. + afterTx func() +} + +// Dispatch binds the ledger to a worker's task token, refusing one that names +// no live task now. The token is checked again on every call, in the call's +// own transaction, so a redispatch that supersedes it later takes effect at +// once. agentID is the agent's Person id, whose own mentions are stripped from +// the instructions handed out. +func (l *Ledger) Dispatch(ctx context.Context, token string, agentID int64) (*TaskDispatch, error) { + if strings.TrimSpace(token) == "" { + return nil, errors.New("connector: a dispatch needs the task token") + } + if agentID <= 0 { + return nil, errors.New("connector: a dispatch needs the agent's Person id") + } + d := &TaskDispatch{ledger: l, hash: tokenHash(token), agentID: agentID} + var live int + if err := l.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM tasks WHERE token_sha256 = ? AND superseded_at IS NULL`, d.hash).Scan(&live); err != nil { + return nil, fmt.Errorf("connector: resolve task token: %w", err) + } + if live == 0 { + return nil, fmt.Errorf("connector: %w", ErrTaskTokenRefused) + } + return d, nil +} + +// Instruction is what get_dispatch hands a worker. It is an allowlist: every +// field is named here, and nothing the ledger holds reaches a worker unless +// it is one of them. No route, no feed position, no token. +type Instruction struct { + EventID int64 `json:"event_id"` + EventType string `json:"event_type"` + Trigger string `json:"trigger"` + Class string `json:"class,omitempty"` + + Recording InstructionRecording `json:"recording"` + ReplyTo InstructionReply `json:"reply_to"` + RequesterID int64 `json:"requester_id"` + + // Acknowledge says a person asked for something: the worker acknowledges + // it and reports the id through ack_dispatch. + Acknowledge bool `json:"acknowledge"` + // GuardAcknowledged says the connector's guard already acknowledged the + // event, so the worker does not acknowledge it again. + GuardAcknowledged bool `json:"guard_acknowledged"` + Delivery Delivery `json:"delivery"` + + // Content is the instruction as admission read it, with the agent's own + // mention stripped. The live recording may be newer. + Content string `json:"content"` + ContentUpdatedAt time.Time `json:"content_updated_at"` +} + +// InstructionRecording points at the recording the event is about. +type InstructionRecording struct { + BucketID int64 `json:"bucket_id"` + RecordingID int64 `json:"recording_id"` + Type string `json:"type"` + Title string `json:"title"` + URL string `json:"url"` +} + +// InstructionReply is where the worker's acknowledgement and reply go. +type InstructionReply struct { + Kind string `json:"kind"` + RecordingID int64 `json:"recording_id"` +} + +// Receipt is the delivery state an acknowledgement or completion left. +type Receipt struct { + EventID int64 `json:"event_id"` + Delivery Delivery `json:"delivery"` + AckID *int64 `json:"ack_id,omitempty"` + Outcome Outcome `json:"outcome,omitempty"` + ReplyID *int64 `json:"reply_id,omitempty"` + Links []string `json:"links,omitempty"` +} + +// Completion is a worker's report for one event. +type Completion struct { + Outcome Outcome + Links []string + ReplyID *int64 +} + +// Completion limits: a report is a handful of links, not a document. +const ( + maxCompletionLinks = 20 + maxLinkLength = 2048 +) + +// Get returns the instruction for eventID, or for the earliest event on the +// task not yet acknowledged when eventID is zero; ok is false when there is +// none. Handing out an event that was never exposed writes exposed before the +// instruction is returned, and cancels an armed guard. A repeat returns the +// same instruction and writes nothing. +// +// Only an event whose record is still on the way to a worker is handed out: +// dispatched, or completed, with its content. The earliest skips any other, +// so one event withdrawn or blocked never hides the rest of the task. +func (d *TaskDispatch) Get(ctx context.Context, eventID int64) (Instruction, bool, error) { + var ( + out Instruction + ok bool + ) + err := retryBusy(func() error { + var err error + out, ok, err = d.get(ctx, eventID) + return err + }) + return out, ok, err +} + +type taskEvent struct { + pulled bool + delivery Delivery + guard string + ackID sql.NullInt64 + outcome string + links string + replyID sql.NullInt64 +} + +// get is one get_dispatch: the ledger work in a transaction, the instruction +// built once it is closed. +func (d *TaskDispatch) get(ctx context.Context, eventID int64) (Instruction, bool, error) { + l := d.ledger + tx, err := l.db.BeginTx(ctx, nil) + if err != nil { + return Instruction{}, false, fmt.Errorf("connector: begin get_dispatch: %w", err) + } + defer func() { _ = tx.Rollback() }() + taskID, err := d.task(ctx, tx) + if err != nil { + return Instruction{}, false, err + } + + if eventID == 0 { + err := tx.QueryRowContext(ctx, ` +SELECT te.event_id FROM task_events te JOIN events e ON e.id = te.event_id +WHERE te.task_id = ? AND te.retired_at IS NULL AND te.delivery IN ('admitted', 'exposed') AND `+servableSQL+` +ORDER BY te.event_id LIMIT 1`, taskID).Scan(&eventID) + if errors.Is(err, sql.ErrNoRows) { + return Instruction{}, false, nil + } + if err != nil { + return Instruction{}, false, fmt.Errorf("connector: get_dispatch: %w", err) + } + } + te, err := loadTaskEvent(ctx, tx, taskID, eventID) + if err != nil { + return Instruction{}, false, err + } + record, err := loadRecord(ctx, tx, eventID) + if err != nil { + return Instruction{}, false, err + } + if !servable(record, te.pulled) { + return Instruction{}, false, fmt.Errorf("connector: event %d: %w", eventID, ErrNotDispatchable) + } + + now := l.timestamp() + wrote := false + if te.delivery == DeliveryAdmitted { + // Written before anything about the event leaves this call: a worker + // handed an instruction may act on it whether or not it reports. + if _, err := tx.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed', exposed_at = ? WHERE task_id = ? AND event_id = ? AND delivery = 'admitted'`, now, taskID, eventID); err != nil { + return Instruction{}, false, fmt.Errorf("connector: expose event %d: %w", eventID, err) + } + wrote = true + } + if !te.pulled { + // The first pull by a worker is recorded even when the dispatcher + // already exposed the event at launch: from here on a worker has the + // instruction, and the exposure can no longer be withdrawn as a + // spawn that failed before any worker existed. + if _, err := tx.ExecContext(ctx, `UPDATE task_events SET pulled_at = ? WHERE task_id = ? AND event_id = ? AND pulled_at IS NULL`, now, taskID, eventID); err != nil { + return Instruction{}, false, fmt.Errorf("connector: record the pull of %d: %w", eventID, err) + } + wrote = true + } + if te.guard == "armed" { + if _, err := tx.ExecContext(ctx, `UPDATE task_events SET guard = 'canceled' WHERE task_id = ? AND event_id = ? AND guard = 'armed'`, taskID, eventID); err != nil { + return Instruction{}, false, fmt.Errorf("connector: cancel guard on %d: %w", eventID, err) + } + wrote = true + } + if wrote { + // Read back what the writes left rather than what was read before + // them: the instruction reports the row, not this call's expectation + // of it. + if te, err = loadTaskEvent(ctx, tx, taskID, eventID); err != nil { + return Instruction{}, false, err + } + if err := tx.Commit(); err != nil { + return Instruction{}, false, fmt.Errorf("connector: commit get_dispatch: %w", err) + } + } else if err := tx.Rollback(); err != nil && !errors.Is(err, sql.ErrTxDone) { + return Instruction{}, false, fmt.Errorf("connector: end get_dispatch: %w", err) + } + // The transaction is over. Every transaction here takes the write lock as + // it opens, so decoding the snapshot and stripping the agent's mention — + // the only work in this call that is not a query — happens with the + // ledger free for the connector and for other workers. + if d.afterTx != nil { + d.afterTx() + } + + var snapshot struct { + Type string `json:"type"` + Title string `json:"title"` + AppURL string `json:"app_url"` + Content string `json:"content"` + UpdatedAt time.Time `json:"updated_at"` + } + if err := json.Unmarshal(record.Decision.Snapshot, &snapshot); err != nil { + return Instruction{}, false, fmt.Errorf("connector: event %d snapshot: %w", eventID, err) + } + return Instruction{ + EventID: record.ID, + EventType: record.EventType, + Trigger: record.Decision.Trigger, + Class: record.Decision.Class, + Recording: InstructionRecording{ + BucketID: record.BucketID, + RecordingID: record.RecordingID, + Type: snapshot.Type, + Title: snapshot.Title, + URL: record.Decision.RecordingURL, + }, + ReplyTo: InstructionReply{Kind: record.Decision.ReplyKind, RecordingID: record.Decision.ReplyRecordingID}, + RequesterID: record.Decision.RequesterID, + Acknowledge: record.Decision.Acknowledge, + GuardAcknowledged: te.guard == "fired", + Delivery: te.delivery, + Content: StripMentionsOf(snapshot.Content, d.agentID), + ContentUpdatedAt: snapshot.UpdatedAt, + }, true, nil +} + +// servable is whether an event on a task is handed to its worker: its record +// is dispatched, or completed after this worker pulled it — finished work is +// never handed out for the first time — and it still has its instruction. servableSQL is the same rule over task_events te and events e, +// for the earliest-event query; the two are kept side by side so they cannot +// drift. +func servable(record Record, pulled bool) bool { + // Exposure at launch is the dispatcher's write, not a worker's pull, so + // finished work is served again only to a worker that already had it. + state := record.State == StateDispatched || (record.State == StateCompleted && pulled) + return state && !record.ContentDropped && len(record.Decision.Snapshot) > 0 +} + +const servableSQL = `(e.state = 'dispatched' OR (e.state = 'completed' AND te.pulled_at IS NOT NULL)) + AND e.content_dropped = 0 AND e.snapshot IS NOT NULL AND length(e.snapshot) > 0` + +// Ack records the worker's acknowledgement: delivery moves to delivered, and +// ackID, when given, is the worker's own boost or comment. A repeat — a lost +// tool response retried — answers the same receipt. +func (d *TaskDispatch) Ack(ctx context.Context, eventID int64, ackID *int64) (Receipt, error) { + var out Receipt + err := retryBusy(func() error { + var err error + out, err = d.report(ctx, eventID, func(ctx context.Context, tx *sql.Tx, taskID int64, te taskEvent) (bool, error) { + if ackID != nil && te.ackID.Valid && te.ackID.Int64 != *ackID { + return false, fmt.Errorf("connector: event %d acknowledged as %d: %w", eventID, te.ackID.Int64, ErrReportConflict) + } + if ackID != nil && !te.ackID.Valid && te.delivery != DeliveryExposed { + // The acknowledgement settles with the delivery, id and all: + // this event was acknowledged with nothing to point at, and + // an id arriving afterwards is a second, different report, + // not the same call retried. + return false, fmt.Errorf("connector: event %d was acknowledged without an acknowledgement id: %w", eventID, ErrReportConflict) + } + if te.delivery != DeliveryExposed { + // Acknowledged already, and by whatever this call says: an + // id equal to the one recorded, or none to record. The + // receipt is the answer. + return false, nil + } + _, err := tx.ExecContext(ctx, ` +UPDATE task_events +SET delivery = CASE WHEN delivery = 'exposed' THEN 'delivered' ELSE delivery END, + delivered_at = COALESCE(delivered_at, ?), + ack_id = COALESCE(ack_id, ?) +WHERE task_id = ? AND event_id = ?`, d.ledger.timestamp(), nullableID(ackID), taskID, eventID) + return true, err + }) + return err + }) + return out, err +} + +// Complete records the worker's outcome and acknowledges the event if it was +// not already. The report is recorded whatever has happened to the record +// since the worker was handed it, because it is what the worker did; the +// record moves to completed when it is dispatched. A repeat of the same report +// answers the same receipt; a different one is refused, because a reported +// outcome stands. +func (d *TaskDispatch) Complete(ctx context.Context, eventID int64, c Completion) (Receipt, error) { + var out Receipt + err := retryBusy(func() error { + var err error + out, err = d.report(ctx, eventID, func(ctx context.Context, tx *sql.Tx, taskID int64, te taskEvent) (bool, error) { + // Validated inside the call's transaction, after the token and + // the exposure are: a worker whose task was superseded is told + // that, whatever it sent. + if c.Outcome != OutcomeSucceeded && c.Outcome != OutcomeFailed { + return false, fmt.Errorf("connector: outcome must be %q or %q: %w", OutcomeSucceeded, OutcomeFailed, ErrInvalidReport) + } + links, err := normalizeLinks(c.Links) + if err != nil { + return false, err + } + encoded, err := json.Marshal(links) + if err != nil { + return false, err + } + if te.delivery == DeliveryCompleted { + if te.outcome == string(c.Outcome) && te.links == string(encoded) && sameID(te.replyID, c.ReplyID) { + return false, nil + } + return false, fmt.Errorf("connector: event %d completed as %s: %w", eventID, te.outcome, ErrReportConflict) + } + // The delivery first, the record after: while the record is still + // dispatched, task_events_exposure_comes_first reads this as a + // worker's completion and holds it to the pull. Completing the + // record first would make every completion look like the + // dispatcher settling one. + now := d.ledger.timestamp() + if _, err = tx.ExecContext(ctx, ` +UPDATE task_events +SET delivery = 'completed', delivered_at = COALESCE(delivered_at, ?), completed_at = ?, + outcome = ?, links = ?, reply_id = ? +WHERE task_id = ? AND event_id = ?`, now, now, string(c.Outcome), string(encoded), nullableID(c.ReplyID), taskID, eventID); err != nil { + return false, err + } + if _, err := d.ledger.move(ctx, tx, transition{id: eventID, state: StateCompleted, from: []RecordState{StateDispatched}}); err != nil { + return false, err + } + return true, nil + }) + return err + }) + return out, err +} + +// report runs an acknowledgement or completion in one transaction: the token +// checked, the event found on the task and known to have been exposed, apply +// deciding whether to write, and the receipt read back. +func (d *TaskDispatch) report(ctx context.Context, eventID int64, apply func(context.Context, *sql.Tx, int64, taskEvent) (bool, error)) (Receipt, error) { + if eventID <= 0 { + return Receipt{}, errors.New("connector: event_id must name an event") + } + tx, err := d.ledger.db.BeginTx(ctx, nil) + if err != nil { + return Receipt{}, fmt.Errorf("connector: begin report: %w", err) + } + defer func() { _ = tx.Rollback() }() + taskID, err := d.task(ctx, tx) + if err != nil { + return Receipt{}, err + } + te, err := loadTaskEvent(ctx, tx, taskID, eventID) + if err != nil { + return Receipt{}, err + } + if !te.pulled { + // Exposure at launch is not a worker having the instruction: the + // pull is. A worker reports only what it pulled. + return Receipt{}, fmt.Errorf("connector: event %d: %w", eventID, ErrNotExposed) + } + wrote, err := apply(ctx, tx, taskID, te) + if err != nil { + // A refusal the worker can read is passed on as it is: wrapping it + // would put this package's name in the middle of the message the + // worker is shown. + for _, refusal := range []error{ErrInvalidReport, ErrReportConflict, ErrNotDispatchable, ErrHeldByWorker} { + if errors.Is(err, refusal) { + return Receipt{}, err + } + } + return Receipt{}, fmt.Errorf("connector: report on event %d: %w", eventID, err) + } + if wrote { + if te, err = loadTaskEvent(ctx, tx, taskID, eventID); err != nil { + return Receipt{}, err + } + if err := tx.Commit(); err != nil { + return Receipt{}, fmt.Errorf("connector: commit report on %d: %w", eventID, err) + } + } else if err := tx.Rollback(); err != nil && !errors.Is(err, sql.ErrTxDone) { + return Receipt{}, fmt.Errorf("connector: end report on %d: %w", eventID, err) + } + // The ledger is free before the receipt is built, as in get. + if d.afterTx != nil { + d.afterTx() + } + receipt := Receipt{EventID: eventID, Delivery: te.delivery, Outcome: Outcome(te.outcome)} + if te.ackID.Valid { + receipt.AckID = &te.ackID.Int64 + } + if te.replyID.Valid { + receipt.ReplyID = &te.replyID.Int64 + } + if err := json.Unmarshal([]byte(te.links), &receipt.Links); err != nil { + return Receipt{}, fmt.Errorf("connector: event %d links: %w", eventID, err) + } + return receipt, nil +} + +// task resolves the token to its task inside tx, or refuses it. +func (d *TaskDispatch) task(ctx context.Context, tx *sql.Tx) (int64, error) { + var id int64 + err := tx.QueryRowContext(ctx, `SELECT id FROM tasks WHERE token_sha256 = ? AND superseded_at IS NULL`, d.hash).Scan(&id) + if errors.Is(err, sql.ErrNoRows) { + return 0, fmt.Errorf("connector: %w", ErrTaskTokenRefused) + } + if err != nil { + return 0, fmt.Errorf("connector: resolve task token: %w", err) + } + return id, nil +} + +func loadTaskEvent(ctx context.Context, tx *sql.Tx, taskID, eventID int64) (taskEvent, error) { + var ( + te taskEvent + delivery string + ) + err := tx.QueryRowContext(ctx, ` +SELECT delivery, guard, ack_id, outcome, links, reply_id, pulled_at IS NOT NULL +FROM task_events WHERE task_id = ? AND event_id = ? AND retired_at IS NULL`, taskID, eventID).Scan(&delivery, &te.guard, &te.ackID, &te.outcome, &te.links, &te.replyID, &te.pulled) + if errors.Is(err, sql.ErrNoRows) { + return te, fmt.Errorf("connector: event %d: %w", eventID, ErrNotOnTask) + } + if err != nil { + return te, fmt.Errorf("connector: load task event %d: %w", eventID, err) + } + te.delivery = Delivery(delivery) + return te, nil +} + +func loadRecord(ctx context.Context, tx *sql.Tx, id int64) (Record, error) { + rows, err := tx.QueryContext(ctx, selectRecords+` WHERE id = ?`, id) + if err != nil { + return Record{}, fmt.Errorf("connector: load event %d: %w", id, err) + } + records, err := scanRecords(rows) + if err != nil { + return Record{}, err + } + if len(records) == 0 { + return Record{}, fmt.Errorf("connector: event %d: %w", id, ErrNoSuchRecord) + } + return records[0], nil +} + +func normalizeLinks(links []string) ([]string, error) { + if len(links) > maxCompletionLinks { + return nil, fmt.Errorf("connector: at most %d links: %w", maxCompletionLinks, ErrInvalidReport) + } + out := make([]string, 0, len(links)) + for _, link := range links { + if len(link) > maxLinkLength { + return nil, fmt.Errorf("connector: a link is at most %d characters: %w", maxLinkLength, ErrInvalidReport) + } + u, err := url.Parse(link) + if err != nil || (u.Scheme != "https" && u.Scheme != "http") || u.Host == "" { + return nil, fmt.Errorf("connector: link %q is not an http(s) URL: %w", link, ErrInvalidReport) + } + out = append(out, link) + } + return out, nil +} + +func nullableID(id *int64) any { + if id == nil { + return nil + } + return *id +} + +func sameID(stored sql.NullInt64, given *int64) bool { + if given == nil { + return !stored.Valid + } + return stored.Valid && stored.Int64 == *given +} + +// StripMentionsOf removes every mention of personID from rich text, and +// leaves the rest as it was. A worker handed its own mention reads an +// instruction addressed to itself, which says nothing the dispatch does not +// already say. +// +// What counts as a mention is exactly what basecamp.MentionedPersonIDs — the +// reader admission decided the trigger with — counts: the tag walk below is +// that reader's, rule for rule (comments, "") { + // A self-closing tag is the whole element: what follows + // is not its content, and a later stray closing tag is + // not its end. + pos = t.end + } else { + pos = mentionEnd(text, t.end) + } + removed = append(removed, [2]int{t.start, pos}) + continue + } + } + out.WriteString(text[pos:t.end]) + pos = t.end + } + out.WriteString(text[pos:]) + return out.String(), removed +} + +// mentionEnd is where the mention whose start tag ends at from ends: after the +// first , or at from when another attachment starts first, +// some other element closes first, or none closes. +func mentionEnd(text string, from int) int { + var open []string + for at := from; ; { + t, ok := nextMarkup(text, at) + if !ok { + return from + } + switch { + case strings.EqualFold(t.name, "bc-attachment"): + if t.isEnd { + return t.end + } + // Another attachment starts: this one was never closed. + return from + case t.isEnd: + // An end tag for something opened inside the mention — a + // mention's own figure closes its parts — is part of it. One that + // closes nothing opened here belongs to an element around the + // mention, so the closing tag further on is not this mention's: + // the start tag stands alone rather than swallowing what follows. + depth := len(open) - 1 + for depth >= 0 && !strings.EqualFold(open[depth], t.name) { + depth-- + } + if depth < 0 { + return from + } + open = open[:depth] + case !isVoidElement(t.name): + open = append(open, t.name) + } + at = t.end + } +} + +// isVoidElement reports the elements of Basecamp's rich text that have no end +// tag, so an unclosed one of them does not look like something still open. +func isVoidElement(name string) bool { + switch strings.ToLower(name) { + case "br", "hr", "img", "source", "input", "meta", "link": + return true + } + return false +} + +// markup is one start or end tag the walk found: where it starts and ends, +// its name, whether it is an end tag, and a start tag's first sgid, decoded. +type markup struct { + start, end int + name string + isEnd bool + sgid string +} + +// nextMarkup returns the next start or end tag at or after pos, walking the +// text as basecamp.MentionedPersonIDs does. ok is false when the markup ends: +// no "<" left, or a comment, declaration or tag left unterminated, after +// which nothing is markup. +func nextMarkup(text string, pos int) (markup, bool) { + for pos < len(text) { + i := strings.IndexByte(text[pos:], '<') + if i < 0 { + return markup{}, false + } + start := pos + i + pos = start + 1 + rest := text[pos:] + switch { + case strings.HasPrefix(rest, "!--"): + stop := strings.Index(rest, "-->") + if stop < 0 { + return markup{}, false + } + pos += stop + 3 + continue + case strings.HasPrefix(rest, "/"): + stop := strings.IndexByte(rest, '>') + if stop < 0 { + return markup{}, false + } + nameEnd := 1 + for nameEnd < len(rest) && isMarkupNameChar(rest[nameEnd]) { + nameEnd++ + } + return markup{start: start, end: pos + stop + 1, name: rest[1:nameEnd], isEnd: true}, true + case strings.HasPrefix(rest, "!"), strings.HasPrefix(rest, "?"): + stop := strings.IndexByte(rest, '>') + if stop < 0 { + return markup{}, false + } + pos += stop + 1 + continue + } + nameEnd := 0 + for nameEnd < len(rest) && isMarkupNameChar(rest[nameEnd]) { + nameEnd++ + } + if nameEnd == 0 { + continue // a bare "<" in text + } + sgid, end, ok := scanAttributes(text, pos+nameEnd) + if !ok { + return markup{}, false + } + return markup{start: start, end: end, name: rest[:nameEnd], sgid: sgid}, true + } + return markup{}, false +} + +// isMarkupNameChar is what may follow "<" in a tag name: everything but space, +// "/", ">", "<", "=" and quotes, so "", and +// returns the first sgid attribute's decoded value (empty when absent or +// empty), the index after the ">", and whether the tag closed. +func scanAttributes(text string, pos int) (sgid string, end int, ok bool) { + seen := false + for pos < len(text) { + for pos < len(text) && (isMarkupSpace(text[pos]) || text[pos] == '/') { + pos++ + } + if pos >= len(text) { + return sgid, pos, false + } + if text[pos] == '>' { + return sgid, pos + 1, true + } + nameStart := pos + for pos < len(text) && !isMarkupSpace(text[pos]) && text[pos] != '=' && text[pos] != '>' && text[pos] != '/' { + pos++ + } + name := text[nameStart:pos] + for pos < len(text) && isMarkupSpace(text[pos]) { + pos++ + } + value := "" + if pos < len(text) && text[pos] == '=' { + pos++ + for pos < len(text) && isMarkupSpace(text[pos]) { + pos++ + } + if pos < len(text) && (text[pos] == '"' || text[pos] == '\'') { + quote := text[pos] + pos++ + closing := strings.IndexByte(text[pos:], quote) + if closing < 0 { + return sgid, len(text), false + } + value = text[pos : pos+closing] + pos += closing + 1 + } else { + valueStart := pos + for pos < len(text) && !isMarkupSpace(text[pos]) && text[pos] != '>' { + pos++ + } + value = text[valueStart:pos] + } + } + if name == "" { + pos++ + continue + } + if !seen && strings.EqualFold(name, "sgid") { + seen = true + sgid = html.UnescapeString(value) + } + } + return sgid, pos, false +} + +// StateDirName is the connector's state directory for one account and agent, +// "-", inside StateRoot. +func StateDirName(accountID string, agentID int64) string { + return accountID + "-" + strconv.FormatInt(agentID, 10) +} + +// ErrNotAStateDir is a directory that is not a connector state directory for +// the account asked about. StateDirError carries why. +var ErrNotAStateDir = errors.New("not the connector's state directory for this account") + +// StateDirError says which rule a state directory failed, in fields a caller +// can build its own message from rather than by reading this one. +type StateDirError struct { + // Dir is the directory as given, made absolute. + Dir string + // Root is the connector's state root, the only place a state directory + // lives. + Root string + // Account is the account the directory names, empty when it names none; + // Want is the account it had to name. + Account, Want string + // Why is the rule it failed. + Why StateDirProblem +} + +// StateDirProblem is why a state directory was refused. +type StateDirProblem string + +const ( + // StateDirElsewhere is a directory outside the state root. + StateDirElsewhere StateDirProblem = "outside the connector's state root" + // StateDirMisnamed is a directory not named -. + StateDirMisnamed StateDirProblem = "not named -" + // StateDirOtherAccount is another account's state directory. + StateDirOtherAccount StateDirProblem = "another account's" +) + +func (e *StateDirError) Error() string { + switch e.Why { + case StateDirElsewhere: + return fmt.Sprintf("%s is not inside %s", e.Dir, e.Root) + case StateDirOtherAccount: + return fmt.Sprintf("%s belongs to account %s, not %s", e.Dir, e.Account, e.Want) + default: + return fmt.Sprintf("%s is not named -", e.Dir) + } +} + +func (e *StateDirError) Unwrap() error { return ErrNotAStateDir } + +// StateRoot is where every connector state directory lives: +// $XDG_STATE_HOME/basecamp/connect, or ~/.local/state/basecamp/connect when +// XDG_STATE_HOME is unset or not absolute, as the XDG specification says. +func StateRoot() (string, error) { + base := os.Getenv("XDG_STATE_HOME") + if !filepath.IsAbs(base) { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "", fmt.Errorf("connector: no state home: %w", err) + } + base = filepath.Join(home, ".local", "state") + } + return filepath.Join(filepath.Clean(base), "basecamp", "connect"), nil +} + +// ResolveStateDir is the one place a state directory is accepted: dir must +// be exactly StateRoot/-, and its account must be +// accountID, compared as numbers. It returns the directory made absolute — +// which is the one every caller should go on to use — and the agent's Person +// id. +// +// The location is part of the check, not only the name. A directory named +// for this account anywhere else — a copy of another account's ledger renamed +// to match — is refused, because the name is what binds a ledger to an +// account and anyone can choose a name. +func ResolveStateDir(dir, accountID string) (string, int64, error) { + root, err := StateRoot() + if err != nil { + return "", 0, err + } + abs, err := filepath.Abs(dir) + if err != nil { + return "", 0, fmt.Errorf("connector: state directory %q: %w", dir, err) + } + refuse := func(why StateDirProblem, account string) (string, int64, error) { + return "", 0, &StateDirError{Dir: abs, Root: root, Account: account, Want: accountID, Why: why} + } + if filepath.Dir(abs) != root { + return refuse(StateDirElsewhere, "") + } + account, agent, ok := strings.Cut(filepath.Base(abs), "-") + agentID, err := strconv.ParseInt(agent, 10, 64) + if !ok || err != nil || agentID <= 0 || agent != strconv.FormatInt(agentID, 10) { + // The agent is spelled one way, so one directory answers to one name: + // "+52007412" and "052007412" are other directories, not this one. + return refuse(StateDirMisnamed, account) + } + given, errGiven := strconv.ParseUint(account, 10, 64) + if errGiven != nil || given == 0 { + // Not an account at all: the name is wrong, not another account's. + return refuse(StateDirMisnamed, account) + } + want, errWant := strconv.ParseUint(accountID, 10, 64) + if errWant != nil || given != want { + return refuse(StateDirOtherAccount, account) + } + return abs, agentID, nil +} + +// LedgerFile is the ledger's file name inside the state directory. +const LedgerFile = "ledger.db" diff --git a/internal/connector/ledger_dispatch_test.go b/internal/connector/ledger_dispatch_test.go new file mode 100644 index 000000000..7042e2141 --- /dev/null +++ b/internal/connector/ledger_dispatch_test.go @@ -0,0 +1,1370 @@ +package connector + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/connector/admission" +) + +const otherPersonID int64 = 1001 + +// dispatchFixture is a task holding an originating mention (event 1) and a +// follow-up queued on the same conversation (event 2), both still admitted on +// the task. +type dispatchFixture struct { + ledger *Ledger + path string + grant TaskGrant + d *TaskDispatch +} + +func newDispatchFixture(t *testing.T) dispatchFixture { + t.Helper() + path := filepath.Join(t.TempDir(), "state", "connector.db") + ledger, err := OpenLedger(path) + require.NoError(t, err) + t.Cleanup(func() { _ = ledger.Close() }) + ctx := context.Background() + for _, id := range []int64{1, 2} { + seenRecord(t, ledger, id) + v := admittedVerdict(id, 0, "recording:10304028989") + v.Snapshot.Content = "
" + mentionMarkup(adapterAgentID) + " please ask " + mentionMarkup(otherPersonID) + " about it
" + _, err := ledger.Admission().Commit(ctx, v) + require.NoError(t, err) + } + require.Equal(t, StateQueued, getRecord(t, ledger, 2).State) + grant, err := ledger.CreateTask(ctx, []int64{1, 2}) + require.NoError(t, err) + d, err := ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + return dispatchFixture{ledger: ledger, path: path, grant: grant, d: d} +} + +type taskEventRow struct { + Delivery string + Guard string + ExposedAt *string + DeliveredAt *string + CompletedAt *string +} + +func (f dispatchFixture) row(t *testing.T, eventID int64) taskEventRow { + t.Helper() + return f.rowContext(context.Background(), t, eventID) +} + +func (f dispatchFixture) rowContext(ctx context.Context, t *testing.T, eventID int64) taskEventRow { + t.Helper() + var r taskEventRow + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT delivery, guard, exposed_at, delivered_at, completed_at FROM task_events WHERE task_id = ? AND event_id = ?`, + f.grant.ID, eventID).Scan(&r.Delivery, &r.Guard, &r.ExposedAt, &r.DeliveredAt, &r.CompletedAt)) + return r +} + +// Done when: get_dispatch writes exposed on first call and nothing on repeats. +func TestGetDispatchExposesOnceAndRepeatsWriteNothing(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + t0 := time.Date(2026, 9, 17, 12, 0, 0, 0, time.UTC) + f.ledger.now = func() time.Time { return t0 } + record := getRecord(t, f.ledger, 2) + require.Equal(t, StateDispatched, record.State) + + first, ok, err := f.d.Get(ctx, 2) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, DeliveryExposed, first.Delivery) + row := f.row(t, 2) + assert.Equal(t, "exposed", row.Delivery) + require.NotNil(t, row.ExposedAt) + assert.Equal(t, "admitted", f.row(t, 1).Delivery, "the other event is not exposed") + + f.ledger.now = func() time.Time { return t0.Add(time.Minute) } + again, ok, err := f.d.Get(ctx, 2) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, first, again, "a repeat returns the same instruction") + assert.Equal(t, row, f.row(t, 2), "and marks nothing further") + assert.Equal(t, record.Revision, getRecord(t, f.ledger, 2).Revision, "exposure is the task's, not the record's") +} + +func TestGetDispatchWithoutAnIDIsTheEarliestNotAcknowledged(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + + got, ok, err := f.d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(1), got.EventID) + + // Exposed but not acknowledged is still the earliest. + got, _, err = f.d.Get(ctx, 0) + require.NoError(t, err) + assert.Equal(t, int64(1), got.EventID) + + _, err = f.d.Ack(ctx, 1, nil) + require.NoError(t, err) + got, ok, err = f.d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(2), got.EventID) + + _, err = f.d.Complete(ctx, 2, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + _, ok, err = f.d.Get(ctx, 0) + require.NoError(t, err) + assert.False(t, ok, "nothing is left to acknowledge") +} + +// The instruction is an allowlist: the fields a worker needs, the agent's own +// mention stripped, and nothing that is a route, a position or a token. +func TestGetDispatchHandsOutOnlyTheAllowlist(t *testing.T) { + f := newDispatchFixture(t) + + got, ok, err := f.d.Get(context.Background(), 1) + require.NoError(t, err) + require.True(t, ok) + + assert.Equal(t, Instruction{ + EventID: 1, EventType: "comment.created", Trigger: "mentioned", Class: "internal", + Recording: InstructionRecording{ + BucketID: adapterBucketID, RecordingID: 10304028972, Type: "Comment", Title: "A comment", + URL: "https://app.basecamp.com/2914079/buckets/48699913/recordings/10304028972", + }, + ReplyTo: InstructionReply{Kind: "comment", RecordingID: 10304028989}, + RequesterID: adapterOperatorID, + Acknowledge: true, + Delivery: DeliveryExposed, + Content: "
please ask " + mentionMarkup(otherPersonID) + " about it
", + ContentUpdatedAt: time.Date(2026, 9, 16, 10, 0, 0, 0, time.UTC), + }, got) + + encoded, err := json.Marshal(got) + require.NoError(t, err) + var fields map[string]any + require.NoError(t, json.Unmarshal(encoded, &fields)) + keys := make([]string, 0, len(fields)) + for k := range fields { + keys = append(keys, k) + } + sort.Strings(keys) + assert.Equal(t, []string{"acknowledge", "class", "content", "content_updated_at", "delivery", "event_id", "event_type", + "guard_acknowledged", "recording", "reply_to", "requester_id", "trigger"}, keys) + assert.NotContains(t, string(encoded), "/work/connector", "no route") + assert.NotContains(t, string(encoded), f.grant.Token, "no token") +} + +func TestGetDispatchCancelsTheGuardAndReportsAFiredOne(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.Equal(t, "armed", f.row(t, 1).Guard, "an acknowledged trigger arms the guard") + + got, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + assert.False(t, got.GuardAcknowledged) + assert.Equal(t, "canceled", f.row(t, 1).Guard) + + // The connector fired the guard on event 2 before the worker asked. + _, err = f.ledger.db.ExecContext(context.Background(), `UPDATE task_events SET guard = 'fired' WHERE task_id = ? AND event_id = 2`, f.grant.ID) + require.NoError(t, err) + got, _, err = f.d.Get(ctx, 2) + require.NoError(t, err) + assert.True(t, got.GuardAcknowledged) + assert.Equal(t, "fired", f.row(t, 2).Guard, "a fired guard stays fired") +} + +func TestGetDispatchCancelsAnArmedGuardOnAnAlreadyExposedEvent(t *testing.T) { + f := newDispatchFixture(t) + // Exposed at launch by the dispatcher, guard still armed. + _, err := f.ledger.db.ExecContext(context.Background(), `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + + _, _, err = f.d.Get(context.Background(), 1) + require.NoError(t, err) + assert.Equal(t, "canceled", f.row(t, 1).Guard) +} + +// Done when: ack_dispatch and complete_dispatch move the delivery state. +func TestAckAndCompleteMoveTheDelivery(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + + ackID := int64(9001) + receipt, err := f.d.Ack(ctx, 1, &ackID) + require.NoError(t, err) + assert.Equal(t, Receipt{EventID: 1, Delivery: DeliveryDelivered, AckID: &ackID, Links: []string{}}, receipt) + delivered := f.row(t, 1) + assert.Equal(t, "delivered", delivered.Delivery) + require.NotNil(t, delivered.DeliveredAt) + + // A lost tool response, retried. + again, err := f.d.Ack(ctx, 1, &ackID) + require.NoError(t, err) + assert.Equal(t, receipt, again) + assert.Equal(t, delivered, f.row(t, 1)) + + replyID := int64(9002) + done, err := f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded, Links: []string{"https://github.com/basecamp/basecamp-cli/pull/1"}, ReplyID: &replyID}) + require.NoError(t, err) + assert.Equal(t, DeliveryCompleted, done.Delivery) + assert.Equal(t, OutcomeSucceeded, done.Outcome) + assert.Equal(t, &replyID, done.ReplyID) + assert.Equal(t, &ackID, done.AckID) + assert.Equal(t, StateCompleted, getRecord(t, f.ledger, 1).State) + completed := f.row(t, 1) + assert.Equal(t, delivered.DeliveredAt, completed.DeliveredAt) + + repeat, err := f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded, Links: []string{"https://github.com/basecamp/basecamp-cli/pull/1"}, ReplyID: &replyID}) + require.NoError(t, err) + assert.Equal(t, done, repeat) + assert.Equal(t, completed, f.row(t, 1)) +} + +func TestCompleteAlsoAcknowledges(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 2) + require.NoError(t, err) + + receipt, err := f.d.Complete(ctx, 2, Completion{Outcome: OutcomeFailed}) + require.NoError(t, err) + assert.Equal(t, DeliveryCompleted, receipt.Delivery) + row := f.row(t, 2) + assert.NotNil(t, row.DeliveredAt) + assert.NotNil(t, row.CompletedAt) +} + +func TestAReportedOutcomeStands(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + first := int64(1) + _, err = f.d.Ack(ctx, 1, &first) + require.NoError(t, err) + second := int64(2) + _, err = f.d.Ack(ctx, 1, &second) + assert.ErrorIs(t, err, ErrReportConflict) + + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + for _, c := range []Completion{ + {Outcome: OutcomeFailed}, + {Outcome: OutcomeSucceeded, Links: []string{"https://example.com/a"}}, + {Outcome: OutcomeSucceeded, ReplyID: &second}, + } { + _, err = f.d.Complete(ctx, 1, c) + assert.ErrorIs(t, err, ErrReportConflict) + } +} + +func TestAReportNeedsTheEventHandedOut(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + + _, err := f.d.Ack(ctx, 2, nil) + assert.ErrorIs(t, err, ErrNotExposed) + _, err = f.d.Complete(ctx, 2, Completion{Outcome: OutcomeSucceeded}) + assert.ErrorIs(t, err, ErrNotExposed) + assert.Equal(t, "admitted", f.row(t, 2).Delivery) +} + +// Done when: a superseded token is refused. +func TestASupersededTokenIsRefused(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + + _, _, err = f.d.Get(ctx, 2) + assert.ErrorIs(t, err, ErrTaskTokenRefused) + _, err = f.d.Ack(ctx, 1, nil) + assert.ErrorIs(t, err, ErrTaskTokenRefused) + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + assert.ErrorIs(t, err, ErrTaskTokenRefused) + assert.Equal(t, "admitted", f.row(t, 2).Delivery, "a refused get exposes nothing") + assert.Equal(t, "exposed", f.row(t, 1).Delivery) + + _, err = f.ledger.Dispatch(ctx, "not-a-token", adapterAgentID) + assert.ErrorIs(t, err, ErrTaskTokenRefused, "refused when bound, not only on use") + _, err = f.ledger.Dispatch(ctx, f.grant.Token, adapterAgentID) + assert.ErrorIs(t, err, ErrTaskTokenRefused) +} + +func TestAWorkerSeesOnlyItsOwnTask(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + seenRecord(t, f.ledger, 3) + _, err := f.ledger.Admission().Commit(ctx, admittedVerdict(3, 0, "recording:other")) + require.NoError(t, err) + _, err = f.ledger.CreateTask(ctx, []int64{3}) + require.NoError(t, err) + + _, _, err = f.d.Get(ctx, 3) + assert.ErrorIs(t, err, ErrNotOnTask) + _, err = f.d.Ack(ctx, 3, nil) + assert.ErrorIs(t, err, ErrNotOnTask) + _, _, err = f.d.Get(ctx, 404) + assert.ErrorIs(t, err, ErrNotOnTask, "an unknown event reads the same as another task's") +} + +func TestAnEventThatLeftThePathIsNotHandedOut(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + + // A record on a live task cannot be taken off the path around its task. + require.ErrorIs(t, f.ledger.SetState(ctx, 2, StateBlocked, "no_route"), ErrOnALiveTask) + + // It can be settled before a worker pulls it: finished work is not + // handed out for the first time. + require.NoError(t, f.ledger.SetState(ctx, 2, StateCompleted, "")) + _, _, err := f.d.Get(ctx, 2) + assert.ErrorIs(t, err, ErrNotDispatchable) + assert.Equal(t, "admitted", f.row(t, 2).Delivery) +} + +// Retention took the instruction: a completed event asked for again answers +// that it can no longer be dispatched, never an empty instruction. +func TestAnEventWhoseContentWasDroppedIsNotHandedOut(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + at := time.Date(2026, 9, 17, 12, 0, 0, 0, time.UTC) + f.ledger.now = func() time.Time { return at } + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + dropped, err := f.ledger.DropContent(ctx, at.Add(time.Hour), at.Add(time.Hour)) + require.NoError(t, err) + require.Equal(t, 1, dropped) + + _, _, err = f.d.Get(ctx, 1) + assert.ErrorIs(t, err, ErrNotDispatchable) +} + +// A record is dispatched exactly while a live task carries it: joining a task +// moves it there, in the task's own transaction. +func TestCreateTaskDispatchesItsRecords(t *testing.T) { + f := newDispatchFixture(t) + assert.Equal(t, StateDispatched, getRecord(t, f.ledger, 1).State) + assert.Equal(t, StateDispatched, getRecord(t, f.ledger, 2).State, "the queued follow-up too") +} + +// An event is on at most one live task. A retried launch is refused and +// writes nothing; after a redispatch supersedes the task, it joins a new one. +func TestAnEventIsOnOneLiveTask(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + countTasks := func() int { + var n int + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM tasks`).Scan(&n)) + return n + } + + _, err := f.ledger.CreateTask(ctx, []int64{1}) + assert.ErrorIs(t, err, ErrEventOnLiveTask) + _, err = f.ledger.CreateTask(ctx, []int64{2, 1}) + assert.ErrorIs(t, err, ErrEventOnLiveTask) + assert.Equal(t, 1, countTasks(), "a refused task leaves no task behind") + + // The database refuses it too, whoever writes. + _, err = f.ledger.db.ExecContext(ctx, `INSERT INTO tasks (id, token_sha256, created_at) VALUES (99, 'x', 'now')`) + require.NoError(t, err) + _, err = f.ledger.db.ExecContext(ctx, `INSERT INTO task_events (task_id, event_id) VALUES (99, 1)`) + require.Error(t, err) + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + grant, err := f.ledger.CreateTask(ctx, []int64{1, 2}) + require.NoError(t, err) + d, err := f.ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + got, ok, err := d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(1), got.EventID) + _, _, err = f.d.Get(ctx, 1) + assert.ErrorIs(t, err, ErrTaskTokenRefused) +} + +// The dispatcher writes a task with its attempt and exposure in one commit; +// a task created in a transaction that rolls back leaves nothing, and does not +// hold the event. +func TestCreateTaskInsideACallersTransaction(t *testing.T) { + ledger := newTestLedger(t) + ctx := context.Background() + seenRecord(t, ledger, 1) + _, err := ledger.Admission().Commit(ctx, admittedVerdict(1, 0, "recording:9")) + require.NoError(t, err) + + tx, err := ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = ledger.createTask(ctx, tx, []int64{1}) + require.NoError(t, err) + require.NoError(t, tx.Rollback()) + assert.Equal(t, StateAdmitted, getRecord(t, ledger, 1).State) + + tx, err = ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = ledger.createTask(ctx, tx, []int64{1}) + require.NoError(t, err) + _, err = ledger.createTask(ctx, tx, []int64{1}) + require.ErrorIs(t, err, ErrEventOnLiveTask, "one live task per event within a transaction too") + require.NoError(t, tx.Rollback()) + + tx, err = ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + grant, err := ledger.createTask(ctx, tx, []int64{1}) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + _, err = ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + assert.Equal(t, StateDispatched, getRecord(t, ledger, 1).State) +} + +// A task is only ever made of instructions a worker can pull. A record that +// lost its snapshot on the way through blocked is refused, not dispatched as +// an empty task. +func TestCreateTaskRefusesARecordWithoutItsInstruction(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + require.NoError(t, f.ledger.SetState(ctx, 1, StateBlocked, "read_failed"), "never handed, it left dispatched with its task") + require.NoError(t, f.ledger.SetState(ctx, 1, StateAdmitted, "")) + + _, err := f.ledger.CreateTask(ctx, []int64{1}) + require.ErrorIs(t, err, ErrNotDispatchable) + assert.Equal(t, StateAdmitted, getRecord(t, f.ledger, 1).State) + + _, err = f.ledger.CreateTask(ctx, []int64{2, 2}) + require.Error(t, err) + assert.NotErrorIs(t, err, ErrEventOnLiveTask, "a duplicate id is not another task's event") +} + +// Superseding a task returns what it never exposed to admitted and leaves +// what a worker was handed dispatched, so no record is left dispatched on no +// task without a worker having seen it. +func TestSupersedingReturnsUnexposedWork(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + + assert.Equal(t, StateDispatched, getRecord(t, f.ledger, 1).State, "a worker saw it") + assert.Equal(t, StateAdmitted, getRecord(t, f.ledger, 2).State, "never exposed, it is work again") + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID), "a repeat is harmless") + assert.Equal(t, StateAdmitted, getRecord(t, f.ledger, 2).State) + + // The conversation is not free while event 1 waits for settlement. + _, err = f.ledger.CreateTask(ctx, []int64{2}) + require.ErrorIs(t, err, ErrConversationBusy) + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + grant, err := f.ledger.CreateTask(ctx, []int64{2}) + require.NoError(t, err) + d, err := f.ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + got, ok, err := d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(2), got.EventID) +} + +// A conversation has one task at a time. Siblings a supersede returned wait +// behind the exposed event a worker was handed, and two of them are never +// launched as two tasks. +func TestAConversationHasOneTaskAtATime(t *testing.T) { + ledger := newTestLedger(t) + ctx := context.Background() + const key = "recording:10304028989" + for _, id := range []int64{1, 2, 3} { + seenRecord(t, ledger, id) + _, err := ledger.Admission().Commit(ctx, admittedVerdict(id, 0, key)) + require.NoError(t, err) + } + grant, err := ledger.CreateTask(ctx, []int64{1, 2, 3}) + require.NoError(t, err) + d, err := ledger.Dispatch(ctx, grant.Token, adapterAgentID) + require.NoError(t, err) + _, _, err = d.Get(ctx, 1) + require.NoError(t, err) + require.NoError(t, ledger.SupersedeTask(ctx, grant.ID)) + + _, err = ledger.CreateTask(ctx, []int64{2}) + require.ErrorIs(t, err, ErrConversationBusy, "event 1 was handed to a worker and is not settled") + _, err = ledger.CreateTask(ctx, []int64{2, 3}) + require.ErrorIs(t, err, ErrConversationBusy) + assert.Equal(t, StateAdmitted, getRecord(t, ledger, 2).State, "a refused task moves nothing") + + // A redispatch that takes the whole conversation is one task. + _, err = ledger.CreateTask(ctx, []int64{1, 2, 3}) + require.NoError(t, err) + + // Two separate launches on a free conversation: the second is refused. + other := newTestLedger(t) + for _, id := range []int64{1, 2} { + seenRecord(t, other, id) + _, err := other.Admission().Commit(ctx, admittedVerdict(id, 0, key)) + require.NoError(t, err) + } + _, err = other.CreateTask(ctx, []int64{1}) + require.NoError(t, err) + _, err = other.CreateTask(ctx, []int64{2}) + require.ErrorIs(t, err, ErrConversationBusy) +} + +// Asked for by id or as the earliest, an event is served by one rule. +func TestTheEarliestAndAnExplicitGetAgree(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + // Settled elsewhere while the worker had it, before it acknowledged. + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + + byID, ok, err := f.d.Get(ctx, 1) + require.NoError(t, err) + require.True(t, ok) + earliest, ok, err := f.d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, byID, earliest) +} + +// Finished work is never handed out for the first time: a record completed +// before this worker was exposed to it is refused, and one it completed +// itself is served again. +func TestCompletedWorkIsServedOnlyToItsWorker(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SetState(ctx, 2, StateCompleted, "")) + + _, _, err := f.d.Get(ctx, 2) + assert.ErrorIs(t, err, ErrNotDispatchable) + assert.Equal(t, "admitted", f.row(t, 2).Delivery) + + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err) + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + got, ok, err := f.d.Get(ctx, 1) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, DeliveryCompleted, got.Delivery) +} + +func TestConcurrentLaunchesOfOneEventMakeOneTask(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "connector.db") + ledger, err := OpenLedger(path) + require.NoError(t, err) + t.Cleanup(func() { _ = ledger.Close() }) + ctx := context.Background() + seenRecord(t, ledger, 1) + _, err = ledger.Admission().Commit(ctx, admittedVerdict(1, 0, "recording:9")) + require.NoError(t, err) + + // Each racer has its own handle on the file, so SQLite sees the race + // rather than database/sql's one connection serializing it. + const racers = 8 + errs := make([]error, racers) + done := make(chan struct{}) + for i := range racers { + go func() { + defer func() { done <- struct{}{} }() + handle, err := OpenExistingLedger(ctx, path) + if err != nil { + errs[i] = err + return + } + defer handle.Close() + _, errs[i] = handle.CreateTask(ctx, []int64{1}) + }() + } + for range racers { + <-done + } + created := 0 + for _, err := range errs { + if err == nil { + created++ + continue + } + assert.ErrorIs(t, err, ErrEventOnLiveTask) + } + assert.Equal(t, 1, created) + var live int + require.NoError(t, ledger.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM task_events WHERE event_id = 1 AND retired_at IS NULL`).Scan(&live)) + assert.Equal(t, 1, live) +} + +// One event that left the path never hides the rest of the task. +func TestTheEarliestSkipsAnEventThatLeftThePath(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, ""), "settled before any worker pulled it") + + got, ok, err := f.d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(2), got.EventID) +} + +// A worker's report is what it did: a record the dispatcher settled as +// completed while the worker was still going keeps its settlement, and the +// report the worker then sends is recorded against it. +func TestAReportIsRecordedAfterSettlement(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 2) + require.NoError(t, err) + require.NoError(t, f.ledger.SetState(ctx, 2, StateCompleted, "")) + + _, err = f.d.Ack(ctx, 2, nil) + require.NoError(t, err) + receipt, err := f.d.Complete(ctx, 2, Completion{Outcome: OutcomeFailed}) + require.NoError(t, err) + assert.Equal(t, DeliveryCompleted, receipt.Delivery) + assert.Equal(t, OutcomeFailed, receipt.Outcome) + assert.Equal(t, StateCompleted, getRecord(t, f.ledger, 2).State) +} + +// Invariant 4: nothing leaves dispatched while a worker may still act on it. +// A handed record cannot be withdrawn, blocked or requeued — not through the +// ledger's write, and not around it — so its conversation stays busy and no +// sibling starts a second task until its outcome is in. +func TestAHandedRecordLeavesDispatchedOnlyWhenCompleted(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID), "superseding does not release what a worker holds") + + for _, to := range []struct { + state RecordState + reason string + }{{StateAdmitted, ""}, {StateBlocked, "read_failed"}} { + err := f.ledger.SetState(ctx, 1, to.state, to.reason) + require.ErrorIs(t, err, ErrHeldByWorker, "to %s", to.state) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE events SET state = ? WHERE id = 1`, string(to.state)) + require.Error(t, err, "the database refuses it too") + } + assert.Equal(t, StateDispatched, getRecord(t, f.ledger, 1).State) + _, err = f.ledger.CreateTask(ctx, []int64{2}) + require.ErrorIs(t, err, ErrConversationBusy, "the sibling waits for the handed event") + + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + _, err = f.ledger.CreateTask(ctx, []int64{2}) + require.NoError(t, err) +} + +// A worker's open never leaves a ledger behind where there was none: not +// through the privacy check, and not through SQLite, whichever the file +// disappears before. +func TestOpenExistingLedgerCreatesNothing(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + require.NoError(t, os.Mkdir(dir, 0o700)) + path := filepath.Join(dir, LedgerFile) + + _, err := OpenExistingLedger(context.Background(), path) + require.ErrorIs(t, err, os.ErrNotExist) + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Empty(t, entries, "the privacy check created nothing") + + // The file gone after the check: SQLite itself must refuse. + db, err := sql.Open("sqlite", ledgerDSN(path, false)) + require.NoError(t, err) + defer db.Close() + require.Error(t, db.PingContext(context.Background())) + entries, err = os.ReadDir(dir) + require.NoError(t, err) + assert.Empty(t, entries, "SQLite created nothing") + + owner, err := sql.Open("sqlite", ledgerDSN(path, true)) + require.NoError(t, err) + defer owner.Close() + require.NoError(t, owner.PingContext(context.Background()), "the connector's own open still creates") +} + +func TestOpenExistingLedgerNeverCreatesOrMigrates(t *testing.T) { + dir := t.TempDir() + "/state" + _, err := OpenExistingLedger(context.Background(), dir+"/missing.db") + require.Error(t, err) + + path := dir + "/connector.db" + all := migrations + migrations = all[:4] + old, err := OpenLedger(path) + migrations = all + require.NoError(t, err) + require.NoError(t, old.Close()) + + _, err = OpenExistingLedger(context.Background(), path) + require.ErrorIs(t, err, ErrLedgerSchema) + again, err := OpenLedger(path) + require.NoError(t, err) + version, err := again.SchemaVersion(context.Background()) + require.NoError(t, err) + require.NoError(t, again.Close()) + assert.Equal(t, len(migrations), version, "migrated only by the owner's open") + + current, err := OpenExistingLedger(context.Background(), path) + require.NoError(t, err) + require.NoError(t, current.Close()) +} + +func TestDeliveryNeverGoesBack(t *testing.T) { + f := newDispatchFixture(t) + _, _, err := f.d.Get(context.Background(), 1) + require.NoError(t, err) + + _, err = f.ledger.db.ExecContext(context.Background(), `UPDATE task_events SET delivery = 'admitted' WHERE event_id = 1`) + require.Error(t, err) + assert.Contains(t, err.Error(), "never goes back") +} + +func TestCreateTaskTakesOnlyWorkWaitingForAWorker(t *testing.T) { + ledger := newTestLedger(t) + ctx := context.Background() + seenRecord(t, ledger, 1) + + _, err := ledger.CreateTask(ctx, []int64{1}) + require.Error(t, err) + _, err = ledger.CreateTask(ctx, []int64{404}) + assert.ErrorIs(t, err, ErrNoSuchRecord) + _, err = ledger.CreateTask(ctx, nil) + require.Error(t, err) + var tasks int + require.NoError(t, ledger.db.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM tasks`).Scan(&tasks)) + assert.Zero(t, tasks, "a refused task leaves nothing behind") + + blocked := blockedVerdict(1, 0, admission.ReasonNoRoute) + _, err = ledger.Admission().Commit(ctx, blocked) + require.NoError(t, err) + _, err = ledger.CreateTask(ctx, []int64{1}) + require.Error(t, err) +} + +func TestTheTokenIsStoredOnlyAsAHash(t *testing.T) { + f := newDispatchFixture(t) + var stored string + require.NoError(t, f.ledger.db.QueryRowContext(context.Background(), `SELECT token_sha256 FROM tasks WHERE id = ?`, f.grant.ID).Scan(&stored)) + assert.NotContains(t, stored, f.grant.Token) + assert.Len(t, stored, 64) + assert.GreaterOrEqual(t, len(f.grant.Token), 43, "32 random bytes") +} + +func TestCompleteRefusesMalformedReports(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + + tooMany := make([]string, maxCompletionLinks+1) + for i := range tooMany { + tooMany[i] = "https://example.com/" + } + for name, c := range map[string]Completion{ + "no outcome": {}, + "unknown outcome": {Outcome: "unknown"}, + "not a URL": {Outcome: OutcomeSucceeded, Links: []string{"javascript:alert(1)"}}, + "too many links": {Outcome: OutcomeSucceeded, Links: tooMany}, + "a link too long": {Outcome: OutcomeSucceeded, Links: []string{"https://example.com/" + strings.Repeat("a", maxLinkLength)}}, + } { + t.Run(name, func(t *testing.T) { + _, err := f.d.Complete(ctx, 1, c) + require.ErrorIs(t, err, ErrInvalidReport) + assert.NotContains(t, err.Error(), "report on event", "what the worker got wrong reaches it without the ledger's own wrapping") + assert.Equal(t, "exposed", f.rowContext(ctx, t, 1).Delivery) + }) + } +} + +func TestStripMentionsOf(t *testing.T) { + agent := mentionMarkup(adapterAgentID) + other := mentionMarkup(otherPersonID) + withFigure := strings.Replace(agent, ">", `>
Agent
`, 1) + file := `` + selfClosing := strings.Replace(agent, ">", " />", 1) + unclosed := strings.Replace(agent, "", "", 1) + + // A removed mention leaves a space, so what was around it cannot join. + for name, tc := range map[string]struct{ in, want string }{ + "the agent's mention": {"
" + agent + " do it
", "
do it
"}, + "with its figure": {"
" + withFigure + " do it
", "
do it
"}, + "another person's mention stays": {"
" + other + " and " + agent + "
", "
" + other + " and
"}, + "a file stays": {file + agent, file + " "}, + "self-closing": {"a" + selfClosing + "b", "a b"}, + "self-closing, before another": {selfClosing + other, " " + other}, + "unclosed, before another": {unclosed + " x " + other, " x " + other}, + "every occurrence": {agent + " and " + agent, " and "}, + "no attachments": {"
plain
", "
plain
"}, + "single-quoted sgid": {"a" + strings.ReplaceAll(agent, `"`, "'") + "b", "a b"}, + "a > inside another attribute": {"a" + strings.Replace(agent, "" + other, "" + other}, + "uppercase": {"a" + strings.ToUpper(agent[:14]) + agent[14:] + "b", "a b"}, + "the first sgid is the one": {strings.Replace(agent, "

thanks

tail", + " " + " please deploy

thanks

tail"}, + "unclosed, then a stray close": {unclosed + " please deploy

thanks

tail", + " " + " please deploy

thanks

tail"}, + } { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.want, StripMentionsOf(tc.in, adapterAgentID)) + }) + } +} + +// entityEncodedSGID writes the mention's sgid with its first character as a +// hex entity, the way a serializer may. +func entityEncodedSGID(mention string) string { + i := strings.Index(mention, `sgid="`) + len(`sgid="`) + return mention[:i] + fmt.Sprintf("&#x%x;", mention[i]) + mention[i+1:] +} + +func TestResolveStateDirAcceptsOnlyTheCanonicalDirectory(t *testing.T) { + home := t.TempDir() + t.Setenv("XDG_STATE_HOME", home) + root, err := StateRoot() + require.NoError(t, err) + assert.Equal(t, filepath.Join(home, "basecamp", "connect"), root) + canonical := filepath.Join(root, StateDirName("999", adapterAgentID)) + + resolved, agentID, err := ResolveStateDir(canonical, "999") + require.NoError(t, err) + assert.Equal(t, adapterAgentID, agentID) + assert.Equal(t, canonical, resolved) + resolved, agentID, err = ResolveStateDir(canonical+"/../"+StateDirName("999", adapterAgentID)+"/", "0999") + require.NoError(t, err, "accounts compare as numbers, and a trailing slash or a .. is the same directory") + assert.Equal(t, adapterAgentID, agentID) + assert.Equal(t, canonical, resolved, "the directory every caller goes on to use is the one that was checked") + + for name, dir := range map[string]string{ + "outside the root": filepath.Join(t.TempDir(), StateDirName("999", adapterAgentID)), + "nested in the root": filepath.Join(root, "x", StateDirName("999", adapterAgentID)), + "another account": filepath.Join(root, StateDirName("1000", adapterAgentID)), + "no agent": filepath.Join(root, "999-"), + "no account": filepath.Join(root, "-52007412"), + "not a number": filepath.Join(root, "999-abc"), + "a signed agent": filepath.Join(root, "999-+52007412"), + "a padded agent": filepath.Join(root, "999-052007412"), + "the root itself": root, + "above the root": filepath.Join(root, "..", StateDirName("999", adapterAgentID)), + } { + t.Run(name, func(t *testing.T) { + _, _, err := ResolveStateDir(dir, "999") + assert.ErrorIs(t, err, ErrNotAStateDir) + var refusal *StateDirError + require.ErrorAs(t, err, &refusal, "the refusal says why in fields, not in a message to be parsed") + assert.Equal(t, root, refusal.Root) + assert.Equal(t, "999", refusal.Want) + if name == "another account" { + assert.Equal(t, StateDirOtherAccount, refusal.Why) + assert.Equal(t, "1000", refusal.Account) + } + if name == "outside the root" { + assert.Equal(t, StateDirElsewhere, refusal.Why) + } + }) + } + + t.Setenv("XDG_STATE_HOME", "relative/state") + root, err = StateRoot() + require.NoError(t, err) + assert.True(t, filepath.IsAbs(root), "a relative XDG_STATE_HOME is ignored, as the specification says") +} + +// The ledger is free while a call builds its answer: every transaction here +// takes the write lock as it opens, and decoding the snapshot and stripping +// the agent's mention must not be done holding it. +func TestGetDispatchDoesNotHoldTheLedgerWhileItBuildsItsAnswer(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + other, err := OpenExistingLedger(ctx, f.path) + require.NoError(t, err) + t.Cleanup(func() { _ = other.Close() }) + + writes := 0 + f.d.afterTx = func() { + // Intake, writing while the worker builds its instruction. It waits + // for the write lock, so a transaction still open here fails this + // (after the busy timeout) rather than deadlocking. + writes++ + fresh, err := other.RecordSeen(ctx, testEvent(int64(100+writes)), LanePoll) + require.NoError(t, err) + require.True(t, fresh) + } + + // The call that writes (exposure), and the repeat that writes nothing. + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err) + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err) + assert.Equal(t, 2, writes) +} + +// Acknowledgements and completions free the ledger before building their +// receipt too — on the retry that writes nothing as much as on the first. +func TestReportsDoNotHoldTheLedgerWhileTheyBuildTheirReceipt(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + other, err := OpenExistingLedger(ctx, f.path) + require.NoError(t, err) + t.Cleanup(func() { _ = other.Close() }) + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err) + + writes := 0 + f.d.afterTx = func() { + writes++ + fresh, err := other.RecordSeen(ctx, testEvent(int64(200+writes)), LanePoll) + require.NoError(t, err) + require.True(t, fresh) + } + for range 2 { + _, err = f.d.Ack(ctx, 1, nil) + require.NoError(t, err) + } + for range 2 { + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + } + assert.Equal(t, 4, writes) +} + +// The spec's automatic retry, as a transition table of its own: the +// dispatcher writes the originating event exposed at launch; the spawn is +// proven to have failed before any worker process existed; the exposure is +// withdrawn and the event launched again, once — and after a second failure it +// is blocked. Without the withdrawal marker, an exposure holds the record in +// dispatched and neither the retry nor the block is possible. +func TestASpawnThatFailedIsRetriedOnceThenBlocked(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + exposeAtLaunch := func(taskID int64) { + t.Helper() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed', exposed_at = 'launch' WHERE task_id = ? AND event_id = 1`, taskID) + require.NoError(t, err) + } + exposeAtLaunch(f.grant.ID) + + // First failure: supersede, withdraw, launch again — one transaction. + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + require.NoError(t, f.ledger.supersedeTask(ctx, tx, f.grant.ID)) + require.NoError(t, f.ledger.withdrawExposure(ctx, tx, f.grant.ID, 1, StateAdmitted, "")) + retry, err := f.ledger.createTask(ctx, tx, []int64{1, 2}) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + var onRetry int + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM task_events WHERE task_id = ? AND event_id = 1 AND retired_at IS NULL`, retry.ID).Scan(&onRetry)) + assert.Equal(t, 1, onRetry, "the retry carries the originating event again") + assert.Equal(t, StateDispatched, getRecord(t, f.ledger, 1).State) + exposeAtLaunch(retry.ID) + + // Second failure: supersede, withdraw, block. + tx, err = f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + require.NoError(t, f.ledger.supersedeTask(ctx, tx, retry.ID)) + require.NoError(t, f.ledger.withdrawExposure(ctx, tx, retry.ID, 1, StateBlocked, "spawn_failed")) + require.NoError(t, tx.Commit()) + record := getRecord(t, f.ledger, 1) + assert.Equal(t, StateBlocked, record.State) + assert.Equal(t, "spawn_failed", record.Reason) +} + +// What the database can see of "no worker existed", it holds: a withdrawal is +// refused once a worker pulled the instruction, and refused while a live task +// carries the event — so the only order is supersede, withdraw, create. +func TestAWithdrawalIsRefusedWhenAWorkerCouldHaveTheInstruction(t *testing.T) { + t.Run("a worker pulled it", func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err, "the worker pulls an event exposed at launch") + + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + require.NoError(t, f.ledger.supersedeTask(ctx, tx, f.grant.ID)) + require.Error(t, f.ledger.withdrawExposure(ctx, tx, f.grant.ID, 1, StateAdmitted, "")) + }) + + t.Run("a live task already carries it", func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed' WHERE event_id = 1`) + require.NoError(t, err) + + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + require.NoError(t, f.ledger.supersedeTask(ctx, tx, f.grant.ID)) + _, err = f.ledger.createTask(ctx, tx, []int64{1, 2}) + require.NoError(t, err) + // The database refuses it whoever writes, and on an untouched row. + _, err = tx.ExecContext(ctx, `UPDATE task_events SET withdrawn_at = 'raw' WHERE task_id = ? AND event_id = 1`, f.grant.ID) + require.Error(t, err) + require.Error(t, f.ledger.withdrawExposure(ctx, tx, f.grant.ID, 1, StateAdmitted, ""), "create before withdraw is the wrong order") + }) + + t.Run("a pull is recorded once, and a repeat writes nothing", func(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + var first string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT pulled_at FROM task_events WHERE event_id = 1`).Scan(&first)) + _, _, err = f.d.Get(ctx, 1) + require.NoError(t, err) + var again string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT pulled_at FROM task_events WHERE event_id = 1`).Scan(&again)) + assert.Equal(t, first, again) + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET pulled_at = 'later' WHERE event_id = 1`) + require.Error(t, err) + }) +} + +// A worker whose task was superseded is told so, whatever it sends: the token +// is checked before the report is. +func TestASupersededWorkerIsRefusedBeforeItsReportIsRead(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + + for name, c := range map[string]Completion{ + "no outcome": {}, + "not a URL": {Outcome: OutcomeSucceeded, Links: []string{"javascript:alert(1)"}}, + } { + t.Run(name, func(t *testing.T) { + _, err := f.d.Complete(ctx, 1, c) + require.ErrorIs(t, err, ErrTaskTokenRefused) + assert.NotErrorIs(t, err, ErrInvalidReport) + }) + } +} + +// dispatchForTest dispatches a record the only way a record is dispatched: on +// a task. A record a test walked to admitted by hand has no instruction, so one +// is attached first; a seen record is admitted first. +func dispatchForTest(t *testing.T, ledger *Ledger, id int64) TaskGrant { + t.Helper() + ctx := context.Background() + _, err := ledger.db.ExecContext(ctx, `UPDATE events +SET snapshot = COALESCE(snapshot, CAST('{"content":"do it"}' AS BLOB)), + conversation_key = CASE WHEN conversation_key = '' THEN 'recording:' || id ELSE conversation_key END +WHERE id = ?`, id) + require.NoError(t, err) + if getRecord(t, ledger, id).State == StateSeen { + require.NoError(t, ledger.SetState(ctx, id, StateAdmitted, "")) + } + grant, err := ledger.CreateTask(ctx, []int64{id}) + require.NoError(t, err) + return grant +} + +// createTask writes nothing when it refuses: the caller's transaction is as +// it found it, whatever the caller does with it next. +func TestCreateTaskWritesNothingWhenItRefuses(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + seenRecord(t, f.ledger, 3) + _, err := f.ledger.Admission().Commit(ctx, admittedVerdict(3, 0, "recording:3")) + require.NoError(t, err) + dispatchForTest(t, f.ledger, 3) + require.NoError(t, f.ledger.SetState(ctx, 3, StateCompleted, ""), "settled, and its snapshot is still there") + + var tasksBefore, rowsBefore int + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM tasks`).Scan(&tasksBefore)) + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM task_events`).Scan(&rowsBefore)) + + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + _, err = f.ledger.createTask(ctx, tx, []int64{3}) + require.Error(t, err) + assert.Contains(t, err.Error(), "completed") + var tasksAfter, rowsAfter int + require.NoError(t, tx.QueryRowContext(ctx, `SELECT COUNT(*) FROM tasks`).Scan(&tasksAfter)) + require.NoError(t, tx.QueryRowContext(ctx, `SELECT COUNT(*) FROM task_events`).Scan(&rowsAfter)) + require.NoError(t, tx.Rollback()) + assert.Equal(t, tasksBefore, tasksAfter, "no task row") + assert.Equal(t, rowsBefore, rowsAfter, "no task event row") +} + +// Exposure written at launch is the dispatcher's word, not a worker's pull. +// Until the worker pulls, finished work is not served to it, and it can +// neither acknowledge nor complete anything. +func TestALaunchExposureIsNotAPull(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + exposeAtLaunch := func(id int64) { + t.Helper() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed', exposed_at = 'launch' WHERE event_id = ?`, id) + require.NoError(t, err) + } + exposeAtLaunch(1) + exposeAtLaunch(2) + + _, err := f.d.Ack(ctx, 1, nil) + assert.ErrorIs(t, err, ErrNotExposed, "nothing was pulled yet") + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + assert.ErrorIs(t, err, ErrNotExposed) + + // Settled before the worker ever pulled it: not served, and not the + // earliest either. + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + _, _, err = f.d.Get(ctx, 1) + assert.ErrorIs(t, err, ErrNotDispatchable) + got, ok, err := f.d.Get(ctx, 0) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(2), got.EventID) + + // Event 2 the worker did pull, just now. Settled after that, it is served + // again to the worker that has it, and its report is taken. + require.NoError(t, f.ledger.SetState(ctx, 2, StateCompleted, "")) + again, ok, err := f.d.Get(ctx, 2) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, int64(2), again.EventID) + _, err = f.d.Ack(ctx, 2, nil) + require.NoError(t, err) +} + +// A task is live until it is superseded, whatever became of its records, so a +// conversation whose only event was settled before the worker pulled it is +// still busy. +func TestALiveTaskKeepsItsConversationBusy(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SetState(ctx, 1, StateCompleted, "")) + require.NoError(t, f.ledger.SetState(ctx, 2, StateCompleted, "")) + seenRecord(t, f.ledger, 3) + _, err := f.ledger.Admission().Commit(ctx, admittedVerdict(3, 0, "recording:10304028989")) + require.NoError(t, err) + + _, err = f.ledger.CreateTask(ctx, []int64{3}) + require.ErrorIs(t, err, ErrConversationBusy, "the first task is still live") + + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err = f.ledger.CreateTask(ctx, []int64{3}) + require.NoError(t, err) +} + +// A task is made of work waiting for a worker, on a live task, and its rows +// stay where they were written — the database says so too. +func TestTheDatabaseRefusesAttachingWorkToTheWrongTask(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + seenRecord(t, f.ledger, 3) + + _, err := f.ledger.db.ExecContext(ctx, `INSERT INTO task_events (task_id, event_id) VALUES (?, 3)`, f.grant.ID) + require.Error(t, err, "a seen record is not work waiting for a worker") + + other, err := f.ledger.CreateTask(ctx, []int64{}) + require.Error(t, err) + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err = f.ledger.Admission().Commit(ctx, admittedVerdict(3, 0, "recording:3")) + require.NoError(t, err) + _, err = f.ledger.db.ExecContext(ctx, `INSERT INTO task_events (task_id, event_id) VALUES (?, 3)`, f.grant.ID) + require.Error(t, err, "a superseded task takes no new work") + _ = other + + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET task_id = 99 WHERE event_id = 1`) + require.Error(t, err, "a task event does not move between tasks") + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET event_id = 3 WHERE event_id = 1`) + require.Error(t, err, "nor between events") +} + +// An instruction is content, not an empty blob: a record with one would be +// dispatched and never servable. +func TestCreateTaskRefusesAnEmptyInstruction(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err := f.ledger.db.ExecContext(ctx, `UPDATE events SET snapshot = CAST('' AS BLOB) WHERE id = 1`) + require.NoError(t, err) + + _, err = f.ledger.CreateTask(ctx, []int64{1}) + require.ErrorIs(t, err, ErrNotDispatchable) +} + +// A reported outcome stands, and so does what came with it: an +// acknowledgement arriving after the completion is refused, not written. +func TestAnAcknowledgementAfterTheOutcomeIsRefused(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + _, err = f.d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + + late := int64(4242) + _, err = f.d.Ack(ctx, 1, &late) + + require.ErrorIs(t, err, ErrReportConflict) + var ack *int64 + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT ack_id FROM task_events WHERE event_id = 1`).Scan(&ack)) + assert.Nil(t, ack, "nothing was written") +} + +// A withdrawn exposure is finished: the record it belonged to may be running +// again on a new task, and the old row does not follow it. +func TestAWithdrawnExposureDoesNotFollowTheRetry(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, err := f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'exposed', exposed_at = 'launch' WHERE event_id = 1`) + require.NoError(t, err) + + tx, err := f.ledger.db.BeginTx(ctx, nil) + require.NoError(t, err) + require.NoError(t, f.ledger.supersedeTask(ctx, tx, f.grant.ID)) + require.NoError(t, f.ledger.withdrawExposure(ctx, tx, f.grant.ID, 1, StateAdmitted, "")) + retry, err := f.ledger.createTask(ctx, tx, []int64{1}) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + + // The retry's worker pulls and completes it, so the record is completed. + d, err := f.ledger.Dispatch(ctx, retry.Token, adapterAgentID) + require.NoError(t, err) + _, _, err = d.Get(ctx, 1) + require.NoError(t, err) + _, err = d.Complete(ctx, 1, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + + // The withdrawn row on the old task stays where it was. + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET delivery = 'completed' WHERE task_id = ? AND event_id = 1`, f.grant.ID) + require.Error(t, err) + var delivery string + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT delivery FROM task_events WHERE task_id = ? AND event_id = 1`, f.grant.ID).Scan(&delivery)) + assert.Equal(t, "exposed", delivery) +} + +// A queued record does not go around the admitted one that holds its +// conversation: admission queued it precisely because that one is next. +func TestAQueuedRecordDoesNotJumpItsConversation(t *testing.T) { + ctx := context.Background() + ledger, err := OpenLedger(filepath.Join(t.TempDir(), "state", "connector.db")) + require.NoError(t, err) + t.Cleanup(func() { _ = ledger.Close() }) + for _, id := range []int64{1, 2} { + seenRecord(t, ledger, id) + _, err := ledger.Admission().Commit(ctx, admittedVerdict(id, 0, "recording:10304028989")) + require.NoError(t, err) + } + require.Equal(t, StateQueued, getRecord(t, ledger, 2).State) + + _, err = ledger.CreateTask(ctx, []int64{2}) + + require.ErrorIs(t, err, ErrConversationBusy) + assert.Contains(t, err.Error(), "event 1 is admitted ahead of event 2") + _, err = ledger.CreateTask(ctx, []int64{1}) + assert.NoError(t, err, "and the record that is next dispatches") +} + +// A task is its worker's until it is superseded, even after retention has +// cleared what its records said: the conversation a task holds is written on +// the task's own rows, where retention does not reach. +func TestRetentionDoesNotFreeALiveTasksConversation(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + for _, id := range []int64{1, 2} { + _, _, err := f.d.Get(ctx, id) + require.NoError(t, err) + _, err = f.d.Complete(ctx, id, Completion{Outcome: OutcomeSucceeded}) + require.NoError(t, err) + } + dropped, err := f.ledger.DropContent(ctx, time.Now().Add(time.Hour), time.Now().Add(time.Hour)) + require.NoError(t, err) + require.Equal(t, 2, dropped, "the records' payloads are gone, conversation and all") + + seenRecord(t, f.ledger, 3) + _, err = f.ledger.Admission().Commit(ctx, admittedVerdict(3, 0, "recording:10304028989")) + require.NoError(t, err) + _, err = f.ledger.CreateTask(ctx, []int64{3}) + + require.ErrorIs(t, err, ErrConversationBusy) + assert.Contains(t, err.Error(), "is live on the conversation of event 3") + require.NoError(t, f.ledger.SupersedeTask(ctx, f.grant.ID)) + _, err = f.ledger.CreateTask(ctx, []int64{3}) + assert.NoError(t, err, "and the conversation is free once the task is not") +} + +// An acknowledgement with an id, after one without, is a second report, not +// the same call retried: the receipt stands and nothing is written. +func TestAnAcknowledgementIdAfterTheAcknowledgementIsRefused(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + _, err = f.d.Ack(ctx, 1, nil) + require.NoError(t, err) + + late := int64(4242) + _, err = f.d.Ack(ctx, 1, &late) + + require.ErrorIs(t, err, ErrReportConflict) + assert.Contains(t, err.Error(), "acknowledged without an acknowledgement id") + var ack *int64 + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT ack_id FROM task_events WHERE event_id = 1`).Scan(&ack)) + assert.Nil(t, ack, "nothing was written") +} + +// A retry that has nothing to point at answers the receipt it already has: no +// id is not a different id. +func TestAnAcknowledgementWithoutAnIdAfterOneWithItIsTheSameReceipt(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + posted := int64(4242) + first, err := f.d.Ack(ctx, 1, &posted) + require.NoError(t, err) + + again, err := f.d.Ack(ctx, 1, nil) + + require.NoError(t, err) + assert.Equal(t, first, again) + var ack *int64 + require.NoError(t, f.ledger.db.QueryRowContext(ctx, `SELECT ack_id FROM task_events WHERE event_id = 1`).Scan(&ack)) + require.NotNil(t, ack) + assert.Equal(t, posted, *ack, "and the id it did report is still there") +} + +// The same rule holds for anything else writing to the file: an id is written +// with the acknowledgement or not at all. +func TestTheSchemaKeepsAnAcknowledgementIdWithItsAcknowledgement(t *testing.T) { + f := newDispatchFixture(t) + ctx := context.Background() + _, _, err := f.d.Get(ctx, 1) + require.NoError(t, err) + _, err = f.d.Ack(ctx, 1, nil) + require.NoError(t, err) + + _, err = f.ledger.db.ExecContext(ctx, `UPDATE task_events SET ack_id = 4242 WHERE event_id = 1`) + + require.Error(t, err) + assert.Contains(t, err.Error(), "written with the acknowledgement, once") +} diff --git a/internal/connector/ledger_events.go b/internal/connector/ledger_events.go index 54d6f3281..4e278c040 100644 --- a/internal/connector/ledger_events.go +++ b/internal/connector/ledger_events.go @@ -217,8 +217,10 @@ var lifecycle = map[RecordState][]RecordState{ StateAdmitted: {StateQueued, StateDispatched, StateBlocked, StateDiscarded}, StateQueued: {StateDispatched, StateBlocked, StateDiscarded}, StateBlocked: {StateAdmitted, StateQueued, StateDispatched, StateDiscarded}, - // A dispatched record whose worker never started has its exposure - // withdrawn and returns to admitted. It is never discarded: a dispatched + // A dispatched record whose spawn failed before any worker process + // existed has its exposure withdrawn (task_events.withdrawn_at) and + // returns to admitted; one a worker was handed otherwise leaves only to + // completed (the dispatch lifecycle, ledger_dispatch.go). It is never discarded: a dispatched // event ends completed, with an outcome, even when the outcome is // unknown. StateDispatched: {StateCompleted, StateBlocked, StateAdmitted}, @@ -367,6 +369,21 @@ func (l *Ledger) move(ctx context.Context, db dbtx, t transition) (bool, error) args = append(args, *t.revision) } query.WriteString(" AND state IN (" + strings.TrimSuffix(strings.Repeat("?, ", len(froms)), ", ") + ")") + switch t.state { + case StateDispatched: + // A record is dispatched exactly while a live task carries it: it + // enters dispatched only with its task row already written + // (createTask), and a repeat is a repeat. + query.WriteString(" AND (state = 'dispatched' OR " + onLiveTask + ")") + case StateCompleted: + default: + // Invariant 4 of the dispatch lifecycle (ledger_dispatch.go): a + // record a worker was handed leaves dispatched only to completed — + // and any other record leaves it only once no live task carries it + // (supersedeTask retires the row first). + query.WriteString(" AND NOT (" + heldByWorker + ")") + query.WriteString(" AND NOT (state = 'dispatched' AND " + onLiveTask + ")") + } for _, from := range froms { args = append(args, from) } @@ -385,6 +402,29 @@ func (l *Ledger) move(ctx context.Context, db dbtx, t transition) (bool, error) return affected > 0, nil } +// heldByWorker is true of a dispatched events row a worker was handed and +// has not reported on: a delivery exposed or delivered, on any task, and not +// withdrawn because the spawn failed before any worker existed. +const heldByWorker = `state = 'dispatched' AND EXISTS ( + SELECT 1 FROM task_events WHERE task_events.event_id = events.id + AND delivery IN ('exposed', 'delivered') AND withdrawn_at IS NULL)` + +// onLiveTask is true of an events row a live task carries. +const onLiveTask = `EXISTS ( + SELECT 1 FROM task_events WHERE task_events.event_id = events.id AND retired_at IS NULL)` + +// ErrNotOnALiveTask is a move into dispatched for a record no live task +// carries. Records are dispatched by creating a task for them. +var ErrNotOnALiveTask = errors.New("no live task carries this event; a record is dispatched by creating a task for it") + +// ErrOnALiveTask is a move out of dispatched, other than to completed, for a +// record a live task still carries. Its task is superseded first. +var ErrOnALiveTask = errors.New("a live task still carries this event; supersede the task first") + +// ErrHeldByWorker is a move out of dispatched for an event a worker was +// handed and has not reported on. Only its outcome moves it. +var ErrHeldByWorker = errors.New("a worker was handed this event; it leaves dispatched only when completed") + // explainRefusal says why an update changed nothing: there is no such record, // or the record is somewhere the lifecycle cannot leave for state. // @@ -399,6 +439,21 @@ func (l *Ledger) explainRefusal(ctx context.Context, id int64, state RecordState case err != nil: return fmt.Errorf("connector: set state of %d: %w", id, err) } + if slices.Contains(enterableFrom(state), current) { + // The edge exists; a dispatch rule refused it. + var held, live bool + if err := l.db.QueryRowContext(ctx, `SELECT EXISTS (SELECT 1 FROM events WHERE id = ? AND `+heldByWorker+`), EXISTS (SELECT 1 FROM events WHERE id = ? AND `+onLiveTask+`)`, id, id).Scan(&held, &live); err != nil { + return fmt.Errorf("connector: set state of %d: %w", id, err) + } + switch { + case held: + return fmt.Errorf("connector: set state of %d: %w", id, ErrHeldByWorker) + case state == StateDispatched && !live: + return fmt.Errorf("connector: set state of %d: %w", id, ErrNotOnALiveTask) + case live: + return fmt.Errorf("connector: set state of %d: %w", id, ErrOnALiveTask) + } + } return fmt.Errorf("connector: set state of %d: %s to %s is %w", id, current, state, ErrNotATransition) } diff --git a/internal/connector/ledger_owner_unix_test.go b/internal/connector/ledger_owner_unix_test.go new file mode 100644 index 000000000..0eaa66167 --- /dev/null +++ b/internal/connector/ledger_owner_unix_test.go @@ -0,0 +1,88 @@ +//go:build unix + +package connector + +import ( + "context" + "os" + "path/filepath" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Ownership is read from the file itself: a ledger this user does not own is +// not one this process may read, whoever else can see it. +func TestOwnedByThisUser(t *testing.T) { + path := filepath.Join(t.TempDir(), "ledger.db") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + info, err := os.Lstat(path) + require.NoError(t, err) + + assert.True(t, ownedByThisUser(info)) + assert.True(t, sameOwner(info, info)) + assert.False(t, sameOwner(info, otherOwner{info}), "the owner changed since the check") + err = verifySameFile(path, otherOwner{info}) + require.Error(t, err, "a second open is held to the owner the check passed") + assert.Contains(t, err.Error(), "no longer owned by the user the check passed") + assert.False(t, ownedByThisUser(otherOwner{info}), "another user's file") + assert.False(t, ownedByThisUser(noOwner{info}), "a file whose owner cannot be read") +} + +type otherOwner struct{ os.FileInfo } + +func (o otherOwner) Sys() any { + stat := *o.FileInfo.Sys().(*syscall.Stat_t) + stat.Uid++ + return &stat +} + +type noOwner struct{ os.FileInfo } + +func (noOwner) Sys() any { return nil } + +// A second open vets the path it is reached through, not only the file: a +// directory chain that is no longer private is refused, without the ledger +// being opened again. +func TestASecondOpenVetsTheDirectoryChain(t *testing.T) { + home := t.TempDir() + dir := filepath.Join(home, "state") + require.NoError(t, os.Mkdir(dir, 0o700)) + path := filepath.Join(dir, "connector.db") + first, err := OpenLedger(path) + require.NoError(t, err) + defer first.Close() + checks := securePathRuns.Load() + + // An ancestor anyone can write is a path anyone can redirect. + require.NoError(t, os.Chmod(home, 0o777)) + t.Cleanup(func() { _ = os.Chmod(home, 0o700) }) + + _, err = OpenExistingLedger(context.Background(), path) + + require.Error(t, err) + assert.Contains(t, err.Error(), "secure the ledger") + assert.Equal(t, checks, securePathRuns.Load(), "and the file was not opened to find that out") +} + +// And a second open vets the ledger file's own mode: a file another user can +// read is not one to read from, whatever it was when the first open checked. +func TestASecondOpenVetsTheFileMode(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + require.NoError(t, os.Mkdir(dir, 0o700)) + path := filepath.Join(dir, "connector.db") + first, err := OpenLedger(path) + require.NoError(t, err) + defer first.Close() + checks := securePathRuns.Load() + + require.NoError(t, os.Chmod(path, 0o644)) + + _, err = OpenExistingLedger(context.Background(), path) + + require.Error(t, err) + assert.Contains(t, err.Error(), "can be read by other users (mode 0644)") + assert.Equal(t, checks, securePathRuns.Load(), "and the file was not opened to find that out") +} diff --git a/internal/connector/ledger_test.go b/internal/connector/ledger_test.go index 0c94ceb8f..50a1eb5d7 100644 --- a/internal/connector/ledger_test.go +++ b/internal/connector/ledger_test.go @@ -238,3 +238,114 @@ func TestLedgerRefusesALooseAncestor(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, setup.ErrNotPrivate) } + +// A second Ledger on a file this process already has open — a status read +// beside a running connector, a promote — must not run the check that opens +// the file: POSIX drops every lock this process holds on a file when any +// descriptor for it is closed, SQLite's included, and the first handle would +// go on believing it still held them. +func TestASecondLedgerOnALiveFileNeitherOpensNorDisturbsIt(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "connector.db") + first, err := OpenLedger(path) + require.NoError(t, err) + defer first.Close() + ctx := context.Background() + _, err = first.RecordSeen(ctx, testEvent(1), LanePoll) + require.NoError(t, err) + checksBefore := securePathRuns.Load() + + second, err := OpenLedger(path) + require.NoError(t, err) + defer second.Close() + + assert.Equal(t, checksBefore, securePathRuns.Load(), "the check that opens the file did not run again") + + // Both handles read and write, in both orders, with the other's locks + // still in place. + _, err = second.RecordSeen(ctx, testEvent(2), LanePoll) + require.NoError(t, err) + record, ok, err := first.Get(ctx, 2) + require.NoError(t, err) + require.True(t, ok, "the first handle reads what the second wrote") + assert.Equal(t, StateSeen, record.State) + require.NoError(t, first.SetState(ctx, 1, StateDiscarded, "untrusted_author")) + record, ok, err = second.Get(ctx, 1) + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, StateDiscarded, record.State) + + // The file has to be the one that was checked. + require.NoError(t, second.Close()) + require.NoError(t, os.Rename(path, path+".moved")) + require.NoError(t, os.WriteFile(path, nil, 0o600)) + _, err = OpenLedger(path) + assert.ErrorIs(t, err, ErrLedgerNotTheSameFile) +} + +// The one-check-per-file rule is about the file, not the name: a hardlink or +// another route to a file this process has open is recognized as that file, +// before the check that would open it. +func TestASecondNameForALiveLedgerIsRefusedWithoutOpeningIt(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + require.NoError(t, os.MkdirAll(dir, 0o700)) + path := filepath.Join(dir, "connector.db") + first, err := OpenLedger(path) + require.NoError(t, err) + defer first.Close() + checks := securePathRuns.Load() + + link := filepath.Join(dir, "hard.db") + require.NoError(t, os.Link(path, link)) + _, err = OpenExistingLedger(context.Background(), link) + + // Refused, because SQLite names its write-ahead log after the path and + // one file under two names is two logs — and refused before the check + // that opens the file, which would have dropped the live handle's locks. + require.ErrorIs(t, err, ErrLedgerUnderAnotherName) + assert.Equal(t, checks, securePathRuns.Load()) + _, err = first.RecordSeen(context.Background(), testEvent(1), LanePoll) + require.NoError(t, err, "the live handle is untouched") + _, ok, err := first.Get(context.Background(), 1) + require.NoError(t, err) + assert.True(t, ok) +} + +// Close releases the file once: a defensive second Close must not take the +// entry away from another Ledger still holding it. +func TestClosingALedgerTwiceReleasesItOnce(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "connector.db") + first, err := OpenLedger(path) + require.NoError(t, err) + defer first.Close() + second, err := OpenLedger(path) + require.NoError(t, err) + require.NoError(t, second.Close()) + require.NoError(t, second.Close()) + checks := securePathRuns.Load() + + third, err := OpenLedger(path) + require.NoError(t, err) + defer third.Close() + + assert.Equal(t, checks, securePathRuns.Load(), "the first handle still holds the file") +} + +// An aliased open that arrives while the first opener's check is still +// running must find that file's entry: that check is the one that opens the +// file, and it is what the whole mechanism exists to hold off. +func TestAnAliasClaimedDuringTheFirstCheckFindsTheSameFile(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + require.NoError(t, os.MkdirAll(dir, 0o700)) + path := filepath.Join(dir, "connector.db") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + link := filepath.Join(dir, "hard.db") + require.NoError(t, os.Link(path, link)) + + // The first opener has claimed the file; its check has not run yet. + first := claimLedger(path) + defer releaseLedger(first) + second := claimLedger(link) + defer releaseLedger(second) + + assert.Same(t, first, second, "one file, one entry, whatever it is called") +} diff --git a/internal/connector/owner_other.go b/internal/connector/owner_other.go new file mode 100644 index 000000000..e0fa3fbb8 --- /dev/null +++ b/internal/connector/owner_other.go @@ -0,0 +1,12 @@ +//go:build !unix + +package connector + +import "os" + +// ownedByThisUser cannot be answered without POSIX owners, and a ledger whose +// privacy cannot be established is refused rather than opened. +func ownedByThisUser(os.FileInfo) bool { return false } + +// sameOwner cannot be answered without POSIX owners. +func sameOwner(os.FileInfo, os.FileInfo) bool { return false } diff --git a/internal/connector/owner_unix.go b/internal/connector/owner_unix.go new file mode 100644 index 000000000..6deb424c8 --- /dev/null +++ b/internal/connector/owner_unix.go @@ -0,0 +1,26 @@ +//go:build unix + +package connector + +import ( + "os" + "syscall" +) + +// ownedByThisUser reports a file this user owns. The connector runs on Unix +// only — its ledger's privacy cannot be established elsewhere — so this is +// where ownership is read. +func ownedByThisUser(info os.FileInfo) bool { + stat, ok := info.Sys().(*syscall.Stat_t) + // The effective uid, which is what setup's private-path check compares + // against: the two must agree, or a first open could pass where a later + // one is refused. + return ok && int(stat.Uid) == os.Geteuid() +} + +// sameOwner reports two stats of a file with the same owner. +func sameOwner(a, b os.FileInfo) bool { + left, okA := a.Sys().(*syscall.Stat_t) + right, okB := b.Sys().(*syscall.Stat_t) + return okA && okB && left.Uid == right.Uid +} diff --git a/internal/connector/round7_test.go b/internal/connector/round7_test.go index b55e73c6d..d294ede2e 100644 --- a/internal/connector/round7_test.go +++ b/internal/connector/round7_test.go @@ -40,8 +40,7 @@ func TestDropContentKeepsARecordUpdatedJustAfterAWholeSecondCutoff(t *testing.T) _, err := ledger.RecordSeen(ctx, testEvent(43), LanePoll) require.NoError(t, err) // The lifecycle has no shortcut to completed, so the record walks there. - require.NoError(t, ledger.SetState(ctx, 43, StateAdmitted, "")) - require.NoError(t, ledger.SetState(ctx, 43, StateDispatched, "")) + dispatchForTest(t, ledger, 43) require.NoError(t, ledger.SetState(ctx, 43, StateCompleted, "")) dropped, err := ledger.DropContent(ctx, cutoff, cutoff) diff --git a/internal/connector/setup/private_state.go b/internal/connector/setup/private_state.go index 81b55d319..e24d3c152 100644 --- a/internal/connector/setup/private_state.go +++ b/internal/connector/setup/private_state.go @@ -147,6 +147,55 @@ func EnsurePrivateFile(path string) error { return checkPrivateReadableFile(f, path) } +// CheckPrivateDir holds a directory to the rules a private file's directory +// must meet — every ancestor this user's own and unwritable by anyone else, +// and the directory itself private — without opening anything inside it. +// +// It exists for a second open of a file this process already holds: the file +// must not be opened again (POSIX drops a process's locks on any close of it), +// but the path it is reached through can still be vetted. +func CheckPrivateDir(dir string) error { + abs, err := filepath.Abs(dir) + if err != nil { + return err + } + if err := checkAncestors(filepath.Dir(abs)); err != nil { + return err + } + if _, err := os.Lstat(abs); err != nil { + return fmt.Errorf("inspect %s: %w", abs, err) + } + return checkPrivateDir(abs) +} + +// CheckPrivateFile holds an existing file to EnsurePrivateFile's rules without +// creating anything: every directory on the way must be this user's alone, the +// file must not be a symlink, and — inspected through the open descriptor — it +// must be this user's own file that nobody else can read. A missing file, or +// a missing directory, is an error satisfying errors.Is(err, os.ErrNotExist). +func CheckPrivateFile(path string) error { + abs, err := filepath.Abs(path) + if err != nil { + return err + } + dir := filepath.Dir(abs) + if err := checkAncestors(filepath.Dir(dir)); err != nil { + return err + } + if _, err := os.Lstat(dir); err != nil { + return fmt.Errorf("inspect %s: %w", dir, err) + } + if err := checkPrivateDir(dir); err != nil { + return err + } + f, err := openNoFollow(abs) + if err != nil { + return err + } + defer f.Close() + return checkPrivateReadableFile(f, abs) +} + func checkPrivateReadableFile(f *os.File, path string) error { if err := checkPrivateFile(f, path); err != nil { return err diff --git a/internal/connector/setup/private_unix_test.go b/internal/connector/setup/private_unix_test.go index 5b8fdb560..c6a0f4bfe 100644 --- a/internal/connector/setup/private_unix_test.go +++ b/internal/connector/setup/private_unix_test.go @@ -187,3 +187,28 @@ func TestLockRefusesASecondSetup(t *testing.T) { require.NoError(t, err) unlockAgain() } + +func TestCheckPrivateFileCreatesNothingAndHoldsTheRules(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + require.NoError(t, os.Mkdir(dir, 0o700)) + path := filepath.Join(dir, "ledger.db") + + err := CheckPrivateFile(path) + require.ErrorIs(t, err, os.ErrNotExist) + _, statErr := os.Lstat(path) + require.ErrorIs(t, statErr, os.ErrNotExist, "nothing was created") + + require.NoError(t, os.WriteFile(path, nil, 0o600)) + require.NoError(t, CheckPrivateFile(path)) + + require.NoError(t, os.Chmod(path, 0o644)) + require.ErrorIs(t, CheckPrivateFile(path), ErrNotPrivate) + require.NoError(t, os.Chmod(path, 0o600)) + + link := filepath.Join(dir, "link.db") + require.NoError(t, os.Symlink(path, link)) + require.Error(t, CheckPrivateFile(link)) + + require.NoError(t, os.Chmod(dir, 0o775)) + require.ErrorIs(t, CheckPrivateFile(path), ErrNotPrivate, "a directory others can write") +} diff --git a/internal/connector/strip_mentions_test.go b/internal/connector/strip_mentions_test.go new file mode 100644 index 000000000..60d543581 --- /dev/null +++ b/internal/connector/strip_mentions_test.go @@ -0,0 +1,214 @@ +package connector + +import ( + "slices" + "strings" + "testing" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" +) + +// StripMentionsOf is held to agreement with the reader admission decides the +// trigger with, basecamp.MentionedPersonIDs, over markup built to make two +// parsers disagree. Four properties, for every input: +// +// 1. no mention of the agent survives; +// 2. every other person the reader found is still found, in order; +// 3. text with no mention of the agent comes back unchanged; +// 4. every removed span is a mention element and nothing more — it closes +// nothing it did not open, so a removal takes a mention and never the +// instruction around it. This is checked against the span's own text +// rather than against what the function meant to remove, so a span that +// is too long cannot hide itself. +// +// The pieces are combined in threes, so each hostile form meets each other in +// both orders and inside or around an element. + +func mentionPieces() []string { + agent := mentionMarkup(adapterAgentID) + other := mentionMarkup(otherPersonID) + sgidOf := func(m string) string { + i := strings.Index(m, `sgid="`) + len(`sgid="`) + return m[i : i+strings.Index(m[i:], `"`)] + } + a, o := sgidOf(agent), sgidOf(other) + return []string{ + agent, + other, + "plain text", + ``, + ``, + ``, + ``, + `>4])+string("0123456789abcdef"[a[0]&15])) + `;` + a[1:] + `">`, + ``, + ``, + ``, + ``, + ``, + ``, + ``, + ``, + ``, + ``, + `