forgejo_admin/pal-e-mcp
1
0
Fork 0
Code Issues 4 Pull requests Projects Releases Packages Wiki Activity Actions

Phase 8f-3 + 8f-4: Param alignment audit + QA nits cleanup #24

New issue
Closed
opened 2026-03-08 00:24:32 +00:00 by forgejo_admin · 1 comment
forgejo_admin commented 2026-03-08 00:24:32 +00:00
Owner
Copy link

Lineage

plan-2026-02-26-tf-modularize-postgres → Phase 8 (SDK + MCP Rewrite) → Phase 8f (MCP Rewrite) → Phase 8f-3 (Param Alignment Audit)

Repo

forgejo_admin/pal-e-docs-mcp

User Story

As an AI agent using pal-e-docs MCP tools
I want param translations between MCP and SDK to be correct and edge-case-safe
So that tool invocations never silently produce wrong results

Context

Phase 8f-2 (PR #23) rewrote all 26 MCP tools to wrap the pal-e-docs SDK instead of raw httpx, and added 7 new tools. The rewrite introduced translation layers where MCP tool params differ from SDK method params (CSV strings → lists, field renames, sentinel handling, default values).

Betty Sue audited every MCP tool on origin/main (commit 2c41b7a) against every SDK method signature. The 8 originally-documented mismatches are all correctly implemented. However, the audit found 5 additional items — 1 at MEDIUM risk that may need a code fix.

Full audit details: phase-postgres-8f3-param-alignment in pal-e-docs.

File Targets

Files the agent should modify:

  • src/pal_e_docs_mcp/tools/notes.py — fix create_note tags edge case if regression confirmed
  • src/pal_e_docs_mcp/tools/sprints.py — add code comment documenting move_sprint_item → update_sprint_item name mapping

Files the agent should NOT touch:

  • src/pal_e_docs_mcp/server.py — no changes needed
  • src/pal_e_docs_mcp/tools/blocks.py — no param translations, all pass-through

Acceptance Criteria

Live verification (all 8 documented translations):

  • search_notes(query=...) correctly passes to SDK search_notes(q=...)
  • create_note(content=..., project=..., tags="a,b") correctly maps to SDK html_content, project_slug, tags=["a","b"]
  • update_note(content=..., project=..., tags="a,b") same mappings as create
  • update_note_links(target_slugs="slug-a, slug-b") splits to ["slug-a","slug-b"]
  • create_sprint() without status defaults to "planning"
  • add_sprint_item() uses position=0 default; labels="a,b" splits correctly
  • bulk_move_items(items='[{"id":1,"column":"done"}]') parses JSON correctly
  • update_sprint() sentinel handling: omitted fields are NOT sent as null to API

Edge case #12 — tags=None vs tags=[] (MEDIUM risk):

  • Call create_note without tags param → verify note is created with no tags (not inherited/default tags)
  • Compare behavior with old MCP (which sent "tags": [] explicitly)
  • If behavioral regression: fix create_note to pass tags=[] when tags param is None (matching old behavior)

Edge case #13 — labels falsy check:

  • Verify add_sprint_item(labels="") is treated as "no labels" (current behavior, should be correct)

CSV edge cases:

  • Trailing comma: tags="sop,active," — should not produce empty string element
  • Whitespace: tags=" sop , active " — should strip to ["sop","active"]
  • Single value: tags="sop" — should produce ["sop"]

Documentation:

  • Code comment on move_sprint_item explaining it maps to SDK update_sprint_item

Test Expectations

  • If code changes needed for #12: add a test case for create_note with tags=None
  • CSV edge case tests: trailing comma, whitespace, single value
  • Run command: pytest tests/ -v

Constraints

  • Backward compatibility is paramount — no MCP tool param names change, no required/optional changes
  • CSV string convention stays — it's the agent interface contract
  • Follow existing code style in tools/notes.py and tools/sprints.py
  • Run ruff format --check before pushing — CI will catch it otherwise

Checklist

  • PR opened
  • Tests pass
  • No unrelated changes
  • Edge case #12 verified or fixed

Related

  • phase-postgres-8f3-param-alignment — full audit results in pal-e-docs
  • phase-postgres-8f-mcp-rewrite — parent phase
  • PR #23 — the 8f-2 rewrite this audits
### Lineage `plan-2026-02-26-tf-modularize-postgres` → Phase 8 (SDK + MCP Rewrite) → Phase 8f (MCP Rewrite) → Phase 8f-3 (Param Alignment Audit) ### Repo `forgejo_admin/pal-e-docs-mcp` ### User Story As an AI agent using pal-e-docs MCP tools I want param translations between MCP and SDK to be correct and edge-case-safe So that tool invocations never silently produce wrong results ### Context Phase 8f-2 (PR #23) rewrote all 26 MCP tools to wrap the pal-e-docs SDK instead of raw httpx, and added 7 new tools. The rewrite introduced translation layers where MCP tool params differ from SDK method params (CSV strings → lists, field renames, sentinel handling, default values). Betty Sue audited every MCP tool on `origin/main` (commit `2c41b7a`) against every SDK method signature. The 8 originally-documented mismatches are all correctly implemented. However, the audit found 5 additional items — 1 at MEDIUM risk that may need a code fix. Full audit details: `phase-postgres-8f3-param-alignment` in pal-e-docs. ### File Targets Files the agent should modify: - `src/pal_e_docs_mcp/tools/notes.py` — fix `create_note` tags edge case if regression confirmed - `src/pal_e_docs_mcp/tools/sprints.py` — add code comment documenting `move_sprint_item` → `update_sprint_item` name mapping Files the agent should NOT touch: - `src/pal_e_docs_mcp/server.py` — no changes needed - `src/pal_e_docs_mcp/tools/blocks.py` — no param translations, all pass-through ### Acceptance Criteria **Live verification (all 8 documented translations):** - [ ] `search_notes(query=...)` correctly passes to SDK `search_notes(q=...)` - [ ] `create_note(content=..., project=..., tags="a,b")` correctly maps to SDK `html_content`, `project_slug`, `tags=["a","b"]` - [ ] `update_note(content=..., project=..., tags="a,b")` same mappings as create - [ ] `update_note_links(target_slugs="slug-a, slug-b")` splits to `["slug-a","slug-b"]` - [ ] `create_sprint()` without status defaults to `"planning"` - [ ] `add_sprint_item()` uses `position=0` default; `labels="a,b"` splits correctly - [ ] `bulk_move_items(items='[{"id":1,"column":"done"}]')` parses JSON correctly - [ ] `update_sprint()` sentinel handling: omitted fields are NOT sent as null to API **Edge case #12 — tags=None vs tags=[] (MEDIUM risk):** - [ ] Call `create_note` without tags param → verify note is created with no tags (not inherited/default tags) - [ ] Compare behavior with old MCP (which sent `"tags": []` explicitly) - [ ] If behavioral regression: fix `create_note` to pass `tags=[]` when tags param is None (matching old behavior) **Edge case #13 — labels falsy check:** - [ ] Verify `add_sprint_item(labels="")` is treated as "no labels" (current behavior, should be correct) **CSV edge cases:** - [ ] Trailing comma: `tags="sop,active,"` — should not produce empty string element - [ ] Whitespace: `tags=" sop , active "` — should strip to `["sop","active"]` - [ ] Single value: `tags="sop"` — should produce `["sop"]` **Documentation:** - [ ] Code comment on `move_sprint_item` explaining it maps to SDK `update_sprint_item` ### Test Expectations - [ ] If code changes needed for #12: add a test case for `create_note` with `tags=None` - [ ] CSV edge case tests: trailing comma, whitespace, single value - Run command: `pytest tests/ -v` ### Constraints - **Backward compatibility is paramount** — no MCP tool param names change, no required/optional changes - CSV string convention stays — it's the agent interface contract - Follow existing code style in `tools/notes.py` and `tools/sprints.py` - Run `ruff format --check` before pushing — CI will catch it otherwise ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No unrelated changes - [ ] Edge case #12 verified or fixed ### Related - `phase-postgres-8f3-param-alignment` — full audit results in pal-e-docs - `phase-postgres-8f-mcp-rewrite` — parent phase - PR #23 — the 8f-2 rewrite this audits
forgejo_admin changed title from Phase 8f-3: Param alignment audit + edge case fixes to Phase 8f-3 + 8f-4: Param alignment audit + QA nits cleanup 2026-03-08 00:25:58 +00:00
forgejo_admin commented 2026-03-08 00:26:06 +00:00
Author
Owner
Copy link

8f-4 Scope (batched into this issue)

Lineage: plan-2026-02-26-tf-modularize-postgres → Phase 8 → Phase 8f → Phase 8f-4 (QA Nits Cleanup)

Two non-blocking nits from PR #23 QA review:

Fix 1: Add .claude/ to .gitignore

Safety measure to prevent worktree artifacts from being staged.

File: .gitignore

Fix 2: Rename l → lbl in sprints.py

Lines 162 and 199 use l as loop variable in CSV split comprehensions, triggering E741 ambiguous variable name lint. Rename to lbl and remove noqa: E741 comments.

File: src/pal_e_docs_mcp/tools/sprints.py

Additional acceptance criteria

  • .claude/ added to .gitignore
  • l renamed to lbl in sprints.py CSV split comprehensions (2 occurrences)
  • # noqa: E741 comments removed from those lines
  • ruff check passes without E741 warnings

This closes out Phase 8f entirely when merged.

## 8f-4 Scope (batched into this issue) Lineage: `plan-2026-02-26-tf-modularize-postgres` → Phase 8 → Phase 8f → Phase 8f-4 (QA Nits Cleanup) Two non-blocking nits from PR #23 QA review: ### Fix 1: Add `.claude/` to .gitignore Safety measure to prevent worktree artifacts from being staged. **File:** `.gitignore` ### Fix 2: Rename `l` → `lbl` in sprints.py Lines 162 and 199 use `l` as loop variable in CSV split comprehensions, triggering E741 ambiguous variable name lint. Rename to `lbl` and remove `noqa: E741` comments. **File:** `src/pal_e_docs_mcp/tools/sprints.py` ### Additional acceptance criteria - [ ] `.claude/` added to `.gitignore` - [ ] `l` renamed to `lbl` in sprints.py CSV split comprehensions (2 occurrences) - [ ] `# noqa: E741` comments removed from those lines - [ ] `ruff check` passes without E741 warnings This closes out Phase 8f entirely when merged.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/pal-e-mcp#24
No description provided.