Skip to content

Instantly share code, notes, and snippets.

@mm-parthy

mm-parthy/boundary-first-tag-resolution.md

Created July 7, 2025 14:50
Show Gist options
  • Save mm-parthy/e32a0aa1cb16b62d9ce6e80bfb436998 to your computer and use it in GitHub Desktop.
Save mm-parthy/e32a0aa1cb16b62d9ce6e80bfb436998 to your computer and use it in GitHub Desktop.
Download ZIP

Revisions

  1. mm-parthy created this gist Jul 7, 2025.
    248 changes: 248 additions & 0 deletions boundary-first-tag-resolution.md
    View file
    Original file line number Diff line number Diff line change
    @@ -0,0 +1,248 @@
    # Boundary-First Tag Resolution

    TLDR: "Any function that re-detects the tag invites race conditions. Resolve it in the CLI/MCP layer, thread { projectRoot, tag } everywhere else, and keep IO dumb."

    _"Boundary resolves, context carries, IO obeys."_

    ## 1 Current Coverage —

    ### 1.1 CLI Commands (`scripts/modules/commands.js`)

    | Status | Command | Notes |
    | :----: | -------------------------------------------------------- | -------------------------------------------------------------- |
    | ✅ | `add-dependency` | Correctly resolves and passes tag. |
    | ✅ | `add-subtask` | Correctly resolves and passes tag. |
    | ❌ | `add-task` | **Passes raw `options.tag`; can be undefined.** |
    | ✅ | `analyze-complexity` | Correctly resolves and passes tag. |
    | ❌ | `clear-subtasks` | Passes unresolved tag value. |
    | ❌ | `expand` / `expand --all` | Passes unresolved tag value. |
    | ✅ | `fix-dependencies` | Correctly resolves and passes tag. |
    | ✅ | `list` | Correctly resolves and passes tag. |
    | ❌ | `move` | Passes unresolved tag value. |
    | ❌ | `next` | Passes unresolved tag value. |
    | ✅ | `parse-prd` | Correctly resolves and passes tag. |
    | ✅ | `remove-dependency` | Correctly resolves and passes tag. |
    | ✅ | `remove-subtask` | Correctly resolves and passes tag. |
    | ❌ | `remove-task` | Passes unresolved tag value. |
    | ❌ | `research` | Passes unresolved tag value. |
    | ✅ | `set-status` | Correctly resolves and passes tag. |
    | ❌ | `show` | Passes unresolved tag value. |
    | ✅ | `update` | Correctly resolves and passes tag. |
    | ✅ | `update-subtask` | Correctly resolves and passes tag. |
    | ✅ | `update-task` | Correctly resolves and passes tag. |
    | ✅ | `validate-dependencies` | Correctly resolves and passes tag. |
    | ❌ | `generate` | Fails to resolve or pass the tag. |
    | ❌ | `sync-readme` | No --tag flag; fails to pass tag. |
    | N/A | `complexity-report`, `init`, `models`, `lang`, `migrate` | No file I/O or not applicable to tag contexts. |
    | N/A | `tags`, `add-tag`, `delete-tag`, `use-tag`, etc. | Tag administration tools operate outside a single tag context. |

    ### 1.2 MCP Tools (`mcp-server/src/tools/*.js`)

    | Status | Tool file | Notes |
    | :----: | ------------------------------------ | ---------------------------------------------------------------- |
    | ✅ | `set-task-status.js` | Correctly passes `args.tag` to the direct function. |
    | ✅ | `update.js` | Correctly passes `args.tag` to the direct function. |
    | ✅ | `fix-dependencies.js` | Correctly passes `args.tag` to the direct function. |
    | ❌ | `validate-dependencies.js` | Fails to resolve or pass the tag. |
    | ✅ | `remove-task.js` | Correctly passes `args.tag` to the direct function. |
    | ❌ | `add-dependency.js` | Fails to resolve or pass the tag. |
    | ❌ | `add-subtask.js` | Fails to resolve or pass the tag. |
    | ❌ | `add-task.js` | Fails to resolve or pass the tag. |
    | ❌ | `analyze.js` | Fails to resolve or pass the tag. |
    | ❌ | `clear-subtasks.js` | Fails to resolve or pass the tag. |
    | ❌ | `expand-all.js` | Fails to resolve or pass the tag. |
    | ❌ | `expand-task.js` | Fails to resolve or pass the tag. |
    | ❌ | `generate.js` | Fails to resolve or pass the tag. |
    | ❌ | `get-task.js` | Fails to resolve or pass the tag. |
    | ❌ | `get-tasks.js` | Fails to resolve or pass the tag. |
    | ❌ | `move-task.js` | Fails to resolve or pass the tag. |
    | ❌ | `next-task.js` | Fails to resolve or pass the tag. |
    | ❌ | `parse-prd.js` | Fails to resolve or pass the tag. |
    | ❌ | `remove-dependency.js` | Fails to resolve or pass the tag. |
    | ❌ | `remove-subtask.js` | Fails to resolve or pass the tag. |
    | ❌ | `research.js` | Fails to resolve or pass the tag. |
    | ❌ | `update-subtask.js` | Fails to resolve or pass the tag. |
    | ❌ | `update-task.js` | Fails to resolve or pass the tag. |
    | N/A | Tag-admin tools (`add-tag.js`, etc.) | Operate across all tags by design, no single tag context needed. |
    | N/A | Other tools (`models.js`, etc.) | No file I/O or not applicable to tag contexts. |

    ### 1.3 MCP Direct Functions (`mcp-server/src/core/direct-functions/*.js`)

    | Status | Direct-function file | Notes |
    | :----: | ---------------------------- | ------------------------------------------------------------------ |
    | ❌ | `add-dependency.js` | Does NOT forward any tag/context object to core `addDependency`. |
    | ✅ | `fix-dependencies.js` | Correct – forwards `{ projectRoot, tag }` in context. |
    | ❌ | `generate-task-files.js` | Calls core `generateTaskFiles` with no tag/context. |
    | ✅ | `list-tags.js` | Correctly operates across all tags by design (no single-tag need). |
    | ❌ | `list-tasks.js` | Passes literal `null` for tag – needs explicit tag forwarding. |
    | ❌ | `parse-prd.js` | Builds context without tag; tag not forwarded. |
    | ❌ | `remove-dependency.js` | Does NOT supply tag in context. |
    | ❌ | `show-task.js` | Uses `readJSON(... projectRoot)` only – tag omitted. |
    | ✅ | `add-subtask.js` | Forwards `{ projectRoot, tag }` correctly. |
    | ❌ | `add-task.js` | Fails to forward tag to core `addTask`. |
    | ❌ | `analyze-task-complexity.js` | Fails to forward tag to core analysis. |
    | ✅ | `clear-subtasks.js` | Passes `{ projectRoot, tag }` to core `clearSubtasks`. |
    | ❌ | `complexity-report.js` | Report path resolution ignores tag. |
    | ❌ | `expand-all-tasks.js` | Does NOT pass tag in context. |
    | ✅ | `expand-task.js` | Forwards tag inside context to core `expandTask`. |
    | ✅ | `move-task.js` | Forwards tag via context to core `moveTask`. |
    | ❌ | `next-task.js` | Tag not forwarded to `findNextTask`. |
    | ❌ | `remove-subtask.js` | No tag/context passed. |
    | ✅ | `remove-task.js` | Forwards tag in context to core `removeTask`. |
    | ❌ | `research.js` | Tag not forwarded to core `performResearch`. |
    | ✅ | `set-task-status.js` | Passes tag as separate arg to core `setTaskStatus`. |
    | ❌ | `update-subtask-by-id.js` | Tag absent in context to core `updateSubtaskById`. |
    | ❌ | `update-task-by-id.js` | Tag absent in context to core `updateTaskById`. |
    | ✅ | `update-tasks.js` | Context `{ projectRoot, tag }` forwarded to core `updateTasks`. |
    | N/A | `cache-stats.js` | No file I/O; not applicable. |
    | N/A | `initialize-project.js` | No file I/O; not applicable. |
    | N/A | `models.js` | No file I/O; not applicable. |
    | N/A | `rules.js` | No file I/O; not applicable. |
    | N/A | `response-language.js` | No file I/O; not applicable. |

    ### 1.4 Core-Modules Tag-Resolution (`scripts/modules/*.js`)

    | Status | Module file | Notes |
    | :----: | ----------------------------------------- | --------------------------------------------------------------- |
    | ✅ | `dependency-manager.js` | Passes `context.tag` from a dedicated context object |
    | ⚠️ | `task-manager/add-subtask.js` | Contains internal fallback logic (`getCurrentTag`) |
    | ⚠️ | `task-manager/analyze-task-complexity.js` | Relies on `readJSON` fallback; does not enforce tag from caller |
    | ⚠️ | `task-manager/expand-all-tasks.js` | Contains internal fallback logic (`getCurrentTag`) |
    | ⚠️ | `task-manager/expand-task.js` | Relies on `readJSON` fallback; does not enforce tag from caller |
    | ⚠️ | `task-manager/clear-subtasks.js` | Relies on `readJSON` fallback; does not enforce tag from caller |
    | ⚠️ | `task-manager/list-tasks.js` | Relies on `readJSON` fallback; does not enforce tag from caller |
    | ⚠️ | `task-manager/update-task-by-id.js` | Contains internal fallback logic (`getCurrentTag`) |
    | ⚠️ | `task-manager/update-subtask-by-id.js` | Contains internal fallback logic (`getCurrentTag`) |
    | ⚠️ | `task-manager/update-tasks.js` | Contains internal fallback logic (`getCurrentTag`) |
    | ⚠️ | `task-manager/remove-task.js` | First `readJSON` omits `tag`; write has tag |
    | ⚠️ | `task-manager/research.js` | First `readJSON` omits `tag`; later calls correct |
    | ❌ | `task-manager/add-task.js` | no tag in initial read / write – defaults to master |
    | ❌ | `task-manager/set-task-status.js` | read & write omit tag |
    | ❌ | `task-manager/move-task.js` | read & write omit tag |
    | ❌ | `task-manager/generate-task-files.js` | `readJSON` omits tag |
    | ❌ | `utils/contextGatherer.js` | `readJSON` omits tag |
    | ❌ | `sync-readme.js` | `listTasks` call omits `tag` |
    | N/A | `task-manager/tag-management.js` | operates across all tags by design (reads raw, un-scoped data) |

    ---

    ## 2 Evaluation

    ### 2.1 Why I like the proposed "Boundary → Context → IO" flow

    **Pros**

    - **Deterministic state** – the tag is chosen exactly once; every layer gets the same value.
    - **Race-condition killer** – no hidden fallback logic means concurrent invocations can't silently overwrite each other.
    - **Debuggable** – when something is wrong you inspect the boundary, not every helper down-stream.
    - **Unit-test friendly** – helpers accept explicit ctx; no guessing inside mocks.
    - **Stupid-fast IO** – `readJSON` / `writeJSON` no longer need extra fs calls to "discover" a tag.
    - **Safe refactors** – changing how/where the tag is stored only touches the resolver.
    - **Better error surfacing** – missing tag triggers an immediate, obvious failure.

    **Cons**

    - **Boiler-plate propagation** – `{ projectRoot, tag }` must be threaded through every call chain.
    - **Legacy breakage** – older code/tests that omitted a tag explode until patched.
    - **Higher cognitive load** – quick scripts must remember to resolve & pass ctx.
    - **Large diff surface** – ripping out fallbacks touches many files.

    ### 2.2 Alternative evaluated – AsyncLocalStorage-based "ambient context"

    ```js
    runWithCtx({ projectRoot, tag }, () => coreFn(...))
    const { projectRoot, tag } = getCtx(); // inside helpers / IO
    ```

    **Why I considered it**

    – removes parameter noise; deep helpers stay untouched.

    **Why I rejected it**

    - **Hidden dependency** – functions rely on ambient state; harder to reason about in isolation.
    - **Testing overhead** – every unit test must wrap calls in `runWithCtx`, or `getCtx()` is undefined.
    - **Async-hook limitations** – spawning worker threads or non-hook-aware libraries loses the context.

    The explicit-context approach stays transparent, predictable and cross-runtime.

    ### 2.3 Why I _don't_ let `readJSON`/`writeJSON` re-resolve the tag

    Even if every caller _tries_ to pass the tag, keeping a "smart" fallback inside the IO layer would re-introduce the bugs I am purging.

    | Problem | What happens in a CLI / MCP world |
    | ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
    | **Two sources of truth** | Boundary resolves `feature-foo`, but `readJSON` looks at `state.json` and decides `master` – the file winds up in the wrong context. |
    | **Race conditions** | In the long-running MCP server two simultaneous requests hit different tags; the one that happens to update `state.json` last wins, corrupting the other request's write. |
    | **Hidden coupling** | Helpers must defensively double-check that the IO-inferred tag equals the ctx tag → more code, more tests. |
    | **Harder tests** | Unit tests would need to fake both context _and_ the config/state files so that the fallback picks the "right" tag. |
    | **Extra I/O** | Every call rereads config/state to "detect" the tag – unnecessary disk churn. |

    Therefore **IO obeys**: it accepts the explicit `tag` and does no further guessing. This keeps behaviour deterministic across both the one-shot CLI process and the multi-request MCP server.

    ---

    ## 3 Rule

    | Step | Responsibility |
    | ---- | ------------------------------------------------------------------------------------------------------------------ |
    | 1 | **Boundary layer** (CLI command handler or MCP tool) calls `resolveEffectiveTag({ projectRoot, explicitTag })`. |
    | 2 | The resolved `tag` plus `projectRoot` are stored in a `ctx` object and forwarded to direct-functions & core logic. |
    | 3 | `readJSON` & `writeJSON` **require** an explicit `tag`; they no longer guess. |
    | 4 | Legacy code can use a thin shim (`@deprecated`) that still calls the resolver. |

    ---

    ## 4 Work Items

    ### 4.1 CLI layer — patch these commands

    `add-task`, `clear-subtasks`, `expand`, `move`, `next`, `remove-task`, `research`, `show`, `generate`, `sync-readme`

    ### 4.2 MCP Tools — patch these tools

    `add-dependency`, `add-subtask`, `add-task`, `analyze`, `clear-subtasks`, `expand-all`, `expand-task`, `generate`, `get-task`, `get-tasks`, `move-task`, `next-task`, `parse-prd`, `remove-dependency`, `remove-subtask`, `research`, `update-subtask`, `update-task`, `validate-dependencies`

    ### 4.3 MCP Direct — patch these direct-functions

    `add-dependency`, `add-subtask`, `add-task`, `analyze-task-complexity`, `clear-subtasks`, `complexity-report`, `expand-all-tasks`, `expand-task`, `generate-task-files`, `list-tasks`, `move-task`, `next-task`, `remove-subtask`, `research`, `update-subtask-by-id`, `update-task-by-id`, `update-tasks`

    ### 4.4 Core Modules — remove fallback logic

    This involves removing `|| getCurrentTag(projectRoot) || 'master'` fallbacks from all the `⚠️` files listed in section 3.4 and ensuring their callers (from CLI, MCP tools, and direct functions) always provide an explicit `tag`.

    ### 4.5 Test-suite — enforce explicit `tag`

    All tests must now prove that the boundary-layer contract is honoured:

    - **Always pass a tag** – every helper, core function, direct-function, CLI handler or MCP tool invoked from tests gets `tag: 'master'` (or another explicit tag).
    - **Mock updates**
    - `readJSON(path, tag)` – second param is mandatory and asserted.
    - `writeJSON(path, data, tag)` – third param is mandatory and asserted.
    - **CLI / shell tests** – append `--tag master` to every command.
    - **Fixture helpers** – sample blobs now include a top-level `tag` key (`{ …, tag: 'master' }`).
    - **Negative guard** – add one test that omits the tag and expects the new "tag required" error.

    _Standard resolver snippet to drop into each boundary layer:_

    ```ts
    const projectRoot = findProjectRoot(); // CLI helper
    const resolvedTag = options.tag || getCurrentTag(projectRoot) || "master";

    await coreFn(...args, { projectRoot, tag: resolvedTag, session });
    ```

    Boundary tools (MCP) do the same but read `args.projectRoot` instead of `findProjectRoot`.

    ---

    ## 5 Aftermath

    - Once all boxes are green, make `readJSON` / `writeJSON` throw if `tag` is
    missing – enforcing the contract.
    - Update tests to supply `{ tag }` where necessary.
    - Remove legacy fallback path in a follow-up release.

    ---

    _Last updated: 2025-07-07_