diff --git a/.claude/settings.json b/.claude/settings.json index 08f7954a..5c631979 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -40,13 +40,9 @@ ], "deny": [ "Edit(types/**/*.gen.go)", - "Write(types/**/*.gen.go)", "Edit(docs/cli/**)", - "Write(docs/cli/**)", "Edit(docs/types/**)", - "Write(docs/types/**)", "Edit(docs/public/schemas/**)", - "Write(docs/public/schemas/**)", "Bash(flow secret get:*)", "Bash(flow secret list:*)", "Bash(env)", @@ -54,5 +50,9 @@ "Read(./.env)", "Read(./.env.*)" ] + }, + "env": { + "FLOW_RUN_CLIENT": "claude-code", + "FLOW_RUN_SESSION": "${CLAUDE_CODE_SESSION_ID}" } } diff --git a/.claude/skills/setup/SKILL.md b/.claude/skills/setup/SKILL.md new file mode 100644 index 00000000..61f66c99 --- /dev/null +++ b/.claude/skills/setup/SKILL.md @@ -0,0 +1,9 @@ +--- +name: setup +description: Set up a fresh clone of the flow repo for development (Go toolchain, workspace registration, dev tools, validation). +--- + +1. Go 1.25+, flow CLI installed +2. `flow workspace add flow . --set` +3. `flow install tools` +4. `flow validate` diff --git a/CLAUDE.md b/CLAUDE.md index 5783599b..50f47fd9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,9 +121,7 @@ The CLI emits a structured error envelope (`{"error":{"code","message","details" `--output json|yaml`, plain text otherwise. Both paths go through `cmd/internal/errors.HandleFatal`. Typed errors in `pkg/errors/errors.go` implement `Code() string`. Extend that set rather than -returning bare `fmt.Errorf` when a stable machine-readable code matters. Codes: `INVALID_INPUT`, -`NOT_FOUND`, `EXECUTION_FAILED`, `TIMEOUT`, `CANCELLED`, `VALIDATION_FAILED`, `INTERNAL_ERROR`, -`PERMISSION_DENIED`. +returning bare `fmt.Errorf` when a stable machine-readable code matters. --- @@ -144,9 +142,3 @@ Run the `validate` executable before marking a PR ready. Commit messages: impera - **`.claude/settings.local.json`** — gitignored, per-user. Where absolute paths belong, e.g. `permissions.additionalDirectories` for the module cache and sibling checkouts. -## Development Setup - -1. Go 1.25+, flow CLI installed -2. `flow workspace add flow . --set` -3. `flow install tools` -4. `flow validate` diff --git a/cmd/internal/browse.go b/cmd/internal/browse.go index b61bc6d3..ffb7065f 100644 --- a/cmd/internal/browse.go +++ b/cmd/internal/browse.go @@ -80,7 +80,7 @@ func executableLibrary(ctx *context.Context, cmd *cobra.Command, _ []string) { wsFilter := flags.ValueFor[string](cmd, *flags.FilterWorkspaceFlag, false) switch wsFilter { case ".": - wsFilter = ctx.Config.CurrentWorkspace + wsFilter = ctx.CurrentWorkspaceName() case executable.WildcardWorkspace: wsFilter = "" } @@ -140,7 +140,7 @@ func executableLibrary(ctx *context.Context, cmd *cobra.Command, _ []string) { func listExecutables(ctx *context.Context, cmd *cobra.Command, _ []string) { wsFilter := flags.ValueFor[string](cmd, *flags.FilterWorkspaceFlag, false) if wsFilter == "." { - wsFilter = ctx.Config.CurrentWorkspace + wsFilter = ctx.CurrentWorkspaceName() } nsFilter := flags.ValueFor[string](cmd, *flags.FilterNamespaceFlag, false) diff --git a/cmd/internal/exec.go b/cmd/internal/exec.go index 3e4d7c87..69edc317 100644 --- a/cmd/internal/exec.go +++ b/cmd/internal/exec.go @@ -92,7 +92,19 @@ func RegisterExecCmd(ctx *context.Context, rootCmd *cobra.Command) { rootCmd.AddCommand(subCmd) } -func execPreRun(_ *context.Context, _ *cobra.Command, _ []string) { +func execPreRun(ctx *context.Context, cmd *cobra.Command, _ []string) { + // --workspace has to move the whole context, not just the transient run's: a named + // executable is looked up by a ref expanded from the current workspace, and running one from + // a different workspace is refused outright. + if explicit := flags.ValueFor[string](cmd, *flags.RunWorkspaceFlag, false); explicit != "" { + res, err := filesystem.ResolveWorkspace(ctx.Config, filesystem.ResolveOptions{Override: explicit}) + if err != nil { + errhandler.HandleUsage(ctx, cmd, "%v", err) + return + } + ctx.SetCurrentWorkspace(res) + } + runner.RegisterRunner(exec.NewRunner()) runner.RegisterRunner(launch.NewRunner()) runner.RegisterRunner(request.NewRunner()) @@ -210,7 +222,7 @@ func resolveExecutableForRun( "executable '%s' belongs to workspace '%s' and cannot be run from the current workspace '%s'", ref, e.Workspace(), - ctx.Config.CurrentWorkspace, + ctx.CurrentWorkspaceName(), )) } return e, ref @@ -357,37 +369,51 @@ func resolveSpecContent(spec string) (string, error) { func setTransientContext(ctx *context.Context, cmd *cobra.Command, e *executable.Executable, runDir string) { wsName, wsPath := resolveRunWorkspace(ctx, cmd, runDir) - currentName := "" - if ctx.CurrentWorkspace != nil { - currentName = ctx.CurrentWorkspace.AssignedName() - } + currentName := ctx.CurrentWorkspaceName() if wsName != "" && wsName != currentName { logger.Log().Infof("Running in workspace '%s' (current workspace is '%s')", wsName, currentName) + } else if wsName != "" && !ctx.WorkspaceIsRegistered() { + logger.Log().Debugf("Running in unregistered workspace '%s' (%s)", wsName, wsPath) } var flowFilePath string if wsPath != "" { - flowFilePath = filepath.Join(wsPath, "flow.yaml") + flowFilePath = filepath.Join(wsPath, filesystem.WorkspaceConfigFileName) } e.SetContext(wsName, wsPath, ctx.Config.CurrentNamespace, flowFilePath) } -// resolveRunWorkspace picks the workspace a transient run should use, in priority order: -// an explicit --workspace flag, then the workspace whose location contains runDir (longest match), -// then the global current workspace. It never changes the global current workspace. +// resolveRunWorkspace picks the workspace a transient run should use, in priority order: an +// explicit --workspace flag (a registered name or a path), then the nearest workspace at or above +// runDir, then a registered workspace containing runDir, then the global current workspace. It +// never changes the global current workspace. func resolveRunWorkspace(ctx *context.Context, cmd *cobra.Command, runDir string) (name, path string) { - wsList, err := ctx.WorkspacesCache.GetWorkspaceConfigList() - if err != nil { - logger.Log().Debugf("unable to load workspaces for transient run resolution: %v", err) + if explicit := flags.ValueFor[string](cmd, *flags.RunWorkspaceFlag, false); explicit != "" { + res, err := filesystem.ResolveWorkspace(ctx.Config, filesystem.ResolveOptions{ + Dir: runDir, Override: explicit, + }) + if err != nil || res == nil { + errhandler.HandleUsage(ctx, cmd, "unable to resolve workspace %q: %v", explicit, err) + return "", "" + } + return res.Name, res.Path } - if explicit := flags.ValueFor[string](cmd, *flags.RunWorkspaceFlag, false); explicit != "" { - if ws := wsList.FindByName(explicit); ws != nil { - return ws.AssignedName(), ws.Location() + // Discovery covers a worktree or clone that is registered nowhere; the cache lookup below + // still handles a registered workspace whose own flow.yaml has gone missing. + if runDir != "" { + res, err := filesystem.ResolveWorkspace(ctx.Config, filesystem.ResolveOptions{Dir: runDir}) + if err != nil { + logger.Log().Debugf("unable to resolve workspace for transient run: %v", err) + } else if res != nil && res.Source != filesystem.SourceCurrent { + return res.Name, res.Path } - errhandler.HandleUsage(ctx, cmd, "unknown workspace %q", explicit) } + wsList, err := ctx.WorkspacesCache.GetWorkspaceConfigList() + if err != nil { + logger.Log().Debugf("unable to load workspaces for transient run resolution: %v", err) + } if ws := workspaceForPath(wsList, runDir); ws != nil { return ws.AssignedName(), ws.Location() } @@ -702,15 +728,15 @@ type provenance struct { // runProvenanceFromEnv resolves run provenance from environment variables set by the caller // (e.g. the MCP server). Source defaults to "cli". func runProvenanceFromEnv() provenance { - source := os.Getenv(store.RunSourceEnv) + source := store.RunEnvValue(store.RunSourceEnv) if source == "" { source = store.RunSourceCLI } wd, _ := os.Getwd() return provenance{ source: source, - client: os.Getenv(store.RunClientEnv), - session: os.Getenv(store.RunSessionEnv), + client: store.RunEnvValue(store.RunClientEnv), + session: store.RunEnvValue(store.RunSessionEnv), dir: wd, } } diff --git a/cmd/internal/helpers.go b/cmd/internal/helpers.go index 493bdd1a..1fef2ebe 100644 --- a/cmd/internal/helpers.go +++ b/cmd/internal/helpers.go @@ -105,22 +105,27 @@ func printContext(ctx *context.Context, cmd *cobra.Command) { } func workspaceOrCurrent(ctx *context.Context, workspaceName string) *workspace.Workspace { - var ws *workspace.Workspace - if workspaceName == "" { - ws = ctx.CurrentWorkspace - workspaceName = ws.AssignedName() - } else { - wsPath, wsFound := ctx.Config.Workspaces[workspaceName] - if !wsFound { + // An empty name, or the name of the workspace already resolved for this command, is answered + // from the context — which is the only place a workspace discovered from the working + // directory exists, since it is in no config. + if workspaceName == "" || ctx.CurrentWorkspaceName() == workspaceName { + if ctx.CurrentWorkspace == nil { return nil } - var err error - ws, err = filesystem.LoadWorkspaceConfig(workspaceName, wsPath) - if err != nil { - logger.Log().WrapError(err, "unable to load workspace config") - } - ws.SetContext(workspaceName, wsPath) + logger.Log().Debugf("'%s' workspace set", ctx.CurrentWorkspaceName()) + return ctx.CurrentWorkspace + } + + wsPath, wsFound := ctx.Config.Workspaces[workspaceName] + if !wsFound { + return nil + } + ws, err := filesystem.LoadWorkspaceConfig(workspaceName, wsPath) + if err != nil { + logger.Log().WrapError(err, "unable to load workspace config") + return nil } + ws.SetContext(workspaceName, wsPath) logger.Log().Debugf("'%s' workspace set", workspaceName) return ws } diff --git a/cmd/internal/vault.go b/cmd/internal/vault.go index 19a6195c..ad16653e 100644 --- a/cmd/internal/vault.go +++ b/cmd/internal/vault.go @@ -76,6 +76,25 @@ func registerCreateVaultCmd(ctx *context.Context, vaultCmd *cobra.Command) { vaultCmd.AddCommand(createCmd) } +// vaultWorkspacePath returns the directory a workspace-relative vault path resolves against. A +// vault outlives the command that created it, so anchoring one in a directory flow only found by +// walking up from here is almost never what the user meant — warn before doing it. +func vaultWorkspacePath(ctx *context.Context, cmd *cobra.Command) string { + if ctx.CurrentWorkspace == nil { + return "" + } + path := ctx.CurrentWorkspace.Location() + // Machine-readable output is parsed by the caller, so a loose warning line would corrupt it. + outputFormat := flags.ValueFor[string](cmd, *flags.OutputFormatFlag, false) + if !ctx.WorkspaceIsRegistered() && outputFormat == "" { + logger.Log().Warnf( + "workspace '%s' is not registered; run 'flow workspace add %s %s' to keep this vault reachable", + ctx.CurrentWorkspaceName(), ctx.CurrentWorkspaceName(), path, + ) + } + return path +} + func createVaultFunc(ctx *context.Context, cmd *cobra.Command, args []string) { vaultName := args[0] vaultType := flags.ValueFor[string](cmd, *flags.VaultTypeFlag, false) @@ -120,9 +139,8 @@ func createVaultFunc(ctx *context.Context, cmd *cobra.Command, args []string) { ctx.Config.Vaults = make(map[string]string) } - curWs := ctx.Config.CurrentWorkspace vaultPath = utils.ExpandDirectory( - vaultPath, ctx.Config.Workspaces[curWs], vault.CacheDirectory(vaultName), nil, + vaultPath, vaultWorkspacePath(ctx, cmd), vault.CacheDirectory(vaultName), nil, ) ctx.Config.Vaults[vaultName] = vaultPath diff --git a/cmd/internal/workspace.go b/cmd/internal/workspace.go index 1c32141b..588c2d25 100644 --- a/cmd/internal/workspace.go +++ b/cmd/internal/workspace.go @@ -298,6 +298,15 @@ func switchWorkspaceFunc(ctx *context.Context, cmd *cobra.Command, args []string ws := args[0] userConfig := ctx.Config if _, found := userConfig.Workspaces[ws]; !found { + // Switching persists a name into the config, so a workspace that exists only because + // flow found its flow.yaml from here cannot be switched to — say so rather than + // reporting it as missing when the user is standing in it. + if ctx.CurrentWorkspaceName() == ws && !ctx.WorkspaceIsRegistered() { + errhandler.HandleUsage(ctx, cmd, + "workspace '%s' was discovered at %s but is not registered - run 'flow workspace add %s %s' first", + ws, ctx.CurrentWorkspace.Location(), ws, ctx.CurrentWorkspace.Location()) + return + } errhandler.HandleFatal(ctx, cmd, flowerrors.WorkspaceNotFoundError{Workspace: ws}) } userConfig.CurrentWorkspace = ws @@ -367,10 +376,19 @@ func removeWorkspaceFunc(ctx *context.Context, cmd *cobra.Command, args []string errhandler.HandleFatal(ctx, cmd, flowerrors.WorkspaceNotFoundError{Workspace: name}) } + wsPath := userConfig.Workspaces[name] delete(userConfig.Workspaces, name) if err := filesystem.WriteConfig(userConfig); err != nil { errhandler.HandleFatal(ctx, cmd, err) } + if filesystem.WorkspaceConfigExists(wsPath) { + // The directory keeps its flow.yaml, so flow will still find it by walking up — just + // under its directory name rather than the name being removed here. + logger.Log().Infof( + "%s still has a %s, so running from %s resolves to the workspace '%s'", + wsPath, filesystem.WorkspaceConfigFileName, wsPath, filepath.Base(wsPath), + ) + } if err := cache.UpdateAll(ctx.DataStore); err != nil { errhandler.HandleFatal(ctx, cmd, errors.Wrap(err, "unable to update cache")) @@ -468,12 +486,20 @@ func getWorkspaceFunc(ctx *context.Context, cmd *cobra.Command, args []string) { if len(args) == 1 { workspaceName = args[0] wsPath = ctx.Config.Workspaces[workspaceName] + if wsPath == "" && ctx.CurrentWorkspaceName() == workspaceName { + wsPath = ctx.CurrentWorkspace.Location() + } } else { if ctx.CurrentWorkspace == nil { errhandler.HandleUsage(ctx, cmd, "no current workspace set — run 'flow workspace add' to get started") + return } workspaceName = ctx.CurrentWorkspace.AssignedName() wsPath = ctx.CurrentWorkspace.Location() + if !ctx.WorkspaceIsRegistered() { + logger.Log().Infof( + "workspace '%s' was discovered at %s and is not registered", workspaceName, wsPath) + } } wsCfg, err := filesystem.LoadWorkspaceConfig(workspaceName, wsPath) diff --git a/cmd/root.go b/cmd/root.go index edc229b2..75aa039c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -39,6 +39,7 @@ func NewRootCmd(ctx *context.Context) *cobra.Command { case "fatal": logger.Log().SetLevel(-1) } + ctx.LogWorkspaceResolution() sync := flags.ValueFor[bool](cmd.Root(), *flags.SyncCacheFlag, true) if sync { if err := cache.UpdateAll(ctx.DataStore); err != nil { diff --git a/docs/.vitepress/config.mts b/docs/.vitepress/config.mts index 95e69132..b22cd9c4 100644 --- a/docs/.vitepress/config.mts +++ b/docs/.vitepress/config.mts @@ -73,6 +73,7 @@ export default defineConfig({ items: [ { text: 'Interactive UI', link: '/guides/interactive' }, { text: 'AI Tools', link: '/guides/ai-tools' }, + { text: 'Run Provenance', link: '/guides/run-provenance' }, { text: 'Integrations', link: '/guides/integrations' }, ] }, diff --git a/docs/guides/ai-tools.md b/docs/guides/ai-tools.md index bb8070de..a86f33b6 100644 --- a/docs/guides/ai-tools.md +++ b/docs/guides/ai-tools.md @@ -57,9 +57,9 @@ below: the `.mcp.json` supplies the tools, the skill tells the assistant to reac | `list_workspaces` | All registered workspaces | | `get_workspace` | Details and config for a specific workspace | | `switch_workspace` | Change the active workspace | -| `list_executables` | Browse executables — filterable by tag, verb, workspace | +| `list_executables` | Browse executables — filterable by tag, verb, workspace, and resolvable from a `dir` | | `get_executable` | Full definition and metadata for a specific executable | -| `execute` | Run a **named** executable by ref | +| `execute` | Run a **named** executable by ref, in a given `dir` or `workspace` | | `run_command` | Run one or more **arbitrary** shell commands through flow (with a `label`, working `dir`, and optional `workspace`) — captured in history like any executable | | `run_executable` | Run a **transient executable of any type** from an inline `spec` — a serial/parallel batch, an HTTP `request`, a `render`, or a `launch` — without saving a file | | `get_execution_logs` | Output from recent runs, filterable by `source`/`session`/`status`, or `mine` for this session's own runs | @@ -68,6 +68,17 @@ below: the `.mcp.json` supplies the tools, the skill tells the assistant to reac The three run tools form a ladder, closest-fit first: **`execute`** for a task you've already named, **`run_command`** for a one-off shell command, **`run_executable`** for something richer than a single command. Reaching for flow before a raw shell tool means every run inherits the workspace's environment and secrets and is recorded — see [Observability](#observability) below. +**Working in a worktree or a fresh clone** + +The MCP server inherits whatever directory it was started in, which is often not where you are +working. Pass `dir` on `execute`, `run_command`, `run_executable`, or `list_executables` and flow +resolves the workspace by walking up from *that* directory to the nearest `flow.yaml` — so a git +worktree or a just-cloned repo works without being registered first. `get_info` reports +`workspaceRegistered` and `workspaceSource` so you can tell which case you are in; an +unregistered workspace runs normally but cannot be switched to. Exporting `FLOW_WORKSPACE` in the +server's environment pins it for every call instead. See +[Unregistered workspaces](workspaces.md#unregistered-workspaces). + **Prompts** Structured prompts the assistant can invoke for common tasks: diff --git a/docs/guides/run-provenance.md b/docs/guides/run-provenance.md new file mode 100644 index 00000000..2e767f22 --- /dev/null +++ b/docs/guides/run-provenance.md @@ -0,0 +1,192 @@ +--- +title: Run Provenance +--- + +# Run Provenance + +Every execution flow records answers *what ran*. Provenance answers **who ran it** — a terminal, +a GUI, or an AI assistant, and which one. + +This page is the contract for setting it. For reading it back, see +[History & Logs](execution-history.md). + +## What gets recorded + +| Field | Meaning | +|---|---| +| `source` | How the run reached flow — `cli`, `desktop`, or `mcp` | +| `clientName` | Who drove it — e.g. `claude-code`, `cursor` | +| `sessionId` | What it groups with, so one assistant's related runs stay together | +| `workingDir` | Where it executed — the directory, not just the workspace | + +`source` is compared as a plain string throughout, so the set is open. The three values above +are what flow itself produces; anything embedding flow can record its own without waiting for +a release. + +## MCP runs: automatic + +Nothing to configure. When an assistant runs something through the +[MCP server](ai-tools.md) — a named `execute`, a `run_command`, or a `run_executable` — the +server tags it before handing off to the CLI: + +```shell +flow logs --source mcp --limit 1 -o json +``` +```json +{ + "ref": "exec myproject/adhoc-run-tests", + "source": "mcp", + "clientName": "claude-code", + "sessionId": "4ce5bfea-7ada-4b20-b59e-bcc0df500181", + "command": "go test ./...", + "label": "run the test suite" +} +``` + +### Reading your own session ID + +A client that wants to group its runs later needs the session ID it will be tagged with. +Over stdio nothing in the transport carries it, so the server reports it in `get_info`: + +```json +{ + "currentContext": { + "workspace": "myproject", + "sessionId": "4ce5bfea-7ada-4b20-b59e-bcc0df500181" + } +} +``` + +Call it once when you connect, keep the value, and query with it later: + +```shell +flow logs --session 4ce5bfea-7ada-4b20-b59e-bcc0df500181 +``` + +### What a session is + +**Whatever the caller says belongs together.** flow resolves it in this order: + +1. a per-connection ID from the transport, when one exists +2. otherwise `FLOW_RUN_SESSION` from the environment — the caller told us +3. otherwise a UUID minted once per `flow mcp` process + +Step 2 is what keeps a single conversation intact. An assistant does not only use MCP; it +also shells out to `flow` directly. If the server invented an ID while the shell inherited a +different one, the same conversation would land in two groups. A caller that tags its +environment gets both paths under one ID. + +The minted UUID is a last resort, and it approximates a conversation rather than being one — +a client that reconnects mid-conversation gets a new one. If you know your own conversation +identity, supply it and flow will use it instead. + +## Everything else: three environment variables + +Runs that arrive any other way — an assistant shelling out, a script, an app embedding flow — +would otherwise be indistinguishable from you typing at a prompt. flow reads provenance from +the environment for **every** run, so anything that can export a variable can attribute itself: + +| Variable | Sets | Default | +|---|---|---| +| `FLOW_RUN_SOURCE` | `source` | `cli` | +| `FLOW_RUN_CLIENT` | `clientName` | empty | +| `FLOW_RUN_SESSION` | `sessionId` | empty | + +```shell +FLOW_RUN_CLIENT=claude-code FLOW_RUN_SESSION=$MY_SESSION_ID flow run build +``` + +```shell +flow logs --client claude-code # finds it, even though source is still "cli" +``` + +Leaving `FLOW_RUN_SOURCE` alone is usually right: the run genuinely did arrive through the +CLI, and `clientName` is what says an assistant drove it. Set it when the surface itself is +distinct — a desktop app sets `desktop` so its runs are not confused with a terminal's. + +> **Export it, don't pass it.** Environment beats a flag the assistant must remember on every +> call. A model can silently omit an argument; it cannot omit a variable it never sees. This +> is also why identity is not a tool parameter — parameters are for intent, which only the +> model knows. + +### Referencing another variable + +A session ID is only known once the assistant is running, so it cannot be written into a +config file as a constant. Most harnesses store that file's values **verbatim** — no `${...}` +interpolation — which would otherwise leave you recording a literal `${...}` on every run. + +So flow resolves the reference itself. Give it `${NAME}` and it reads `NAME` from the +environment: + +```shell +FLOW_RUN_SESSION='${MY_HARNESS_SESSION_ID}' flow run build +# records the value of MY_HARNESS_SESSION_ID +``` + +If `NAME` is unset the session is recorded empty, never as the literal — one shared constant +across every run would group unrelated work together, which is worse than recording nothing. + +### Example: an editor or agent harness + +Most agent tools let you define environment variables for the commands they run. Point flow at +the variable the tool already exports for its own session: + +```json +{ + "env": { + "FLOW_RUN_CLIENT": "claude-code", + "FLOW_RUN_SESSION": "${CLAUDE_CODE_SESSION_ID}" + } +} +``` + +That works whether or not the tool expands `${...}` itself — flow handles it either way. The +variable's name lives in your config rather than in flow, so when a vendor renames theirs you +edit one line here instead of waiting for a flow release. + +### Example: embedding flow behind a UI + +Set the variables on the process you spawn: + +```go +cmd := exec.Command("flow", "run", "build") +cmd.Env = append(os.Environ(), + "FLOW_RUN_SOURCE=desktop", + "FLOW_RUN_CLIENT=my-app", +) +``` + +A wrapper that also runs an MCP server can set `FLOW_RUN_SESSION` on it too, and the server +will use that instead of minting its own — so runs the assistant makes over MCP and runs the +wrapper makes directly share one session. + +## What flow deliberately does not do + +**No client registry.** flow does not sniff for `CLAUDE_CODE_SESSION_ID`, `CURSOR_*`, or any +other vendor's variables. Those are undocumented internals that get renamed, and detection +built on them fails silently — history quietly stops grouping and nobody notices. Each tool +maps its own variables onto the contract above; flow stays neutral. + +**No conversation identity of its own.** A session is a grouping key, not a claim about what a +conversation is. flow will happily record a conversation ID you hand it — that is what step 2 +above is for — but it never goes looking for one, and it stores nothing that points back at a +chat. When nobody supplies an identity, the minted per-process UUID is an approximation, and +flow treats it as one. Deciding what a conversation *is* stays with the client, which is the +only thing that can know. + +## Querying it + +```shell +flow logs --source mcp # only assistant-launched runs +flow logs --client cursor # one client +flow logs --session # one connection's runs +flow logs --source mcp --status failed # what an assistant broke +``` + +Assistants can review their own activity with `get_execution_logs`, which takes the same +filters plus `mine: true` — scoped to the calling session without needing to know its ID. + +## What's Next? + +- [History & Logs](execution-history.md) — reading, filtering, and clearing history +- [AI Tools](ai-tools.md) — setting up the MCP server diff --git a/docs/guides/workspaces.md b/docs/guides/workspaces.md index 3d7f1b12..cc365abb 100644 --- a/docs/guides/workspaces.md +++ b/docs/guides/workspaces.md @@ -203,6 +203,66 @@ flow workspace switch my-project # Now flow always uses my-project, regardless of directory ``` +## Unregistered Workspaces + +Registration is an optimization, not a prerequisite. In dynamic mode flow finds its workspace by +walking up from your current directory to the nearest `flow.yaml` — the same way `make` and +`bazel` find their root. Clone a repo and its executables work immediately: + +```shell +git clone https://github.com/acme/service && cd service +flow test # works — no `flow workspace add` needed +``` + +The same rule is what makes git worktrees work, whether the worktree sits inside a registered +workspace or somewhere else entirely: + +```shell +git worktree add .worktrees/feature +cd .worktrees/feature +flow build # runs against the worktree's flow files, not the main checkout's +``` + +An unregistered workspace is named after its directory, runs normally, and is never written +anywhere — not to your config, not to the shared executable cache. What you give up by not +registering it: you can't `flow workspace switch` to it, and other workspaces can't reference its +executables by name. + +Register it when you want those: + +```shell +flow workspace add service /path/to/service +``` + +### Resolution order + +1. `--workspace` or `$FLOW_WORKSPACE` — a registered name **or** a path. Honored in both modes. +2. The nearest `flow.yaml` at or above the current directory (dynamic mode only). If that + directory is registered, its registered name is used. +3. A registered workspace whose directory contains the current one. +4. The workspace set by `flow workspace switch`. + +`$FLOW_WORKSPACE` is the escape hatch in fixed mode, and the convenient one for agents and CI — +export it once rather than remembering a flag on every command: + +```shell +export FLOW_WORKSPACE=/path/to/checkout +``` + +### Nested workspaces + +A directory containing its own `flow.yaml` is a workspace boundary. The closest one wins, and a +parent workspace does not scan into it — so a worktree checked out inside a repo doesn't +duplicate every executable of its parent. Override this for a specific directory by naming it in +the parent's `executables.included`. + +Two directories flow deliberately walks *past* when they sit inside a registered workspace: +vendored dependencies (`vendor/`, `node_modules/`, `third_party/`, `external/`) and repo copies +(`.git/`, `.claude/`). A `flow.yaml` in there belongs to that copy, not to your project. + +> **Note**: there is no stopping point above your home directory. A `flow.yaml` in `~` makes your +> entire home directory a workspace, exactly as registering `~` would. + ## Multi-Workspace Workflows ### Cross-Workspace References diff --git a/internal/mcp/command_executor.go b/internal/mcp/command_executor.go index a4962874..59db1526 100644 --- a/internal/mcp/command_executor.go +++ b/internal/mcp/command_executor.go @@ -36,6 +36,27 @@ func provenanceFromContext(ctx context.Context) (runProvenance, bool) { return p, ok } +// runDirCtxKey keys the working directory the flow subprocess should run in. +type runDirCtxKey struct{} + +// withRunDir returns a context carrying the directory to launch the flow subprocess from. +// +// This matters because flow resolves its workspace by walking up from the working directory. The +// server process inherits whatever directory the MCP client was started in, which is routinely +// not where the caller is working — an agent in a git worktree, say. Without this, every run +// would silently execute against the server's directory instead of the caller's. +func withRunDir(ctx context.Context, dir string) context.Context { + if dir == "" { + return ctx + } + return context.WithValue(ctx, runDirCtxKey{}, dir) +} + +func runDirFromContext(ctx context.Context) string { + dir, _ := ctx.Value(runDirCtxKey{}).(string) + return dir +} + // stdioSessionID is what mcp-go reports as the session ID for every stdio connection — a // package constant, not a per-connection value (see mcp-go server/stdio.go). Taken at face // value it collapses every run from every client into one "session". @@ -45,6 +66,9 @@ const stdioSessionID = "stdio" // is stdio-only and each client spawns its own `flow mcp` process, so one process is exactly one // client connection — the session boundary mcp-go's constant fails to draw. It is resolved once // and never changes, which is what makes it a usable grouping key in execution history. +// +// It is the last resort: a caller that supplies its own identifier knows better than we do what +// belongs together. var processSessionID = sync.OnceValue(uuid.NewString) // mcpProvenance builds run provenance for an MCP-originated command, capturing the calling client's @@ -57,10 +81,21 @@ func mcpProvenance(ctx context.Context) runProvenance { prov.Client = withInfo.GetClientInfo().Name } } - // Only substitute when the transport gave us nothing usable, so a transport that does issue - // genuine per-connection IDs keeps them. + // A transport that issues genuine per-connection IDs keeps them; only substitute when it + // gave us nothing usable. if prov.Session == "" || prov.Session == stdioSessionID { - prov.Session = processSessionID() + // An inherited session beats one we invent. A harness that tags its environment wants + // the runs it makes over MCP grouped with the ones it makes by shelling out to the CLI + // — minting our own here would split a single conversation across two IDs. + if inherited := store.RunEnvValue(store.RunSessionEnv); inherited != "" { + prov.Session = inherited + } else { + prov.Session = processSessionID() + } + } + // Same reasoning for the client, for a transport that does not report one. + if prov.Client == "" { + prov.Client = store.RunEnvValue(store.RunClientEnv) } return prov } @@ -89,6 +124,7 @@ func (c *FlowCLIExecutor) ExecuteContext(ctx context.Context, args ...string) (s name = envName } cmd := exec.CommandContext(ctx, name, args...) // #nosec G204,G702 + cmd.Dir = runDirFromContext(ctx) if p, ok := provenanceFromContext(ctx); ok { cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", store.RunSourceEnv, p.Source), diff --git a/internal/mcp/output_types.go b/internal/mcp/output_types.go index 5cfe9a51..e98e153f 100644 --- a/internal/mcp/output_types.go +++ b/internal/mcp/output_types.go @@ -37,6 +37,14 @@ type CurrentContext struct { Vault string `json:"vault"` WorkspaceMode string `json:"workspaceMode"` WorkspacePath string `json:"workspacePath"` + // WorkspaceRegistered is false when the workspace was found by walking up from the working + // directory rather than being registered in the user config — a git worktree or a fresh + // clone. Such a workspace runs normally but cannot be switched to, and other directories + // cannot reference its executables. + WorkspaceRegistered bool `json:"workspaceRegistered"` + // WorkspaceSource records how the workspace was chosen: "override", "registered", + // "discovered", "prefix", or "current". + WorkspaceSource string `json:"workspaceSource,omitempty"` // SessionID tags every run this connection launches, and is what `flow logs --session` // filters on. A client reads it once here rather than inferring which records are its own: // the value is knowable only to the server, since the stdio transport carries no identity. diff --git a/internal/mcp/provenance_internal_test.go b/internal/mcp/provenance_internal_test.go index fc91f616..c16646de 100644 --- a/internal/mcp/provenance_internal_test.go +++ b/internal/mcp/provenance_internal_test.go @@ -20,9 +20,33 @@ func (f fakeSession) NotificationChannel() chan<- mcp.JSONRPCNotification { } func (f fakeSession) SessionID() string { return f.id } +// fakeSessionWithInfo also reports a client name, as a transport that completed the initialize +// handshake does. +type fakeSessionWithInfo struct { + fakeSession + name string +} + +func (f fakeSessionWithInfo) GetClientInfo() mcp.Implementation { + return mcp.Implementation{Name: f.name} +} +func (f fakeSessionWithInfo) SetClientInfo(mcp.Implementation) {} +func (f fakeSessionWithInfo) GetClientCapabilities() mcp.ClientCapabilities { + return mcp.ClientCapabilities{} +} +func (f fakeSessionWithInfo) SetClientCapabilities(mcp.ClientCapabilities) {} + func TestMCPProvenance(t *testing.T) { srv := server.NewMCPServer("test", "1.0.0") + // Start from an environment that inherits nothing. mcpProvenance deliberately prefers an + // inherited session over one it mints, so a developer running these tests from inside a + // harness that exports FLOW_RUN_SESSION — an agent session, or CI — would otherwise see the + // fallback cases fail on their machine and pass on everyone else's. Subtests that want an + // inherited value set it themselves. + t.Setenv(store.RunSessionEnv, "") + t.Setenv(store.RunClientEnv, "") + t.Run("replaces mcp-go's stdio constant with the process session ID", func(t *testing.T) { ctx := srv.WithContext(context.Background(), fakeSession{id: stdioSessionID}) prov := mcpProvenance(ctx) @@ -40,6 +64,41 @@ func TestMCPProvenance(t *testing.T) { } }) + t.Run("prefers an inherited session over one it mints", func(t *testing.T) { + // A harness that tags its environment wants its MCP runs grouped with the ones it + // makes by shelling out; minting our own would split one conversation across two IDs. + t.Setenv(store.RunSessionEnv, "harness-conversation-9") + ctx := srv.WithContext(context.Background(), fakeSession{id: stdioSessionID}) + if prov := mcpProvenance(ctx); prov.Session != "harness-conversation-9" { + t.Errorf("expected the inherited session, got %q", prov.Session) + } + }) + + t.Run("resolves an inherited session given as a ${NAME} reference", func(t *testing.T) { + t.Setenv("HARNESS_SESSION_ID", "resolved-conversation") + t.Setenv(store.RunSessionEnv, "${HARNESS_SESSION_ID}") + if prov := mcpProvenance(context.Background()); prov.Session != "resolved-conversation" { + t.Errorf("expected the referenced value, got %q", prov.Session) + } + }) + + t.Run("falls back to the client the environment names", func(t *testing.T) { + t.Setenv(store.RunClientEnv, "some-harness") + // No transport session, so nothing reports a client name. + if prov := mcpProvenance(context.Background()); prov.Client != "some-harness" { + t.Errorf("expected the inherited client, got %q", prov.Client) + } + }) + + t.Run("keeps the transport's client over the environment's", func(t *testing.T) { + t.Setenv(store.RunClientEnv, "stale-value") + sess := fakeSessionWithInfo{fakeSession: fakeSession{id: "real-7"}, name: "cursor"} + ctx := srv.WithContext(context.Background(), sess) + if prov := mcpProvenance(ctx); prov.Client != "cursor" { + t.Errorf("expected the transport's client, got %q", prov.Client) + } + }) + t.Run("keeps a genuine per-connection session ID", func(t *testing.T) { ctx := srv.WithContext(context.Background(), fakeSession{id: "real-session-7"}) if prov := mcpProvenance(ctx); prov.Session != "real-session-7" { diff --git a/internal/mcp/resources.go b/internal/mcp/resources.go index 76314a2c..c73a0b7f 100644 --- a/internal/mcp/resources.go +++ b/internal/mcp/resources.go @@ -13,6 +13,7 @@ import ( "github.com/mark3labs/mcp-go/server" "github.com/flowexec/flow/v2/pkg/filesystem" + "github.com/flowexec/flow/v2/types/config" ) func addServerResources(srv *server.MCPServer) { @@ -119,11 +120,15 @@ func executableResourceHandler(_ context.Context, request mcp.ReadResourceReques return nil, fmt.Errorf("failed to load config: %w", err) } + resolved, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to resolve current workspace: %w", err) + } if parts.workspace == "" { - if cfg.CurrentWorkspace == "" { + if resolved == nil { return nil, fmt.Errorf("workspace is empty in URI and no current workspace is set") } - parts.workspace = cfg.CurrentWorkspace + parts.workspace = resolved.Name } if parts.namespace == "" { parts.namespace = cfg.CurrentNamespace @@ -131,7 +136,12 @@ func executableResourceHandler(_ context.Context, request mcp.ReadResourceReques wsPath, ok := cfg.Workspaces[parts.workspace] if !ok { - return nil, fmt.Errorf("workspace %q not found", parts.workspace) + // The named workspace may be one discovered from the working directory, which is absent + // from the config but still perfectly runnable. + if resolved == nil || resolved.Name != parts.workspace { + return nil, fmt.Errorf("workspace %q not found", parts.workspace) + } + wsPath = resolved.Path } ws, err := filesystem.LoadWorkspaceConfig(parts.workspace, wsPath) @@ -198,19 +208,19 @@ func flowfileResourceHandler(_ context.Context, request mcp.ReadResourceRequest) return nil, fmt.Errorf("failed to load config: %w", err) } + resolved, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to resolve current workspace: %w", err) + } + absPath := path - if !filepath.IsAbs(path) { - // Try to resolve relative to current workspace - if cfg.CurrentWorkspace != "" { - if wsPath, ok := cfg.Workspaces[cfg.CurrentWorkspace]; ok { - absPath = filepath.Join(wsPath, path) - } - } + if !filepath.IsAbs(path) && resolved != nil { + absPath = filepath.Join(resolved.Path, path) } - // Security check: path must be within a registered workspace - if !isPathInWorkspace(absPath, cfg.Workspaces) { - return nil, fmt.Errorf("path %q is not within a registered workspace", path) + // Security check: path must be within a workspace flow can see from here + if !isPathInWorkspace(absPath, accessibleWorkspaceRoots(cfg, resolved)) { + return nil, fmt.Errorf("path %q is not within a known workspace", path) } data, err := os.ReadFile(filepath.Clean(absPath)) @@ -289,11 +299,27 @@ func extractExecutableURIParts(uri string) executableURIParts { } // isPathInWorkspace checks if an absolute path is within any registered workspace. -func isPathInWorkspace(absPath string, workspaces map[string]string) bool { - for _, wsPath := range workspaces { - if strings.HasPrefix(absPath, wsPath) { +// isPathInWorkspace reports whether absPath lies inside one of the given workspace roots. The +// comparison is path-segment aware, so "/src/wsX" is not treated as being inside "/src/ws". +func isPathInWorkspace(absPath string, roots []string) bool { + for _, root := range roots { + if filesystem.IsPathWithin(absPath, root) { return true } } return false } + +// accessibleWorkspaceRoots returns every directory a resource may be read from: the registered +// workspaces plus the workspace resolved from the working directory, which may be a discovered +// one that appears nowhere in the config. +func accessibleWorkspaceRoots(cfg *config.Config, resolved *filesystem.ResolvedWorkspace) []string { + roots := make([]string, 0, len(cfg.Workspaces)+1) + for _, wsPath := range cfg.Workspaces { + roots = append(roots, wsPath) + } + if resolved != nil { + roots = append(roots, resolved.Path) + } + return roots +} diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index a97ba270..b43612df 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -281,7 +281,7 @@ var _ = Describe("MCP Server", func() { It("should call executor with correct arguments", func() { expectedOutput := "list execs execution results" mockExecutor.EXPECT(). - Execute("browse", "--output", "json", "--workspace", "*", "--namespace", "*"). + ExecuteContext(gomock.Any(), "browse", "--output", "json", "--workspace", "*", "--namespace", "*"). Return(expectedOutput, nil) result, err := mcpClient.CallTool(ctx, newCallToolRequest("list_executables", nil)) @@ -598,7 +598,7 @@ executables: cliJSON, _ := json.Marshal(cliResp) mockExecutor.EXPECT(). - Execute("browse", "--output", "json", "--workspace", "*", "--namespace", "*"). + ExecuteContext(gomock.Any(), "browse", "--output", "json", "--workspace", "*", "--namespace", "*"). Return(string(cliJSON), nil) result, err := mcpClient.CallTool(ctx, newCallToolRequest("list_executables", nil)) diff --git a/internal/mcp/tools_executable.go b/internal/mcp/tools_executable.go index ffa3dcea..00e7cdf9 100644 --- a/internal/mcp/tools_executable.go +++ b/internal/mcp/tools_executable.go @@ -47,6 +47,9 @@ func addExecutableTools(srv *server.MCPServer, executor CommandExecutor) { mcp.WithString("keyword", mcp.Description("Keyword filter (optional)")), mcp.WithString("tag", mcp.Description("Tag filter (optional)")), mcp.WithString("cursor", mcp.Description("Pagination cursor for next page of results")), + mcp.WithString("dir", mcp.Description( + "Directory to resolve the workspace from. Set this to the directory you are working in to see the "+ + "executables of the nearest workspace at or above it, even one that is not registered.")), ) listExecutables.Annotations = mcp.ToolAnnotation{ Title: "List executables", @@ -69,6 +72,13 @@ func addExecutableTools(srv *server.MCPServer, executor CommandExecutor) { "If the executable does not have a name, you can specify just the workspace (`ws/`), namespace (`ns:`) "+ "both (`ws/ns:`) or neither if the current workspace/namespace should be used.")), mcp.WithString("args", mcp.Description("Arguments to pass")), + mcp.WithString("dir", mcp.Description( + "Directory to resolve the workspace from and run in. Set this to the directory you are working in "+ + "— a git worktree or a freshly cloned repo — and flow uses the nearest workspace at or above it, "+ + "registered or not. Defaults to the server's own directory, which is often not yours.")), + mcp.WithString("workspace", mcp.Description( + "Workspace to run in, by registered name or by path. Overrides `dir` resolution. Does not change "+ + "the global current workspace.")), mcp.WithBoolean("sync", mcp.Description("Sync executable changes before execution")), mcp.WithOutputSchema[ExecutionOutput](), ) @@ -125,7 +135,7 @@ func getExecutableHandler(executor CommandExecutor) server.ToolHandlerFunc { } func listExecutablesHandler(executor CommandExecutor) server.ToolHandlerFunc { - return func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { wsFilter := request.GetString("workspace", executable.WildcardWorkspace) nsFilter := request.GetString("namespace", executable.WildcardNamespace) verbFilter := request.GetString("verb", "") @@ -144,7 +154,7 @@ func listExecutablesHandler(executor CommandExecutor) server.ToolHandlerFunc { cmdArgs = append(cmdArgs, "--tag", tagFilter) } - output, err := executor.Execute(cmdArgs...) + output, err := executor.ExecuteContext(withRunDir(ctx, request.GetString("dir", "")), cmdArgs...) if err != nil { return toolError(ErrCodeExecutionFailed, fmt.Sprintf("Failed to list executables: %s", output)), nil } @@ -193,12 +203,18 @@ func executeFlowHandler(srv *server.MCPServer, executor CommandExecutor) server. if args != "" { cmdArgs = append(cmdArgs, strings.Fields(args)...) } + if ws := request.GetString("workspace", ""); ws != "" { + cmdArgs = append(cmdArgs, "--workspace", ws) + } if syncFlag { cmdArgs = append(cmdArgs, "--sync") } // Capture the MCP caller's identity so the resulting execution record records who ran it. ctx = withProvenance(ctx, mcpProvenance(ctx)) + // The subprocess resolves its workspace from its working directory, so the caller's + // directory has to reach it — the server's own is rarely the right one. + ctx = withRunDir(ctx, request.GetString("dir", "")) sendProgress(srv, ctx, progressToken, 0, 2, "Preparing execution") output, err := executor.ExecuteContext(ctx, cmdArgs...) @@ -301,6 +317,9 @@ func addRunExecutableTool(srv *server.MCPServer, executor CommandExecutor) { "{\"execs\":[{\"cmd\":\"...\"},{\"cmd\":\"...\"}]}}).")), mcp.WithString("label", mcp.Description("Short human-readable label recorded in history (defaults to the executable's name).")), + mcp.WithString("dir", + mcp.Description("Directory to resolve the workspace from and run in (defaults to the server's directory). "+ + "Set this to the directory you are working in — flow uses the nearest workspace at or above it.")), mcp.WithString("workspace", mcp.Description("Workspace whose environment to use for this run. Defaults to the workspace containing "+ "the working directory, then the current workspace. Does not change the global current workspace.")), @@ -348,6 +367,9 @@ func runTransientTool( // Tag the run as MCP-originated with the caller's identity. ctx = withProvenance(ctx, mcpProvenance(ctx)) + // `dir` is forwarded as --dir for the command itself; it also has to be the subprocess's + // working directory so workspace resolution starts from where the caller is working. + ctx = withRunDir(ctx, request.GetString("dir", "")) sendProgress(srv, ctx, progressToken, 0, 2, "Preparing execution") output, err := executor.ExecuteContext(ctx, cmdArgs...) @@ -434,10 +456,14 @@ func resolveFlowfilePath(path string) (string, *mcp.CallToolResult) { if err != nil { return "", toolError(ErrCodeInternal, fmt.Sprintf("failed to load config: %s", err)) } - if cfg.CurrentWorkspace != "" { - if wsPath, ok := cfg.Workspaces[cfg.CurrentWorkspace]; ok { - return filepath.Join(wsPath, path), nil - } + // Resolve against the workspace flow would actually run in, which may be one discovered from + // the working directory rather than the one persisted in the config. + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + if err != nil { + return "", toolError(ErrCodeInvalidInput, err.Error()) + } + if res != nil { + return filepath.Join(res.Path, path), nil } return path, nil } diff --git a/internal/mcp/tools_system.go b/internal/mcp/tools_system.go index 80c88e01..2858b97a 100644 --- a/internal/mcp/tools_system.go +++ b/internal/mcp/tools_system.go @@ -93,23 +93,30 @@ func getInfoHandler(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolRe } cfg.SetDefaults() - var wsName, wsPath string - if len(cfg.Workspaces) > 0 { - wsName, err = cfg.CurrentWorkspaceName() - if err != nil { - return toolError(ErrCodeInternal, fmt.Sprintf("failed to get current workspace name: %s", err)), nil - } - wsPath = cfg.Workspaces[wsName] + // Resolution walks up from this process's directory, so a workspace found there but absent + // from the config still reports correctly. Callers need to know which case they are in: + // an unregistered workspace cannot be switched to, and its executables live only here. + var wsName, wsPath, wsSource string + var wsRegistered bool + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + if err != nil { + return toolError(ErrCodeInternal, fmt.Sprintf("failed to resolve current workspace: %s", err)), nil + } + if res != nil { + wsName, wsPath = res.Name, res.Path + wsSource, wsRegistered = string(res.Source), res.Registered } output := FlowInfoOutput{ CurrentContext: CurrentContext{ - Workspace: wsName, - Namespace: cfg.CurrentNamespace, - Vault: cfg.CurrentVaultName(), - WorkspaceMode: string(cfg.WorkspaceMode), - WorkspacePath: wsPath, - SessionID: mcpProvenance(ctx).Session, + Workspace: wsName, + Namespace: cfg.CurrentNamespace, + Vault: cfg.CurrentVaultName(), + WorkspaceMode: string(cfg.WorkspaceMode), + WorkspacePath: wsPath, + WorkspaceRegistered: wsRegistered, + WorkspaceSource: wsSource, + SessionID: mcpProvenance(ctx).Session, }, Summary: flowInfoSummary, DocsURL: docsBaseURL, diff --git a/internal/runner/engine/engine.go b/internal/runner/engine/engine.go index 5bda9e97..edfbd8e3 100644 --- a/internal/runner/engine/engine.go +++ b/internal/runner/engine/engine.go @@ -2,6 +2,7 @@ package engine import ( "context" + "errors" "fmt" "golang.org/x/sync/errgroup" @@ -30,6 +31,18 @@ func (rs ResultSummary) HasErrors() bool { return false } +// Err returns the failures as one error, naming the executable behind each, or nil when +// everything succeeded. +func (rs ResultSummary) Err() error { + var errs []error + for _, r := range rs.Results { + if r.Error != nil { + errs = append(errs, fmt.Errorf("%s: %w", r.ID, r.Error)) + } + } + return errors.Join(errs...) +} + func (rs ResultSummary) String() string { var res string if rs.HasErrors() { diff --git a/internal/runner/parallel/parallel.go b/internal/runner/parallel/parallel.go index 375bfd4b..1184aedf 100644 --- a/internal/runner/parallel/parallel.go +++ b/internal/runner/parallel/parallel.go @@ -288,7 +288,7 @@ func handleExec( } } if results.HasErrors() { - return fmt.Errorf("parallel execution failed") + return errors.Wrap(results.Err(), "parallel execution failed") } return nil } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index dff57e89..2c375184 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -96,7 +96,7 @@ func ExpressionEnv( envMap, "store", dataMap, "ctx", &CtxData{ - Workspace: ctx.CurrentWorkspace.AssignedName(), + Workspace: ctx.CurrentWorkspaceName(), Namespace: ctx.Config.CurrentNamespace, WorkspacePath: executable.WorkspacePath(), FlowFileName: fn, diff --git a/internal/runner/serial/serial.go b/internal/runner/serial/serial.go index 94c303f8..e8e61f02 100644 --- a/internal/runner/serial/serial.go +++ b/internal/runner/serial/serial.go @@ -268,7 +268,7 @@ func handleExec( } } if results.HasErrors() { - return fmt.Errorf("serial execution failed") + return errors.Wrap(results.Err(), "serial execution failed") } return nil } diff --git a/internal/utils/env/env.go b/internal/utils/env/env.go index c9f4d98d..345b1ca3 100644 --- a/internal/utils/env/env.go +++ b/internal/utils/env/env.go @@ -253,7 +253,7 @@ func EnvListToEnvMap(envList []string) map[string]string { func DefaultEnv(ctx *context.Context, executable *executable.Executable) map[string]string { envMap := make(map[string]string) envMap["FLOW_RUNNER"] = "true" - envMap["FLOW_CURRENT_WORKSPACE"] = ctx.CurrentWorkspace.AssignedName() + envMap["FLOW_CURRENT_WORKSPACE"] = ctx.CurrentWorkspaceName() envMap["FLOW_CURRENT_NAMESPACE"] = ctx.Config.CurrentNamespace if ctx.ProcessTmpDir != "" { envMap["FLOW_TMP_DIRECTORY"] = ctx.ProcessTmpDir diff --git a/pkg/cache/executables_cache.go b/pkg/cache/executables_cache.go index 90837379..e6ce527c 100644 --- a/pkg/cache/executables_cache.go +++ b/pkg/cache/executables_cache.go @@ -53,96 +53,112 @@ type ExecutableCacheImpl struct { func NewExecutableCache(wsCache WorkspaceCache, s store.DataStore) ExecutableCache { return &ExecutableCacheImpl{ - Store: s, - Data: &ExecutableCacheData{ - ExecutableMap: make(map[executable.Ref]string), - AliasMap: make(map[executable.Ref]executable.Ref), - ConfigMap: make(map[string]WorkspaceInfo), - }, + Store: s, + Data: newExecutableCacheData(), WorkspaceCache: wsCache, } } -func (c *ExecutableCacheImpl) Update() error { //nolint:gocognit - logger.Log().Debugf("Updating executable cache data") - wsCacheData, err := c.WorkspaceCache.GetLatestData() +// newExecutableCacheData returns an empty, ready-to-populate index. +func newExecutableCacheData() *ExecutableCacheData { + return &ExecutableCacheData{ + ExecutableMap: make(map[executable.Ref]string), + AliasMap: make(map[executable.Ref]executable.Ref), + ConfigMap: make(map[string]WorkspaceInfo), + } +} + +// indexWorkspaceExecutables walks one workspace's flow files and records every visible, valid +// executable (and its aliases) in data. wsCfg must already carry its name and location. +// +// This is shared by the persisted cache and the in-memory overlay built for a workspace +// discovered from the working directory, so the two agree on visibility, validation, generated +// imports, and alias expansion. +func indexWorkspaceExecutables(data *ExecutableCacheData, wsCfg *workspace.Workspace) { //nolint:gocognit + name := wsCfg.AssignedName() + flowFiles, err := filesystem.LoadWorkspaceFlowFiles(wsCfg) if err != nil { - return fmt.Errorf("failed to get workspace cache data\n%w", err) + logger.Log().Error("failed to load workspace executable configs", "workspace", name, "err", err) + return } + for _, flowFile := range flowFiles { + if len(flowFile.Imports) > 0 { + generated, err := fileparser.ExecutablesFromImports(name, flowFile) + if err != nil { + logger.Log().Error( + "failed to generate executables from files", + "flowFilePath", flowFile.ConfigPath(), + "err", err, + ) + } + flowFile.Executables = append(flowFile.Executables, generated...) + } - cacheData := c.Data - for name, wsCfg := range wsCacheData.Workspaces { - wsCfg.SetContext(name, wsCacheData.WorkspaceLocations[name]) - flowFiles, err := filesystem.LoadWorkspaceFlowFiles(wsCfg) - if err != nil { - logger.Log().Error("failed to load workspace executable configs", "workspace", wsCfg.AssignedName(), "err", err) + if flowFile.Visibility == nil || + common.Visibility(*flowFile.Visibility).IsHidden() || + len(flowFile.Executables) == 0 { continue } - for _, flowFile := range flowFiles { - if len(flowFile.Imports) > 0 { - generated, err := fileparser.ExecutablesFromImports(name, flowFile) - if err != nil { - logger.Log().Error( - "failed to generate executables from files", - "flowFilePath", flowFile.ConfigPath(), - "err", err, - ) - } - flowFile.Executables = append(flowFile.Executables, generated...) + for _, e := range flowFile.Executables { + if vErr := e.Validate(); vErr != nil { + logger.Log().Warn( + "invalid executable found during cache update", + "ref", e.Ref().String(), + "workspace", name, + "err", vErr, + ) + continue } - if flowFile.Visibility == nil || - common.Visibility(*flowFile.Visibility).IsHidden() || - len(flowFile.Executables) == 0 { + if e == nil || (e.Visibility != nil && common.Visibility(*e.Visibility).IsHidden()) { continue } - for _, e := range flowFile.Executables { - if vErr := e.Validate(); vErr != nil { - logger.Log().Warn( - "invalid executable found during cache update", - "ref", e.Ref().String(), - "workspace", wsCfg.AssignedName(), - "err", vErr, - ) - continue - } - if e == nil || (e.Visibility != nil && common.Visibility(*e.Visibility).IsHidden()) { - continue - } + if existingPath, exists := data.ExecutableMap[e.Ref()]; exists && existingPath != flowFile.ConfigPath() { + logger.Log().Warn( + "duplicate executable found during cache update", + "ref", e.Ref().String(), + "conflictPath", existingPath, + "newPath", flowFile.ConfigPath(), + "workspace", name, + ) + } + + data.ExecutableMap[e.Ref()] = flowFile.ConfigPath() - if existingPath, exists := cacheData.ExecutableMap[e.Ref()]; exists && existingPath != flowFile.ConfigPath() { + for _, ref := range enumerateExecutableAliasRefs(e, wsCfg.VerbAliases) { + if existingPrimaryRef, exists := data.AliasMap[ref]; exists && existingPrimaryRef != e.Ref() { logger.Log().Warn( - "duplicate executable found during cache update", - "ref", e.Ref().String(), - "conflictPath", existingPath, - "newPath", flowFile.ConfigPath(), - "workspace", wsCfg.AssignedName(), + "duplicate executable alias found during cache update", + "aliasRef", ref.String(), + "conflictRef", existingPrimaryRef.String(), + "primaryRef", e.Ref().String(), + "workspace", name, ) } + data.AliasMap[ref] = e.Ref() + } - cacheData.ExecutableMap[e.Ref()] = flowFile.ConfigPath() - - for _, ref := range enumerateExecutableAliasRefs(e, wsCfg.VerbAliases) { - if existingPrimaryRef, exists := cacheData.AliasMap[ref]; exists && existingPrimaryRef != e.Ref() { - logger.Log().Warn( - "duplicate executable alias found during cache update", - "aliasRef", ref.String(), - "conflictRef", existingPrimaryRef.String(), - "primaryRef", e.Ref().String(), - "workspace", wsCfg.AssignedName(), - ) - } - cacheData.AliasMap[ref] = e.Ref() - } - - cacheData.ConfigMap[flowFile.ConfigPath()] = WorkspaceInfo{ - WorkspaceName: wsCfg.AssignedName(), - WorkspacePath: wsCfg.Location(), - } + data.ConfigMap[flowFile.ConfigPath()] = WorkspaceInfo{ + WorkspaceName: name, + WorkspacePath: wsCfg.Location(), } } } +} + +func (c *ExecutableCacheImpl) Update() error { + logger.Log().Debugf("Updating executable cache data") + wsCacheData, err := c.WorkspaceCache.GetLatestData() + if err != nil { + return fmt.Errorf("failed to get workspace cache data\n%w", err) + } + + cacheData := c.Data + for name, wsCfg := range wsCacheData.Workspaces { + wsCfg.SetContext(name, wsCacheData.WorkspaceLocations[name]) + indexWorkspaceExecutables(cacheData, wsCfg) + } data, err := json.Marshal(cacheData) if err != nil { @@ -157,71 +173,91 @@ func (c *ExecutableCacheImpl) Update() error { //nolint:gocognit return nil } -func (c *ExecutableCacheImpl) GetExecutableByRef(ref executable.Ref) (*executable.Executable, error) { - err := c.initExecutableCacheData() - if err != nil { - return nil, err - } else if c.Data == nil { - return nil, errors.New("no cached executables found") - } - - if c.Data.loadedExecutables == nil { - c.Data.loadedExecutables = make(map[string]*executable.Executable) - } else if exec, found := c.Data.loadedExecutables[ref.String()]; found { +// lookupExecutable resolves ref against an index, following the alias map when the ref is not a +// primary one, and loading the owning flow file to return the executable itself. +func lookupExecutable(data *ExecutableCacheData, ref executable.Ref) (*executable.Executable, error) { + if data.loadedExecutables == nil { + data.loadedExecutables = make(map[string]*executable.Executable) + } else if exec, found := data.loadedExecutables[ref.String()]; found { return exec, nil } - var primaryRef executable.Ref - cfgPath, found := c.Data.ExecutableMap[ref] - //nolint:nestif + primaryRef := ref + cfgPath, found := data.ExecutableMap[ref] if !found { - if aliasedPrimaryRef, aliasFound := c.Data.AliasMap[ref]; aliasFound { - primaryRef = aliasedPrimaryRef - cfgPath, found = c.Data.ExecutableMap[primaryRef] - if !found { - return nil, flowErrors.NewExecutableNotFoundError(ref.String()) - } - } else { + aliasedPrimaryRef, aliasFound := data.AliasMap[ref] + if !aliasFound { + return nil, flowErrors.NewExecutableNotFoundError(ref.String()) + } + primaryRef = aliasedPrimaryRef + if cfgPath, found = data.ExecutableMap[primaryRef]; !found { return nil, flowErrors.NewExecutableNotFoundError(ref.String()) } - } else { - primaryRef = ref } - cfg, err := filesystem.LoadFlowFile(cfgPath) + wsInfo, found := data.ConfigMap[cfgPath] + if !found { + return nil, errors.Errorf("unable to find workspace info for config %s", cfgPath) + } + + cfg, err := loadFlowFileWithImports(cfgPath, wsInfo) if err != nil { - return nil, errors.Wrap(err, "unable to load executable config") + return nil, err } - wsInfo, found := c.Data.ConfigMap[cfgPath] - if !found { - return nil, errors.Wrap(err, "unable to find workspace info for config") + exec, err := cfg.Executables.FindByVerbAndID(primaryRef.Verb(), primaryRef.ID()) + if err != nil { + return nil, err + } else if exec == nil { + return nil, flowErrors.NewExecutableNotFoundError(ref.String()) + } + + data.loadedExecutables[ref.String()] = exec + + return exec, nil +} + +// listExecutables returns every executable in an index, ordered by flow file path. Callers +// paginate this list across separate calls, so map order would silently drop entries. +func listExecutables(data *ExecutableCacheData) executable.ExecutableList { + list := make(executable.ExecutableList, 0) + for _, cfgPath := range slices.Sorted(maps.Keys(data.ConfigMap)) { + cfg, err := loadFlowFileWithImports(cfgPath, data.ConfigMap[cfgPath]) + if err != nil { + logger.Log().Error("unable to load executable config", "cfgPath", cfgPath, "err", err) + continue + } + list = append(list, cfg.Executables...) } + return list +} +// loadFlowFileWithImports reads a flow file, attaches its workspace context, and appends the +// executables generated from its imports. +func loadFlowFileWithImports(cfgPath string, wsInfo WorkspaceInfo) (*executable.FlowFile, error) { + cfg, err := filesystem.LoadFlowFile(cfgPath) + if err != nil { + return nil, errors.Wrap(err, "unable to load executable config") + } cfg.SetDefaults() cfg.SetContext(wsInfo.WorkspaceName, wsInfo.WorkspacePath, cfgPath) generated, err := fileparser.ExecutablesFromImports(wsInfo.WorkspaceName, cfg) if err != nil { - logger.Log().Warn( - "failed to generate executables from files", - "cfgPath", cfgPath, - "err", err, - ) + logger.Log().Warn("failed to generate executables from files", "cfgPath", cfgPath, "err", err) } cfg.Executables = append(cfg.Executables, generated...) + return cfg, nil +} - execs := cfg.Executables - exec, err := execs.FindByVerbAndID(primaryRef.Verb(), primaryRef.ID()) +func (c *ExecutableCacheImpl) GetExecutableByRef(ref executable.Ref) (*executable.Executable, error) { + err := c.initExecutableCacheData() if err != nil { return nil, err - } else if exec == nil { - return nil, flowErrors.NewExecutableNotFoundError(ref.String()) + } else if c.Data == nil { + return nil, errors.New("no cached executables found") } - - c.Data.loadedExecutables[ref.String()] = exec - - return exec, nil + return lookupExecutable(c.Data, ref) } func (c *ExecutableCacheImpl) GetExecutableList() (executable.ExecutableList, error) { @@ -231,36 +267,7 @@ func (c *ExecutableCacheImpl) GetExecutableList() (executable.ExecutableList, er } else if c.Data == nil { return nil, errors.New("no cached executables found") } - - // Sorted: callers paginate this list across separate calls, so map order would drop entries. - list := make(executable.ExecutableList, 0) - for _, cfgPath := range slices.Sorted(maps.Keys(c.Data.ConfigMap)) { - cfg, err := filesystem.LoadFlowFile(cfgPath) - if err != nil { - logger.Log().Error("unable to load executable config", "cfgPath", cfgPath, "err", err) - continue - } - wsInfo, found := c.Data.ConfigMap[cfgPath] - if !found { - logger.Log().Error("unable to find workspace info for config", "cfgPath", cfgPath) - continue - } - cfg.SetDefaults() - cfg.SetContext(wsInfo.WorkspaceName, wsInfo.WorkspacePath, cfgPath) - - generated, err := fileparser.ExecutablesFromImports(wsInfo.WorkspaceName, cfg) - if err != nil { - logger.Log().Warn( - "failed to generate executables from files", - "cfgPath", cfgPath, - "err", err, - ) - } - cfg.Executables = append(cfg.Executables, generated...) - - list = append(list, cfg.Executables...) - } - return list, nil + return listExecutables(c.Data), nil } func (c *ExecutableCacheImpl) initExecutableCacheData() error { diff --git a/pkg/cache/overlay.go b/pkg/cache/overlay.go new file mode 100644 index 00000000..d4683298 --- /dev/null +++ b/pkg/cache/overlay.go @@ -0,0 +1,195 @@ +package cache + +import ( + "maps" + "sync" + + "github.com/flowexec/flow/v2/types/executable" + "github.com/flowexec/flow/v2/types/workspace" +) + +// The overlay caches let flow run inside a workspace the user never registered — a git worktree, +// a fresh clone — by indexing that one workspace in memory and layering it over the persisted +// cache. +// +// Nothing here is ever written to the data store. The persisted cache is shared by every flow +// invocation on the machine and is keyed by workspace name; writing an ad-hoc workspace into it +// would let a throwaway clone shadow a real workspace for every future command. The overlay +// lives and dies with the process instead, which is affordable because indexing one workspace +// costs about what the per-command cache refresh already costs. +// +// Where a discovered workspace's name collides with a registered one, the overlay wins. That is +// the same "closest wins" rule that selected it in the first place. + +// NewLocalWorkspaceCache layers a discovered workspace over the registered ones so that path +// lookups and workspace listings can see it. +func NewLocalWorkspaceCache(base WorkspaceCache, ws *workspace.Workspace) WorkspaceCache { + return &localWorkspaceCache{base: base, ws: ws} +} + +type localWorkspaceCache struct { + base WorkspaceCache + ws *workspace.Workspace +} + +func (c *localWorkspaceCache) Update() error { return c.base.Update() } + +func (c *localWorkspaceCache) GetData() *WorkspaceCacheData { + return c.inject(c.base.GetData()) +} + +func (c *localWorkspaceCache) GetLatestData() (*WorkspaceCacheData, error) { + data, err := c.base.GetLatestData() + if err != nil { + return nil, err + } + return c.inject(data), nil +} + +// inject copies the base data before adding the discovered workspace. The base implementation +// marshals its own struct straight into the data store on Update, so mutating it here would +// persist the discovered workspace — exactly what this type exists to avoid. +func (c *localWorkspaceCache) inject(data *WorkspaceCacheData) *WorkspaceCacheData { + if data == nil { + data = &WorkspaceCacheData{} + } + merged := &WorkspaceCacheData{ + Workspaces: maps.Clone(data.Workspaces), + WorkspaceLocations: maps.Clone(data.WorkspaceLocations), + } + if merged.Workspaces == nil { + merged.Workspaces = make(map[string]*workspace.Workspace) + } + if merged.WorkspaceLocations == nil { + merged.WorkspaceLocations = make(map[string]string) + } + merged.Workspaces[c.ws.AssignedName()] = c.ws + merged.WorkspaceLocations[c.ws.AssignedName()] = c.ws.Location() + return merged +} + +func (c *localWorkspaceCache) GetWorkspaceConfigList() (workspace.WorkspaceList, error) { + base, err := c.base.GetWorkspaceConfigList() + if err != nil { + return nil, err + } + list := make(workspace.WorkspaceList, 0, len(base)+1) + for _, ws := range base { + if ws.AssignedName() != c.ws.AssignedName() { + list = append(list, ws) + } + } + return append(list, c.ws), nil +} + +// NewLocalExecutableCache resolves executables from a discovered workspace first, falling back to +// the persisted cache for every other workspace. +func NewLocalExecutableCache(base ExecutableCache, ws *workspace.Workspace) ExecutableCache { + return &localExecutableCache{base: base, ws: ws} +} + +type localExecutableCache struct { + base ExecutableCache + ws *workspace.Workspace + + mu sync.Mutex + data *ExecutableCacheData +} + +// index builds the in-memory index on first use. Deferring it keeps commands that never touch +// executables (`flow config`, `flow workspace list`) from walking the workspace tree. +func (c *localExecutableCache) index() *ExecutableCacheData { + c.mu.Lock() + defer c.mu.Unlock() + if c.data == nil { + c.data = newExecutableCacheData() + indexWorkspaceExecutables(c.data, c.ws) + } + return c.data +} + +func (c *localExecutableCache) Update() error { + c.mu.Lock() + c.data = nil + c.mu.Unlock() + return c.base.Update() +} + +func (c *localExecutableCache) GetExecutableByRef(ref executable.Ref) (*executable.Executable, error) { + if exec, err := lookupExecutable(c.index(), ref); err == nil { + return exec, nil + } + return c.base.GetExecutableByRef(ref) +} + +func (c *localExecutableCache) GetExecutableList() (executable.ExecutableList, error) { + base, err := c.base.GetExecutableList() + if err != nil { + return nil, err + } + name := c.ws.AssignedName() + list := make(executable.ExecutableList, 0, len(base)) + for _, e := range base { + if e.Workspace() != name { + list = append(list, e) + } + } + return append(list, listExecutables(c.index())...), nil +} + +// NewLocalTemplateCache resolves templates from a discovered workspace first, falling back to the +// persisted cache. +func NewLocalTemplateCache(base TemplateCache, ws *workspace.Workspace) TemplateCache { + return &localTemplateCache{base: base, ws: ws} +} + +type localTemplateCache struct { + base TemplateCache + ws *workspace.Workspace + + mu sync.Mutex + data *TemplateCacheData +} + +func (c *localTemplateCache) index() *TemplateCacheData { + c.mu.Lock() + defer c.mu.Unlock() + if c.data == nil { + c.data = newTemplateCacheData() + indexWorkspaceTemplates(c.data, c.ws) + } + return c.data +} + +func (c *localTemplateCache) Update() error { + c.mu.Lock() + c.data = nil + c.mu.Unlock() + return c.base.Update() +} + +func (c *localTemplateCache) GetTemplate(name string) (*executable.Template, error) { + if tmpl, err := lookupTemplate(c.index(), name); err == nil { + return tmpl, nil + } + return c.base.GetTemplate(name) +} + +func (c *localTemplateCache) GetTemplateList() (executable.TemplateList, error) { + base, err := c.base.GetTemplateList() + if err != nil { + return nil, err + } + local := listTemplates(c.index()) + localNames := make(map[string]struct{}, len(local)) + for _, tmpl := range local { + localNames[tmpl.Name()] = struct{}{} + } + list := make(executable.TemplateList, 0, len(base)+len(local)) + for _, tmpl := range base { + if _, shadowed := localNames[tmpl.Name()]; !shadowed { + list = append(list, tmpl) + } + } + return append(list, local...), nil +} diff --git a/pkg/cache/overlay_test.go b/pkg/cache/overlay_test.go new file mode 100644 index 00000000..81269bf2 --- /dev/null +++ b/pkg/cache/overlay_test.go @@ -0,0 +1,191 @@ +package cache_test + +import ( + "encoding/json" + "os" + "path/filepath" + + "github.com/flowexec/tuikit/io/mocks" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + + "github.com/flowexec/flow/v2/pkg/cache" + cacheMocks "github.com/flowexec/flow/v2/pkg/cache/mocks" + "github.com/flowexec/flow/v2/pkg/filesystem" + "github.com/flowexec/flow/v2/pkg/logger" + "github.com/flowexec/flow/v2/pkg/store" + "github.com/flowexec/flow/v2/types/common" + "github.com/flowexec/flow/v2/types/executable" + "github.com/flowexec/flow/v2/types/workspace" +) + +var _ = Describe("Local workspace overlay", func() { + var ( + ds store.DataStore + baseExecCache cache.ExecutableCache + baseWsCache cache.WorkspaceCache + registeredWs, localWs *workspace.Workspace + tmpDir string + ) + + // writeWorkspace creates a workspace rooted at tmpDir/rel holding one executable named + // execName, and returns its loaded config. + writeWorkspace := func(name, rel, execName string) *workspace.Workspace { + path := filepath.Join(tmpDir, rel) + Expect(filesystem.InitWorkspaceConfig(name, path)).To(Succeed()) + wsCfg, err := filesystem.LoadWorkspaceConfig(name, path) + Expect(err).NotTo(HaveOccurred()) + + v := executable.FlowFileVisibility(common.VisibilityPrivate) + flowFile := &executable.FlowFile{ + Namespace: "ns", + Visibility: &v, + Executables: executable.ExecutableList{{Verb: "run", Name: execName, Exec: &executable.ExecExecutableType{}}}, + } + flowFile.SetContext(name, path, filepath.Join(path, "test"+executable.FlowFileExt)) + Expect(filesystem.WriteFlowFile(flowFile.ConfigPath(), flowFile)).To(Succeed()) + return wsCfg + } + + BeforeEach(func() { + mockLogger := mocks.NewMockLogger(gomock.NewController(GinkgoT())) + logger.Init(logger.InitOptions{Logger: mockLogger, TestingTB: GinkgoTB()}) + mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + mockLogger.EXPECT().Debugf(gomock.Any()).AnyTimes() + mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + mockLogger.EXPECT().Warn(gomock.Any(), gomock.Any()).AnyTimes() + + var err error + tmpDir, err = os.MkdirTemp("", "flow-overlay-test") + Expect(err).NotTo(HaveOccurred()) + Expect(os.Setenv(filesystem.FlowCacheDirEnvVar, tmpDir)).To(Succeed()) + + ds, err = store.NewDataStore(filepath.Join(tmpDir, "test.db")) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { _ = ds.Close() }) + + registeredWs = writeWorkspace("registered", "registered", "build") + localWs = writeWorkspace("worktree", "worktree", "deploy") + + mockWsCache := cacheMocks.NewMockWorkspaceCache(gomock.NewController(GinkgoT())) + mockWsCache.EXPECT().GetLatestData().Return(&cache.WorkspaceCacheData{ + Workspaces: map[string]*workspace.Workspace{"registered": registeredWs}, + WorkspaceLocations: map[string]string{"registered": registeredWs.Location()}, + }, nil).AnyTimes() + mockWsCache.EXPECT().GetData().Return(&cache.WorkspaceCacheData{ + Workspaces: map[string]*workspace.Workspace{"registered": registeredWs}, + WorkspaceLocations: map[string]string{"registered": registeredWs.Location()}, + }).AnyTimes() + mockWsCache.EXPECT().GetWorkspaceConfigList(). + Return(workspace.WorkspaceList{registeredWs}, nil).AnyTimes() + mockWsCache.EXPECT().Update().Return(nil).AnyTimes() + baseWsCache = mockWsCache + + baseExecCache = cache.NewExecutableCache(baseWsCache, ds) + Expect(baseExecCache.Update()).To(Succeed()) + }) + + AfterEach(func() { + Expect(os.RemoveAll(tmpDir)).To(Succeed()) + Expect(os.Unsetenv(filesystem.FlowCacheDirEnvVar)).To(Succeed()) + }) + + Describe("executables", func() { + It("resolves an executable from the discovered workspace", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + exec, err := overlay.GetExecutableByRef(executable.NewRef("worktree/ns:deploy", "run")) + Expect(err).NotTo(HaveOccurred()) + Expect(exec.Name).To(Equal("deploy")) + }) + + It("resolves a verb alias from the discovered workspace", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + // "exec" is a related verb of "run", so the alias map has to be populated too. + exec, err := overlay.GetExecutableByRef(executable.NewRef("worktree/ns:deploy", "exec")) + Expect(err).NotTo(HaveOccurred()) + Expect(exec.Name).To(Equal("deploy")) + }) + + It("falls through to the persisted cache for other workspaces", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + exec, err := overlay.GetExecutableByRef(executable.NewRef("registered/ns:build", "run")) + Expect(err).NotTo(HaveOccurred()) + Expect(exec.Name).To(Equal("build")) + }) + + It("returns not-found for a ref in neither", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + _, err := overlay.GetExecutableByRef(executable.NewRef("worktree/ns:absent", "run")) + Expect(err).To(HaveOccurred()) + }) + + It("lists executables from both the overlay and the persisted cache", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + list, err := overlay.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + Expect(list.FilterByWorkspace("worktree")).To(HaveLen(1)) + Expect(list.FilterByWorkspace("registered")).To(HaveLen(1)) + }) + + It("shadows a persisted workspace of the same name", func() { + // A clone whose directory name matches an existing workspace resolves to itself, + // so the persisted entries for that name must not leak through. + collide := writeWorkspace("registered", "clone-of-registered", "deploy") + overlay := cache.NewLocalExecutableCache(baseExecCache, collide) + + list, err := overlay.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + execs := list.FilterByWorkspace("registered") + Expect(execs).To(HaveLen(1)) + Expect(execs[0].Name).To(Equal("deploy")) + Expect(execs[0].WorkspacePath()).To(Equal(collide.Location())) + }) + }) + + Describe("persistence", func() { + It("never writes the discovered workspace into the data store", func() { + overlay := cache.NewLocalExecutableCache(baseExecCache, localWs) + wsOverlay := cache.NewLocalWorkspaceCache(baseWsCache, localWs) + + // Exercise every path that could plausibly persist: reads, then an explicit Update. + _, err := overlay.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + _, err = wsOverlay.GetLatestData() + Expect(err).NotTo(HaveOccurred()) + Expect(overlay.Update()).To(Succeed()) + Expect(wsOverlay.Update()).To(Succeed()) + + raw, err := ds.GetCacheEntry("executables") + Expect(err).NotTo(HaveOccurred()) + var persisted cache.ExecutableCacheData + Expect(json.Unmarshal(raw, &persisted)).To(Succeed()) + for ref := range persisted.ExecutableMap { + Expect(ref.String()).NotTo(ContainSubstring("worktree/")) + } + for _, info := range persisted.ConfigMap { + Expect(info.WorkspaceName).NotTo(Equal("worktree")) + } + }) + }) + + Describe("workspaces", func() { + It("includes the discovered workspace in the config list", func() { + overlay := cache.NewLocalWorkspaceCache(baseWsCache, localWs) + list, err := overlay.GetWorkspaceConfigList() + Expect(err).NotTo(HaveOccurred()) + Expect(list.FindByName("worktree")).NotTo(BeNil()) + Expect(list.FindByName("registered")).NotTo(BeNil()) + }) + + It("does not mutate the base cache data", func() { + overlay := cache.NewLocalWorkspaceCache(baseWsCache, localWs) + _, err := overlay.GetLatestData() + Expect(err).NotTo(HaveOccurred()) + + base, err := baseWsCache.GetLatestData() + Expect(err).NotTo(HaveOccurred()) + Expect(base.Workspaces).NotTo(HaveKey("worktree")) + }) + }) +}) diff --git a/pkg/cache/templates_cache.go b/pkg/cache/templates_cache.go index 05e104c4..0569da03 100644 --- a/pkg/cache/templates_cache.go +++ b/pkg/cache/templates_cache.go @@ -13,6 +13,7 @@ import ( "github.com/flowexec/flow/v2/pkg/logger" "github.com/flowexec/flow/v2/pkg/store" "github.com/flowexec/flow/v2/types/executable" + "github.com/flowexec/flow/v2/types/workspace" ) const tmplCacheKey = "templates" @@ -43,15 +44,59 @@ type TemplateCacheImpl struct { func NewTemplateCache(wsCache WorkspaceCache, s store.DataStore) TemplateCache { return &TemplateCacheImpl{ - Store: s, - Data: &TemplateCacheData{ - TemplateMap: make(map[string]string), - LocationMap: make(map[string]WorkspaceInfo), - }, + Store: s, + Data: newTemplateCacheData(), WorkspaceCache: wsCache, } } +// newTemplateCacheData returns an empty, ready-to-populate index. +func newTemplateCacheData() *TemplateCacheData { + return &TemplateCacheData{ + TemplateMap: make(map[string]string), + LocationMap: make(map[string]WorkspaceInfo), + } +} + +// indexWorkspaceTemplates records one workspace's valid templates in data. Shared by the +// persisted cache and the in-memory overlay built for a discovered workspace. +func indexWorkspaceTemplates(data *TemplateCacheData, wsCfg *workspace.Workspace) { + templates, err := filesystem.LoadWorkspaceFlowFileTemplates(wsCfg) + if err != nil { + logger.Log().Error("failed to load workspace templates", "workspace", wsCfg.AssignedName(), "err", err) + return + } + for _, tmpl := range templates { + if vErr := tmpl.Validate(); vErr != nil { + logger.Log().Warn( + "invalid template found during cache update", + "template", tmpl.Name(), + "path", tmpl.Location(), + "workspace", wsCfg.AssignedName(), + "err", vErr, + ) + continue + } + + if existingPath, exists := data.TemplateMap[tmpl.Name()]; exists && existingPath != tmpl.Location() { + logger.Log().Warn( + "duplicate template name found during cache update; "+ + "use a workspace/name qualified reference to disambiguate", + "template", tmpl.Name(), + "conflictPath", existingPath, + "newPath", tmpl.Location(), + "workspace", wsCfg.AssignedName(), + ) + } + + data.TemplateMap[tmpl.Name()] = tmpl.Location() + data.LocationMap[tmpl.Location()] = WorkspaceInfo{ + WorkspaceName: wsCfg.AssignedName(), + WorkspacePath: wsCfg.Location(), + } + } +} + func (c *TemplateCacheImpl) Update() error { logger.Log().Debugf("Updating template cache data") wsCacheData, err := c.WorkspaceCache.GetLatestData() @@ -62,40 +107,7 @@ func (c *TemplateCacheImpl) Update() error { cacheData := c.Data for name, wsCfg := range wsCacheData.Workspaces { wsCfg.SetContext(name, wsCacheData.WorkspaceLocations[name]) - templates, err := filesystem.LoadWorkspaceFlowFileTemplates(wsCfg) - if err != nil { - logger.Log().Error("failed to load workspace templates", "workspace", wsCfg.AssignedName(), "err", err) - continue - } - for _, tmpl := range templates { - if vErr := tmpl.Validate(); vErr != nil { - logger.Log().Warn( - "invalid template found during cache update", - "template", tmpl.Name(), - "path", tmpl.Location(), - "workspace", wsCfg.AssignedName(), - "err", vErr, - ) - continue - } - - if existingPath, exists := cacheData.TemplateMap[tmpl.Name()]; exists && existingPath != tmpl.Location() { - logger.Log().Warn( - "duplicate template name found during cache update; "+ - "use a workspace/name qualified reference to disambiguate", - "template", tmpl.Name(), - "conflictPath", existingPath, - "newPath", tmpl.Location(), - "workspace", wsCfg.AssignedName(), - ) - } - - cacheData.TemplateMap[tmpl.Name()] = tmpl.Location() - cacheData.LocationMap[tmpl.Location()] = WorkspaceInfo{ - WorkspaceName: wsCfg.AssignedName(), - WorkspacePath: wsCfg.Location(), - } - } + indexWorkspaceTemplates(cacheData, wsCfg) } data, err := json.Marshal(cacheData) @@ -118,13 +130,18 @@ func (c *TemplateCacheImpl) GetTemplate(name string) (*executable.Template, erro return nil, errors.New("no cached templates found") } - if c.Data.loadedTemplates == nil { - c.Data.loadedTemplates = make(map[string]*executable.Template) - } else if tmpl, found := c.Data.loadedTemplates[name]; found { + return lookupTemplate(c.Data, name) +} + +// lookupTemplate resolves a template reference against an index and loads it. +func lookupTemplate(data *TemplateCacheData, name string) (*executable.Template, error) { + if data.loadedTemplates == nil { + data.loadedTemplates = make(map[string]*executable.Template) + } else if tmpl, found := data.loadedTemplates[name]; found { return tmpl, nil } - path, err := c.resolvePath(name) + path, err := resolveTemplatePath(data, name) if err != nil { return nil, err } @@ -139,17 +156,17 @@ func (c *TemplateCacheImpl) GetTemplate(name string) (*executable.Template, erro if err != nil { return nil, errors.Wrap(err, "unable to load template") } - c.Data.loadedTemplates[name] = tmpl + data.loadedTemplates[name] = tmpl return tmpl, nil } -// resolvePath maps a template reference to a discovered file path. A bare name is looked up -// directly; a "workspace/name" reference is matched against the owning workspace so callers +// resolveTemplatePath maps a template reference to a discovered file path. A bare name is looked +// up directly; a "workspace/name" reference is matched against the owning workspace so callers // can disambiguate the same template name discovered in multiple workspaces. -func (c *TemplateCacheImpl) resolvePath(name string) (string, error) { +func resolveTemplatePath(data *TemplateCacheData, name string) (string, error) { if idx := strings.LastIndex(name, "/"); idx >= 0 { wsName, tmplName := name[:idx], name[idx+1:] - for path, wsInfo := range c.Data.LocationMap { + for path, wsInfo := range data.LocationMap { if wsInfo.WorkspaceName != wsName { continue } @@ -163,7 +180,7 @@ func (c *TemplateCacheImpl) resolvePath(name string) (string, error) { return "", fmt.Errorf("template %s not found", name) } - path, found := c.Data.TemplateMap[name] + path, found := data.TemplateMap[name] if !found { return "", fmt.Errorf("template %s not found", name) } @@ -177,9 +194,14 @@ func (c *TemplateCacheImpl) GetTemplateList() (executable.TemplateList, error) { return nil, errors.New("no cached templates found") } - list := make(executable.TemplateList, 0, len(c.Data.TemplateMap)) - for _, name := range slices.Sorted(maps.Keys(c.Data.TemplateMap)) { - path := c.Data.TemplateMap[name] + return listTemplates(c.Data), nil +} + +// listTemplates returns every template in an index, ordered by name. +func listTemplates(data *TemplateCacheData) executable.TemplateList { + list := make(executable.TemplateList, 0, len(data.TemplateMap)) + for _, name := range slices.Sorted(maps.Keys(data.TemplateMap)) { + path := data.TemplateMap[name] tmpl, err := filesystem.LoadFlowFileTemplate(name, path) if err != nil { logger.Log().Error("unable to load template", "path", path, "err", err) @@ -187,7 +209,7 @@ func (c *TemplateCacheImpl) GetTemplateList() (executable.TemplateList, error) { } list = append(list, tmpl) } - return list, nil + return list } func (c *TemplateCacheImpl) initTemplateCacheData() error { diff --git a/pkg/context/context.go b/pkg/context/context.go index 8c3c25d5..451dc024 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -30,16 +30,21 @@ type Context struct { ctx context.Context cancelFunc context.CancelFunc stdOut, stdIn, stdErr *os.File - callbacks []func(*Context) error + callbacks *callbackList tuiOnce sync.Once tuiContainer *tuikit.Container Config *config.Config CurrentWorkspace *workspace.Workspace - WorkspacesCache cache.WorkspaceCache - ExecutableCache cache.ExecutableCache - TemplateCache cache.TemplateCache - DataStore store.DataStore + + // WorkspaceResolution records how CurrentWorkspace was chosen, including whether it is + // registered in the user config at all. Nil when no workspace could be resolved. + WorkspaceResolution *filesystem.ResolvedWorkspace + + WorkspacesCache cache.WorkspaceCache + ExecutableCache cache.ExecutableCache + TemplateCache cache.TemplateCache + DataStore store.DataStore // RootExecutable is the executable that is being run in the current context. // This will be nil if the context is not associated with an executable run. @@ -86,14 +91,16 @@ func NewContext(ctx context.Context, cancelFunc context.CancelFunc, opts ...Opti // This is a temporary solution until the config handling is refactored a bit _ = os.Setenv(executable.TimeoutOverrideEnv, cfg.DefaultTimeout.String()) } + // Resolution walks up from the working directory, so it succeeds in a workspace the user + // never registered — a worktree or a fresh clone. A nil result is ordinary (a fresh install + // has no workspaces at all) and must not be fatal; commands that need a workspace say so. + resolved, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + if err != nil { + panic(errors.Wrap(err, "workspace config load error")) + } var wsConfig *workspace.Workspace - if len(cfg.Workspaces) > 0 { - wsConfig, err = currentWorkspace(cfg) - if err != nil { - panic(errors.Wrap(err, "workspace config load error")) - } else if wsConfig == nil { - panic(fmt.Errorf("workspace config not found in current workspace (%s)", cfg.CurrentWorkspace)) - } + if resolved != nil { + wsConfig = resolved.Workspace } ds, err := store.NewDataStore(store.Path()) @@ -102,22 +109,31 @@ func NewContext(ctx context.Context, cancelFunc context.CancelFunc, opts ...Opti } workspaceCache := cache.NewWorkspaceCache(ds) + // The base workspace cache is what feeds the persisted executable and template caches. A + // discovered workspace is layered on afterwards so it stays out of the shared data store. executableCache := cache.NewExecutableCache(workspaceCache, ds) templateCache := cache.NewTemplateCache(workspaceCache, ds) + if resolved != nil && !resolved.Registered { + workspaceCache = cache.NewLocalWorkspaceCache(workspaceCache, wsConfig) + executableCache = cache.NewLocalExecutableCache(executableCache, wsConfig) + templateCache = cache.NewLocalTemplateCache(templateCache, wsConfig) + } c := &Context{ - appName: "flow", - ctx: ctx, - cancelFunc: cancelFunc, - stdOut: os.Stdout, - stdIn: os.Stdin, - stdErr: os.Stderr, - Config: cfg, - CurrentWorkspace: wsConfig, - WorkspacesCache: workspaceCache, - ExecutableCache: executableCache, - TemplateCache: templateCache, - DataStore: ds, + appName: "flow", + callbacks: &callbackList{}, + ctx: ctx, + cancelFunc: cancelFunc, + stdOut: os.Stdout, + stdIn: os.Stdin, + stdErr: os.Stderr, + Config: cfg, + CurrentWorkspace: wsConfig, + WorkspaceResolution: resolved, + WorkspacesCache: workspaceCache, + ExecutableCache: executableCache, + TemplateCache: templateCache, + DataStore: ds, } for _, opt := range opts { opt(c) @@ -131,22 +147,26 @@ func NewContext(ctx context.Context, cancelFunc context.CancelFunc, opts ...Opti // fields (CurrentTask, ProcessTmpDir, etc.) func (ctx *Context) ShallowCopy() *Context { cp := &Context{ - appName: ctx.appName, - ctx: ctx.ctx, - cancelFunc: ctx.cancelFunc, - stdOut: ctx.stdOut, - stdIn: ctx.stdIn, - stdErr: ctx.stdErr, - tuiContainer: ctx.tuiContainer, // share already-initialized container (if any) - Config: ctx.Config, - CurrentWorkspace: ctx.CurrentWorkspace, - WorkspacesCache: ctx.WorkspacesCache, - ExecutableCache: ctx.ExecutableCache, - TemplateCache: ctx.TemplateCache, - DataStore: ctx.DataStore, - RootExecutable: ctx.RootExecutable, - ProcessTmpDir: ctx.ProcessTmpDir, - LogArchiveID: ctx.LogArchiveID, + appName: ctx.appName, + // Shared, not copied: cleanup registered by a parallel branch running on this copy + // must reach the root's Finalize. Copying the slice dropped it on the floor. + callbacks: ctx.callbacks, + ctx: ctx.ctx, + cancelFunc: ctx.cancelFunc, + stdOut: ctx.stdOut, + stdIn: ctx.stdIn, + stdErr: ctx.stdErr, + tuiContainer: ctx.tuiContainer, // share already-initialized container (if any) + Config: ctx.Config, + CurrentWorkspace: ctx.CurrentWorkspace, + WorkspaceResolution: ctx.WorkspaceResolution, + WorkspacesCache: ctx.WorkspacesCache, + ExecutableCache: ctx.ExecutableCache, + TemplateCache: ctx.TemplateCache, + DataStore: ctx.DataStore, + RootExecutable: ctx.RootExecutable, + ProcessTmpDir: ctx.ProcessTmpDir, + LogArchiveID: ctx.LogArchiveID, } // If the parent has already initialized the TUI container, mark the copy // as initialized too so it won't re-create one. @@ -183,11 +203,11 @@ func (ctx *Context) Value(key any) any { } func (ctx *Context) String() string { - var ws string - if ctx.CurrentWorkspace != nil { - ws = ctx.CurrentWorkspace.AssignedName() + ws := ctx.CurrentWorkspaceName() + var ns string + if ctx.Config != nil { + ns = ctx.Config.CurrentNamespace } - ns := ctx.Config.CurrentNamespace if ws == "" { ws = "unk" } @@ -281,18 +301,50 @@ func (ctx *Context) SetView(view tuikit.View) error { return ctx.TUIContainer().SetView(view) } +// callbackList collects deferred cleanup registered on a context and on every shallow copy of +// it. Copies share one list by pointer, because a copy is what gets handed to a parallel +// branch: cleanup registered in there (temporary env files, for one) has to survive back to +// whoever finalizes the root. Guarded, since those branches register concurrently. +type callbackList struct { + mu sync.Mutex + funcs []func(*Context) error +} + +func (l *callbackList) add(callback func(*Context) error) { + l.mu.Lock() + defer l.mu.Unlock() + l.funcs = append(l.funcs, callback) +} + +// drain returns the registered callbacks and empties the list, so finalizing more than once +// cannot run the same cleanup twice. +func (l *callbackList) drain() []func(*Context) error { + if l == nil { + return nil + } + l.mu.Lock() + defer l.mu.Unlock() + funcs := l.funcs + l.funcs = nil + return funcs +} + func (ctx *Context) AddCallback(callback func(*Context) error) { if callback == nil { return } - ctx.callbacks = append(ctx.callbacks, callback) + if ctx.callbacks == nil { + // Only reachable for a hand-built Context; NewContext and ShallowCopy both set it. + ctx.callbacks = &callbackList{} + } + ctx.callbacks.add(callback) } func (ctx *Context) Finalize() { _ = ctx.stdIn.Close() _ = ctx.stdOut.Close() - for _, cb := range ctx.callbacks { + for _, cb := range ctx.callbacks.drain() { if err := cb(ctx); err != nil { logger.Log().WrapError(err, "callback execution error") } @@ -340,17 +392,70 @@ func ExpandRefFromParent(parent *executable.Executable, ref executable.Ref) exec return executable.NewRef(executable.NewExecutableID(ws, ns, name), ref.Verb()) } -func currentWorkspace(cfg *config.Config) (*workspace.Workspace, error) { - ws, err := cfg.CurrentWorkspaceName() - if err != nil { - return nil, err +// LogWorkspaceResolution records which workspace was chosen and why. It is separate from +// NewContext because the global logger is not initialized yet at construction time — tools that +// build a context outside the CLI (docs generation) never call Init at all. +func (ctx *Context) LogWorkspaceResolution() { + if ctx.WorkspaceResolution == nil { + logger.Log().Debug("no workspace resolved") + return + } + logger.Log().Debug( + "resolved workspace", + "workspace", ctx.WorkspaceResolution.Name, + "path", ctx.WorkspaceResolution.Path, + "source", string(ctx.WorkspaceResolution.Source), + "registered", ctx.WorkspaceResolution.Registered, + ) +} + +// SetCurrentWorkspace re-points the context at another workspace for the rest of the process, +// rebuilding the caches so an unregistered workspace's executables become resolvable (and a +// registered one stops being shadowed). It does not touch the user config — a per-invocation +// override should not change what the next command does. +func (ctx *Context) SetCurrentWorkspace(resolved *filesystem.ResolvedWorkspace) { + if resolved == nil { + return } - wsPath := cfg.Workspaces[ws] - if ws == "" || wsPath == "" { - return nil, fmt.Errorf("current workspace not found") + ctx.CurrentWorkspace = resolved.Workspace + ctx.WorkspaceResolution = resolved + + wsCache := cache.NewWorkspaceCache(ctx.DataStore) + execCache := cache.NewExecutableCache(wsCache, ctx.DataStore) + tmplCache := cache.NewTemplateCache(wsCache, ctx.DataStore) + if !resolved.Registered { + wsCache = cache.NewLocalWorkspaceCache(wsCache, resolved.Workspace) + execCache = cache.NewLocalExecutableCache(execCache, resolved.Workspace) + tmplCache = cache.NewLocalTemplateCache(tmplCache, resolved.Workspace) } + ctx.WorkspacesCache = wsCache + ctx.ExecutableCache = execCache + ctx.TemplateCache = tmplCache +} - return filesystem.LoadWorkspaceConfig(ws, wsPath) +// WorkspaceIsRegistered reports whether the current workspace exists in the user config. A +// workspace discovered by walking up from the working directory does not, which changes what +// commands that mutate the config (workspace switch, remove) can meaningfully do. +func (ctx *Context) WorkspaceIsRegistered() bool { + if ctx.WorkspaceResolution != nil { + return ctx.WorkspaceResolution.Registered + } + // A context assembled without going through NewContext carries no resolution metadata. + // Absent metadata is not evidence of an unregistered workspace, so fall back to the config. + if ctx.Config == nil || ctx.CurrentWorkspace == nil { + return false + } + _, found := ctx.Config.Workspaces[ctx.CurrentWorkspace.AssignedName()] + return found +} + +// CurrentWorkspaceName returns the resolved workspace name, or an empty string when no workspace +// resolved. +func (ctx *Context) CurrentWorkspaceName() string { + if ctx.CurrentWorkspace == nil { + return "" + } + return ctx.CurrentWorkspace.AssignedName() } func overrideThemeColor(theme themes.Theme, palette *config.ColorPalette) themes.Theme { diff --git a/pkg/context/context_test.go b/pkg/context/context_test.go index 7f9a5878..36a189f3 100644 --- a/pkg/context/context_test.go +++ b/pkg/context/context_test.go @@ -4,6 +4,7 @@ package context import ( "os" "path/filepath" + "sync" "testing" "charm.land/lipgloss/v2" @@ -11,7 +12,9 @@ import ( "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/flowexec/flow/v2/pkg/filesystem" "github.com/flowexec/flow/v2/types/config" + "github.com/flowexec/flow/v2/types/workspace" ) func TestContext(t *testing.T) { @@ -20,18 +23,18 @@ func TestContext(t *testing.T) { } var _ = ginkgo.Describe("Context", func() { - ginkgo.Describe("currentWorkspace", func() { + ginkgo.Describe("workspace resolution", func() { var ( cfg *config.Config tmpDir string ) ginkgo.BeforeEach(func() { - tmpDir = ginkgo.GinkgoT().TempDir() + tmpDir = filesystem.NormalizePath(ginkgo.GinkgoT().TempDir()) cfg = &config.Config{ Workspaces: map[string]string{ - "ws1": filepath.Clean(filepath.Join(tmpDir, "ws1")), - "ws2": filepath.Clean(filepath.Join(tmpDir, "ws2")), + "ws1": filepath.Join(tmpDir, "ws1"), + "ws2": filepath.Join(tmpDir, "ws2"), }, CurrentWorkspace: "ws1", WorkspaceMode: config.ConfigWorkspaceModeFixed, @@ -46,28 +49,67 @@ var _ = ginkgo.Describe("Context", func() { }) ginkgo.It("should return the current workspace in fixed mode", func() { - ws, err := currentWorkspace(cfg) + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) Expect(err).NotTo(HaveOccurred()) - Expect(ws.AssignedName()).To(Equal("ws1")) - Expect(ws.Location()).To(Equal(filepath.Join(tmpDir, "ws1"))) + Expect(res.Workspace.AssignedName()).To(Equal("ws1")) + Expect(res.Workspace.Location()).To(Equal(filepath.Join(tmpDir, "ws1"))) + Expect(res.Registered).To(BeTrue()) }) ginkgo.It("should return the current workspace in dynamic mode", func() { cfg.WorkspaceMode = config.ConfigWorkspaceModeDynamic Expect(os.Mkdir(filepath.Join(tmpDir, "ws2"), 0750)).To(Succeed()) - // os.Setenv("PWD", filepath.Join(tmpDir, "ws2")) Expect(os.Chdir(filepath.Join(tmpDir, "ws2"))).To(Succeed()) - ws, err := currentWorkspace(cfg) + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) Expect(err).NotTo(HaveOccurred()) - Expect(ws.AssignedName()).To(Equal("ws2")) - Expect(ws.Location()).To(Equal(filepath.Join(tmpDir, "ws2"))) + Expect(res.Workspace.AssignedName()).To(Equal("ws2")) + Expect(res.Workspace.Location()).To(Equal(filepath.Join(tmpDir, "ws2"))) }) - ginkgo.It("should return an error if the current workspace is not found", func() { + ginkgo.It("should resolve nothing if the current workspace is not found", func() { cfg.CurrentWorkspace = "ws3" - _, err := currentWorkspace(cfg) - Expect(err).To(HaveOccurred()) + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeNil()) + }) + + ginkgo.It("should discover an unregistered workspace by walking up", func() { + cfg.WorkspaceMode = config.ConfigWorkspaceModeDynamic + clone := filepath.Join(tmpDir, "cloned-repo") + Expect(os.MkdirAll(filepath.Join(clone, "sub"), 0750)).To(Succeed()) + Expect(os.WriteFile( + filepath.Join(clone, filesystem.WorkspaceConfigFileName), []byte("{}\n"), 0600, + )).To(Succeed()) + Expect(os.Chdir(filepath.Join(clone, "sub"))).To(Succeed()) + + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Workspace.AssignedName()).To(Equal("cloned-repo")) + Expect(res.Registered).To(BeFalse()) + }) + }) + + ginkgo.Describe("workspace accessors", func() { + ginkgo.It("reports an unresolved workspace without panicking", func() { + ctx := &Context{} + Expect(ctx.CurrentWorkspaceName()).To(BeEmpty()) + Expect(ctx.WorkspaceIsRegistered()).To(BeFalse()) + Expect(ctx.String()).To(Equal("unk/*")) + }) + + ginkgo.It("reports a discovered workspace as unregistered", func() { + ws := &workspace.Workspace{} + ws.SetContext("discovered", "/src/discovered") + ctx := &Context{ + CurrentWorkspace: ws, + WorkspaceResolution: &filesystem.ResolvedWorkspace{ + Workspace: ws, Name: "discovered", Path: "/src/discovered", + Registered: false, Source: filesystem.SourceDiscovered, + }, + } + Expect(ctx.CurrentWorkspaceName()).To(Equal("discovered")) + Expect(ctx.WorkspaceIsRegistered()).To(BeFalse()) }) }) @@ -102,3 +144,59 @@ var _ = ginkgo.Describe("Context", func() { func strPtr(s string) *string { return &s } + +func TestCallbacksSurviveShallowCopy(t *testing.T) { + t.Run("a copy's callbacks reach the original", func(t *testing.T) { + // A parallel branch runs on a shallow copy. Cleanup it registers — temporary env + // files, for one — used to land on the copy and never run, because only the root + // context is ever finalized. + root := &Context{callbacks: &callbackList{}} + var ran []string + + root.AddCallback(func(*Context) error { ran = append(ran, "root"); return nil }) + branch := root.ShallowCopy() + branch.AddCallback(func(*Context) error { ran = append(ran, "branch"); return nil }) + + for _, cb := range root.callbacks.drain() { + _ = cb(root) + } + + if len(ran) != 2 { + t.Fatalf("expected both callbacks to run, got %v", ran) + } + }) + + t.Run("draining twice does not run cleanup twice", func(t *testing.T) { + root := &Context{callbacks: &callbackList{}} + count := 0 + root.AddCallback(func(*Context) error { count++; return nil }) + + for range 2 { + for _, cb := range root.callbacks.drain() { + _ = cb(root) + } + } + + if count != 1 { + t.Errorf("expected the callback to run once, ran %d times", count) + } + }) + + t.Run("concurrent branches can register at once", func(t *testing.T) { + // Run with -race: parallel branches register from their own goroutines. + root := &Context{callbacks: &callbackList{}} + var wg sync.WaitGroup + for range 32 { + wg.Add(1) + go func() { + defer wg.Done() + root.ShallowCopy().AddCallback(func(*Context) error { return nil }) + }() + } + wg.Wait() + + if got := len(root.callbacks.drain()); got != 32 { + t.Errorf("expected 32 callbacks, got %d", got) + } + }) +} diff --git a/pkg/filesystem/discovery.go b/pkg/filesystem/discovery.go new file mode 100644 index 00000000..cad84b1b --- /dev/null +++ b/pkg/filesystem/discovery.go @@ -0,0 +1,103 @@ +package filesystem + +import ( + "path/filepath" + "runtime" + "strings" +) + +// MaxWalkUpDepth bounds the ancestor walk. The volume root normally terminates it; this is a +// backstop against pathological paths (deep symlink cycles resolved into a path, mount loops). +const MaxWalkUpDepth = 128 + +// NormalizePath cleans a path and, on macOS, strips the "/private" prefix. Paths under /tmp, +// /var, and /etc are symlinks into /private there, and the OS hands back either form depending +// on how it was derived — os.Getwd() returns the /private form while a path the user typed +// usually doesn't. Comparisons must normalize both sides or the same directory compares unequal +// to itself. +func NormalizePath(p string) string { + if p == "" { + return "" + } + p = filepath.Clean(p) + if runtime.GOOS == "darwin" { + if p == "/private" { + return "/" + } + if strings.HasPrefix(p, "/private/") { + p = strings.TrimPrefix(p, "/private") + } + } + return p +} + +// SamePath reports whether two paths refer to the same directory. It compares the normalized +// forms first, then the symlink-resolved forms, so a workspace registered through a symlink +// still matches the real path a walk-up produced. +func SamePath(a, b string) bool { + if a == "" || b == "" { + return false + } + if NormalizePath(a) == NormalizePath(b) { + return true + } + ra, err := filepath.EvalSymlinks(a) + if err != nil { + return false + } + rb, err := filepath.EvalSymlinks(b) + if err != nil { + return false + } + return NormalizePath(ra) == NormalizePath(rb) +} + +// IsPathWithin reports whether path is base or lives underneath it. Unlike a raw string prefix +// check, "/a/wsX" is not within "/a/ws". +func IsPathWithin(path, base string) bool { + if path == "" || base == "" { + return false + } + rel, err := filepath.Rel(NormalizePath(base), NormalizePath(path)) + if err != nil { + return false + } + return rel == "." || (!strings.HasPrefix(rel, ".."+string(filepath.Separator)) && rel != "..") +} + +// FindWorkspaceRoot walks up from startDir and returns the first directory containing a +// flow.yaml. This is how a workspace is located without consulting the user config at all — +// the same way make and bazel find their root. +func FindWorkspaceRoot(startDir string) (string, bool) { + return FindWorkspaceRootExcluding(startDir, nil) +} + +// FindWorkspaceRootExcluding is FindWorkspaceRoot but continues walking upward past any +// candidate for which skip returns true. Used to keep a flow.yaml that sits inside a tree an +// ancestor workspace already excludes (vendor/, node_modules/, ...) from becoming a root of +// its own. +func FindWorkspaceRootExcluding(startDir string, skip func(root string) bool) (string, bool) { + if startDir == "" { + return "", false + } + dir := NormalizePath(startDir) + if !filepath.IsAbs(dir) { + abs, err := filepath.Abs(dir) + if err != nil { + return "", false + } + dir = NormalizePath(abs) + } + + for range MaxWalkUpDepth { + if WorkspaceConfigExists(dir) && (skip == nil || !skip(dir)) { + return dir, true + } + parent := filepath.Dir(dir) + if parent == dir { + break + } + dir = parent + } + return "", false +} diff --git a/pkg/filesystem/discovery_test.go b/pkg/filesystem/discovery_test.go new file mode 100644 index 00000000..e3336103 --- /dev/null +++ b/pkg/filesystem/discovery_test.go @@ -0,0 +1,152 @@ +package filesystem_test + +import ( + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/flowexec/flow/v2/pkg/filesystem" +) + +var _ = Describe("Discovery", func() { + var tmpDir string + + // mkWorkspace creates dir (relative to tmpDir) and marks it a workspace root. + mkWorkspace := func(rel string) string { + path := filepath.Join(tmpDir, rel) + Expect(os.MkdirAll(path, 0750)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(path, filesystem.WorkspaceConfigFileName), []byte("{}\n"), 0600)).To(Succeed()) + return path + } + mkDir := func(rel string) string { + path := filepath.Join(tmpDir, rel) + Expect(os.MkdirAll(path, 0750)).To(Succeed()) + return path + } + + BeforeEach(func() { + var err error + tmpDir, err = os.MkdirTemp("", "flow-discovery-test") + Expect(err).NotTo(HaveOccurred()) + tmpDir, err = filepath.EvalSymlinks(tmpDir) + Expect(err).NotTo(HaveOccurred()) + tmpDir = filesystem.NormalizePath(tmpDir) + }) + + AfterEach(func() { + Expect(os.RemoveAll(tmpDir)).To(Succeed()) + }) + + Describe("FindWorkspaceRoot", func() { + It("finds a flow.yaml in the starting directory", func() { + root := mkWorkspace("ws") + found, ok := filesystem.FindWorkspaceRoot(root) + Expect(ok).To(BeTrue()) + Expect(found).To(Equal(filesystem.NormalizePath(root))) + }) + + It("walks up to an ancestor", func() { + root := mkWorkspace("ws") + deep := mkDir("ws/a/b/c") + found, ok := filesystem.FindWorkspaceRoot(deep) + Expect(ok).To(BeTrue()) + Expect(found).To(Equal(filesystem.NormalizePath(root))) + }) + + It("prefers the closest root when workspaces nest", func() { + mkWorkspace("ws") + nested := mkWorkspace("ws/sub/worktree") + deep := mkDir("ws/sub/worktree/pkg") + found, ok := filesystem.FindWorkspaceRoot(deep) + Expect(ok).To(BeTrue()) + Expect(found).To(Equal(filesystem.NormalizePath(nested))) + }) + + It("returns false when no flow.yaml exists above the directory", func() { + // tmpDir has no flow.yaml, and neither do the system directories above it. + _, ok := filesystem.FindWorkspaceRoot(mkDir("plain/nested")) + Expect(ok).To(BeFalse()) + }) + + It("returns false for an empty start directory", func() { + _, ok := filesystem.FindWorkspaceRoot("") + Expect(ok).To(BeFalse()) + }) + + It("ignores a directory named flow.yaml", func() { + path := filepath.Join(tmpDir, "notaws") + Expect(os.MkdirAll(filepath.Join(path, filesystem.WorkspaceConfigFileName), 0750)).To(Succeed()) + _, ok := filesystem.FindWorkspaceRoot(path) + Expect(ok).To(BeFalse()) + }) + + It("walks past candidates the skip function rejects", func() { + outer := mkWorkspace("ws") + inner := mkWorkspace("ws/vendor/dep") + found, ok := filesystem.FindWorkspaceRootExcluding(inner, func(root string) bool { + return filepath.Base(filepath.Dir(root)) == "vendor" + }) + Expect(ok).To(BeTrue()) + Expect(found).To(Equal(filesystem.NormalizePath(outer))) + }) + }) + + Describe("IsPathWithin", func() { + It("matches a directory against itself and its descendants", func() { + Expect(filesystem.IsPathWithin("/a/ws", "/a/ws")).To(BeTrue()) + Expect(filesystem.IsPathWithin("/a/ws/pkg/x", "/a/ws")).To(BeTrue()) + }) + + It("does not match a sibling that shares a name prefix", func() { + Expect(filesystem.IsPathWithin("/a/wsX", "/a/ws")).To(BeFalse()) + Expect(filesystem.IsPathWithin("/a", "/a/ws")).To(BeFalse()) + }) + + It("is false for empty operands", func() { + Expect(filesystem.IsPathWithin("", "/a")).To(BeFalse()) + Expect(filesystem.IsPathWithin("/a", "")).To(BeFalse()) + }) + }) + + Describe("SamePath", func() { + It("matches a symlinked path to its target", func() { + target := mkDir("real") + link := filepath.Join(tmpDir, "link") + Expect(os.Symlink(target, link)).To(Succeed()) + Expect(filesystem.SamePath(link, target)).To(BeTrue()) + }) + + It("does not match distinct directories", func() { + Expect(filesystem.SamePath(mkDir("a"), mkDir("b"))).To(BeFalse()) + }) + }) + + Describe("ReadWorkspaceConfig", func() { + It("does not create anything when the file is missing", func() { + path := filepath.Join(tmpDir, "absent") + _, err := filesystem.ReadWorkspaceConfig("absent", path) + Expect(err).To(HaveOccurred()) + _, statErr := os.Stat(path) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "must not create the workspace directory") + }) + + It("reads an existing config and attaches the given name and location", func() { + root := mkWorkspace("ws") + cfg, err := filesystem.ReadWorkspaceConfig("custom", root) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.AssignedName()).To(Equal("custom")) + Expect(cfg.Location()).To(Equal(root)) + }) + + It("accepts an empty flow.yaml as a valid workspace marker", func() { + path := filepath.Join(tmpDir, "empty") + Expect(os.MkdirAll(path, 0750)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(path, filesystem.WorkspaceConfigFileName), nil, 0600)).To(Succeed()) + cfg, err := filesystem.ReadWorkspaceConfig("empty", path) + Expect(err).NotTo(HaveOccurred()) + Expect(cfg.AssignedName()).To(Equal("empty")) + }) + }) +}) diff --git a/pkg/filesystem/executables.go b/pkg/filesystem/executables.go index efd5c786..6370136b 100644 --- a/pkg/filesystem/executables.go +++ b/pkg/filesystem/executables.go @@ -141,6 +141,7 @@ func findFiles( } excludedPaths = append(excludedPaths, defaultExcutablePaths...) + root := workspaceCfg.Location() var cfgPaths []string walkDirFunc := func(path string, entry fs.DirEntry, err error) error { if err != nil { @@ -150,6 +151,14 @@ func findFiles( } return err } + if isNestedWorkspaceRoot(path, root, entry, filter) { + logger.Log().Debug( + "skipping nested workspace root", + "path", path, + "workspace", workspaceCfg.AssignedName(), + ) + return filepath.SkipDir + } if isPathIncluded(path, workspaceCfg.Location(), includePaths) { if isPathExcluded(path, workspaceCfg.Location(), excludedPaths) { if entry.IsDir() { @@ -171,6 +180,27 @@ func findFiles( return cfgPaths, nil } +// isNestedWorkspaceRoot reports whether path is a subdirectory that holds its own flow.yaml. +// Such a directory is a workspace in its own right — a git worktree, a submodule, an embedded +// example project — and its executables belong to it, not to the tree being scanned. Without +// this, a worktree checked out inside a workspace gets indexed twice and every one of its +// executables collides with the original. +// +// A directory named explicitly in the workspace's `executables.included` is scanned anyway: that +// is a deliberate instruction to pull those executables in, and it should outrank the default. +func isNestedWorkspaceRoot(path, root string, entry fs.DirEntry, filter *workspace.ExecutableFilter) bool { + if !entry.IsDir() || filepath.Clean(path) == filepath.Clean(root) { + return false + } + if !WorkspaceConfigExists(path) { + return false + } + if filter != nil && len(filter.Included) > 0 && pathMatches(path, root, filter.Included) { + return false + } + return true +} + func pathMatches(path, basePath string, patterns []string) bool { if len(patterns) == 0 { return false diff --git a/pkg/filesystem/executables_test.go b/pkg/filesystem/executables_test.go index e681574b..11c76951 100644 --- a/pkg/filesystem/executables_test.go +++ b/pkg/filesystem/executables_test.go @@ -153,5 +153,53 @@ var _ = Describe("Executables", func() { Expect(err).NotTo(HaveOccurred()) Expect(definitions).To(BeEmpty()) }) + + Context("with a nested workspace", func() { + // nestedSetup writes one flow file at the workspace root and another inside a + // subdirectory that is itself a workspace root (a git worktree, say). + nestedSetup := func() string { + def := &executable.FlowFile{ + Namespace: "test", + Executables: executable.ExecutableList{{Verb: "exec", Name: "test-executable"}}, + } + Expect(filesystem.WriteFlowFile(filepath.Join(tmpDir, "root"+executable.FlowFileExt), def)).To(Succeed()) + + nested := filepath.Join(tmpDir, "worktree") + Expect(os.MkdirAll(nested, 0750)).To(Succeed()) + Expect(os.WriteFile( + filepath.Join(nested, filesystem.WorkspaceConfigFileName), []byte("{}\n"), 0600, + )).To(Succeed()) + Expect(filesystem.WriteFlowFile(filepath.Join(nested, "nested"+executable.FlowFileExt), def)).To(Succeed()) + + ctrl := gomock.NewController(GinkgoT()) + mockLogger := mocks.NewMockLogger(ctrl) + logger.Init(logger.InitOptions{Logger: mockLogger, TestingTB: GinkgoTB()}) + mockLogger.EXPECT().Debug(gomock.Any(), gomock.Any()).AnyTimes() + return nested + } + + It("does not load executables from a subdirectory that is its own workspace", func() { + nestedSetup() + workspaceCfg := &workspace.Workspace{} + workspaceCfg.SetContext("test", tmpDir) + + definitions, err := filesystem.LoadWorkspaceFlowFiles(workspaceCfg) + Expect(err).NotTo(HaveOccurred()) + Expect(definitions).To(HaveLen(1)) + Expect(definitions[0].ConfigPath()).To(Equal(filepath.Join(tmpDir, "root"+executable.FlowFileExt))) + }) + + It("still loads them when the nested path is explicitly included", func() { + nested := nestedSetup() + workspaceCfg := &workspace.Workspace{ + Executables: &workspace.ExecutableFilter{Included: []string{tmpDir, nested}}, + } + workspaceCfg.SetContext("test", tmpDir) + + definitions, err := filesystem.LoadWorkspaceFlowFiles(workspaceCfg) + Expect(err).NotTo(HaveOccurred()) + Expect(definitions).To(HaveLen(2)) + }) + }) }) }) diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go new file mode 100644 index 00000000..2b916ef3 --- /dev/null +++ b/pkg/filesystem/resolve.go @@ -0,0 +1,239 @@ +package filesystem + +import ( + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/flowexec/flow/v2/types/config" + "github.com/flowexec/flow/v2/types/workspace" +) + +// WorkspaceOverrideEnv pins the workspace for a single invocation, by registered name or by +// path. Exporting it beats threading a flag through every call, which matters most for agents +// and CI where the caller can set the environment once but can't remember a flag every time. +const WorkspaceOverrideEnv = "FLOW_WORKSPACE" + +// WorkspaceSource records how a workspace was resolved, so callers can explain the choice +// (`flow workspace get`, MCP get_info) instead of leaving the user to guess. +type WorkspaceSource string + +const ( + // SourceOverride: named explicitly via --workspace or FLOW_WORKSPACE. + SourceOverride WorkspaceSource = "override" + // SourceRegistered: walking up from the working directory found a registered workspace root. + SourceRegistered WorkspaceSource = "registered" + // SourceDiscovered: walking up found a flow.yaml that is not registered anywhere. + SourceDiscovered WorkspaceSource = "discovered" + // SourcePrefix: no flow.yaml above the working directory, but it sits inside a registered + // workspace's tree. + SourcePrefix WorkspaceSource = "prefix" + // SourceCurrent: fell back to the workspace persisted in the user config. + SourceCurrent WorkspaceSource = "current" +) + +// ResolvedWorkspace is the workspace a command should operate in, plus how it was chosen. +// Registered is false for a workspace found only by walking up from the working directory — +// it is valid to run in, but it exists nowhere in the user config and is never persisted. +type ResolvedWorkspace struct { + Workspace *workspace.Workspace + Name string + Path string + Registered bool + Source WorkspaceSource +} + +// ResolveOptions tunes workspace resolution. The zero value resolves from the process working +// directory with no override, which is what almost every caller wants. +type ResolveOptions struct { + // Dir is the directory to resolve from. Defaults to the process working directory. + Dir string + // Override is an explicit workspace name or path (a --workspace flag value). When empty, + // WorkspaceOverrideEnv is consulted instead. + Override string +} + +// discoveryBoundaryDirs are directory names whose contents are copies of, or dependencies of, +// some other project. A flow.yaml inside one of them belongs to that copy, not to the tree the +// user is working in, so discovery walks past it rather than adopting it as a root. These mirror +// the paths executable discovery already refuses to scan. +var discoveryBoundaryDirs = []string{ + ".git", + ".claude", + "vendor", + "third_party", + "external", + "node_modules", +} + +// ResolveWorkspace picks the workspace for a directory. +// +// Precedence: an explicit override always wins; otherwise, in dynamic mode, the nearest +// flow.yaml at or above dir wins — the same way make and bazel locate their root — falling back +// to a registered workspace containing dir and finally to the workspace persisted in the config. +// Fixed mode skips discovery entirely, because pinning a workspace and then auto-switching on +// every cd would defeat the point of pinning it. +// +// Returns (nil, nil) when nothing resolves. That is an ordinary state — a fresh install has no +// workspaces at all — so callers must handle a nil result rather than treating it as an error. +// A sentinel error would invert that: every caller would have to remember to special-case it, +// and forgetting once turns "no workspace yet" into a fatal. +// +//nolint:nilnil +func ResolveWorkspace(cfg *config.Config, opts ResolveOptions) (*ResolvedWorkspace, error) { + if cfg == nil { + return nil, nil + } + + dir := opts.Dir + if dir == "" { + // A working directory that no longer exists is not an error here; resolution just falls + // through to the config-based paths below. + if wd, err := os.Getwd(); err == nil { + dir = wd + } + } + if dir != "" { + if abs, err := filepath.Abs(dir); err == nil { + dir = abs + } + dir = NormalizePath(dir) + } + + override := opts.Override + if override == "" { + override = os.Getenv(WorkspaceOverrideEnv) + } + if override != "" { + return resolveOverride(cfg, override) + } + + if cfg.WorkspaceMode == config.ConfigWorkspaceModeDynamic && dir != "" { + if root, found := FindWorkspaceRootExcluding(dir, discoveryBoundary(cfg)); found { + if name, registered := cfg.NameForWorkspacePath(root); registered { + return loadResolved(name, cfg.Workspaces[name], SourceRegistered) + } + return readResolved(root, SourceDiscovered) + } + + // No flow.yaml anywhere above dir. A registered workspace may still claim this path — + // its own flow.yaml could have been deleted, or the workspace root may be unreachable. + if name, found := cfg.WorkspaceForPath(dir); found { + return loadResolved(name, cfg.Workspaces[name], SourcePrefix) + } + } + + if path, found := cfg.Workspaces[cfg.CurrentWorkspace]; found && path != "" { + return loadResolved(cfg.CurrentWorkspace, path, SourceCurrent) + } + + return nil, nil +} + +// resolveOverride handles an explicit --workspace / FLOW_WORKSPACE value, which may be either a +// registered name or a path to any directory holding a flow.yaml. Overrides are honored in fixed +// mode too — they are how a fixed-mode user opts into a different workspace for one command. +func resolveOverride(cfg *config.Config, override string) (*ResolvedWorkspace, error) { + if path, found := cfg.Workspaces[override]; found && path != "" { + return loadResolved(override, path, SourceOverride) + } + + if !looksLikePath(override) { + return nil, fmt.Errorf( + "unknown workspace %q - not a registered workspace name, and not a path to a directory with a %s", + override, WorkspaceConfigFileName, + ) + } + + path, err := expandOverridePath(override) + if err != nil { + return nil, err + } + if !WorkspaceConfigExists(path) { + return nil, fmt.Errorf("no %s found in %s", WorkspaceConfigFileName, path) + } + if name, found := cfg.NameForWorkspacePath(path); found { + return loadResolved(name, cfg.Workspaces[name], SourceOverride) + } + return readResolved(path, SourceOverride) +} + +func looksLikePath(v string) bool { + return v == "." || v == ".." || + filepath.IsAbs(v) || + strings.HasPrefix(v, "~") || + strings.ContainsRune(v, filepath.Separator) || + strings.ContainsRune(v, '/') +} + +func expandOverridePath(v string) (string, error) { + if strings.HasPrefix(v, "~") { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("unable to expand %q - %w", v, err) + } + v = filepath.Join(home, strings.TrimPrefix(strings.TrimPrefix(v, "~"), "/")) + } + abs, err := filepath.Abs(v) + if err != nil { + return "", fmt.Errorf("unable to resolve workspace path %q - %w", v, err) + } + return NormalizePath(abs), nil +} + +// discoveryBoundary rejects candidate roots that a registered workspace already refuses to scan +// — a vendored dependency or a repo copy checked out inside it. Without this, `cd vendor/somelib` +// would silently make that dependency the workspace. +// +// The check applies only *below* a registered workspace root. A standalone clone that happens to +// live in ~/external/ is a perfectly good workspace; only a copy sitting inside a workspace that +// deliberately excludes it is not. +func discoveryBoundary(cfg *config.Config) func(string) bool { + return func(root string) bool { + // An explicitly registered path is always a legitimate root, wherever it lives. + if _, registered := cfg.NameForWorkspacePath(root); registered { + return false + } + parent, found := cfg.WorkspaceForPath(root) + if !found { + return false + } + rel, err := filepath.Rel(NormalizePath(cfg.Workspaces[parent]), root) + if err != nil { + return false + } + for _, segment := range strings.Split(filepath.ToSlash(rel), "/") { + if slices.Contains(discoveryBoundaryDirs, segment) { + return true + } + } + return false + } +} + +// loadResolved builds a result for a registered workspace, creating its flow.yaml if the user +// registered a path that does not have one yet. +func loadResolved(name, path string, src WorkspaceSource) (*ResolvedWorkspace, error) { + ws, err := LoadWorkspaceConfig(name, path) + if err != nil { + return nil, err + } + return &ResolvedWorkspace{ + Workspace: ws, Name: name, Path: NormalizePath(path), Registered: true, Source: src, + }, nil +} + +// readResolved builds a result for a workspace found on disk but absent from the user config, +// reading its flow.yaml without writing anything into a directory the user never registered. +func readResolved(path string, src WorkspaceSource) (*ResolvedWorkspace, error) { + name := filepath.Base(path) + ws, err := ReadWorkspaceConfig(name, path) + if err != nil { + return nil, err + } + return &ResolvedWorkspace{ + Workspace: ws, Name: name, Path: path, Registered: false, Source: src, + }, nil +} diff --git a/pkg/filesystem/resolve_test.go b/pkg/filesystem/resolve_test.go new file mode 100644 index 00000000..eb8727c0 --- /dev/null +++ b/pkg/filesystem/resolve_test.go @@ -0,0 +1,229 @@ +package filesystem_test + +import ( + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/flowexec/flow/v2/pkg/filesystem" + "github.com/flowexec/flow/v2/types/config" +) + +var _ = Describe("ResolveWorkspace", func() { + var tmpDir string + + mkWorkspace := func(rel string) string { + path := filepath.Join(tmpDir, rel) + Expect(os.MkdirAll(path, 0750)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(path, filesystem.WorkspaceConfigFileName), []byte("{}\n"), 0600)).To(Succeed()) + return path + } + mkDir := func(rel string) string { + path := filepath.Join(tmpDir, rel) + Expect(os.MkdirAll(path, 0750)).To(Succeed()) + return path + } + dynamic := func(workspaces map[string]string, current string) *config.Config { + return &config.Config{ + Workspaces: workspaces, + CurrentWorkspace: current, + WorkspaceMode: config.ConfigWorkspaceModeDynamic, + } + } + + BeforeEach(func() { + var err error + tmpDir, err = os.MkdirTemp("", "flow-resolve-test") + Expect(err).NotTo(HaveOccurred()) + tmpDir, err = filepath.EvalSymlinks(tmpDir) + Expect(err).NotTo(HaveOccurred()) + // Resolution reports normalized paths; normalize the fixture root so the two forms of a + // macOS temp dir (/private/var/... and /var/...) don't make every comparison fail. + tmpDir = filesystem.NormalizePath(tmpDir) + Expect(os.Unsetenv(filesystem.WorkspaceOverrideEnv)).To(Succeed()) + }) + + AfterEach(func() { + Expect(os.RemoveAll(tmpDir)).To(Succeed()) + Expect(os.Unsetenv(filesystem.WorkspaceOverrideEnv)).To(Succeed()) + }) + + Context("discovery", func() { + It("resolves an unregistered root by walking up, named for its directory", func() { + root := mkWorkspace("cloned-repo") + res, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{ + Dir: mkDir("cloned-repo/internal/pkg"), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(res).NotTo(BeNil()) + Expect(res.Name).To(Equal("cloned-repo")) + Expect(res.Path).To(Equal(root)) + Expect(res.Registered).To(BeFalse()) + Expect(res.Source).To(Equal(filesystem.SourceDiscovered)) + }) + + It("uses the registered name when the discovered root is registered", func() { + root := mkWorkspace("checkout") + cfg := dynamic(map[string]string{"myproject": root}, "myproject") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: mkDir("checkout/sub")}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("myproject"), "registered name wins over the directory basename") + Expect(res.Registered).To(BeTrue()) + Expect(res.Source).To(Equal(filesystem.SourceRegistered)) + }) + + It("prefers a nested worktree over its registered parent workspace", func() { + parent := mkWorkspace("repo") + worktree := mkWorkspace("repo/.worktrees/feature") + cfg := dynamic(map[string]string{"repo": parent}, "repo") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{ + Dir: mkDir("repo/.worktrees/feature/cmd"), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("feature")) + Expect(res.Path).To(Equal(worktree)) + Expect(res.Registered).To(BeFalse()) + }) + + It("resolves a worktree that lives outside every registered workspace", func() { + registered := mkWorkspace("repo") + worktree := mkWorkspace("elsewhere/repo-feature") + cfg := dynamic(map[string]string{"repo": registered}, "repo") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: worktree}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("repo-feature")) + Expect(res.Registered).To(BeFalse()) + }) + + It("walks past a vendored copy inside a registered workspace", func() { + parent := mkWorkspace("repo") + mkWorkspace("repo/vendor/dep") + cfg := dynamic(map[string]string{"repo": parent}, "repo") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: mkDir("repo/vendor/dep")}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("repo")) + Expect(res.Path).To(Equal(parent)) + }) + + It("still discovers a standalone clone under a directory named external", func() { + // The boundary only applies inside a registered workspace; an unrelated path that + // happens to contain "external" is a legitimate root. + root := mkWorkspace("external/standalone") + res, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{Dir: root}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("standalone")) + }) + + It("falls back to a registered workspace containing the directory", func() { + // Registered, but its flow.yaml is gone, so there is nothing to walk up to. + root := mkDir("bare") + cfg := dynamic(map[string]string{"bare": root}, "bare") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: mkDir("bare/sub")}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("bare")) + Expect(res.Source).To(Equal(filesystem.SourcePrefix)) + }) + + It("falls back to the configured current workspace outside any workspace", func() { + root := mkWorkspace("home-ws") + cfg := dynamic(map[string]string{"home-ws": root}, "home-ws") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: mkDir("unrelated")}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("home-ws")) + Expect(res.Source).To(Equal(filesystem.SourceCurrent)) + }) + + It("returns nil when nothing resolves", func() { + res, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{ + Dir: mkDir("nowhere"), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeNil()) + }) + }) + + Context("fixed mode", func() { + It("ignores discovery and uses the configured workspace", func() { + pinned := mkWorkspace("pinned") + mkWorkspace("other") + cfg := &config.Config{ + Workspaces: map[string]string{"pinned": pinned}, + CurrentWorkspace: "pinned", + WorkspaceMode: config.ConfigWorkspaceModeFixed, + } + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Dir: filepath.Join(tmpDir, "other")}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("pinned")) + Expect(res.Source).To(Equal(filesystem.SourceCurrent)) + }) + + It("still honors an explicit override", func() { + pinned := mkWorkspace("pinned") + other := mkWorkspace("other") + cfg := &config.Config{ + Workspaces: map[string]string{"pinned": pinned}, + CurrentWorkspace: "pinned", + WorkspaceMode: config.ConfigWorkspaceModeFixed, + } + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Override: other}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("other")) + Expect(res.Source).To(Equal(filesystem.SourceOverride)) + }) + }) + + Context("override", func() { + It("accepts a registered workspace name", func() { + root := mkWorkspace("ws") + cfg := dynamic(map[string]string{"named": root}, "named") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{ + Override: "named", Dir: mkDir("unrelated"), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("named")) + Expect(res.Registered).To(BeTrue()) + }) + + It("accepts an absolute path to an unregistered workspace", func() { + root := mkWorkspace("adhoc") + res, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{Override: root}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("adhoc")) + Expect(res.Registered).To(BeFalse()) + Expect(res.Source).To(Equal(filesystem.SourceOverride)) + }) + + It("maps a path that is registered back to its registered name", func() { + root := mkWorkspace("dir-name") + cfg := dynamic(map[string]string{"registered-name": root}, "registered-name") + res, err := filesystem.ResolveWorkspace(cfg, filesystem.ResolveOptions{Override: root}) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("registered-name")) + Expect(res.Registered).To(BeTrue()) + }) + + It("reads the override from the environment when no value is passed", func() { + root := mkWorkspace("env-ws") + Expect(os.Setenv(filesystem.WorkspaceOverrideEnv, root)).To(Succeed()) + res, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{ + Dir: mkDir("unrelated"), + }) + Expect(err).NotTo(HaveOccurred()) + Expect(res.Name).To(Equal("env-ws")) + }) + + It("errors on an unknown name", func() { + _, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{Override: "nope"}) + Expect(err).To(MatchError(ContainSubstring("unknown workspace"))) + }) + + It("errors on a path with no flow.yaml", func() { + _, err := filesystem.ResolveWorkspace(dynamic(nil, ""), filesystem.ResolveOptions{ + Override: mkDir("plain"), + }) + Expect(err).To(MatchError(ContainSubstring("no flow.yaml found"))) + }) + }) +}) diff --git a/pkg/filesystem/workspace.go b/pkg/filesystem/workspace.go index ec7c46bd..6afdc2b8 100644 --- a/pkg/filesystem/workspace.go +++ b/pkg/filesystem/workspace.go @@ -1,6 +1,7 @@ package filesystem import ( + "io" "os" "path/filepath" @@ -25,9 +26,15 @@ func InitWorkspaceConfig(name, path string) error { return nil } +// WorkspaceConfigExists reports whether workspacePath holds a flow.yaml file. Workspace +// discovery walks up through arbitrary directories testing this, so a directory that merely +// happens to be named flow.yaml must not count as a workspace root. func WorkspaceConfigExists(workspacePath string) bool { - _, err := os.Stat(filepath.Join(workspacePath, WorkspaceConfigFileName)) - return !os.IsNotExist(err) + info, err := os.Stat(filepath.Join(workspacePath, WorkspaceConfigFileName)) + if err != nil { + return false + } + return !info.IsDir() } func EnsureWorkspaceDir(workspacePath string) error { @@ -71,13 +78,22 @@ func WriteWorkspaceConfig(workspacePath string, config *workspace.Workspace) err return nil } +// LoadWorkspaceConfig reads a registered workspace's config, creating the directory and a +// default flow.yaml if either is missing. Only use this for paths the user has explicitly +// registered — see ReadWorkspaceConfig for the read-only variant discovery must use. func LoadWorkspaceConfig(workspaceName, workspacePath string) (*workspace.Workspace, error) { if err := EnsureWorkspaceDir(workspacePath); err != nil { return nil, errors.Wrap(err, "unable to ensure workspace directory") } else if err := EnsureWorkspaceConfig(workspaceName, workspacePath); err != nil { return nil, errors.Wrap(err, "unable to ensure workspace config file") } + return ReadWorkspaceConfig(workspaceName, workspacePath) +} +// ReadWorkspaceConfig reads a workspace's flow.yaml without creating anything. Workspace +// discovery resolves roots the user never registered, so it must never write into them the way +// LoadWorkspaceConfig does. +func ReadWorkspaceConfig(workspaceName, workspacePath string) (*workspace.Workspace, error) { wsCfg := &workspace.Workspace{} wsFile := filepath.Join(workspacePath, WorkspaceConfigFileName) file, err := os.Open(filepath.Clean(wsFile)) @@ -87,7 +103,8 @@ func LoadWorkspaceConfig(workspaceName, workspacePath string) (*workspace.Worksp defer file.Close() err = yaml.NewDecoder(file).Decode(wsCfg) - if err != nil { + if err != nil && !errors.Is(err, io.EOF) { + // An empty flow.yaml is a valid workspace marker with all-default settings. return nil, errors.Wrap(err, "unable to decode workspace config file") } diff --git a/pkg/logger/discard_internal_test.go b/pkg/logger/discard_internal_test.go new file mode 100644 index 00000000..d4854b0f --- /dev/null +++ b/pkg/logger/discard_internal_test.go @@ -0,0 +1,47 @@ +package logger + +import ( + "os" + "testing" +) + +// TestDiscardOutputIsNotStandardInput pins the exact defect: the fallback sink used to be +// os.NewFile(0, os.DevNull), which adopts descriptor 0 rather than opening anything. +// +// In-package because the descriptor is the whole assertion, and it is not reachable from +// outside. An external test can only observe the consequence — a descriptor closed by a +// finalizer at an arbitrary later moment — which is exactly what made this so hard to +// trace the first time. +func TestDiscardOutputIsNotStandardInput(t *testing.T) { + sink := discardOutput() + + if sink == nil { + t.Fatal("discardOutput returned nil") + } + if sink.Fd() == 0 { + t.Error("the fallback log sink is standard input; it must open the null device instead") + } +} + +// TestDiscardOutputIsReused guards the second half of the fix. One file per process means +// one finalizer; a file per call leaves a finalizer for every call, each waiting to close +// whichever descriptor number it ends up holding. +func TestDiscardOutputIsReused(t *testing.T) { + first := discardOutput() + for range 50 { + if discardOutput() != first { + t.Fatal("discardOutput allocated a new file; every call must share one") + } + } +} + +// TestDiscardOutputIsWritable confirms the sink is usable, so switching away from +// standard input did not simply move the silence somewhere else. +func TestDiscardOutputIsWritable(t *testing.T) { + if discardOutput() == os.Stdout { + t.Skip("null device unavailable; the fallback is stdout by design") + } + if _, err := discardOutput().WriteString("discarded\n"); err != nil { + t.Errorf("the fallback sink is not writable: %v", err) + } +} diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 37cb178a..3d1850cf 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -17,8 +17,28 @@ var ( once sync.Once loggerMutex sync.RWMutex testLoggerEnvKey = "FLOW_TEST_LOGGER" + + discardOnce sync.Once + discardFile *os.File ) +// discardOutput returns a writer that throws log output away, for tests that have not +// registered a logger of their own. +// +// Falls back to stdout if the device cannot be opened: a noisy test logger is a smaller +// problem than one writing to an arbitrary descriptor. +func discardOutput() *os.File { + discardOnce.Do(func() { + f, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + discardFile = os.Stdout + return + } + discardFile = f + }) + return discardFile +} + type InitOptions struct { StdOut *os.File ArchiveDirectory string @@ -79,7 +99,7 @@ func Log() io.Logger { return logger.(io.Logger) //nolint:errcheck } } - return io.NewLogger(io.WithOutput(os.NewFile(0, os.DevNull))) + return io.NewLogger(io.WithOutput(discardOutput())) } if globalLogger == nil { diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index a2f1a3ed..b901ba48 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -2,6 +2,7 @@ package logger_test import ( "os" + "runtime" "testing" "github.com/flowexec/tuikit/io" @@ -34,3 +35,38 @@ var _ = Describe("Global Logger", func() { Expect(logger1).ToNot(Equal(logger2)) }) }) + +// The test-mode fallback logger once wrapped descriptor 0 with os.NewFile, which adopts +// standard input rather than opening the null device, and attaches a finalizer that closes +// whatever descriptor it holds. Every Log() call left another finalizer waiting to close +// fd 0, so the runtime would close standard input part-way through a test run and the next +// file the process opened inherited the freed number — under -coverprofile, that was the +// coverage profile, and the write failed with EBADF while every test passed. +// +// The descriptor the sink actually holds is asserted in discard_internal_test.go, which +// can reach it; this covers the consequence, which is what anyone hitting the bug sees. +var _ = Describe("Test-mode fallback logger", func() { + + // A fresh file per call is what accumulates finalizers. Reusing one keeps the count at + // one for the life of the process, however many times Log() is called. + It("survives repeated use with the garbage collector running", func() { + for range 200 { + logger.Log().Debugf("keeping the fallback busy") + } + runtime.GC() + runtime.GC() + + // Standard input must still be open. If a finalizer had closed it, this fails — + // and so would anything the process opened afterwards. + _, err := os.Stdin.Stat() + Expect(err).ToNot(HaveOccurred(), "standard input was closed by a finalizer") + + // A file opened now must be usable, which is precisely what the coverage writer + // needs at process exit. + f, err := os.CreateTemp(GinkgoT().TempDir(), "after-gc-*") + Expect(err).ToNot(HaveOccurred()) + defer f.Close() + _, err = f.WriteString("still writable") + Expect(err).ToNot(HaveOccurred(), "a file opened after GC was closed underneath us") + }) +}) diff --git a/pkg/store/provenance_test.go b/pkg/store/provenance_test.go new file mode 100644 index 00000000..c1c3de7e --- /dev/null +++ b/pkg/store/provenance_test.go @@ -0,0 +1,50 @@ +package store_test + +import ( + "testing" + + "github.com/flowexec/flow/v2/pkg/store" +) + +func TestRunEnvValue(t *testing.T) { + t.Run("returns a plain value unchanged", func(t *testing.T) { + t.Setenv(store.RunClientEnv, "claude-code") + if got := store.RunEnvValue(store.RunClientEnv); got != "claude-code" { + t.Errorf("expected %q, got %q", "claude-code", got) + } + }) + + t.Run("resolves a ${NAME} reference from the environment", func(t *testing.T) { + // A harness that cannot interpolate its settings file stores the reference verbatim. + t.Setenv("HARNESS_SESSION_ID", "3f9a-conversation") + t.Setenv(store.RunSessionEnv, "${HARNESS_SESSION_ID}") + if got := store.RunEnvValue(store.RunSessionEnv); got != "3f9a-conversation" { + t.Errorf("expected the referenced value, got %q", got) + } + }) + + t.Run("yields empty when the reference points at nothing", func(t *testing.T) { + // Never the literal: a constant `${...}` on every run would group unrelated runs + // together, which is worse than recording no session at all. + t.Setenv(store.RunSessionEnv, "${NOT_SET_ANYWHERE}") + if got := store.RunEnvValue(store.RunSessionEnv); got != "" { + t.Errorf("expected empty, got %q", got) + } + }) + + t.Run("leaves values that only look like references alone", func(t *testing.T) { + for _, value := range []string{"${}", "$NAME", "{NAME}", "prefix-${NAME}", "${NAME}-suffix"} { + t.Setenv(store.RunSessionEnv, value) + if got := store.RunEnvValue(store.RunSessionEnv); got != value { + t.Errorf("expected %q unchanged, got %q", value, got) + } + } + }) + + t.Run("is empty when the variable is unset", func(t *testing.T) { + t.Setenv(store.RunSessionEnv, "") + if got := store.RunEnvValue(store.RunSessionEnv); got != "" { + t.Errorf("expected empty, got %q", got) + } + }) +} diff --git a/pkg/store/store.go b/pkg/store/store.go index e9d8f287..3519c280 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -87,6 +87,35 @@ type DataStore interface { //nolint:interfacebloat // single backing store with Close() error } +// RunEnvValue reads a provenance environment variable, resolving a `${NAME}` value by looking +// NAME up in the environment. +// +// Agent harnesses commonly let you set a static environment value but not interpolate one — +// their settings file stores what you write verbatim. Without this, mapping a harness's own +// session variable onto RunSessionEnv would record the literal `${...}` on every run: a +// constant masquerading as an identity, silently grouping unrelated runs together. That is a +// worse failure than having no session at all, because the grouping looks like it worked. +// +// Resolving here rather than recognizing specific variables keeps flow neutral: the name of +// the harness's variable lives in that harness's config, where it can be corrected when the +// vendor renames it. An unresolvable reference yields empty, since no identity beats a false one. +func RunEnvValue(key string) string { + value := os.Getenv(key) + if name, ok := envReference(value); ok { + return os.Getenv(name) + } + return value +} + +// envReference reports whether a value is a `${NAME}` reference and the name it points at. +func envReference(value string) (string, bool) { + if !strings.HasPrefix(value, "${") || !strings.HasSuffix(value, "}") { + return "", false + } + name := strings.TrimSuffix(strings.TrimPrefix(value, "${"), "}") + return name, name != "" +} + // RunStatus represents the lifecycle state of an execution record. type RunStatus string diff --git a/tests/container_exec_e2e_test.go b/tests/container_exec_e2e_test.go index b7b5d775..da5cce87 100644 --- a/tests/container_exec_e2e_test.go +++ b/tests/container_exec_e2e_test.go @@ -6,6 +6,7 @@ import ( stdCtx "context" "os/exec" "runtime" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -14,21 +15,34 @@ import ( ) // canRunLinuxContainers reports whether a runtime capable of running the Linux -// test image is available. Windows hosts are excluded: docker.exe may be on PATH -// there (e.g. Windows CI), but it cannot run a Linux image, so the test would -// fail rather than exercise the feature. +// test image is available and actually reachable. Windows hosts are excluded: +// docker.exe may be on PATH there (e.g. Windows CI), but it cannot run a Linux +// image, so the test would fail rather than exercise the feature. +// +// Mirrors the runner's own "auto" precedence (docker before podman, see +// internal/services/run/container.go ResolveRuntime): a binary on PATH isn't +// enough (Docker Desktop can be installed but not running), so the runtime +// that "auto" would actually pick also has to respond before we call it +// available — otherwise the test fails instead of skipping. func canRunLinuxContainers() bool { if runtime.GOOS == "windows" { return false } for _, rt := range []string{"docker", "podman"} { - if _, err := exec.LookPath(rt); err == nil { - return true + if _, err := exec.LookPath(rt); err != nil { + continue } + return runtimeIsLive(rt) } return false } +func runtimeIsLive(rt string) bool { + ctx, cancel := stdCtx.WithTimeout(stdCtx.Background(), 3*time.Second) + defer cancel() + return exec.CommandContext(ctx, rt, "info").Run() == nil +} + var _ = Describe("container exec e2e", func() { var ctx *utils.Context diff --git a/tests/discovery_e2e_test.go b/tests/discovery_e2e_test.go new file mode 100644 index 00000000..66a5ddcd --- /dev/null +++ b/tests/discovery_e2e_test.go @@ -0,0 +1,142 @@ +//go:build e2e + +package tests_test + +import ( + stdCtx "context" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/flowexec/flow/v2/pkg/filesystem" + "github.com/flowexec/flow/v2/tests/utils" + "github.com/flowexec/flow/v2/types/config" +) + +// These specs build their context through context.NewContext from a real working directory, +// which is the only way to exercise workspace discovery. That changes the process working +// directory, hence Serial. +var _ = Describe("workspace discovery e2e", Serial, func() { + var root string + + dynamicCtx := func(dir string, registered map[string]string) *utils.DiscoveryContext { + return utils.NewDiscoveryContext( + stdCtx.Background(), GinkgoTB(), dir, registered, config.ConfigWorkspaceModeDynamic, + ) + } + + BeforeEach(func() { + root = filesystem.NormalizePath(GinkgoTB().TempDir()) + }) + + It("runs in a freshly cloned repo with no workspaces registered at all", func() { + clone := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "cloned-repo"), "greet") + nested := filepath.Join(clone, "internal", "pkg") + Expect(os.MkdirAll(nested, 0750)).To(Succeed()) + + ctx := dynamicCtx(nested, nil) + + Expect(ctx.CurrentWorkspaceName()).To(Equal("cloned-repo")) + Expect(ctx.WorkspaceIsRegistered()).To(BeFalse()) + Expect(ctx.CurrentWorkspace.Location()).To(Equal(clone)) + + list, err := ctx.ExecutableCache.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + Expect(list.FilterByWorkspace("cloned-repo")).To(HaveLen(1)) + }) + + It("prefers a worktree nested inside a registered workspace", func() { + parent := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "repo"), "build") + worktree := utils.WriteWorkspace(GinkgoTB(), filepath.Join(parent, ".worktrees", "feature"), "deploy") + + ctx := dynamicCtx(worktree, map[string]string{"repo": parent}) + + Expect(ctx.CurrentWorkspaceName()).To(Equal("feature")) + Expect(ctx.WorkspaceIsRegistered()).To(BeFalse()) + + list, err := ctx.ExecutableCache.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + Expect(list.FilterByWorkspace("feature")).To(HaveLen(1)) + }) + + It("does not index a nested worktree's executables into its parent workspace", func() { + parent := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "repo"), "build") + utils.WriteWorkspace(GinkgoTB(), filepath.Join(parent, ".worktrees", "feature"), "deploy") + + ctx := dynamicCtx(parent, map[string]string{"repo": parent}) + Expect(ctx.CurrentWorkspaceName()).To(Equal("repo")) + Expect(ctx.ExecutableCache.Update()).To(Succeed()) + + list, err := ctx.ExecutableCache.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + execs := list.FilterByWorkspace("repo") + Expect(execs).To(HaveLen(1)) + Expect(execs[0].Name).To(Equal("build")) + }) + + It("resolves a worktree that lives outside every registered workspace", func() { + registered := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "repo"), "build") + worktree := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "worktrees", "repo-feature"), "deploy") + + ctx := dynamicCtx(worktree, map[string]string{"repo": registered}) + Expect(ctx.CurrentWorkspaceName()).To(Equal("repo-feature")) + Expect(ctx.WorkspaceIsRegistered()).To(BeFalse()) + }) + + It("keeps the registered name when the directory basename differs", func() { + dir := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "dir-name"), "build") + + ctx := dynamicCtx(dir, map[string]string{"registered-name": dir}) + Expect(ctx.CurrentWorkspaceName()).To(Equal("registered-name")) + Expect(ctx.WorkspaceIsRegistered()).To(BeTrue()) + }) + + It("never writes a discovered workspace to the user config or the shared cache", func() { + clone := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "throwaway"), "greet") + registered := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "kept"), "build") + + ctx := dynamicCtx(clone, map[string]string{"kept": registered}) + Expect(ctx.CurrentWorkspaceName()).To(Equal("throwaway")) + + // Exercise the paths that populate and persist the caches. + _, err := ctx.ExecutableCache.GetExecutableList() + Expect(err).NotTo(HaveOccurred()) + Expect(ctx.ExecutableCache.Update()).To(Succeed()) + Expect(ctx.WorkspacesCache.Update()).To(Succeed()) + + persisted, err := filesystem.LoadConfig() + Expect(err).NotTo(HaveOccurred()) + Expect(persisted.Workspaces).To(HaveKey("kept")) + Expect(persisted.Workspaces).NotTo(HaveKey("throwaway")) + + raw, err := ctx.DataStore.GetCacheEntry("workspaces") + Expect(err).NotTo(HaveOccurred()) + Expect(string(raw)).NotTo(ContainSubstring("throwaway")) + }) + + Context("overrides", func() { + It("honors FLOW_WORKSPACE pointing at an unregistered path", func() { + other := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "other"), "deploy") + here := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "here"), "build") + GinkgoTB().Setenv(filesystem.WorkspaceOverrideEnv, other) + + ctx := dynamicCtx(here, nil) + Expect(ctx.CurrentWorkspaceName()).To(Equal("other")) + Expect(ctx.WorkspaceResolution.Source).To(Equal(filesystem.SourceOverride)) + }) + + It("is the only thing that moves the workspace in fixed mode", func() { + pinned := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "pinned"), "build") + elsewhere := utils.WriteWorkspace(GinkgoTB(), filepath.Join(root, "elsewhere"), "deploy") + + fixed := utils.NewDiscoveryContext( + stdCtx.Background(), GinkgoTB(), elsewhere, + map[string]string{"pinned": pinned}, config.ConfigWorkspaceModeFixed, + ) + Expect(fixed.CurrentWorkspaceName()).To(Equal("pinned"), + "fixed mode must ignore the working directory") + }) + }) +}) diff --git a/tests/utils/context.go b/tests/utils/context.go index 02a567fa..6225f5b6 100644 --- a/tests/utils/context.go +++ b/tests/utils/context.go @@ -281,6 +281,95 @@ func newTestContext( return ctxx, configDir, cacheDir, wsDir } +// DiscoveryContext is a test context built the way the real CLI builds one — through +// context.NewContext, from a working directory — rather than assembled field by field. That is +// the only way to exercise workspace discovery, which resolves by walking up from the working +// directory and is bypassed entirely by the hand-built contexts above. +// +// Because it changes the process working directory, suites using it must run Serial. +type DiscoveryContext struct { + *context.Context + Dir string +} + +// NewDiscoveryContext points flow's config and cache at fresh temp directories, registers the +// given workspaces (name -> path, may be empty), chdirs to dir, and builds a real context from +// there. +func NewDiscoveryContext( + ctx stdCtx.Context, tb testing.TB, dir string, registered map[string]string, mode config.ConfigWorkspaceMode, +) *DiscoveryContext { + tb.Helper() + root := tb.TempDir() + configDir := filepath.Join(root, userConfigSubdir) + cacheDir := filepath.Join(root, cacheSubdir) + setTestEnv(tb, configDir, cacheDir) + + if err := filesystem.InitConfig(); err != nil { + tb.Fatalf("unable to init config: %v", err) + } + userCfg, err := filesystem.LoadConfig() + if err != nil { + tb.Fatalf("unable to load config: %v", err) + } + userCfg.DefaultLogMode = tuikitIO.Text + userCfg.Interactive = &config.Interactive{Enabled: false} + userCfg.WorkspaceMode = mode + userCfg.Workspaces = map[string]string{} + userCfg.CurrentWorkspace = "" + for name, path := range registered { + userCfg.Workspaces[name] = path + if userCfg.CurrentWorkspace == "" { + userCfg.CurrentWorkspace = name + } + } + if err := filesystem.WriteConfig(userCfg); err != nil { + tb.Fatalf("unable to write config: %v", err) + } + + stdOut, stdIn := createTempIOFiles(tb) + logger.Init(logger.InitOptions{ + Logger: tuikitIO.NewLogger( + tuikitIO.WithOutput(stdOut), + tuikitIO.WithTheme(logger.Theme("")), + tuikitIO.WithMode(tuikitIO.Text), + ), + TestingTB: tb, + }) + + tb.Chdir(dir) + cancel := func() { <-ctx.Done() } + ctxx := context.NewContext(ctx, cancel, context.WithStdIn(stdIn), context.WithStdOut(stdOut)) + tb.Cleanup(func() { + if ctxx.DataStore != nil { + _ = ctxx.DataStore.Close() + } + }) + return &DiscoveryContext{Context: ctxx, Dir: dir} +} + +// WriteWorkspace marks dir as a workspace root and gives it one `run`-verb executable that +// echoes its own name, so a test can prove which workspace an executable resolved from. +func WriteWorkspace(tb testing.TB, dir, execName string) string { + tb.Helper() + if err := os.MkdirAll(dir, 0750); err != nil { + tb.Fatalf("unable to create workspace dir: %v", err) + } + wsFile := filepath.Join(dir, filesystem.WorkspaceConfigFileName) + if err := os.WriteFile(wsFile, []byte("displayName: "+filepath.Base(dir)+"\n"), 0600); err != nil { + tb.Fatalf("unable to write workspace config: %v", err) + } + if execName != "" { + flowFile := fmt.Sprintf( + "namespace: ns\nexecutables:\n - verb: run\n name: %s\n exec:\n cmd: echo %s\n", + execName, execName, + ) + if err := os.WriteFile(filepath.Join(dir, "test.flow"), []byte(flowFile), 0600); err != nil { + tb.Fatalf("unable to write flow file: %v", err) + } + } + return filesystem.NormalizePath(dir) +} + func initTestDirectories(tb testing.TB) (string, string, string) { replacer := strings.NewReplacer("-", "", "'", "-", "/", "-", " ", "_") suiteName := getSuiteName() diff --git a/types/config/config.go b/types/config/config.go index 20e19e7c..f49e58cf 100644 --- a/types/config/config.go +++ b/types/config/config.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "fmt" + "maps" "os" "path/filepath" "runtime" @@ -10,7 +11,6 @@ import ( "strings" tuikitIO "github.com/flowexec/tuikit/io" - "golang.org/x/exp/maps" "gopkg.in/yaml.v3" ) @@ -39,7 +39,9 @@ func (c *Config) SetDefaults() { c.Workspaces = make(map[string]string) } if c.CurrentWorkspace == "" && len(c.Workspaces) > 0 { - c.CurrentWorkspace = maps.Keys(c.Workspaces)[0] + // Sorted, not arbitrary map order: an unset current workspace would otherwise land on a + // different workspace each run. + c.CurrentWorkspace = slices.Sorted(maps.Keys(c.Workspaces))[0] } if c.WorkspaceMode == "" { c.WorkspaceMode = ConfigWorkspaceModeDynamic @@ -60,40 +62,79 @@ func (c *Config) CurrentVaultName() string { return *c.CurrentVault } +// normalizePath cleans a path and, on macOS, strips the "/private" prefix. Paths under /tmp and +// friends are symlinks into /private there and the OS hands back either form, so both sides of +// a comparison have to be normalized or the same directory compares unequal to itself. +// +// pkg/filesystem.NormalizePath is the same function; it cannot be reused here because +// pkg/filesystem imports this package. +func normalizePath(p string) string { + if p == "" { + return "" + } + p = filepath.Clean(p) + if runtime.GOOS == "darwin" { + if p == "/private" { + return "/" + } + p = strings.TrimPrefix(p, "/private/") + if !strings.HasPrefix(p, "/") { + p = "/" + p + } + } + return p +} + +// NameForWorkspacePath returns the registered workspace whose path is exactly path. +func (c *Config) NameForWorkspacePath(path string) (string, bool) { + target := normalizePath(path) + if target == "" { + return "", false + } + for _, name := range slices.Sorted(maps.Keys(c.Workspaces)) { + if normalizePath(c.Workspaces[name]) == target { + return name, true + } + } + return "", false +} + +// WorkspaceForPath returns the registered workspace containing dir, preferring the longest +// matching path so a workspace nested inside another wins. Iteration is over sorted names +// because Go map order would otherwise make ties nondeterministic between runs. +func (c *Config) WorkspaceForPath(dir string) (string, bool) { + target := normalizePath(dir) + if target == "" { + return "", false + } + + var bestName, bestPath string + for _, name := range slices.Sorted(maps.Keys(c.Workspaces)) { + path := normalizePath(c.Workspaces[name]) + if path == "" { + continue + } + rel, err := filepath.Rel(path, target) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + continue + } + if bestName == "" || len(path) > len(bestPath) { + bestName, bestPath = name, path + } + } + return bestName, bestName != "" +} + func (c *Config) CurrentWorkspaceName() (string, error) { var ws string - mode := c.WorkspaceMode - - switch mode { - case ConfigWorkspaceModeDynamic: + if c.WorkspaceMode == ConfigWorkspaceModeDynamic { wd, err := os.Getwd() if err != nil { return "", err } - if runtime.GOOS == "darwin" { - // On macOS, paths that start with /tmp (and some other system directories) - // are actually symbolic links to paths under /private. The OS may return - // either form of the path - e.g., both "/tmp/file" and "/private/tmp/file" - // refer to the same location. We strip the "/private" prefix for consistent - // path comparison, while preserving the original paths for filesystem operations. - wd = strings.TrimPrefix(wd, "/private") - } - - for wsName, path := range c.Workspaces { - rel, err := filepath.Rel(filepath.Clean(path), filepath.Clean(wd)) - if err != nil { - return "", err - } - if !strings.HasPrefix(rel, "..") { - ws = wsName - break - } - } - fallthrough - case ConfigWorkspaceModeFixed: - if ws != "" { - break - } + ws, _ = c.WorkspaceForPath(wd) + } + if ws == "" { ws = c.CurrentWorkspace } if ws == "" { diff --git a/types/config/config_test.go b/types/config/config_test.go new file mode 100644 index 00000000..125f9f5f --- /dev/null +++ b/types/config/config_test.go @@ -0,0 +1,109 @@ +package config_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/flowexec/flow/v2/types/config" +) + +func TestConfig(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Config Suite") +} + +var _ = Describe("Workspace path lookups", func() { + Describe("WorkspaceForPath", func() { + It("returns the workspace containing the path", func() { + cfg := &config.Config{Workspaces: map[string]string{"a": "/src/a", "b": "/src/b"}} + name, found := cfg.WorkspaceForPath("/src/a/pkg/deep") + Expect(found).To(BeTrue()) + Expect(name).To(Equal("a")) + }) + + It("prefers the longest match when workspaces nest", func() { + // Map iteration order used to decide this, so the nested workspace won only by luck. + cfg := &config.Config{Workspaces: map[string]string{ + "outer": "/src/repo", + "inner": "/src/repo/sub/worktree", + }} + for range 20 { + name, found := cfg.WorkspaceForPath("/src/repo/sub/worktree/cmd") + Expect(found).To(BeTrue()) + Expect(name).To(Equal("inner")) + } + }) + + It("does not match a sibling sharing a name prefix", func() { + cfg := &config.Config{Workspaces: map[string]string{"ws": "/src/ws"}} + _, found := cfg.WorkspaceForPath("/src/wsX") + Expect(found).To(BeFalse()) + }) + + It("matches the workspace root itself", func() { + cfg := &config.Config{Workspaces: map[string]string{"ws": "/src/ws"}} + name, found := cfg.WorkspaceForPath("/src/ws") + Expect(found).To(BeTrue()) + Expect(name).To(Equal("ws")) + }) + + It("returns false for an empty path or an empty config", func() { + cfg := &config.Config{Workspaces: map[string]string{"ws": "/src/ws"}} + _, found := cfg.WorkspaceForPath("") + Expect(found).To(BeFalse()) + _, found = (&config.Config{}).WorkspaceForPath("/src/ws") + Expect(found).To(BeFalse()) + }) + }) + + Describe("NameForWorkspacePath", func() { + It("matches an exact root only", func() { + cfg := &config.Config{Workspaces: map[string]string{"ws": "/src/ws"}} + name, found := cfg.NameForWorkspacePath("/src/ws") + Expect(found).To(BeTrue()) + Expect(name).To(Equal("ws")) + + _, found = cfg.NameForWorkspacePath("/src/ws/sub") + Expect(found).To(BeFalse()) + }) + + It("tolerates unclean paths", func() { + cfg := &config.Config{Workspaces: map[string]string{"ws": "/src/ws/"}} + _, found := cfg.NameForWorkspacePath("/src/ws/./") + Expect(found).To(BeTrue()) + }) + }) + + Describe("CurrentWorkspaceName", func() { + It("falls back to the configured workspace in fixed mode", func() { + cfg := &config.Config{ + Workspaces: map[string]string{"pinned": "/src/pinned"}, + CurrentWorkspace: "pinned", + WorkspaceMode: config.ConfigWorkspaceModeFixed, + } + name, err := cfg.CurrentWorkspaceName() + Expect(err).NotTo(HaveOccurred()) + Expect(name).To(Equal("pinned")) + }) + + It("errors when nothing is configured", func() { + cfg := &config.Config{WorkspaceMode: config.ConfigWorkspaceModeFixed} + _, err := cfg.CurrentWorkspaceName() + Expect(err).To(MatchError(ContainSubstring("current workspace not found"))) + }) + }) + + Describe("SetDefaults", func() { + It("picks the first workspace by sort order, not map order", func() { + for range 20 { + cfg := &config.Config{Workspaces: map[string]string{ + "zulu": "/z", "alpha": "/a", "mike": "/m", + }} + cfg.SetDefaults() + Expect(cfg.CurrentWorkspace).To(Equal("alpha")) + } + }) + }) +})