Skip to content

fix: create-task asks for the vault instead of silently defaulting - #295

Merged
bborbe merged 5 commits into
masterfrom
fix/create-task-vault-resolution
Oct 15, 2026
Merged

bborbe merged 5 commits into
masterfrom
fix/create-task-vault-resolution

Conversation

@bborbe

@bborbe bborbe commented Oct 15, 2026

Copy link
Copy Markdown
Owner

/vault-cli:create-task and its task-creator agent both fell back silently to the configured default vault, so a task belonging to one vault could land in another by inference — and the fallback is a third vault again, wrong for work belonging to any other.

What changes

  • commands/create-task.md — new Resolve the vault section: use --vault, or the vault the cwd sits in; when neither determines it, ask. Never infer from the topic, never fall through to default_vault.
  • agents/task-creator.md — the constraint, the Defaults: line and §2 all lose the silent default; the caller resolves the vault.

This matches the convention two siblings already carry:

  • commands/reflect.md — "If cwd is not inside any vault path, ask the user which vault to write to (AskUserQuestion with vault names from the config)."
  • agents/graph-auditor.md — "ask user which vault to audit; do not guess."

Why

Observed 2026-10-15: a task belonging to seibert-personal was created in seibert-brogrammers by inference and had to be deleted and re-created — two task-creator runs (~105k tokens).

Verification

make precommit passes locally (format · generate · test · check · addlicense).

@bborbe
bborbe marked this pull request as ready for review October 15, 2026 15:41

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No scenario touches create-task or task-creator. The changed files are all command/agent definitions — no binary-relevant Go code changed. Per the scenario-skip rule, no run needed.

Review complete

Changes reviewed (3 files, all non-Go):

File What changed
commands/create-task.md Added "Resolve the vault" section with explicit 3-case logic: --vault given, cwd in vault, or neither (ask). MODE=non_interactive now fails headlessly instead of falling through to default_vault.
agents/task-creator.md Constraint updated: --vault or ask when absent, never default_vault. Step 2 notes same. Defaults: line updated to remove the VAULT = configured default vault statement.
CHANGELOG.md ## Unreleased bullet added with full incident write-up (observed wrong-vault task creation on 2026-10-15, ~105k tokens wasted).

Correctness concerns from the plan:

  • commands/create-task.md: The documented contract is explicit and unambiguous — three cases, no fallthrough, non_interactive fails rather than guesses. ✅
  • agents/task-creator.md: Both the constraint and the step-2 note now say "ask when absent / never fall back to default_vault". ✅

Version alignment: All four files agree on 0.169.0. ✅

No issues found.


{
  "verdict": "approve",
  "summary": "Fix correctly replaces the silent `default_vault` fallback with explicit ask-or-fail logic in both the command and the agent. The contract is documented in three clear cases, consistent with sibling conventions (`reflect.md`, `graph-auditor.md`), and the CHANGELOG entry is thorough.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "correctness: commands/create-task.md — new vault resolution logic honoring the documented ask/fail contract",
      "disposition": "addressed",
      "detail": "Verified: 'Resolve the vault' section (lines 19-25) provides three explicit cases, no fallthrough to default_vault, and non_interactive mode fails with a clear error message"
    },
    {
      "concern": "correctness: agents/task-creator.md — agent passes vault through when caller provides it, prompts when absent, rather than falling back to default_vault",
      "disposition": "addressed",
      "detail": "Verified: constraint (line 20) and step 2 note (line 48) both state 'never fall through to default_vault' and 'ask the caller when it is absent'"
    }
  ]
}

@ben-s-pull-request-reviewer
ben-s-pull-request-reviewer Bot dismissed their stale review October 15, 2026 16:10

superseded by new automated review

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

{
  "verdict": "request-changes",
  "summary": "PR #295 fixes a vault-resolution bug in create-task but introduces a version-alignment violation: it reverts plugin.json and marketplace.json from v0.169.1 to v0.169.0, stranding the plugin one release behind the already-released vault-cli binary. The CHANGELOG adds an Unreleased section but never bumps to a new version, so the merged state has mismatched version strings that `make precommit`'s `check-versions` target would catch post-merge only if the manifests had already been bumped — they have not been.",
  "comments": [
    {
      "file": ".claude-plugin/plugin.json",
      "line": 4,
      "severity": "critical",
      "blocking": true,
      "blocking_reason": "post-merge `make precommit` will fail the `check-versions` gate: CHANGELOG.md has no v0.169.2 section, and plugin.json/marketplace.json both read 0.169.0 while the binary is already at 0.169.1 on master",
      "message": "Version regresses from 0.169.1 (master) to 0.169.0. The vault-cli binary was already released at v0.169.1 on master (commit 3d4b003); the plugin manifests must not go backwards. Either bump all four version strings to 0.169.2 (Unreleased → v0.169.2 in CHANGELOG, all three JSON fields to 0.169.2), or — since this is a bugfix PR targeting the already-released 0.169.1 state — cherry-pick onto origin/master after the release commit and bump from there."
    },
    {
      "file": ".claude-plugin/marketplace.json",
      "line": 8,
      "severity": "critical",
      "blocking": true,
      "blocking_reason": "same version-alignment violation as plugin.json: metadata.version reverts to 0.169.0 while CHANGELOG has no corresponding v0.169.2 entry",
      "message": "metadata.version regresses from 0.169.1 to 0.169.0 — same issue as plugin.json."
    },
    {
      "file": ".claude-plugin/marketplace.json",
      "line": 15,
      "severity": "critical",
      "blocking": true,
      "blocking_reason": "same version-alignment violation: plugins[0].version reverts to 0.169.0 while the binary is already at 0.169.1",
      "message": "plugins[0].version regresses from 0.169.1 to 0.169.0 — same issue as plugin.json."
    },
    {
      "file": "CHANGELOG.md",
      "line": 11,
      "severity": "major",
      "blocking": false,
      "blocking_reason": "",
      "message": "CHANGELOG correctly adds an `## Unreleased` section for the vault-resolution fix, but no new version number is assigned — the four-way version alignment is incomplete without a corresponding version bump in the JSON manifests."
    }
  ],
  "concerns_addressed": [
    {
      "concern": "correctness: new AskUserQuestion path for vault resolution — ensure question phrasing and vault name list are tested",
      "disposition": "not-verified",
      "detail": "vault-cli binary not available in review environment; cannot run scenarios against a built binary per CLAUDE.md § After completing a spec or major refactor, walk the relevant scenarios/*.md to verify end-to-end behavior. No scenarios/ directory found in this repo."
    },
    {
      "concern": "correctness: new MODE=non_interactive fail-fast — verify callers handle the error correctly",
      "disposition": "not-verified",
      "detail": "Cannot run vault-cli to verify; the error message contract (`❌ Pass --vault — the vault cannot be inferred headlessly.`) and JSON error shape are correctly specified in commands/create-task.md:23 and :49, but not exercised by any test file in this PR."
    },
    {
      "concern": "correctness: task-creator.md §2 still references default_vault after removal from Defaults: line",
      "disposition": "not-an-issue",
      "detail": "Verified in diff: the phrase 'default_vault' was removed from the Defaults: line and §2 was updated to read 'the caller resolves it (--vault, or the vault the cwd sits in); this agent never falls back to default_vault' — no stale default_vault reference remains in §2 after the change."
    },
    {
      "concern": "tests: no test file changes in this PR — confirm automated coverage exists",
      "disposition": "not-verified",
      "detail": "No test files changed in this PR. The vault-cli project has no scenarios/ directory to exercise the behavioral changes. precommit skipped per selector-mode review convention — CI runs lint+test, but the behavioral paths (interactive vault ask, non-interactive fail, cwd-based resolution) are not exercised by any file in the diff."
    }
  ]
}

