Skip to content

Add Crow-Session attribution trailer to commits#249

Merged
dhilgaertner merged 1 commit intomainfrom
feature/crow-246-attribution-trailers
May 7, 2026
Merged

Add Crow-Session attribution trailer to commits#249
dhilgaertner merged 1 commit intomainfrom
feature/crow-246-attribution-trailers

Conversation

@dhilgaertner
Copy link
Copy Markdown
Contributor

Summary

  • setup.sh writes a per-worktree .claude/settings.local.json that overrides Claude Code's native attribution.commit to include a Crow-Session: <session-uuid> trailer alongside the standard Co-Authored-By: Claude line. The session UUID is a stable handle back to session metadata via crow get-session <uuid>.
  • New top-level attributionTrailers flag on AppConfig (default true) toggles the behavior. Exposed in Settings → Automation → Attribution.
  • setup.sh also appends .claude/settings.local.json to the per-worktree git exclude file so the override never gets committed accidentally, even when the repo's tracked .gitignore doesn't already cover it.

The implementation switched from the originally-proposed CLAUDE.md / prompt-injection approach to Claude Code's native attribution.commit setting after research — it's deterministic (no relying on the LLM to remember an instruction every commit) and survives prompt compaction by virtue of being a config file Claude Code re-reads.

The trailer body chosen:

🤖 Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: <UUID>

Test plan

  • make app builds cleanly
  • swift test --package-path Packages/CrowCore (145 tests, including 2 new ones)
  • swift test --package-path Packages/CrowPersistence (24 tests)
  • Smoke tested write_settings_local against a scratch git worktree:
    • default-on (no config) writes the file with the real UUID interpolated
    • "attributionTrailers": false skips the file
    • whitespace-tolerant config parsing ("attributionTrailers" : false)
    • per-worktree exclude entry added; git status reports the file as untracked-and-excluded
    • generated JSON parses cleanly via python3 -c 'json.load(...)' and decodes to the expected three-line trailer body
  • Manual end-to-end check on a real workspace: launch a crow session, ask Claude to commit, verify git log -1 --format='%B' shows both Co-Authored-By: Claude and Crow-Session: <real-uuid> lines
  • Settings UI smoke test: Settings → Automation → Attribution toggle persists to {devRoot}/.claude/config.json

Closes #246

Wire setup.sh to write a per-worktree .claude/settings.local.json that
overrides Claude Code's attribution.commit so commits include a
Crow-Session: <uuid> trailer alongside the standard Co-Authored-By:
Claude line. The session UUID is a stable handle back to crow session
metadata via `crow get-session <uuid>`.

A new top-level attributionTrailers flag on AppConfig (default true)
toggles the behavior, exposed in Settings → Automation. setup.sh adds
the local settings file to the per-worktree git exclude list so it
never gets committed accidentally.

Closes #246

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@dgershman dgershman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Critical Issues

None blocking.

Security Review

Strengths:

  • The settings.local.json is excluded from version control via the per-worktree git exclude file, preventing accidental commits of session metadata.
  • The is_attribution_trailers_enabled function defaults to on (return 0) when the config is missing or malformed, matching AppConfig's decodeIfPresent default — good parity.
  • The write_settings_local function gracefully handles missing SESSION_ID and disabled config without failing the overall setup.

Concerns:

  • setup.sh:349-354SESSION_ID is interpolated directly into a heredoc without sanitization. When supplied via --session-id from the CLI, a malicious value could inject arbitrary keys into the JSON. In practice, the value comes from crow new-session (a trusted binary), and the --session-id path is only used internally by the LLM orchestrator, so the risk is low. Still, a UUID format check (e.g., [[ "$SESSION_ID" =~ ^[a-f0-9-]{36}$ ]]) before interpolation would be a cheap hardening measure.
  • setup.sh:83-84 — The grep-based config parsing (is_attribution_trailers_enabled) searches for a literal string pattern rather than parsing JSON. A contrived config with "attributionTrailers": false inside a string value (e.g., a "note" field) would produce a false negative. Using jq would be more robust, but the current approach is consistent with the existing is_remote_control_enabled pattern and unlikely to cause issues with normal Crow-managed configs.

Code Quality

  • AppConfig.swift: Clean addition following the established pattern — decodeIfPresent with a default, CodingKeys updated, init parameter added. Consistent with experimentalTmuxBackend which was the last addition.
  • Tests: Two new tests (appConfigAttributionTrailersRoundTrip, appConfigAttributionTrailersDefaultsTrueWhenKeyMissing) cover the round-trip and legacy-config-upgrade paths. The empty-JSON decode test also asserts the new default. All 145 CrowCore tests pass.
  • UI wiring: AutomationSettingsView correctly takes an attributionTrailers binding and saves on toggle. SettingsView passes it through. The help text ("Applies to new worktrees only") is accurate and sets user expectations correctly.
  • SKILL.md: Documentation is well-placed and includes opt-out instructions.
  • setup.sh: The write_settings_local function is well-structured — it runs for all worktrees (not just --primary), handles edge cases gracefully, and the git-exclude logic is idempotent (checks grep -qxF before appending).

Summary Table

Priority Issue
🟡 Yellow Consider adding UUID format validation for SESSION_ID before JSON interpolation in write_settings_local (setup.sh:338-339)
🟢 Green Consider using jq for config flag parsing instead of grep (lower priority — consistent with existing is_remote_control_enabled pattern)

Recommendation: Approve — clean, well-tested feature with good UX (default-on, easy opt-out, clear help text). The yellow item is a hardening suggestion for a future pass, not a blocker.

@dhilgaertner dhilgaertner merged commit 8086084 into main May 7, 2026
3 checks passed
@dhilgaertner dhilgaertner deleted the feature/crow-246-attribution-trailers branch May 7, 2026 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

design: add crow attribution to Claude Code commits

2 participants