From 8e25efea2a6e1ef95511e78462c8d5575dad8471 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 14 Oct 2026 19:26:26 +0200 Subject: [PATCH 1/2] fix(trace): register checkouts that shell commands work in A session that works in another checkout through shell commands only, for example in a fresh clone, never registered that checkout. Agent hooks run from the directory the session started in, so the clone got no git hooks, and its commits and pushes carried no session. The pre and post shell hooks now read the command text and find the checkouts it names: the target of cd or pushd, the path of git -C, and the folder that git clone or gh repo clone creates. Each one is handled as a file tool edit there is: the git hooks are installed, the session is recorded, and the checkout is added to the session's checkouts. Claude Code and OpenCode send the command text. Cursor does not, as the provider installs no shell hooks yet. Refs: PFM-7614 Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 2256cda0-6d1f-4a81-ae6a-f07a6a67c3dc --- app/cli/internal/trace/README.md | 10 +- app/cli/internal/trace/claude/hooks.go | 2 + app/cli/internal/trace/claude/hooks_test.go | 13 + app/cli/internal/trace/opencode/hooks.go | 11 +- app/cli/internal/trace/opencode/hooks_test.go | 13 +- .../trace/opencode/testdata/plugin_full.ts | 5 +- .../opencode/testdata/plugin_tracerun.ts | 5 +- app/cli/internal/trace/provider.go | 3 + app/cli/pkg/action/trace_agent_hook.go | 43 ++- app/cli/pkg/action/trace_agent_hook_test.go | 4 +- app/cli/pkg/action/trace_command_dirs.go | 337 ++++++++++++++++++ app/cli/pkg/action/trace_command_dirs_test.go | 78 ++++ .../pkg/action/trace_shell_checkout_test.go | 194 ++++++++++ 13 files changed, 699 insertions(+), 19 deletions(-) create mode 100644 app/cli/pkg/action/trace_command_dirs.go create mode 100644 app/cli/pkg/action/trace_command_dirs_test.go create mode 100644 app/cli/pkg/action/trace_shell_checkout_test.go diff --git a/app/cli/internal/trace/README.md b/app/cli/internal/trace/README.md index 6072463ae..67a8d0141 100644 --- a/app/cli/internal/trace/README.md +++ b/app/cli/internal/trace/README.md @@ -39,7 +39,7 @@ Which lines of a commit the AI wrote. Why not Yes: - **Line attribution, Cursor:** Cursor has only the `afterFileEdit` hook, which gives the edits but no snapshot before the edit. The provider rebuilds the content before the edit from the edits. -- **Shell changes attribution, Cursor:** Cursor has no hook before and after a shell command. +- **Shell changes attribution, Cursor:** the provider does not install the shell hooks of Cursor (`preToolUse` and `postToolUse` for the shell tool). ## User experience @@ -53,7 +53,7 @@ What the user sees in the agent. Why not Yes: - **Welcome message, Cursor and OpenCode:** the provider gives the session-start context to the model only. It has no channel that shows a message to the user at session start, so the welcome message is dropped. -- **Session link after push, Cursor:** Cursor has no hook after a shell command, so there is no point at which to show the link. +- **Session link after push, Cursor:** the provider does not install a Cursor hook that runs after a shell command, so there is no point at which to show the link. - **Session link after push, OpenCode:** the plugin hook fires after a shell command, but the response that shows a message to the user is not verified yet. The link stays pending until it expires. ## Specs and skills @@ -100,13 +100,17 @@ How the sessions are linked to the commits and attested. |---------|---------------|-------------|--------|--------------|------------| | Commit trailer | Each commit gets a `Chainloop-Trace-Sessions` trailer with the sessions that contributed to it. | Yes | Yes | Yes | Yes | | Attestation at push | The `pre-push` hook attests the sessions of the pushed commits. | Yes | Yes | Yes | Yes | +| Shell commands in another checkout | A session can work in a checkout other than the one it started in through shell commands only, for example in a fresh clone. The checkout is found from the command text: the target of `cd` or `pushd`, the path of `git -C`, and the folder that `git clone` or `gh repo clone` creates. The git hooks are installed there and the session is recorded there, the same as for a file tool edit in that checkout. A clone gets the git hooks only after the command that created it, so a commit in that same command does not carry the session. | Yes | No | Yes | Yes | +Why not Yes: + +- **Shell commands in another checkout, Cursor:** the provider does not install the shell hooks of Cursor. A file tool edit in the other checkout still registers it. ## How each agent is hooked in | Agent | Hook installation | Hooks | |-------|-------------------|-------| | Claude Code | `.claude/settings.json` | Session start and end, user prompt, and before and after each file and shell tool and the Skill tool. | -| Cursor | `.cursor/hooks.json` | Session start and end, and `afterFileEdit`. There is no shell hook and no prompt hook. | +| Cursor | `.cursor/hooks.json` | Session start and end, and `afterFileEdit`. The provider installs no shell hook and no prompt hook. | | OpenCode | `.opencode/plugins/chainloop-trace.ts` | Session start and end, user prompt, and before and after each tool. One plugin covers both versions: `server()` for 1.x (1.3.4 or later) and `setup()` for 2. The provider picks the export command from `opencode --version`. | All agents share the git hooks `commit-msg`, `post-commit` and `pre-push`. diff --git a/app/cli/internal/trace/claude/hooks.go b/app/cli/internal/trace/claude/hooks.go index b443255f8..e30138e17 100644 --- a/app/cli/internal/trace/claude/hooks.go +++ b/app/cli/internal/trace/claude/hooks.go @@ -205,6 +205,7 @@ func (p *Provider) ReadHookInput(r io.Reader) (*trace.HookInput, error) { trace.HookInput ToolInput struct { FilePath string `json:"file_path"` + Command string `json:"command"` } `json:"tool_input"` CursorVersion string `json:"cursor_version"` } @@ -226,6 +227,7 @@ func (p *Provider) ReadHookInput(r io.Reader) (*trace.HookInput, error) { if raw.ToolInput.FilePath != "" { input.FilePath = raw.ToolInput.FilePath } + input.Command = raw.ToolInput.Command input.ToolFailed = input.HookEventName == eventPostToolUseFailure return &input, nil diff --git a/app/cli/internal/trace/claude/hooks_test.go b/app/cli/internal/trace/claude/hooks_test.go index 0c7a3825e..3a7155f7e 100644 --- a/app/cli/internal/trace/claude/hooks_test.go +++ b/app/cli/internal/trace/claude/hooks_test.go @@ -257,6 +257,19 @@ func TestReadHookInput(t *testing.T) { assert.Equal(t, "/some/file.go", input.FilePath) }) + t.Run("extracts the shell command", func(t *testing.T) { + r := bytes.NewBufferString(`{ + "session_id": "abc-123", + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": {"command": "cd /tmp/repo-b && git commit -m x", "description": "commit"} + }`) + input, err := provider.ReadHookInput(r) + require.NoError(t, err) + assert.Equal(t, "cd /tmp/repo-b && git commit -m x", input.Command) + assert.Empty(t, input.FilePath) + }) + t.Run("handles missing tool_input gracefully", func(t *testing.T) { r := bytes.NewBufferString(`{"session_id":"abc-123","hook_event_name":"SessionStart"}`) input, err := provider.ReadHookInput(r) diff --git a/app/cli/internal/trace/opencode/hooks.go b/app/cli/internal/trace/opencode/hooks.go index 772db1e5b..c373f8523 100644 --- a/app/cli/internal/trace/opencode/hooks.go +++ b/app/cli/internal/trace/opencode/hooks.go @@ -203,8 +203,9 @@ async function toolEvent(directory: string, hook: string, hookEventName: string, } if (commandTools.includes(tool)) { // The call ID pairs this hook with the other hook of the same call, so - // overlapping commands keep their own snapshots. - await fire(directory, hook, { ...payload, tool_use_id: callID }) + // overlapping commands keep their own snapshots. The command line names + // the checkouts that the command works in. + await fire(directory, hook, { ...payload, tool_use_id: callID, command: args?.command ?? "" }) return } if (!fileWritingTools.includes(tool)) return @@ -422,8 +423,8 @@ const maxHookPayloadBytes = 16 * 1024 * 1024 // ReadHookInput reads and parses the opencode hook JSON from the given reader. // The plugin pipes JSON to stdin with fields: session_id, hook_event_name, -// tool_name, file_path. A relative file_path is resolved against the working -// directory. +// tool_name, file_path, and for shell tools tool_use_id and command. A +// relative file_path is resolved against the working directory. func (p *Provider) ReadHookInput(r io.Reader) (*trace.HookInput, error) { data, err := io.ReadAll(io.LimitReader(r, maxHookPayloadBytes)) if err != nil { @@ -437,6 +438,7 @@ func (p *Provider) ReadHookInput(r io.Reader) (*trace.HookInput, error) { FilePath string `json:"file_path"` ToolUseID string `json:"tool_use_id"` SkillDir string `json:"skill_dir"` + Command string `json:"command"` } if err := json.Unmarshal(data, &raw); err != nil { return nil, err @@ -466,6 +468,7 @@ func (p *Provider) ReadHookInput(r io.Reader) (*trace.HookInput, error) { FilePath: filePath, ToolUseID: raw.ToolUseID, SkillDir: skillDir, + Command: raw.Command, }, nil } diff --git a/app/cli/internal/trace/opencode/hooks_test.go b/app/cli/internal/trace/opencode/hooks_test.go index 7def22013..0371b130b 100644 --- a/app/cli/internal/trace/opencode/hooks_test.go +++ b/app/cli/internal/trace/opencode/hooks_test.go @@ -216,7 +216,8 @@ func TestReadHookInputParsesValidInput(t *testing.T) { // TestPluginSendsShellCallID pins that both entry points send the tool call ID // as tool_use_id on shell hooks, so that the pre and post hooks of one command -// pair their snapshots. OpenCode 1.x names it callID, and OpenCode 2 id. +// pair their snapshots, and the command line, which names the checkouts that +// the command works in (PFM-7614). OpenCode 1.x names it callID, and OpenCode 2 id. func TestPluginSendsShellCallID(t *testing.T) { repoRoot := t.TempDir() require.NoError(t, New().InstallHooks(repoRoot)) @@ -225,7 +226,7 @@ func TestPluginSendsShellCallID(t *testing.T) { require.NoError(t, err) content := string(data) - assert.Contains(t, content, "{ ...payload, tool_use_id: callID }") + assert.Contains(t, content, `{ ...payload, tool_use_id: callID, command: args?.command ?? "" }`) // OpenCode 1.x. assert.Contains(t, content, `toolEvent(directory, "pre-tool-use", "tool.execute.before", input.sessionID, input.tool, input.callID, output.args)`) assert.Contains(t, content, `toolEvent(directory, "post-tool-use", "tool.execute.after", input.sessionID, input.tool, input.callID, input.args, input.tool === skillTool ? skillDirFrom(output) : "")`) @@ -281,13 +282,15 @@ func TestReadHookInputParsesSkillDir(t *testing.T) { } // The plugin sends opencode's callID as tool_use_id on shell hooks, so that -// the pre and post hooks of one command pair their snapshots. -func TestReadHookInputParsesToolUseID(t *testing.T) { - r := bytes.NewBufferString(`{"session_id":"ses_1","hook_event_name":"tool.execute.before","tool_name":"bash","tool_use_id":"call_01"}`) +// the pre and post hooks of one command pair their snapshots. It also sends +// the command line. +func TestReadHookInputParsesShellCall(t *testing.T) { + r := bytes.NewBufferString(`{"session_id":"ses_1","hook_event_name":"tool.execute.before","tool_name":"bash","tool_use_id":"call_01","command":"cd /tmp/repo-b && git commit -m x"}`) p := New() input, err := p.ReadHookInput(r) require.NoError(t, err) assert.Equal(t, "call_01", input.ToolUseID) + assert.Equal(t, "cd /tmp/repo-b && git commit -m x", input.Command) } func TestReadHookInputApplyPatchSingleFile(t *testing.T) { diff --git a/app/cli/internal/trace/opencode/testdata/plugin_full.ts b/app/cli/internal/trace/opencode/testdata/plugin_full.ts index 2eb4e23d2..71f2a458e 100644 --- a/app/cli/internal/trace/opencode/testdata/plugin_full.ts +++ b/app/cli/internal/trace/opencode/testdata/plugin_full.ts @@ -143,8 +143,9 @@ async function toolEvent(directory: string, hook: string, hookEventName: string, } if (commandTools.includes(tool)) { // The call ID pairs this hook with the other hook of the same call, so - // overlapping commands keep their own snapshots. - await fire(directory, hook, { ...payload, tool_use_id: callID }) + // overlapping commands keep their own snapshots. The command line names + // the checkouts that the command works in. + await fire(directory, hook, { ...payload, tool_use_id: callID, command: args?.command ?? "" }) return } if (!fileWritingTools.includes(tool)) return diff --git a/app/cli/internal/trace/opencode/testdata/plugin_tracerun.ts b/app/cli/internal/trace/opencode/testdata/plugin_tracerun.ts index 43e717716..74749d73e 100644 --- a/app/cli/internal/trace/opencode/testdata/plugin_tracerun.ts +++ b/app/cli/internal/trace/opencode/testdata/plugin_tracerun.ts @@ -139,8 +139,9 @@ async function toolEvent(directory: string, hook: string, hookEventName: string, } if (commandTools.includes(tool)) { // The call ID pairs this hook with the other hook of the same call, so - // overlapping commands keep their own snapshots. - await fire(directory, hook, { ...payload, tool_use_id: callID }) + // overlapping commands keep their own snapshots. The command line names + // the checkouts that the command works in. + await fire(directory, hook, { ...payload, tool_use_id: callID, command: args?.command ?? "" }) return } if (!fileWritingTools.includes(tool)) return diff --git a/app/cli/internal/trace/provider.go b/app/cli/internal/trace/provider.go index 5cda563a1..fba5c355b 100644 --- a/app/cli/internal/trace/provider.go +++ b/app/cli/internal/trace/provider.go @@ -307,6 +307,9 @@ type HookInput struct { ToolName string `json:"tool_name,omitempty"` // FilePath is the absolute path of the file being edited, set by provider's ReadHookInput. FilePath string `json:"-"` + // Command is the command line of a shell command tool, set by provider's + // ReadHookInput. Empty when the provider does not report it. + Command string `json:"-"` // Cwd is the directory the agent session runs in, which is where the // agent files its transcripts. It can differ from the checkout that owns // FilePath, e.g. for an edit in a linked git worktree. Empty when the diff --git a/app/cli/pkg/action/trace_agent_hook.go b/app/cli/pkg/action/trace_agent_hook.go index ace543ffc..71caaa153 100644 --- a/app/cli/pkg/action/trace_agent_hook.go +++ b/app/cli/pkg/action/trace_agent_hook.go @@ -283,7 +283,9 @@ func HandleAgentPreToolUse(provider trace.Provider, log zerolog.Logger) error { switch { case provider.IsCommandTool(input.ToolName): // Shell command: snapshot the whole worktree so the post hook can diff - // it and attribute the command's file changes to the AI. + // it and attribute the command's file changes to the AI. Register the + // checkouts the command names first, so the snapshot covers them. + registerCommandCheckouts(provider, input, repoRoot, log) captureWorktreeSnapshot(store, repoRoot, shellCallKey(input), log) case provider.IsFileWritingTool(input.ToolName): if input.FilePath == "" { @@ -391,6 +393,41 @@ func registerSessionCheckout(sessionID, fileRoot string, log zerolog.Logger) { } } +// registerCommandCheckouts handles each checkout that a shell command names as +// a file tool edit there is handled: it installs the git hooks, writes the +// session record, and adds the checkout to the session's checkout list +// (PFM-7614). Shell hooks run in the session's directory, so without this a +// session that works in another checkout through the shell only, for example +// in a fresh clone, never reaches it, and its commits there carry no session. +// homeRoot is the session's own checkout, which needs none of this. +func registerCommandCheckouts(provider trace.Provider, input *trace.HookInput, homeRoot string, log zerolog.Logger) { + if input.Command == "" { + return + } + + cwd := input.Cwd + if cwd == "" { + cwd, _ = os.Getwd() + } + + for _, dir := range commandCheckoutDirs(input.Command, cwd) { + // A directory that does not exist yet (the target of a clone, at the + // pre hook) must not resolve to a repository above it. + if _, err := os.Stat(dir); err != nil { + continue + } + + store, root, err := state.LocateFrom(dir) + if err != nil || !store.IsGit() || sameDir(root, homeRoot) { + continue + } + + log.Debug().Str("root", root).Msg("shell command works in another checkout") + ensureSessionTracked(provider, store, root, input, log) + registerSessionCheckout(input.SessionID, root, log) + } +} + // sameDir reports whether a and b name the same directory, ignoring symlinks // (e.g. /var and /private/var on macOS). func sameDir(a, b string) bool { @@ -613,6 +650,10 @@ func HandleAgentPostToolUse(provider trace.Provider, log zerolog.Logger) error { // every file the command changed to the AI. recordCommandLineRanges(provider, input, store, repoRoot, log) + // A checkout that the command created (`git clone`) did not exist + // at the pre hook. + registerCommandCheckouts(provider, input, repoRoot, log) + // The command may have been a `git push`, whose pre-push hook attested // a session and left its link behind. Show it now: the pre-push output // went to this tool call's captured stderr, which the user does not diff --git a/app/cli/pkg/action/trace_agent_hook_test.go b/app/cli/pkg/action/trace_agent_hook_test.go index ee3b95651..83aac5ab0 100644 --- a/app/cli/pkg/action/trace_agent_hook_test.go +++ b/app/cli/pkg/action/trace_agent_hook_test.go @@ -891,8 +891,8 @@ func TestHandleAgentCommandTool_OtherCheckout(t *testing.T) { homeAttr := homeStore.LoadAILineAttribution(sid) assert.Empty(t, homeAttr.Files, "the session's own checkout did not change") - // Accepted gap: without a file tool edit there, a checkout is not - // snapshotted. + // Accepted gap: a checkout that no file tool edited and that the command + // does not name is not snapshotted. assert.Empty(t, untouchedStore.LoadAILineAttribution(sid).Files) assert.False(t, untouchedStore.SessionRecordExists(sid)) diff --git a/app/cli/pkg/action/trace_command_dirs.go b/app/cli/pkg/action/trace_command_dirs.go new file mode 100644 index 000000000..da01c01de --- /dev/null +++ b/app/cli/pkg/action/trace_command_dirs.go @@ -0,0 +1,337 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "os" + "path/filepath" + "regexp" + "slices" + "strings" +) + +var ( + // shellAssignment matches a leading variable assignment ("GIT_TRACE=1 git …"). + shellAssignment = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*=`) + + // gitValueOptions are the git options before the subcommand that take + // their value as the next word. + gitValueOptions = []string{"-c", "--git-dir", "--work-tree", "--namespace", "--config-env"} + + // gitCloneValueOptions are the git clone options that take their value as + // the next word. + gitCloneValueOptions = []string{ + "-o", "--origin", "-b", "--branch", "-u", "--upload-pack", "--reference", + "--reference-if-able", "--separate-git-dir", "--depth", "--shallow-since", + "--shallow-exclude", "-c", "--config", "-j", "--jobs", "--template", + "--filter", "--server-option", "--bundle-uri", "--ref-format", + } + + // ghCloneValueOptions are the gh repo clone options that take their value + // as the next word. + ghCloneValueOptions = []string{"-u", "--upstream-remote-name"} +) + +// commandCheckoutDirs returns the directories that a shell command works in, +// as far as the command text tells them (PFM-7614): the target of each +// `cd`/`pushd`, the path of `git -C`, and the directory that `git clone` or +// `gh repo clone` creates. Relative paths resolve against cwd and the `cd` +// targets before them. A path that holds an expansion ($VAR, $(…)) is unknown +// and is skipped, and so is every relative path after an unknown `cd`. +// +// It is a heuristic, not a shell interpreter. The caller checks each +// directory, so a wrong guess costs a lookup only. +func commandCheckoutDirs(command, cwd string) []string { + var dirs []string + add := func(dir string) { + if !slices.Contains(dirs, dir) { + dirs = append(dirs, dir) + } + } + + dir := cwd + for _, words := range splitShellCommands(command) { + for len(words) > 0 && !words[0].dynamic && shellAssignment.MatchString(words[0].text) { + words = words[1:] + } + if len(words) == 0 { + continue + } + + name, args := words[0].text, words[1:] + switch { + case name == "cd" || name == "pushd": + // Without a usable target ("cd", "cd -", "cd $X") the directory + // is unknown from here on. + base := dir + dir = "" + if i := slices.IndexFunc(args, func(w shellWord) bool { return !strings.HasPrefix(w.text, "-") }); i >= 0 { + if d, ok := resolveShellPath(base, args[i]); ok { + dir = d + add(d) + } + } + case name == "git": + for _, d := range gitCommandDirs(dir, args) { + add(d) + } + case name == "gh" && len(args) >= 2 && args[0].text == "repo" && args[1].text == "clone": + args = args[2:] + // The words after "--" are flags for git, not operands. + if i := slices.IndexFunc(args, func(w shellWord) bool { return w.text == "--" }); i >= 0 { + args = args[:i] + } + if d, ok := cloneDir(dir, args, ghCloneValueOptions); ok { + add(d) + } + } + } + + return dirs +} + +// gitCommandDirs returns the directories of one git command: the -C path, +// and for a clone the directory it creates. +func gitCommandDirs(dir string, args []shellWord) []string { + var dirs []string + + withC := false + i := 0 + for ; i < len(args) && strings.HasPrefix(args[i].text, "-"); i++ { + switch a := args[i].text; { + case a == "-C": + if i+1 == len(args) { + return nil + } + i++ + d, ok := resolveShellPath(dir, args[i]) + if !ok { + return nil + } + dir, withC = d, true + case slices.Contains(gitValueOptions, a): + i++ + } + } + + if withC { + dirs = append(dirs, dir) + } + + if i < len(args) && args[i].text == "clone" { + if d, ok := cloneDir(dir, args[i+1:], gitCloneValueOptions); ok { + dirs = append(dirs, d) + } + } + + return dirs +} + +// cloneDir returns the directory that a clone creates: its second operand, +// or else the name of the repository without ".git", as git clone does. +func cloneDir(base string, args []shellWord, valueOptions []string) (string, bool) { + var operands []shellWord + for i := 0; i < len(args); i++ { + switch a := args[i].text; { + case a == "--": + operands = append(operands, args[i+1:]...) + i = len(args) + case slices.Contains(valueOptions, a): + i++ + case strings.HasPrefix(a, "-"): + default: + operands = append(operands, args[i]) + } + } + + switch len(operands) { + case 0: + return "", false + case 1: + if operands[0].dynamic { + return "", false + } + name := repositoryDirName(operands[0].text) + if name == "" { + return "", false + } + return resolveShellPath(base, shellWord{text: name}) + default: + return resolveShellPath(base, operands[1]) + } +} + +// repositoryDirName returns the default clone directory for a repository +// URL, path or OWNER/REPO name: its last path element without ".git". +func repositoryDirName(repo string) string { + name := strings.TrimRight(repo, "/") + name = strings.TrimSuffix(name, "/.git") + name = strings.TrimSuffix(name, ".git") + if i := strings.LastIndexAny(name, "/:"); i >= 0 { + name = name[i+1:] + } + + if name == "." || name == ".." { + return "" + } + + return name +} + +// resolveShellPath returns the absolute path that w names, from base. It +// fails for a word with an expansion, and for a relative path when base is +// unknown. +func resolveShellPath(base string, w shellWord) (string, bool) { + if w.dynamic || w.text == "" { + return "", false + } + + p := w.text + if p == "~" || strings.HasPrefix(p, "~/") { + home, err := os.UserHomeDir() + if err != nil { + return "", false + } + p = filepath.Join(home, p[1:]) + } + + if filepath.IsAbs(p) { + return filepath.Clean(p), true + } + if base == "" { + return "", false + } + + return filepath.Join(base, p), true +} + +// shellWord is one word of a shell command, after quote removal. +type shellWord struct { + text string + // dynamic is set when the word holds an expansion ($VAR, $(…), `…`), + // whose value is not known without running the shell. + dynamic bool +} + +// splitShellCommands splits a shell command line into its simple commands, +// each one a list of words. It knows quotes, backslash escapes, comments, +// command separators and redirections, which it drops with their target. +// It does not expand anything: commandCheckoutDirs only needs the words. +func splitShellCommands(line string) [][]shellWord { + var ( + cmds [][]shellWord + cmd []shellWord + word strings.Builder + inWord, dynamic, skipWord bool + ) + + endWord := func() { + if !inWord { + return + } + if skipWord { + skipWord = false + } else { + cmd = append(cmd, shellWord{text: word.String(), dynamic: dynamic}) + } + word.Reset() + inWord, dynamic = false, false + } + endCommand := func() { + endWord() + skipWord = false + if len(cmd) > 0 { + cmds = append(cmds, cmd) + } + cmd = nil + } + + runes := []rune(line) + for i := 0; i < len(runes); i++ { + switch c := runes[i]; { + case c == ' ' || c == '\t': + endWord() + case strings.ContainsRune("\n;&|()", c): + endCommand() + case c == '>' || c == '<': + // A file descriptor number before the operator ("2>") belongs + // to it, not to the command. + if inWord && isDigits(word.String()) { + word.Reset() + inWord, dynamic = false, false + } else { + endWord() + } + for i+1 < len(runes) && strings.ContainsRune(">&|", runes[i+1]) { + i++ + } + skipWord = true + case c == '#' && !inWord: + for i+1 < len(runes) && runes[i+1] != '\n' { + i++ + } + case c == '\\': + if i+1 < len(runes) { + i++ + if runes[i] == '\n' { + continue + } + word.WriteRune(runes[i]) + inWord = true + } + case c == '\'': + inWord = true + for i+1 < len(runes) { + i++ + if runes[i] == '\'' { + break + } + word.WriteRune(runes[i]) + } + case c == '"': + inWord = true + for i+1 < len(runes) { + i++ + d := runes[i] + if d == '"' { + break + } + if d == '\\' && i+1 < len(runes) && strings.ContainsRune("\"\\$`\n", runes[i+1]) { + i++ + word.WriteRune(runes[i]) + continue + } + if d == '$' || d == '`' { + dynamic = true + } + word.WriteRune(d) + } + default: + if c == '$' || c == '`' { + dynamic = true + } + word.WriteRune(c) + inWord = true + } + } + endCommand() + + return cmds +} + +func isDigits(s string) bool { + return s != "" && strings.Trim(s, "0123456789") == "" +} diff --git a/app/cli/pkg/action/trace_command_dirs_test.go b/app/cli/pkg/action/trace_command_dirs_test.go new file mode 100644 index 000000000..ed8bbe375 --- /dev/null +++ b/app/cli/pkg/action/trace_command_dirs_test.go @@ -0,0 +1,78 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCommandCheckoutDirs(t *testing.T) { + const cwd = "/work/home" + t.Setenv("HOME", "/users/dev") + + cases := []struct { + name string + cmd string + want []string + }{ + {name: "empty", cmd: "", want: nil}, + {name: "no directory", cmd: "make test", want: nil}, + {name: "cd absolute", cmd: "cd /tmp/repo-b && git commit -m x", want: []string{"/tmp/repo-b"}}, + {name: "cd relative to cwd", cmd: "cd sub && ls", want: []string{"/work/home/sub"}}, + {name: "cd relative to previous cd", cmd: "cd /a; cd b", want: []string{"/a", "/a/b"}}, + {name: "cd home", cmd: "cd ~/src/x && ls", want: []string{"/users/dev/src/x"}}, + {name: "cd on its own line", cmd: "cd /a\ngit commit", want: []string{"/a"}}, + {name: "cd in subshell", cmd: "(cd /a && make)", want: []string{"/a"}}, + {name: "pushd", cmd: "pushd /a >/dev/null && git status", want: []string{"/a"}}, + {name: "quoted path", cmd: `cd "/tmp/my repo" && git commit`, want: []string{"/tmp/my repo"}}, + {name: "single-quoted path", cmd: `cd '/tmp/my repo'`, want: []string{"/tmp/my repo"}}, + {name: "escaped space", cmd: `cd /tmp/my\ repo`, want: []string{"/tmp/my repo"}}, + {name: "cd text in an argument", cmd: `echo "cd /tmp/x"`, want: nil}, + {name: "comment", cmd: "ls # cd /tmp/x", want: nil}, + {name: "cd with variable", cmd: "cd $REPO && git commit", want: nil}, + {name: "relative cd after unknown cd", cmd: "cd $REPO && cd sub", want: nil}, + {name: "absolute cd after unknown cd", cmd: "cd $REPO && cd /a", want: []string{"/a"}}, + {name: "cd back", cmd: "cd - && ls", want: nil}, + {name: "duplicates", cmd: "cd /a && git -C /a commit", want: []string{"/a"}}, + {name: "git -C", cmd: `git -C /tmp/repo-b commit -m "x"`, want: []string{"/tmp/repo-b"}}, + {name: "git -C twice", cmd: "git -C /a -C b status", want: []string{"/a/b"}}, + {name: "git -c then -C", cmd: "git -c user.name=x -C /a commit", want: []string{"/a"}}, + {name: "env assignment", cmd: "GIT_TRACE=1 git -C /a status", want: []string{"/a"}}, + {name: "git without -C", cmd: "git commit -m x", want: nil}, + {name: "git clone with dir", cmd: "git clone git@github.com:o/repo-b.git /tmp/repo-b", want: []string{"/tmp/repo-b"}}, + {name: "git clone without dir", cmd: "git clone https://github.com/o/repo-b.git", want: []string{"/work/home/repo-b"}}, + {name: "git clone without dir, trailing slash", cmd: "git clone /srv/git/repo-b/", want: []string{"/work/home/repo-b"}}, + {name: "git clone after cd", cmd: "cd /tmp && git clone --depth 1 -b main https://github.com/o/repo-b", want: []string{"/tmp", "/tmp/repo-b"}}, + {name: "git clone option with equals", cmd: "git clone --branch=main https://github.com/o/repo-b dir", want: []string{"/work/home/dir"}}, + {name: "git clone with redirection", cmd: "git clone https://github.com/o/repo-b /tmp/b 2>&1 | tail -1", want: []string{"/tmp/b"}}, + {name: "git -C clone", cmd: "git -C /tmp clone https://github.com/o/repo-b", want: []string{"/tmp", "/tmp/repo-b"}}, + {name: "gh repo clone with dir", cmd: "gh repo clone o/repo-b /tmp/repo-b -- --depth 1", want: []string{"/tmp/repo-b"}}, + {name: "gh repo clone without dir", cmd: "gh repo clone o/repo-b", want: []string{"/work/home/repo-b"}}, + {name: "gh other command", cmd: "gh pr create --title x", want: nil}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, commandCheckoutDirs(tc.cmd, cwd)) + }) + } +} + +func TestCommandCheckoutDirs_UnknownCwd(t *testing.T) { + assert.Equal(t, []string{"/a"}, commandCheckoutDirs("cd sub && cd /a", "")) +} diff --git a/app/cli/pkg/action/trace_shell_checkout_test.go b/app/cli/pkg/action/trace_shell_checkout_test.go new file mode 100644 index 000000000..be50dfaf0 --- /dev/null +++ b/app/cli/pkg/action/trace_shell_checkout_test.go @@ -0,0 +1,194 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/chainloop-dev/chainloop/app/cli/internal/trace" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/claude" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/hooks" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/opencode" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// PFM-7614: a Claude Code session started in a traced checkout clones a second +// repository and works there with shell commands only: no file tool touches a +// path in the clone. Every agent hook runs from the directory the session +// started in, so the clone is found from the command text. +// +// The steps follow the subagent case in the ticket: the agent writes a file +// outside any repository, clones the repository in one Bash call, copies the +// file into the clone in a second call, and commits in a third. +func TestClaudeSessionCommitsInClonedCheckoutThroughShell(t *testing.T) { + const sid = "7614a0c2-1a2b-4c3d-8e9f-0a1b2c3d4e5f" + + home := t.TempDir() + setTestHome(t, home) + + // The repository that the agent clones. It is traced: it has a project + // name, which the git hook install requires. + upstream := resolvedTempGitRepo(t) + configureGitUser(t, upstream) + require.NoError(t, os.WriteFile(filepath.Join(upstream, ".chainloop.yml"), []byte("projectName: repo-b\n"), 0600)) + require.NoError(t, os.MkdirAll(filepath.Join(upstream, "docs"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(upstream, "docs", "README.md"), []byte("docs\n"), 0600)) + runGit(t, upstream, "add", ".") + runGit(t, upstream, "commit", "-m", "initial") + + mainRoot := chdirToResolvedGitRepo(t) + require.NoError(t, os.WriteFile(filepath.Join(mainRoot, ".chainloop.yml"), []byte("projectName: repo-a\n"), 0600)) + + scratch, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + clone := filepath.Join(scratch, "repo-b") + notes := filepath.Join(scratch, "notes") + require.NoError(t, os.MkdirAll(notes, 0755)) + + p := claude.New() + hook := func(event, tool, toolUseID, toolInput string, handler func(trace.Provider, zerolog.Logger) error) { + t.Helper() + withStdin(t, fmt.Sprintf(`{"session_id":%q,"cwd":%q,"hook_event_name":%q,"tool_name":%q,"tool_use_id":%q,"tool_input":%s}`, + sid, mainRoot, event, tool, toolUseID, toolInput)) + require.NoError(t, handler(p, zerolog.Nop())) + } + // bash replays one Bash call: the pre hook, the effect of the command, + // then the post hook. Both hooks run from the session's directory. + bash := func(toolUseID, command string, effect func()) { + t.Helper() + input := fmt.Sprintf(`{"command":%q}`, command) + hook("PreToolUse", "Bash", toolUseID, input, HandleAgentPreToolUse) + effect() + hook("PostToolUse", "Bash", toolUseID, input, HandleAgentPostToolUse) + } + + withStdin(t, fmt.Sprintf(`{"session_id":%q,"cwd":%q,"hook_event_name":"SessionStart","source":"startup"}`, sid, mainRoot)) + require.NoError(t, HandleAgentSessionStart(p, zerolog.Nop())) + + // 1. The agent writes a file in a folder that is not a git repository. + spec := filepath.Join(notes, "spec.md") + writeInput := fmt.Sprintf(`{"file_path":%q,"content":"# Spec\n\nline\n"}`, spec) + hook("PreToolUse", "Write", "toolu_w1", writeInput, HandleAgentPreToolUse) + require.NoError(t, os.WriteFile(spec, []byte("# Spec\n\nline\n"), 0600)) + hook("PostToolUse", "Write", "toolu_w1", writeInput, HandleAgentPostToolUse) + + // 2. The agent clones the repository. The clone does not exist at the + // pre hook, so the post hook registers it. + bash("toolu_b1", fmt.Sprintf("git clone %s %s", upstream, clone), func() { + runGit(t, scratch, "clone", upstream, clone) + }) + + cloneGitDir := filepath.Join(clone, ".git") + assert.True(t, hooks.IsInstalled(cloneGitDir, false), "the git hooks are installed in the clone") + + homeStore := state.NewGitStore(filepath.Join(mainRoot, ".git")) + rec, err := homeStore.LoadSessionRecord(sid) + require.NoError(t, err) + require.NotNil(t, rec) + assert.Equal(t, []string{clone}, rec.Checkouts, "the clone is registered on the session record") + + // 3. The agent copies the file into the clone with a shell command. + bash("toolu_b2", fmt.Sprintf("cd %s && cp %s docs/", clone, spec), func() { + content, err := os.ReadFile(spec) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(clone, "docs", "spec.md"), content, 0600)) + }) + + cloneStore := state.NewGitStore(cloneGitDir) + assert.Contains(t, cloneStore.LoadAILineAttribution(sid).Files, "docs/spec.md", + "the shell change is recorded in the ledger of the clone") + assert.Empty(t, homeStore.LoadAILineAttribution(sid).Files, "the session's own checkout did not change") + + // 4. The agent commits in the clone. Git runs the commit-msg and + // post-commit hooks inside the clone. + bash("toolu_b3", fmt.Sprintf(`cd %s && git add docs/spec.md && git commit -s -m "docs: add spec"`, clone), func() { + t.Chdir(clone) + defer t.Chdir(mainRoot) + + configureGitUser(t, clone) + runGit(t, clone, "add", "docs/spec.md") + msgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") + require.NoError(t, os.WriteFile(msgFile, []byte("docs: add spec\n"), 0600)) + require.NoError(t, handleCommitMsg(msgFile, zerolog.Nop())) + + msg, err := os.ReadFile(msgFile) + require.NoError(t, err) + assert.Contains(t, string(msg), state.TrailerKey+": "+sid, "commit-msg stamps the session") + + runGit(t, clone, "commit", "--no-verify", "-F", msgFile) + require.NoError(t, handlePostCommit(t.Context(), zerolog.Nop())) + }) + + // What pre-push reads in the clone. + commits, err := cloneStore.LoadAllCommitRecords() + require.NoError(t, err) + require.Len(t, commits, 1) + assert.Equal(t, []string{sid}, commits[0].SessionIDs, "the commit is credited to the session") + + sessions, err := cloneStore.LoadAllSessionRecords() + require.NoError(t, err) + require.Contains(t, sessions, sid, "pre-push resolves the provider from the session record of the clone") + assert.Equal(t, mainRoot, sessions[sid].Cwd, "the record keeps the directory the agent runs in") +} + +// The OpenCode plugin sends the command line of a shell call too. A checkout +// that exists before the call is registered at the pre hook, so the changes +// that the same call makes there are recorded in its ledger. +func TestOpenCodeShellCommandInOtherCheckout(t *testing.T) { + const sid = "ses_7614" + + mainRoot := chdirToResolvedGitRepo(t) + other := resolvedTempGitRepo(t) + require.NoError(t, os.WriteFile(filepath.Join(other, ".chainloop.yml"), []byte("projectName: repo-b\n"), 0600)) + + p := opencode.New() + payload := fmt.Sprintf(`"tool_name":"bash","tool_use_id":"call_01","command":"cd %s && sed -i '' s/a/b/ notes.md"`, other) + + withStdin(t, fmt.Sprintf(`{"session_id":%q,"hook_event_name":"session.created"}`, sid)) + require.NoError(t, HandleAgentSessionStart(p, zerolog.Nop())) + + withStdin(t, fmt.Sprintf(`{"session_id":%q,"hook_event_name":"tool.execute.before",%s}`, sid, payload)) + require.NoError(t, HandleAgentPreToolUse(p, zerolog.Nop())) + require.NoError(t, os.WriteFile(filepath.Join(other, "notes.md"), []byte("b\n"), 0600)) + withStdin(t, fmt.Sprintf(`{"session_id":%q,"hook_event_name":"tool.execute.after",%s}`, sid, payload)) + require.NoError(t, HandleAgentPostToolUse(p, zerolog.Nop())) + + otherGitDir := filepath.Join(other, ".git") + assert.True(t, hooks.IsInstalled(otherGitDir, false), "the git hooks are installed in the other checkout") + + rec, err := state.NewGitStore(filepath.Join(mainRoot, ".git")).LoadSessionRecord(sid) + require.NoError(t, err) + require.NotNil(t, rec) + assert.Equal(t, []string{other}, rec.Checkouts) + + otherStore := state.NewGitStore(otherGitDir) + assert.True(t, otherStore.SessionRecordExists(sid)) + assert.Contains(t, otherStore.LoadAILineAttribution(sid).Files, "notes.md", + "the change of the same call is recorded in the other checkout") +} + +func configureGitUser(t *testing.T, dir string) { + t.Helper() + runGit(t, dir, "config", "user.email", "test@test.com") + runGit(t, dir, "config", "user.name", "Test") + runGit(t, dir, "config", "commit.gpgsign", "false") +} From eb0de90e070f33ede12efc548cab3cbdd42157b0 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 14 Oct 2026 22:26:04 +0200 Subject: [PATCH 2/2] fix(trace): handle subshells, bare cd and expanded assignments in shell dir detection Address review findings on the command parser: - A cd inside parentheses applies only until the closing parenthesis. - A cd without arguments moves to the home directory. - A leading assignment with an expansion in its value is skipped, so the command after it is still read. Split the tokenizer into a small lexer type to bring its complexity under the lint limit, and fix the other lint findings in the tests. The trace README tells OpenCode users to install the hooks again. Refs: PFM-7614 Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 2256cda0-6d1f-4a81-ae6a-f07a6a67c3dc --- app/cli/internal/trace/README.md | 2 +- app/cli/pkg/action/trace_command_dirs.go | 342 +++++++++++------- app/cli/pkg/action/trace_command_dirs_test.go | 64 ++-- .../pkg/action/trace_shell_checkout_test.go | 9 +- 4 files changed, 259 insertions(+), 158 deletions(-) diff --git a/app/cli/internal/trace/README.md b/app/cli/internal/trace/README.md index 67a8d0141..025ceaee2 100644 --- a/app/cli/internal/trace/README.md +++ b/app/cli/internal/trace/README.md @@ -100,7 +100,7 @@ How the sessions are linked to the commits and attested. |---------|---------------|-------------|--------|--------------|------------| | Commit trailer | Each commit gets a `Chainloop-Trace-Sessions` trailer with the sessions that contributed to it. | Yes | Yes | Yes | Yes | | Attestation at push | The `pre-push` hook attests the sessions of the pushed commits. | Yes | Yes | Yes | Yes | -| Shell commands in another checkout | A session can work in a checkout other than the one it started in through shell commands only, for example in a fresh clone. The checkout is found from the command text: the target of `cd` or `pushd`, the path of `git -C`, and the folder that `git clone` or `gh repo clone` creates. The git hooks are installed there and the session is recorded there, the same as for a file tool edit in that checkout. A clone gets the git hooks only after the command that created it, so a commit in that same command does not carry the session. | Yes | No | Yes | Yes | +| Shell commands in another checkout | A session can work in a checkout other than the one it started in through shell commands only, for example in a fresh clone. The checkout is found from the command text: the target of `cd` or `pushd`, the path of `git -C`, and the folder that `git clone` or `gh repo clone` creates. The git hooks are installed there and the session is recorded there, the same as for a file tool edit in that checkout. A clone gets the git hooks only after the command that created it, so a commit in that same command does not carry the session. For OpenCode, a plugin installed by an earlier version does not send the command text: install the hooks again to update it. | Yes | No | Yes | Yes | Why not Yes: diff --git a/app/cli/pkg/action/trace_command_dirs.go b/app/cli/pkg/action/trace_command_dirs.go index da01c01de..fa2a2c41a 100644 --- a/app/cli/pkg/action/trace_command_dirs.go +++ b/app/cli/pkg/action/trace_command_dirs.go @@ -50,7 +50,8 @@ var ( // `cd`/`pushd`, the path of `git -C`, and the directory that `git clone` or // `gh repo clone` creates. Relative paths resolve against cwd and the `cd` // targets before them. A path that holds an expansion ($VAR, $(…)) is unknown -// and is skipped, and so is every relative path after an unknown `cd`. +// and is skipped, and so is every relative path after an unknown `cd`. A `cd` +// inside parentheses applies only until the closing parenthesis. // // It is a heuristic, not a shell interpreter. The caller checks each // directory, so a wrong guess costs a lookup only. @@ -63,46 +64,79 @@ func commandCheckoutDirs(command, cwd string) []string { } dir := cwd - for _, words := range splitShellCommands(command) { - for len(words) > 0 && !words[0].dynamic && shellAssignment.MatchString(words[0].text) { - words = words[1:] - } - if len(words) == 0 { - continue - } - - name, args := words[0].text, words[1:] - switch { - case name == "cd" || name == "pushd": - // Without a usable target ("cd", "cd -", "cd $X") the directory - // is unknown from here on. - base := dir - dir = "" - if i := slices.IndexFunc(args, func(w shellWord) bool { return !strings.HasPrefix(w.text, "-") }); i >= 0 { - if d, ok := resolveShellPath(base, args[i]); ok { - dir = d - add(d) - } - } - case name == "git": - for _, d := range gitCommandDirs(dir, args) { - add(d) - } - case name == "gh" && len(args) >= 2 && args[0].text == "repo" && args[1].text == "clone": - args = args[2:] - // The words after "--" are flags for git, not operands. - if i := slices.IndexFunc(args, func(w shellWord) bool { return w.text == "--" }); i >= 0 { - args = args[:i] - } - if d, ok := cloneDir(dir, args, ghCloneValueOptions); ok { - add(d) + // outer holds the directory of each enclosing shell, one per open + // parenthesis. + var outer []string + for _, cmd := range splitShellCommands(command) { + switch cmd.subshell { + case '(': + outer = append(outer, dir) + case ')': + if n := len(outer); n > 0 { + dir, outer = outer[n-1], outer[:n-1] } + default: + dir = applyShellCommand(cmd.words, dir, add) } } return dirs } +// applyShellCommand passes to add each directory that one simple command +// names, and returns the directory that the shell is in after it. +func applyShellCommand(words []shellWord, dir string, add func(string)) string { + // The name of an assignment is never an expansion, whatever its value. + for len(words) > 0 && shellAssignment.MatchString(words[0].text) { + words = words[1:] + } + if len(words) == 0 { + return dir + } + + name, args := words[0].text, words[1:] + switch { + case name == "cd" || name == "pushd": + // Without a usable target ("cd -", "pushd", "cd $X") the directory + // is unknown from here on. + d, ok := cdTarget(dir, args, name == "cd") + if !ok { + return "" + } + add(d) + return d + case name == "git": + for _, d := range gitCommandDirs(dir, args) { + add(d) + } + case name == "gh" && len(args) >= 2 && args[0].text == "repo" && args[1].text == "clone": + args = args[2:] + // The words after "--" are flags for git, not operands. + if i := slices.IndexFunc(args, func(w shellWord) bool { return w.text == "--" }); i >= 0 { + args = args[:i] + } + if d, ok := cloneDir(dir, args, ghCloneValueOptions); ok { + add(d) + } + } + + return dir +} + +// cdTarget returns the directory that a cd or pushd with args moves to. A cd +// without arguments moves to the home directory, when homeIfBare is set. +func cdTarget(dir string, args []shellWord, homeIfBare bool) (string, bool) { + i := slices.IndexFunc(args, func(w shellWord) bool { return !strings.HasPrefix(w.text, "-") }) + switch { + case i >= 0: + return resolveShellPath(dir, args[i]) + case len(args) == 0 && homeIfBare: + return resolveShellPath(dir, shellWord{text: "~"}) + default: + return "", false + } +} + // gitCommandDirs returns the directories of one git command: the -C path, // and for a clone the directory it creates. func gitCommandDirs(dir string, args []shellWord) []string { @@ -226,110 +260,164 @@ type shellWord struct { dynamic bool } -// splitShellCommands splits a shell command line into its simple commands, -// each one a list of words. It knows quotes, backslash escapes, comments, +// shellCommand is one simple command of a command line, or the start or end +// of a subshell. +type shellCommand struct { + words []shellWord + // subshell is '(' or ')' for the start or end of a subshell, which has + // no words. It is 0 for a simple command. + subshell rune +} + +// splitShellCommands splits a shell command line into its simple commands +// and subshell boundaries. It knows quotes, backslash escapes, comments, // command separators and redirections, which it drops with their target. // It does not expand anything: commandCheckoutDirs only needs the words. -func splitShellCommands(line string) [][]shellWord { - var ( - cmds [][]shellWord - cmd []shellWord - word strings.Builder - inWord, dynamic, skipWord bool - ) - - endWord := func() { - if !inWord { - return - } - if skipWord { - skipWord = false - } else { - cmd = append(cmd, shellWord{text: word.String(), dynamic: dynamic}) +func splitShellCommands(line string) []shellCommand { + l := &shellLexer{runes: []rune(line)} + for ; l.pos < len(l.runes); l.pos++ { + l.step(l.runes[l.pos]) + } + l.endCommand() + + return l.cmds +} + +// shellLexer holds the state of splitShellCommands. +type shellLexer struct { + runes []rune + pos int + cmds []shellCommand + words []shellWord + word strings.Builder + // inWord is set while a word is open, also an empty quoted one. + inWord bool + // dynamic is set when the open word holds an expansion. + dynamic bool + // skipWord drops the next word, the target of a redirection. + skipWord bool +} + +func (l *shellLexer) step(c rune) { + switch { + case c == ' ' || c == '\t': + l.endWord() + case c == '(' || c == ')': + l.endCommand() + l.cmds = append(l.cmds, shellCommand{subshell: c}) + case strings.ContainsRune("\n;&|", c): + l.endCommand() + case c == '>' || c == '<': + l.redirection() + case c == '#' && !l.inWord: + // A comment runs to the end of the line. + for l.peek() != 0 && l.peek() != '\n' { + l.pos++ } - word.Reset() - inWord, dynamic = false, false + case c == '\\': + l.escape() + case c == '\'': + l.singleQuoted() + case c == '"': + l.doubleQuoted() + default: + l.dynamic = l.dynamic || c == '$' || c == '`' + l.add(c) } - endCommand := func() { - endWord() - skipWord = false - if len(cmd) > 0 { - cmds = append(cmds, cmd) +} + +// peek returns the rune after the current one, or 0 at the end. +func (l *shellLexer) peek() rune { + if l.pos+1 < len(l.runes) { + return l.runes[l.pos+1] + } + + return 0 +} + +func (l *shellLexer) add(c rune) { + l.word.WriteRune(c) + l.inWord = true +} + +func (l *shellLexer) endWord() { + if !l.inWord { + return + } + if l.skipWord { + l.skipWord = false + } else { + l.words = append(l.words, shellWord{text: l.word.String(), dynamic: l.dynamic}) + } + l.word.Reset() + l.inWord, l.dynamic = false, false +} + +func (l *shellLexer) endCommand() { + l.endWord() + l.skipWord = false + if len(l.words) > 0 { + l.cmds = append(l.cmds, shellCommand{words: l.words}) + } + l.words = nil +} + +// redirection drops a redirection operator and marks its target for +// dropping. A file descriptor number before the operator ("2>") belongs to +// it, not to the command. +func (l *shellLexer) redirection() { + if l.inWord && isDigits(l.word.String()) { + l.word.Reset() + l.inWord, l.dynamic = false, false + } else { + l.endWord() + } + for strings.ContainsRune(">&|", l.peek()) { + l.pos++ + } + l.skipWord = true +} + +// escape adds the rune after a backslash as it is. A backslash before a +// line break joins the lines. +func (l *shellLexer) escape() { + next := l.peek() + if next == 0 { + return + } + l.pos++ + if next != '\n' { + l.add(next) + } +} + +func (l *shellLexer) singleQuoted() { + l.inWord = true + for l.peek() != 0 { + l.pos++ + if l.runes[l.pos] == '\'' { + return } - cmd = nil + l.word.WriteRune(l.runes[l.pos]) } +} - runes := []rune(line) - for i := 0; i < len(runes); i++ { - switch c := runes[i]; { - case c == ' ' || c == '\t': - endWord() - case strings.ContainsRune("\n;&|()", c): - endCommand() - case c == '>' || c == '<': - // A file descriptor number before the operator ("2>") belongs - // to it, not to the command. - if inWord && isDigits(word.String()) { - word.Reset() - inWord, dynamic = false, false - } else { - endWord() - } - for i+1 < len(runes) && strings.ContainsRune(">&|", runes[i+1]) { - i++ - } - skipWord = true - case c == '#' && !inWord: - for i+1 < len(runes) && runes[i+1] != '\n' { - i++ - } - case c == '\\': - if i+1 < len(runes) { - i++ - if runes[i] == '\n' { - continue - } - word.WriteRune(runes[i]) - inWord = true - } - case c == '\'': - inWord = true - for i+1 < len(runes) { - i++ - if runes[i] == '\'' { - break - } - word.WriteRune(runes[i]) - } +func (l *shellLexer) doubleQuoted() { + l.inWord = true + for l.peek() != 0 { + l.pos++ + c := l.runes[l.pos] + switch { case c == '"': - inWord = true - for i+1 < len(runes) { - i++ - d := runes[i] - if d == '"' { - break - } - if d == '\\' && i+1 < len(runes) && strings.ContainsRune("\"\\$`\n", runes[i+1]) { - i++ - word.WriteRune(runes[i]) - continue - } - if d == '$' || d == '`' { - dynamic = true - } - word.WriteRune(d) - } + return + case c == '\\' && strings.ContainsRune("\"\\$`\n", l.peek()): + l.pos++ + l.word.WriteRune(l.runes[l.pos]) default: - if c == '$' || c == '`' { - dynamic = true - } - word.WriteRune(c) - inWord = true + l.dynamic = l.dynamic || c == '$' || c == '`' + l.word.WriteRune(c) } } - endCommand() - - return cmds } func isDigits(s string) bool { diff --git a/app/cli/pkg/action/trace_command_dirs_test.go b/app/cli/pkg/action/trace_command_dirs_test.go index ed8bbe375..e61c09f89 100644 --- a/app/cli/pkg/action/trace_command_dirs_test.go +++ b/app/cli/pkg/action/trace_command_dirs_test.go @@ -22,8 +22,18 @@ import ( ) func TestCommandCheckoutDirs(t *testing.T) { - const cwd = "/work/home" - t.Setenv("HOME", "/users/dev") + const ( + cwd = "/work/home" + home = "/users/dev" + repoB = "/tmp/repo-b" + myRepo = "/tmp/my repo" + cwdRepoB = "/work/home/repo-b" + repoBURL = "https://github.com/o/repo-b" + otherRepo = "/a" + otherSub = "/a/b" + tmp = "/tmp" + ) + t.Setenv("HOME", home) cases := []struct { name string @@ -32,37 +42,41 @@ func TestCommandCheckoutDirs(t *testing.T) { }{ {name: "empty", cmd: "", want: nil}, {name: "no directory", cmd: "make test", want: nil}, - {name: "cd absolute", cmd: "cd /tmp/repo-b && git commit -m x", want: []string{"/tmp/repo-b"}}, + {name: "cd absolute", cmd: "cd " + repoB + " && git commit -m x", want: []string{repoB}}, {name: "cd relative to cwd", cmd: "cd sub && ls", want: []string{"/work/home/sub"}}, - {name: "cd relative to previous cd", cmd: "cd /a; cd b", want: []string{"/a", "/a/b"}}, + {name: "cd relative to previous cd", cmd: "cd /a; cd b", want: []string{otherRepo, otherSub}}, {name: "cd home", cmd: "cd ~/src/x && ls", want: []string{"/users/dev/src/x"}}, - {name: "cd on its own line", cmd: "cd /a\ngit commit", want: []string{"/a"}}, - {name: "cd in subshell", cmd: "(cd /a && make)", want: []string{"/a"}}, - {name: "pushd", cmd: "pushd /a >/dev/null && git status", want: []string{"/a"}}, - {name: "quoted path", cmd: `cd "/tmp/my repo" && git commit`, want: []string{"/tmp/my repo"}}, - {name: "single-quoted path", cmd: `cd '/tmp/my repo'`, want: []string{"/tmp/my repo"}}, - {name: "escaped space", cmd: `cd /tmp/my\ repo`, want: []string{"/tmp/my repo"}}, + {name: "bare cd goes home", cmd: "cd && git clone " + repoBURL, want: []string{home, "/users/dev/repo-b"}}, + {name: "cd on its own line", cmd: "cd /a\ngit commit", want: []string{otherRepo}}, + {name: "cd in subshell", cmd: "(cd /a && make)", want: []string{otherRepo}}, + {name: "cd in subshell does not leak", cmd: "(cd /tmp) && git clone " + repoBURL, want: []string{tmp, cwdRepoB}}, + {name: "nested subshells", cmd: "(cd /a && (cd b) && git clone " + repoBURL + ")", want: []string{otherRepo, otherSub, "/a/repo-b"}}, + {name: "pushd", cmd: "pushd /a >/dev/null && git status", want: []string{otherRepo}}, + {name: "quoted path", cmd: `cd "` + myRepo + `" && git commit`, want: []string{myRepo}}, + {name: "single-quoted path", cmd: `cd '` + myRepo + `'`, want: []string{myRepo}}, + {name: "escaped space", cmd: `cd /tmp/my\ repo`, want: []string{myRepo}}, {name: "cd text in an argument", cmd: `echo "cd /tmp/x"`, want: nil}, {name: "comment", cmd: "ls # cd /tmp/x", want: nil}, {name: "cd with variable", cmd: "cd $REPO && git commit", want: nil}, {name: "relative cd after unknown cd", cmd: "cd $REPO && cd sub", want: nil}, - {name: "absolute cd after unknown cd", cmd: "cd $REPO && cd /a", want: []string{"/a"}}, + {name: "absolute cd after unknown cd", cmd: "cd $REPO && cd /a", want: []string{otherRepo}}, {name: "cd back", cmd: "cd - && ls", want: nil}, - {name: "duplicates", cmd: "cd /a && git -C /a commit", want: []string{"/a"}}, - {name: "git -C", cmd: `git -C /tmp/repo-b commit -m "x"`, want: []string{"/tmp/repo-b"}}, - {name: "git -C twice", cmd: "git -C /a -C b status", want: []string{"/a/b"}}, - {name: "git -c then -C", cmd: "git -c user.name=x -C /a commit", want: []string{"/a"}}, - {name: "env assignment", cmd: "GIT_TRACE=1 git -C /a status", want: []string{"/a"}}, + {name: "duplicates", cmd: "cd /a && git -C /a commit", want: []string{otherRepo}}, + {name: "git -C", cmd: `git -C ` + repoB + ` commit -m "x"`, want: []string{repoB}}, + {name: "git -C twice", cmd: "git -C /a -C b status", want: []string{otherSub}}, + {name: "git -c then -C", cmd: "git -c user.name=x -C /a commit", want: []string{otherRepo}}, + {name: "env assignment", cmd: "GIT_TRACE=1 git -C /a status", want: []string{otherRepo}}, + {name: "env assignment with expansion", cmd: `FOO="$HOME" git -C /a status`, want: []string{otherRepo}}, {name: "git without -C", cmd: "git commit -m x", want: nil}, - {name: "git clone with dir", cmd: "git clone git@github.com:o/repo-b.git /tmp/repo-b", want: []string{"/tmp/repo-b"}}, - {name: "git clone without dir", cmd: "git clone https://github.com/o/repo-b.git", want: []string{"/work/home/repo-b"}}, - {name: "git clone without dir, trailing slash", cmd: "git clone /srv/git/repo-b/", want: []string{"/work/home/repo-b"}}, - {name: "git clone after cd", cmd: "cd /tmp && git clone --depth 1 -b main https://github.com/o/repo-b", want: []string{"/tmp", "/tmp/repo-b"}}, - {name: "git clone option with equals", cmd: "git clone --branch=main https://github.com/o/repo-b dir", want: []string{"/work/home/dir"}}, - {name: "git clone with redirection", cmd: "git clone https://github.com/o/repo-b /tmp/b 2>&1 | tail -1", want: []string{"/tmp/b"}}, - {name: "git -C clone", cmd: "git -C /tmp clone https://github.com/o/repo-b", want: []string{"/tmp", "/tmp/repo-b"}}, - {name: "gh repo clone with dir", cmd: "gh repo clone o/repo-b /tmp/repo-b -- --depth 1", want: []string{"/tmp/repo-b"}}, - {name: "gh repo clone without dir", cmd: "gh repo clone o/repo-b", want: []string{"/work/home/repo-b"}}, + {name: "git clone with dir", cmd: "git clone git@github.com:o/repo-b.git " + repoB, want: []string{repoB}}, + {name: "git clone without dir", cmd: "git clone " + repoBURL + ".git", want: []string{cwdRepoB}}, + {name: "git clone without dir, trailing slash", cmd: "git clone /srv/git/repo-b/", want: []string{cwdRepoB}}, + {name: "git clone after cd", cmd: "cd /tmp && git clone --depth 1 -b main " + repoBURL, want: []string{tmp, repoB}}, + {name: "git clone option with equals", cmd: "git clone --branch=main " + repoBURL + " dir", want: []string{"/work/home/dir"}}, + {name: "git clone with redirection", cmd: "git clone " + repoBURL + " /tmp/b 2>&1 | tail -1", want: []string{"/tmp/b"}}, + {name: "git -C clone", cmd: "git -C /tmp clone " + repoBURL, want: []string{tmp, repoB}}, + {name: "gh repo clone with dir", cmd: "gh repo clone o/repo-b " + repoB + " -- --depth 1", want: []string{repoB}}, + {name: "gh repo clone without dir", cmd: "gh repo clone o/repo-b", want: []string{cwdRepoB}}, {name: "gh other command", cmd: "gh pr create --title x", want: nil}, } diff --git a/app/cli/pkg/action/trace_shell_checkout_test.go b/app/cli/pkg/action/trace_shell_checkout_test.go index be50dfaf0..c9d464e48 100644 --- a/app/cli/pkg/action/trace_shell_checkout_test.go +++ b/app/cli/pkg/action/trace_shell_checkout_test.go @@ -85,10 +85,11 @@ func TestClaudeSessionCommitsInClonedCheckoutThroughShell(t *testing.T) { require.NoError(t, HandleAgentSessionStart(p, zerolog.Nop())) // 1. The agent writes a file in a folder that is not a git repository. + const specBody = "# Spec\n\nline\n" spec := filepath.Join(notes, "spec.md") - writeInput := fmt.Sprintf(`{"file_path":%q,"content":"# Spec\n\nline\n"}`, spec) + writeInput := fmt.Sprintf(`{"file_path":%q,"content":%q}`, spec, specBody) hook("PreToolUse", "Write", "toolu_w1", writeInput, HandleAgentPreToolUse) - require.NoError(t, os.WriteFile(spec, []byte("# Spec\n\nline\n"), 0600)) + require.NoError(t, os.WriteFile(spec, []byte(specBody), 0600)) hook("PostToolUse", "Write", "toolu_w1", writeInput, HandleAgentPostToolUse) // 2. The agent clones the repository. The clone does not exist at the @@ -108,9 +109,7 @@ func TestClaudeSessionCommitsInClonedCheckoutThroughShell(t *testing.T) { // 3. The agent copies the file into the clone with a shell command. bash("toolu_b2", fmt.Sprintf("cd %s && cp %s docs/", clone, spec), func() { - content, err := os.ReadFile(spec) - require.NoError(t, err) - require.NoError(t, os.WriteFile(filepath.Join(clone, "docs", "spec.md"), content, 0600)) + require.NoError(t, os.WriteFile(filepath.Join(clone, "docs", "spec.md"), []byte(specBody), 0600)) }) cloneStore := state.NewGitStore(cloneGitDir)