Skip to content

feat: knowledge bases — list, plain text CRUD, and access probe#427

Merged
mocha06 merged 2 commits into
devfrom
rc-dev/feat/kb-plain-text
Jul 18, 2026
Merged

feat: knowledge bases — list, plain text CRUD, and access probe#427
mocha06 merged 2 commits into
devfrom
rc-dev/feat/kb-plain-text

Conversation

@mocha06

@mocha06 mocha06 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Closes #412. Third phase of the AI Agents milestone (knowledge bases, capabilities & model selection); builds on the provider-discovery SDK error classifier from #411.

What ships

New pipe-scoped knowledge base surface with full SDK + MCP + CLI parity:

MCP CLI
get_ai_knowledge_bases pipefy kb list
get_ai_knowledge_base_plain_text pipefy kb plain-text get
create_ai_knowledge_base_plain_text pipefy kb plain-text create
update_ai_knowledge_base_plain_text pipefy kb plain-text update
delete_ai_knowledge_base_plain_text pipefy kb plain-text delete
validate_knowledge_base_access pipefy kb validate-access

validate_ai_agent_behaviors / pipefy agent validate-behaviors gain an optional data_source_ids (--data-source-id, repeatable): agent-level IDs are unioned with each behavior's dataSourceIds and checked against the pipe's knowledge bases. Unknown IDs are warnings only (valid stays true); a failed list probe skips the check with a single warning.

Contract (validated against the live schema, Core master resolvers, and the downstream data-source model)

  • Every operation is scoped by the pipe UUID (pipeUuid), never the numeric id. plain_text_id / knowledge base id are data-source UUIDs from the list.
  • Mutations use the Relay input: wrapper. Auth: reads require read_ai_agents; writes require manage_ai_agents. A green probe proves read access only, never write entitlement.
  • Plain-text limits enforced client-side to fail fast: content 1-3500 and description 1-900 characters. description is required on create — the GraphQL schema marks it optional, but the backend rejects a blank one, so the toolkit requires it and surfaces that as a clear client-side error rather than a 422.
  • KB content is ordinary resource data (the one encrypted field is never exposed on the type), so no secret-path inputs or redaction; KB tools stay off the remote profile (writes unreviewed for remote; reads follow the provider-read precedent).

Behavior boundaries

  • CLI gates writes on the read-access probe (fails with the classified problem before the mutation). MCP tools stay explicit-validate-first: create/update do not auto-probe. Deletes require confirmation (MCP confirm=true; CLI --yes or prompt).

Docs & skills

docs/parity.md (6 rows + counts), README.md (168 tools, new Knowledge bases domain), docs/mcp/tools/knowledge-bases.md, docs/mcp/README.md index, and the pipefy-ai-agents skill (validate-access → create plain text → attach via dataSourceIds flow). Skill-ref allowlist gains kb.

Testing

uv run pytest -m "not integration" (3573 passed, PIPEFY_* unset) + ruff check/format + import-linter + skill-ref linter + bump_version.py verify + parity row-count (168) + remote-seed drift guard green. /simplify + /code-review run and findings applied. Exercised end-to-end against dev pipe 303088927 through the MCP server running this branch's code:

# Scenario Expected Observed Verdict
1 validate_knowledge_base_access Green probe with count and read-only caveat ok: true, knowledge_base_count: 3, note states read access only (read_ai_agents), never manage_ai_agents ✅ pass
2 get_ai_knowledge_bases All items with id / type for dataSourceIds 3 items, each with id / type / name / description / updatedAt ✅ pass
3 create_ai_knowledge_base_plain_text Created plain text with id 54208963-…, all fields echoed ✅ pass
4 get_ai_knowledge_base_plain_text Full-content round-trip exact round-trip ✅ pass
5 update_ai_knowledge_base_plain_text (content only) Content changes, name / description preserved content updated, name + description unchanged ✅ pass
6 validate_ai_agent_behaviors with data_source_ids (real + bogus) Bogus → warning, real → none, valid unaffected by membership 1 warning for the bogus id, real id silent, valid set only by an unrelated field problem ✅ pass
7 Delete preview (confirm=false) Preview, nothing deleted requires_confirmation: true, mutation not run ✅ pass
8 Delete execute (confirm=true) Deleted, gone from list deleted_id returned, absent from the follow-up list ✅ pass
9 Update with no fields Rejected before the wire "Provide at least one of name, content, or description to update." ✅ pass (expected rejection)
10 Create missing description Boundary rejection INVALID_ARGUMENTS, path: ["description"], "Field required" ✅ pass (expected rejection)
11 get_ai_knowledge_base_plain_text (bogus id) Structured not-found + discovery hint kind: not_found, AI_CHAT_REQUEST_FAILED, "Use 'get_ai_knowledge_bases'…" appended, correlation_id carried ✅ pass (expected rejection)
12 validate_knowledge_base_access (bogus pipe uuid) Classified failure RESOURCE_NOT_FOUND, kind: not_found, correlation_id carried ✅ pass (expected rejection)
13 Create content 3501 chars Rejected client-side before the wire ValueError "content must be at most 3500 characters (got 3501)"; 3500 reaches the wire (proven in-process + unit-tested) ✅ pass (expected rejection)

Rows 9–13 are the client-side and classified-error acceptance criteria exercised live; row 6 is the data_source_ids membership criterion (unknown IDs warn, never block).

Add get_ai_knowledge_bases, get_ai_knowledge_base_plain_text,
create/update/delete_ai_knowledge_base_plain_text, and
validate_knowledge_base_access with full SDK + MCP + CLI parity.

All operations are pipe-scoped by pipe UUID (not the numeric id). Plain
text limits are enforced client-side to fail fast: content 1-3500 and
description 1-900 characters, both required (the backend rejects a blank
description even though the GraphQL schema marks it optional). CLI gates
writes on the read-access probe; MCP stays explicit-validate-first;
deletes require confirmation.

validate_ai_agent_behaviors gains optional data_source_ids: agent-level
IDs are unioned with each behavior's dataSourceIds and checked against
the pipe's knowledge bases, warning (never blocking) on unknown IDs and
skipping the check with a single warning when the list cannot be read.

Closes #412
@mocha06 mocha06 self-assigned this Jul 17, 2026

@adriannoes adriannoes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Solid delivery of the #412 knowledge-base surface: SDK + MCP + CLI parity, probe semantics, docs/skills, and CI are in good shape (checkout + targeted tests green on tip). Two contract gaps should land before merge so agents and CLI scripts do not get false success or a self-referential discovery hint.

What worked well

  • Thin shells over PipefyClient, static gql() queries, and the shared GraphQL problem classifier.
  • Client-side content/description limits and required description called out at point of use.
  • Probe documents read-only entitlement; CLI create/update gate and MCP explicit-validate-first match the docs.

Also noted

  • create/update still report success when _unwrap_plain_text yields {} (no GraphQL errors but null nested payload). get in the same module rejects empty, and create_ai_agent raises on a missing nested payload. Worth the same empty-payload guard (or an SDK ValueError) when convenient; fine as a follow-up if the two inline threads land first.

Review path

  • Checked out aa21c95, ran ruff on touched KB paths and 55 targeted SDK/MCP/CLI/preflight tests; GitHub CI green on tip.
  • Adversarial passes and refutation kept the CLI empty-get parity gap and the list-tool self-referential not_found hint; create/update empty unwrap stayed as a non-blocking note.

Comment thread packages/cli/src/pipefy_cli/commands/knowledge_base.py Outdated
Comment thread packages/mcp/src/pipefy_mcp/tools/knowledge_base_tools.py Outdated
…pty-write guard

The CLI `kb plain-text get` now mirrors the MCP tool's not-found handling: an
empty SDK result returns success=false and exits 1 instead of reporting success.

The MCP not-found discovery hint is scoped to per-id tools; the list tool's own
failure no longer tells the caller to retry the call that just failed.

A write mutation returning no errors but a null nested payload now raises in the
SDK instead of unwrapping to an empty success, so create/update can never report
success for a write that may not have persisted.
@mocha06

mocha06 commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator Author

@adriannoes All three review points landed in 98c3a8d — the two inline threads plus the non-blocking empty-unwrap note: _unwrap_plain_text now raises when a write returns no errors but a null nested payload, so create/update can never report success for a write that may not have persisted (SDK-level, covering MCP and CLI in one place; test_null_payload_without_errors_is_a_failure on both create and update).

@adriannoes adriannoes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Rechecked tip 98c3a8d. The follow-up commit addresses the review notes: CLI empty-get now fails closed with exit 1, the list-tool discovery hint is no longer self-referential, and write unwraps raise when the nested plain-text payload is missing. Targeted KB tests and CI are green on tip.

Thanks for addressing those points cleanly, including the regression coverage.

Review path

  • Compared the fix commit against the earlier REQUEST_CHANGES threads.
  • Re-ran the KB SDK/MCP/CLI unit suite locally (48 passed); GitHub checks green on tip.

@mocha06
mocha06 merged commit d0b9a93 into dev Jul 18, 2026
3 checks passed
@mocha06
mocha06 deleted the rc-dev/feat/kb-plain-text branch July 18, 2026 03:28
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.

2 participants