Skip to content

Move plan-manager as a script#48

Open
isolomatov-gd wants to merge 5 commits intomainfrom
feature/plan-manager-script
Open

Move plan-manager as a script#48
isolomatov-gd wants to merge 5 commits intomainfrom
feature/plan-manager-script

Conversation

@isolomatov-gd
Copy link
Copy Markdown
Collaborator

@isolomatov-gd isolomatov-gd commented Mar 27, 2026

Problem: MCP is server hosted, plan may contain sensitive information
Solution: Move plan_manager as skill
Decision to be made:

  • Use typescript npm package instead of the script: easier to update and use (npx)
  • Implement Rosetta Local MCP using typescript and STDIO - can access MCP sampling and add other local features.
  • Continue to use *.js file in skill

@github-actions
Copy link
Copy Markdown
Contributor

Rosetta Triage Review

Summary: This PR migrates the plan_manager functionality from a server-hosted Redis-backed MCP tool to a client-side JavaScript skill (plan_manager.js), allowing agents to track execution plans locally without sending potentially sensitive plan data to the Rosetta MCP server.

Findings:

  • Bug — Dangling reference in SKILL.md: The <resources> section of instructions/r2/core/skills/plan-manager/SKILL.md contains USE FLOW adhoc-flow-with-plan-manager``, but that workflow file is deleted in this same PR. Any agent that loads the skill and tries to follow this reference will fail to resolve it.
  • Open architectural decision unresolved: The PR body explicitly flags an undecided question — whether to ship the skill as a plain JS file or as an npx-invocable TypeScript package. This ambiguity may make the implementation hard to evolve post-merge; the decision should be made and recorded before merging.
  • Plans folder committed: plans/plan-manager-skills/ (discovery notes, plan, specs) is committed to the repo. Per Rosetta workspace conventions, plans/ holds temporary execution artifacts and should be excluded from SCM. These files add repository noise.
  • ARCHITECTURE.md MCP tools table not updated: The table still describes plan_manager (opt-in) as storing plans in Redis. Since this PR adds a local-file alternative, the table should clarify the relationship between the MCP tool and the new skill (e.g., plan_manager MCP remains for users with Redis; the skill is the fallback).
  • Spec deviation: plans/plan-manager-skills/plan-manager-PLAN.md Task 7 says to update the three adhoc-flow-with-plan-manager.md copies; the actual implementation deletes them instead and folds the feature into adhoc-flow.md. The decision to delete vs. update is reasonable but should be documented.
  • Test coverage: 802 lines of unit tests covering 401 lines of implementation — solid coverage included. ✓
  • No npm dependencies: The script uses only Node.js built-ins, no install step needed. ✓
  • Plugin sync: All plugin copies (core-claude, core-cursor) are correctly auto-synced via scripts/pre_commit.py. ✓

Suggestions:

  • Fix the dangling USE FLOW adhoc-flow-with-plan-manager reference in SKILL.md before merging — either remove the line or replace it with the correct flow name.
  • Resolve the TypeScript-vs-JS decision in the PR body and update the description accordingly.
  • Remove plans/plan-manager-skills/ from the commit (add to .gitignore or delete from the branch).

Automated triage by Rosetta agent

<resources>

- Asset: ACQUIRE `plan-manager/assets/pm-schema.md` FROM KB — plan JSON structure
- Flow: USE FLOW `adhoc-flow-with-plan-manager`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agree with a comment from Rosetta Triage Reviewer. The file with workflow flow adhoc-flow-with-plan-manager was deleted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants