Port: port/pr-32-add-managedby-tag

[briansumma]

Ported from upstream cytostack/openwolf. Needs review before merge.

[briansumma]

Now I have a complete picture. Here's my review:

---

PR #8 — Port: _managedBy Tag on Hook Entries

State: DRAFT | Author: briansumma | Changes: +32 / -12 across src/cli/init.ts and src/cli/update.ts

---

Overview

This ports upstream cytostack/openwolf#31, adding a _managedBy: "openwolf" tag to every hook object written to Claude Code's settings.json. The goal: let replaceOpenWolfHooks() identify its own entries by tag (primary) rather than by path substring (fallback). The fallback .wolf/hooks/ substring match is preserved for backward compatibility with pre-tag installs.

The logic is structurally sound and the two-file change is consistent. No blockers. A few things worth your attention before merging.

---

⚠️ Warnings

1. Silent custom-hook loss on co-located entries (pre-existing, but worth documenting)

The filter removes the entire outer matcher entry if any inner hook matches — either by tag or by path. Both pre- and post-PR behavior.

// entry = { matcher: "", hooks: [...inner hooks...] }
const isOpenWolfHook = entry.hooks?.some(
  (h) => h._managedBy === "openwolf" || h.command?.includes(".wolf/hooks/")
);
return !isOpenWolfHook; // drops ENTIRE entry if any inner hook matches

If a user hand-edited settings.json to add a second command to the same outer SessionStart entry as an OpenWolf hook, that second command gets silently dropped on the next openwolf init/update. This PR doesn't make it worse, but doesn't fix it either. Consider a comment in replaceOpenWolfHooks documenting this assumption: "OpenWolf writes exactly one inner hook per outer entry; co-location with user hooks is unsupported."

2. Undocumented reliance on Claude Code's settings round-tripper

The comment in update.ts (lines 46–49) asserts that Claude Code's settings round-tripper "preserves entries with this provenance tag" — but _managedBy is not a documented Claude Code field. If Claude Code performs schema-validated serialization in a future release and strips unknown fields, the tag silently disappears from settings.json and this entire mechanism falls back to the .wolf/hooks/ substring match with no warning. The PR is betting on observed behavior that isn't guaranteed.

Recommendation: Either add a comment noting this is an empirically observed behavior (not a documented guarantee), or add a verify step in openwolf update that checks whether _managedBy survived the last Claude Code round-trip and warns if the tag is absent on entries it wrote.

---

ℹ️ Info

3. Code duplication between init.ts and update.ts

HOOK_SETTINGS and replaceOpenWolfHooks() are defined separately in both files. This PR correctly updates both — they are in sync now. But any future hook change needs two edits in two files. The risk is they drift again. This is pre-existing tech debt, not introduced here, but the PR is a good opportunity to note it.

4. Inconsistent formatting of HOOK_SETTINGS

• init.ts: multi-line, expanded style (matches project's 80-char rule)
• update.ts: compressed single-line style (lines 52–62 exceed 80 chars significantly)

Both were pre-existing, but this PR adds _managedBy to the single-line form in update.ts, making those lines even longer. Minor, but yapf/prettier equivalents would flag this in a Python/JS project. If you run prettier on the TS files, it'll reformat update.ts.

---

✅ What's Correct

• Tag placement is right: _managedBy goes on the inner hook object (h), and the filter checks h._managedBy — structurally consistent.
• Backward compatibility is handled: legacy substring match kept as fallback.
• Both files updated consistently — init and update agree on tag value and filter logic.
• The change is minimal and surgical. No unrelated edits.

---

Verdict

Approve with comments — logic is correct, backward-compatible, and the two-file sync is clean. Address Warning #2 (document the round-tripper assumption) before merging out of DRAFT. Warning #1 and the Info items can be tracked separately.

[briansumma]

Autofixer skipped: merge conflicts with main detected.

[briansumma]

Autofixer skipped: merge conflicts with main detected.

[briansumma]

Autofixer skipped: merge conflicts with main detected.

[briansumma]

Autofixer skipped: merge conflicts with main detected.

[briansumma]

✅ Autofixer applied. Build and tests passed. Promoting to Ready for Review.