Repository navigation
fix(trace): attribute commits made inside Bash calls - #3631
waveywaves wants to merge 3 commits into
Conversation
AI Session Checks —
|
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
iac-scan |
no IaC files changed |
Security context
This change touches code with 4 recorded security-fix advisories. These are pointers to what past fixes established, not findings in this diff, and they never fail the check.
View in Chainloop ↗ · How this works ↗
Powered by Chainloop and Chainloop Trace
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
View guided diff | Re-trigger cubic
d65514e to
26f4d6d
Compare
|
@jiparis can you take a look at this one? |
Tag the exact Claude Bash call so commit-msg can attribute staged content before PostToolUse runs. Assisted-by: Vibhav's Pi Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
26f4d6d to
0b7e872
Compare
Honor the effective Git index, preserve recreated paths after staged deletions, and distinguish conflict-stage signatures. Assisted-by: Vibhav's Pi Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
View guided diff | Re-trigger cubic
|
Thanks for this fix. I tested the branch in real headless Claude Code sessions (Claude Code 2.1.301, Results
Scenarios and suggested approach1. Permission check on the rewritten command (blocker)Claude Code applies Suggested approach: do not rewrite the command. Use the snapshot files that this PR already writes (
There is no Notes:
2. Another hook that also returns
|
| Current PR | Minimal version | |
|---|---|---|
| Bash calls with a narrow allowlist | Denied, or a prompt on each call | No change in permissions |
| Another hook that rewrites the command | Trailer lost, with no warning | No conflict |
| Token in the transcript and in prompts | Yes, on each Bash call | No |
| Attribution precision | Exact call | Session. This is sufficient for the trailer. Line ranges can be less exact when calls overlap in one session. |
| Dependency | updatedInput (documented) |
CLAUDE_CODE_SESSION_ID (not documented, as far as we know). If Claude Code stops setting it, the behavior is the same as on the base commit. |
| Background Bash calls | Not attributed | Not attributed |
Estimated size of the change:
| Change | Size |
|---|---|
Delete TagShellCommand, ShellCommandTagger, RawToolInput, the ReadHookInput changes, EncodeShellCallKey/DecodeShellCallKey, and the ShellCallEnv export |
Approximately −100 lines |
Add store.OpenShellSnapshotKeys(sessionID). It lists the shell-index-* files in snapshots/<session>/ and makes each key from the file name. |
Approximately +30 lines |
In checkpointActiveShellAttribution, read the environment variable instead of the token, and do the checkpoint for each key. The diff and checkpoint logic does not change. |
Approximately +15 lines |
Tests: set CLAUDE_CODE_SESSION_ID instead of the token. Add one test with two overlapping calls in the same session. |
Small |
The snapshot, index, checkpoint and cleanup logic of this PR stays. Thus we expect the result to be smaller than the current PR.
Other items
- The
Chainloop AI PoliciesandChainloop PR Validationchecks fail. A possible cause is theAssisted-by: Vibhav's Pitrailer. We did not examine the failure details. - The repository rules require an update to the feature table and the compatibility matrix in
app/cli/internal/trace/README.mdwhen a Trace feature changes. This PR does not change that file.
Claude Code helped with this review.
Read staged blob content from the exact object recorded in the index, even when refs/replace maps it to another object. Assisted-by: Vibhav's Pi Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
| } | ||
|
|
||
| status, err := wt.Status() | ||
| output, err := commandOutput(exec.Command("git", "-C", repoRoot, "diff", "--cached", |
There was a problem hiding this comment.
This is gogit implementation, so this command should be run with the library.
There was a problem hiding this comment.
Also, why are these changes needed? wasn't StagedFiles working properly before?
|
|
||
| // SnapshotIndex returns the content signature currently staged in git's index. | ||
| // It excludes symlinks and submodules to match SnapshotWorktree's file scope. | ||
| func (c *GoGitClient) SnapshotIndex(repoRoot string) (map[string]string, error) { |
There was a problem hiding this comment.
All these methods belong to GoGitClient, however use exec.Command("git"...) which would belong to the Exec implementation (not used in production).
|
Thanks @waveywaves, sorry for the last long ie-vibed response.
|
Closes #3625
Claude Code can edit, stage, commit, and push within one Bash tool call. The
commit-msghook runs before Claude'sPostToolUsehook, so the staged files previously had no recorded AI attribution and the commit lost itsChainloop-Trace-Sessionstrailer.This change:
tool_inputcommit-msgattribute the exact staged blob changes from that call beforePostToolUseTesting
go test -race -shuffle=on -count=1 ./internal/trace/state ./internal/trace/claude ./internal/trace/git ./pkg/actionGOTOOLCHAIN=go1.27.2 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.14.0 run --new-from-rev=upstream/maingo build ./...commit-msgCLI help and invalid-argument pathsAI disclosure: this change was made with Vibhav's Pi.