Skip to content

archivist: resolve trunk root before searching .notes/ (closes #59)#68

Merged
SnowboardTechie merged 3 commits into
mainfrom
issue-59-archivist-worktree-fix
May 1, 2026
Merged

archivist: resolve trunk root before searching .notes/ (closes #59)#68
SnowboardTechie merged 3 commits into
mainfrom
issue-59-archivist-worktree-fix

Conversation

@SnowboardTechie
Copy link
Copy Markdown
Owner

Summary

The archivist agent's prompt body searched .notes/ with bare relative paths, so every Glob/Grep failed when the agent ran inside a git worktree (the .notes symlink lives only in the trunk). Six parallel archivist calls during #57's /pr-self-review Phase 1.2 all returned "vault not accessible" — the regression #59 documented.

This PR grants archivist Bash and adds a Startup section that runs git rev-parse --path-format=absolute --git-common-dir once per invocation, drops the trailing /.git, and uses the result as \$TRUNK_ROOT for every search-strategy path. The single command is equivalent to the canonical two-step resolve_trunk_root from agent-workspace (--git-common-dir always points at the trunk's .git from either side), expressed inline because archivist's bash-hygiene rule forbids shell functions and pipes.

The five archivist callers (pr-self-review, session-review, meeting-sync, workday-planning, athena.md) stay untouched — their existing "archivist owns path resolution" contract is now actually true.

The plan considered Option B (each caller passes `trunk_root:` in the prompt) and Option C (hybrid) but went with Option A (agent self-resolves) for blast-radius reasons: five caller-site edits avoided, existing `workday-planning` contract preserved, and the new Bash surface is bounded to `git rev-parse` by archivist's existing hygiene rule.

Test plan

No agent test harness exists in the repo, so verification is manual.

  • Direct archivist invocation from this worktree (executed during `/pr-self-review` Phase 1.2 of this PR's own self-review): pre-fix returned "vault not accessible" string responses; post-fix archivist resolved the trunk, located the project vault, and scanned end-to-end. Result was "0 matches" only because the project vault is genuinely empty of related-topic content — the resolution itself succeeded.
  • `git rev-parse --path-format=absolute --git-common-dir` returns the trunk's `.git` from both the trunk and a worktree on this machine — verified live during planning.
  • Three-lens self-review (correctness / security / simplicity) ran clean after one fix-up commit. Pass 1 surfaced 1 Major + 5 Minor + 2 Nit; all 7 actionable findings accepted in the second commit. Pass 2 surfaced two non-blocker wording nits, both intentionally skipped.

Acceptance criteria from #59:

  • ✅ `@archivist` invoked from a worktree successfully reads `.notes/` content from the trunk.
  • ✅ At least one existing skill-invocation path returns archivist results from the resolved vault — `/pr-self-review` Phase 1.2 of this PR demonstrates it.
  • ✅ `archivist.md` references the `agent-workspace` trunk-resolution pattern explicitly in the new Startup section.

Changelog

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

Grant archivist Bash and add a Startup section that runs
`git rev-parse --path-format=absolute --git-common-dir` once per
invocation, dropping the trailing `/.git` to get $TRUNK_ROOT. Every
search-strategy path becomes $TRUNK_ROOT/.notes/... so Glob/Grep work
from a worktree, where the bare `.notes/` symlink doesn't exist.

The five archivist callers (pr-self-review, session-review, meeting-sync,
workday-planning, athena.md) stay untouched — their existing "archivist
owns path resolution" contract is now actually true.

Replaces the misleading "this is transparent to you" claim with a pointer
to the canonical resolve_trunk_root pattern in agent-workspace.
Pre-PR review pass against the trunk-resolution fix surfaced one Major
and several Minor findings; all addressed in this commit.

- Clarify in Startup why the single command (--path-format=absolute
  --git-common-dir) is equivalent to the two-step canonical resolve —
  --git-common-dir always points at the trunk's .git from either side.
  Prevents an LLM-archivist from second-guessing the shortcut and
  re-introducing a conditional branch.
- Spec three error paths instead of one: Bash failed, output didn't
  end in /.git (bare repo / unrecognized layout), or the resolved
  \$TRUNK_ROOT/.notes/ doesn't exist. Each gets a distinct message so
  callers can tell "no matches" from "no vault."
- Trim the Smoke-test paragraph to a one-line parenthetical for
  editors (the plan asked for one line; original draft was five).
- Prefix scope-filtering prose with \$TRUNK_ROOT so the text matches
  the strategy code blocks.
- Collapse Notes Architecture Awareness to a single pointer line —
  Startup already states the rationale; the bottom section was a
  near-verbatim repeat.
@SnowboardTechie SnowboardTechie added bug Something isn't working quick win Small, self-contained scope — good candidate for a focused session labels May 1, 2026
@SnowboardTechie SnowboardTechie marked this pull request as ready for review May 1, 2026 20:07
@SnowboardTechie SnowboardTechie merged commit 4f4a826 into main May 1, 2026
3 checks passed
@SnowboardTechie SnowboardTechie deleted the issue-59-archivist-worktree-fix branch May 1, 2026 20:07
SnowboardTechie added a commit that referenced this pull request May 1, 2026
)

## Summary

Two follow-up edits from `/code-review` on
[#68](#68) — the PR
that landed the original
[#59](#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`](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

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

## Changelog

- [x] **Non-release PR** — added a bullet under `## [Unreleased]` in
`CHANGELOG.md`.
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

bug Something isn't working quick win Small, self-contained scope — good candidate for a focused session

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant