Skip to content

archivist: tool-native vault check; {TRUNK_ROOT} placeholder syntax#72

Merged
SnowboardTechie merged 1 commit into
mainfrom
archivist-followup-issue-59
May 1, 2026
Merged

archivist: tool-native vault check; {TRUNK_ROOT} placeholder syntax#72
SnowboardTechie merged 1 commit into
mainfrom
archivist-followup-issue-59

Conversation

@SnowboardTechie
Copy link
Copy Markdown
Owner

Summary

Two follow-up edits from /code-review on #68 — the PR that landed the original #59 fix.

  • Error path 3 — vault-not-found check now specifies Glob(pattern="{TRUNK_ROOT}/.notes") rather than leaving the verification mechanism implicit. Bash is reserved for trunk-resolution per the agent's hygiene rule, so the existence check has to be tool-native; without explicit guidance a haiku-class model could skip the check or reach for Bash again.
  • Placeholder syntax — switched $TRUNK_ROOT{TRUNK_ROOT} across the Startup section and all five search-strategy code blocks, matching the convention in plugins/athena-notes/skills/agent-workspace/SKILL.md. Both forms work for LLM substitution; mixed conventions inside one plugin invite future drift.

No behavior change beyond what archivist's prompt instructs the LLM to do — instruction-text tightenings only.

Test plan

  • Reviewed diff line by line; both edits match the suggestions in the #68 review thread.
  • grep -n '\$TRUNK_ROOT' against plugins/athena-notes/agents/archivist.md returns zero matches; {TRUNK_ROOT} appears 17 times.

Changelog

  • Non-release PR — added a bullet under ## [Unreleased] in CHANGELOG.md.

Two follow-up fixes from /code-review on #68:

- Error path 3 (vault not found) now specifies a Glob check —
  Bash is reserved for trunk-resolution per the agent's hygiene
  rule, so the existence check has to be tool-native. Without
  explicit guidance a haiku-class model could either skip the
  check or reach for Bash again.
- Placeholder syntax switched from $TRUNK_ROOT to {TRUNK_ROOT}
  to match skills/agent-workspace/SKILL.md. Both forms work for
  LLM substitution, but mixed conventions inside one plugin
  invite future drift.

Follow-up to #59.
@SnowboardTechie SnowboardTechie merged commit 61bfe60 into main May 1, 2026
3 checks passed
@SnowboardTechie SnowboardTechie deleted the archivist-followup-issue-59 branch May 1, 2026 20:23
SnowboardTechie added a commit that referenced this pull request May 3, 2026
…loses #61) (#76)

## Summary

`issue-work` Phase 1.6 instructed the LLM to call `EnterWorktree` with a
`base_branch` parameter that doesn't exist on the tool. Result: every
worktree created by `/issue-work` branched off the session's current
HEAD instead of the default branch — silently wrong whenever the skill
was invoked from inside another worktree (the common case in this repo)
or from a trunk parked on a feature branch.

This PR rewrites Phase 1.6 to a three-step pattern: resolve
`{TRUNK_ROOT}` (now bound explicitly at the end of Phase 1.4), run `git
-C "{TRUNK_ROOT}" worktree add -b {branch} {path}
"origin/$DEFAULT_BRANCH"` against the trunk, then `EnterWorktree(path:
...)` to switch in. Mirrors the precedent set by
[#68](#68) and the
`{TRUNK_ROOT}` placeholder convention from
[#72](#72). Adjacent
drift fixed alongside: Phase 1.5's two `$TRUNK` shell-var references
rename to `{TRUNK_ROOT}` for consistency, and
`references/repo-resolution.md:113` corrects its description of
`EnterWorktree`'s contract.

## Test plan

- [x] `frontmatter-lint.yml` — runs locally clean (14 agents, 18 skills
validated)
- [x] `docs-lint.yml` — relative trailing-slash pattern returns no
matches
- [x] `version-check.yml` — `CHANGELOG.md` entry under `[Unreleased] /
Fixed`; `plugin.json` version unchanged so non-release path applies
- [ ] **Smoke test (post-merge, per ticket AC):** invoke `/issue-work
{ticket}` from inside another worktree, confirm `git log --oneline -1
issue-{N}-{slug}^` resolves to the tip of `origin/main`

Three review passes via `/pr-self-review` (correctness / security /
simplicity) ran in pre-pr mode after the initial commit; 8 findings
accepted in-loop, 2 pushed back as out-of-scope (kebab-slug
path-traversal tightening — threat model is the user's own tickets, `git
check-ref-format` rejects `..`-style branch names; 60-char cap
truncation under-spec — over-specifying for a non-issue). No critical or
major findings outstanding. Full triage record in the state dir's
`summary.md`.

Acceptance criteria from #61, all met:

- Phase 1.6 no longer references a non-existent `EnterWorktree`
`base_branch` or `branch_name` parameter.
- `{TRUNK_ROOT}` bound explicitly in Phase 1.4 before `git worktree add`
runs.
- Default branch pinned via `origin/\$DEFAULT_BRANCH`.
- Resume case documented in Phase 1.6 + the Edge Cases table row.

## Changelog

- [x] **Non-release PR** — added a bullet under \`## [Unreleased]\` /
\`### Fixed\` in \`CHANGELOG.md\`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant