Skip to content

fix(mcp): accept provenance on ctx_add and ctx_watch_update - #199

Open
bilersan wants to merge 6 commits into
ActiveMemory:mainfrom
bilersan:fix/mcp-add-provenance
Open

bilersan wants to merge 6 commits into
ActiveMemory:mainfrom
bilersan:fix/mcp-add-provenance

Conversation

@bilersan

Copy link
Copy Markdown
Contributor

Summary

Over MCP, ctx_add and ctx_watch_update could not add a task, decision,
or learning in a project without a relaxing .ctxrc: entry.Validate
requires session_id, branch, and commit (and section for tasks), but
the tool schemas never advertised them, so schema-driven clients never sent
them and every such call failed with
missing required fields: --session-id, --branch, --commit. This branch
advertises every field validation can require, resolves branch and commit
from git on the server, keeps session_id caller-supplied and required, and
flattens the single-line fields that end up inside TASKS.md lines and
headings. Decision and learning provenance is validated but never persisted;
that is raised on #190 and left out of this PR.

Fixes #190.

Changes

  • Schema (internal/mcp/server/def/tool): EntryAttrProps declares
    section, priority, session_id, branch, and commit, so both entry
    tools take them from one definition; ctx_watch_update gains
    section/priority, which its handler already read. Serves
    specs/fix-mcp-add-provenance.md.
  • Git provenance (internal/mcp/server/route/tool): when the caller
    gives neither branch nor commit (each empty or "unknown"),
    ctx_add and ctx_watch_update resolve both with the git calls the hook
    relay makes in provenance.Emit and its "unknown" fallback. Explicit
    values win, and the pair is resolved as a whole, so one checkout never
    supplies the branch and another the commit. session_id stays required:
    its description points at the relayed Session value or an explicit
    "unknown", and the decision/learning prompt footers ask for it. Serves
    specs/fix-mcp-add-provenance.md.
  • Sanitization (internal/sanitize, internal/mcp/server/extract): the
    new sanitize.Line strips every control character, including the tab, CR,
    and LF that StripControl keeps, and the Unicode line/paragraph separators,
    then truncates on a rune boundary. SanitizedOpts applies it to
    priority, section, branch, and commit with MaxOptsLineLen (256),
    the cap specs/sanitize-hardening-followup.md deferred for these fields.
    Serves specs/fix-mcp-add-provenance.md.
  • Docs (docs/cli/mcp.md, docs/home/configuration.md): the argument
    tables of both tools list every advertised argument, with a provenance
    note, and describe section by what validation requires; the
    provenance_required rows say how each key applies over MCP. Serves
    specs/fix-mcp-add-provenance.md.

Specs

  • specs/fix-mcp-add-provenance.md — how the MCP entry tools advertise,
    resolve, and sanitize the provenance fields entry validation requires
    (committed on-branch)

Verification

  • internal/mcp/server/def/tool: TestDefsEntryToolsAdvertiseEntryFields
    checks that both entry tools advertise every field validation can require.
  • internal/mcp/server, end to end through tools/call:
    TestToolEntryResolvesGitProvenance (both tools, in a temporary git
    repository, expected values read from git), TestToolAddKeepsExplicitProvenance,
    TestToolAddBranchWithoutCommit (a branch without a commit fails, naming
    only --commit, instead of taking the server's HEAD),
    TestToolAddLearningUnknownSession (the ctx_add MCP tool requires provenance fields its own schema doesn't expose (task/decision/learning unusable over MCP) #190 probe with
    "session_id": "unknown"), and TestToolAddRequiresSessionID (omission
    still fails, naming only --session-id).
  • internal/sanitize: TestLine; internal/mcp/server/extract:
    TestSanitizedOptsFlattensLineFields.
  • Per the spec's Tests section, each test fails before its fix except
    TestToolAddKeepsExplicitProvenance, which guards the explicit-value rule.

Entry validation (internal/entry/validate.go) requires session_id,
branch, and commit for tasks, decisions, and learnings unless .ctxrc
relaxes them, and section for tasks. extract.Opts reads all of them
from the MCP arguments, but EntryAttrProps never declared the
provenance fields, and the ctx_watch_update schema declared neither
section nor priority. MCP clients only send what a tool schema
advertises, so every ctx_add and ctx_watch_update call for those
entry types failed with "missing required fields: --session-id,
--branch, --commit" (ActiveMemory#190).

Declare section, priority, session_id, branch, and commit in
EntryAttrProps, so both entry tools take them from one definition,
and test that both schemas advertise every field validation can
require.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
Only the client knows its session ID, but the server can read branch
and commit itself, and its values beat relayed ones: read at write
time (a relayed commit predates any commit made since the prompt) and
never retyped by a model.

ctx_add and ctx_watch_update now fill branch and commit when the
caller leaves them empty or "unknown", with the git calls the hook
relay makes in provenance.Emit, falling back to "unknown" as it does,
so MCP-written provenance matches the CLI path. An explicit value
still wins, for an agent working in another checkout.

session_id stays caller-supplied and required. Its description points
the agent at the Session value ctx relays, or an explicit "unknown"
when it has none, and the decision and learning prompts ask for it:
their "all fields above" footers never listed it.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
SanitizedOpts promised sanitization for every text field but passed
priority, section, branch, and commit through raw. format.Task writes
priority, branch, and commit into the task line, and an unmatched
section becomes a new "### " heading, so a line break in any of them
forged task lines or headings in TASKS.md. Advertising branch and
commit makes schema-driven clients send them.

sanitize.Line removes every control character, including the tab and
line breaks that StripControl keeps for multi-line content, and the
Unicode line and paragraph separators, then truncates on a rune
boundary. SanitizedOpts applies it to the four fields with a new
MaxOptsLineLen (256). Over-long values are truncated, not rejected,
as session IDs already are: this is the length cap that
specs/sanitize-hardening-followup.md deferred for these fields.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
docs/cli/mcp.md listed neither section nor the provenance arguments
for ctx_add, and ctx_watch_update also lacked priority and section,
although validation requires section for tasks and session_id for
tasks, decisions, and learnings. Both tables now list every argument
the tools advertise, with a note on what to pass for session_id and
when branch and commit come from git.

The provenance_required rows in the configuration reference now say
how each key applies to the MCP entry tools.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
Filling branch and commit independently let two checkouts share one
provenance pair: an agent in another checkout that passed its branch
but not its commit got the server's HEAD stored next to it, a pair no
checkout ever had, in exactly the case explicit values exist for.

Branch and commit are now resolved from git only when the caller gave
neither (each empty or "unknown"). A caller that passes one passes
both, or validation names the missing one, as it does for an omitted
session_id. The schema descriptions and the MCP reference say so, and
the provenance_required rows mention the "unknown" recorded when git
cannot answer. The test helper also stops discarding its json.Marshal
error.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
The ctx_add and ctx_watch_update rows called section "Target section
in TASKS.md (tasks only)", but conventions take it too: it picks the
## section of CONVENTIONS.md the bullet goes under. Describe it as the
schema does, required for tasks, without claiming it is tasks-only.

Spec: specs/fix-mcp-add-provenance.md
Signed-off-by: Ersan Bilik <ersanbilik@gmail.com>
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.

ctx_add MCP tool requires provenance fields its own schema doesn't expose (task/decision/learning unusable over MCP)

1 participant