@ben-s-pull-request-reviewer
ben-s-pull-request-reviewer Bot dismissed their stale review October 15, 2026 16:25

superseded by new automated review

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is already the PR head worktree — no temporary worktree was created. The review is complete.


PR Review: fix: create-task asks for the vault instead of silently defaulting

Verdict: approve

Changes reviewed

File Δ
CHANGELOG.md +11 lines (detailed fix entry under ## Unreleased)
agents/task-creator.md +2 / −2 lines (constraints + defaults + vault config step)
commands/create-task.md +10 lines (## Resolve the vault section)

Must Fix (Critical)

None.

Should Fix (Important)

None.

Nice to Have (Optional)

None.


Plan Concerns — Verified

Concern Disposition Detail
agents/task-creator.md: removed silent default vault fallback — verify caller now always resolves vault before invoking the agent addressed commands/create-task.md now owns the resolution logic: it tries --vault or cwd, asks via AskUserQuestion if neither applies, and fails headlessly with a clear error. The agent received no vault of its own; it reads what the command passes.
commands/create-task.md: new AskUserQuestion flow for vault resolution — verify ask triggered only when neither --vault nor cwd-based vault detection resolves addressed The ## Resolve the vault section is explicit: --vault or cwd → use it; neither → ask; MODE=non_interactive → hard fail. No silent fallthrough remains.
commands/create-task.md: verify AskUserQuestion presents only vaults from the config, matching sibling convention addressed The new section says "offering the vault names from vault-cli config list", matching commands/reflect.md line 28 and agents/graph-auditor.md line 25.

Notes

The three changed files are all markdown (.md) — no Go code or test files were touched. The PR is a documentation/agent-instruction fix; there are no unit tests to run, and make precommit in selector mode is skipped per the workflow (CI covers lint+test).

The CHANGELOG entry is unusually detailed (11 lines) but the incident it documents (seibert-personal task placed in seibert-brogrammers) is concrete and the rationale is sound. The sibling convention citations (commands/reflect.md and agents/graph-auditor.md) are accurate — both documents confirm the same "resolve, or ask; do not guess" pattern.

{
  "verdict": "approve",
  "summary": "Bugfix removes silent fallback to default vault in create-task flow, replacing it with an explicit AskUserQuestion when neither --vault nor cwd resolves the vault. The change is correctly scoped to the two files that own the decision (command + agent), follows the existing sibling convention from reflect.md and graph-auditor.md, and the CHANGELOG entry documents the incident and rationale clearly.",
  "comments": [],
  "concerns_addressed": [
    {
      "concern": "agents/task-creator.md: removed silent default vault fallback — verify caller now always resolves vault before invoking the agent",
      "disposition": "addressed",
      "detail": "commands/create-task.md owns resolution: --vault or cwd → use it; neither → AskUserQuestion; non-interactive → hard fail. Agent receives no vault of its own; it reads what the command passes."
    },
    {
      "concern": "commands/create-task.md: new AskUserQuestion flow for vault resolution — verify the ask is triggered only when neither --vault flag nor cwd-based vault detection resolves a vault",
      "disposition": "addressed",
      "detail": "## Resolve the vault section is explicit: --vault or cwd → use; neither → ask; non-interactive → fail. No silent fallthrough remains."
    },
    {
      "concern": "commands/create-task.md: verify the AskUserQuestion presents only vaults from the config, matching the sibling commands reflect.md and graph-auditor.md convention",
      "disposition": "addressed",
      "detail": "New section says 'offering the vault names from vault-cli config list', matching reflect.md:28 and graph-auditor.md:25."
    }
  ]
}

@bborbe
bborbe merged commit afac401 into master Oct 15, 2026
3 checks passed
@bborbe
bborbe deleted the fix/create-task-vault-resolution branch October 15, 2026 16:26
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