feat(mcp): a board ticket can be edited, not only filed and moved - #675
Closed
serge-ivo wants to merge 1 commit into
Closed
feat(mcp): a board ticket can be edited, not only filed and moved#675serge-ivo wants to merge 1 commit into
serge-ivo wants to merge 1 commit into
Conversation
…oappstore-online/platform#137) `create_instance_ticket` put a card on the board and `set_board_item_status` moved it between columns, but nothing could amend one. Correcting a single word meant filing a second, corrected ticket and cancelling the first, leaving a dead card on the board for a one-field fix. ── The API had no edit route `instances-tasks.ts` carried twelve task routes and not one PUT or PATCH, so this could not be a thin MCP proxy the way `set_board_item_status` is. POSTing `/tasks/direct` with an existing `id` looks like it already does the job — `mirrorRuntimeTask` is an upsert (`ON CONFLICT(id) DO UPDATE`) — but it is a REPLACE. That route rebuilds the task from the request body, so amending one field blanks `description` and `reasoning`, resets `status` to completed, drops an actionable ticket's `action`, and moves `createdAt`. It would have looked like an edit right up until the fields nobody mentioned came back empty. `PATCH /:instanceId/tasks/:taskId` merges instead: absent means leave alone, `""` means clear. `id`, `type` and `createdAt` are pinned from the stored row. `status` is deliberately NOT patchable. Moving a card is `POST /board/status`, which validates the target against the agent's configured columns; accepting a status here would be a second, unvalidated way to move a card, and that validation is the whole reason the other route exists. ── jobKey is resolved, not assumed The issue asks for the card to be addressed by `jobKey`, and the ticket routes are keyed by `taskId`. Nothing mapped between them: `/board/status` writes a SEPARATE jobKey-keyed overlay table and never touches the task, and the grouping that turns tasks into cards lives inside `buildInstanceBoard`. So the tool asks the board and reads the card's `latestTaskId`. For a ticket the two happen to coincide — `jobKeyForTask` falls back to the task id for anything without a job URL — but relying on that would be a latent bug: it is a fallback in a function whose other branches key by URL, not a contract. A card that exists only as a status overlay carries an EMPTY `latestTaskId` and is reported as its own failure rather than sent on as a request to patch task "". ── Dry run First written resolving the jobKey before the dry-run branch, so a preview could reject an unknown key. `contract.test.ts` rejected that: a declared dry run must not reach the network. The preview now answers first and reports a templated endpoint — a preview that quietly makes a request is not a preview. ── Surface bump A new tool NAME is the first bullet of server-version.ts's bump list, and the one a caching host is most exposed to: a host holding the 0.1.6 list does not know this tool exists, and unlike a widened inputSchema there is no degraded call it can still make. 0.1.6 → 0.1.7, appended to SURFACE_LOCK, never edited. Counts: MCP_TOOL_COUNT 136 → 137, MCP_TOOL_ALWAYS_ON 117 → 118 (board tools are registered unconditionally). `pnpm docs:drift` named the ten doc claims that restate them. ── Verification `pnpm build` clean, `pnpm typecheck` clean, `biome check` clean, `pnpm docs:drift` no drift. Suite 545 files / 9496 passing. Eight new tests drive the merge against the real schema: the fields nobody mentioned survive, `""` clears where omission does not, identity and createdAt are pinned, an empty patch is a 400 rather than a cheerful 200, and a field cannot be grown past its create-time cap by editing it. `workers/api/src/lib/sql-time-writers.test.ts` (#634) fails on clean `origin/main` and still fails here — pre-existing, untouched by this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
serge-ivo
pushed a commit
that referenced
this pull request
Aug 16, 2026
…or is halted and the repository is named (#676) Instance `e4d2d031` ("PAS Coder") has exactly one registered repo, `proappstore-online/platform`. Given an objective to implement that repo's issue #137, session `csess_f686f1ff` ran, at 2026-08-16T01:55:44Z: gh pr create --repo ProAgentStore/platform … opening PR #675 in a different GitHub organisation. The intended repo received nothing, and the run reported SUCCESS — its closing summary described the target as "the PAGS platform repo". The Engine's model of where it was working had drifted and no layer beneath it disagreed. A registered repo was a working-directory default; it was never an authorization boundary. WHAT THIS DOES `lib/repo-write-scope.ts` is the second authority gate in the Pilot loop, and it is deliberately the same shape as `coding-authority.ts`'s: the observed acts (#294) are protocol fact — a `tool_use` event carrying the literal command — and a remote write naming a repository outside the instance's registered set halts the run, writes an `error` event and raises a board card. Merge policy bounds WHAT a run may make irreversible; this bounds WHERE. They are independent: csess_f686f1ff was inside its merge policy the whole time. The stop reason leads with the refused target — `Attempted write to "X", not permitted: this agent is registered for Y` — because a silent refusal reproduces the defect with the opposite sign, and because the repository is the fact that was wrong. WHAT IT DOES NOT DO, WHICH MATTERS MORE This is detect-and-halt, NOT containment, and #676 stays open for that reason. The Engine runs `claude --dangerously-skip-permissions` on the owner's machine under the machine's own git and `gh` credentials, so the FIRST wrong-repo write still lands and is then stopped and named. Measured, not assumed: both remotes are `git@github-personal:…` (ssh, unreachable by any token) and `gh` is authed from the keyring with account-wide `repo` + `admin:org`. Nothing has ever set `GH_TOKEN` for the engine — `grep` returns zero injection sites. The one genuine credential-layer narrowing is here too: `resolveCloneCredential` now mints a token GitHub itself restricts to the one repo being cloned (`repoScopedInstallationToken`). That credential is embedded in the checkout's `origin`, so an installation-wide token left every managed clone able to push to any sibling repo in the org. It is never cached — the cache is one slot per installation, shared with `github-issues.ts` and the connectors, and a scoped token written there would narrow all of them. Falls back to the wide token when scoping is unavailable, so no clone that works today starts failing. READS STAY BROAD, ON PURPOSE Verified from the live run rather than reasoned: the cross-repo reference read the owner cited was the Engine running `gh pr view 138 --repo proappstore-online/platform`. Only consequential acts reach this gate and #294 records writes alone, so a read is never examined. `git-credentials.test.ts` pins that asymmetry. UNKNOWN IS NEVER A VIOLATION This halts runs, so every unanswerable input permits: a command naming no repository (`git push -u origin feat/x`) is not judged, because the repo would have to be inferred from a working directory the act record does not carry, and a guess that stops a working run costs more than the gap it closes. An instance with no registered GitHub slug is likewise left alone — which is the answer for a local-path repo that was never given GitHub coordinates. `coding_diagnostics` now reports `githubApp.writeScope` plus an `enforcement` field that says this is an acts-observed halt and that the first write still lands, so an owner cannot read the scope list as a guarantee it is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
This PR was pushed here by mistake — it was created by a coding instance registered to a different repository (proappstore-online/platform) and implements a board-ticket update feature belonging to proappstore-online/platform issue #137. CI fails because this PR sits on the wrong repository. It is being reimplemented natively on the correct repository. Closing unmerged as misfiled. See also issue #676. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements proappstore-online/platform#137 — "MCP connector: board tickets can be created but never edited".
Problem
create_instance_ticketputs a card on the board andset_board_item_statusmoves it between columns, but nothing could amend one. Correcting a single word meant filing a second, corrected ticket and cancelling the first — a dead card left on the board for a one-field fix. Per the issue, this happened in practice.The API had no edit route
instances-tasks.tscarried twelve task routes and not onePUTorPATCH, so this could not be a thin MCP proxy the wayset_board_item_statusis.POST /tasks/directwith an existingidlooks like it already does the job —mirrorRuntimeTaskis an upsert (ON CONFLICT(id) DO UPDATE). But it is a replace: the route rebuilds the task from the request body, so amending one field blanksdescriptionandreasoning, resetsstatusto completed, drops an actionable ticket'saction, and movescreatedAt. It would have looked like an edit right up until the fields nobody mentioned came back empty.New
PATCH /:instanceId/tasks/:taskIdmerges: absent means leave alone,""means clear.id,typeandcreatedAtare pinned from the stored row.statusis deliberately not patchable. Moving a card isPOST /board/status, which validates the target against the agent's configured columns; accepting a status here would be a second, unvalidated way to move a card — and that validation is the whole reason the other route exists.jobKeyis resolved, not assumedThe issue asks for the card to be addressed by
jobKey; the ticket routes are keyed bytaskId. Nothing mapped between them —/board/statuswrites a separate jobKey-keyed overlay table and never touches the task, and the grouping that turns tasks into cards lives insidebuildInstanceBoard. So the tool asks the board and reads the card'slatestTaskId.For a ticket the two happen to coincide (
jobKeyForTaskfalls back to the task id for anything without a job URL), but relying on that would be a latent bug: it is a fallback in a function whose other branches key by URL, not a contract. A card that exists only as a status overlay carries an emptylatestTaskIdand is reported as its own failure rather than sent on as a request to patch task"".Dry run
First written resolving the
jobKeybefore the dry-run branch, so a preview could reject an unknown key.contract.test.tsrejected that — a declared dry run must not reach the network. The preview now answers first and reports a templated endpoint. A preview that quietly makes a request is not a preview.Surface bump — 0.1.6 → 0.1.7
A new tool name is the first bullet of
server-version.ts's bump list, and the one a caching host is most exposed to: a host holding the 0.1.6 list does not know this tool exists, and unlike a widenedinputSchemathere is no degraded call it can still make. Appended toSURFACE_LOCK, never edited.Counts:
MCP_TOOL_COUNT136 → 137,MCP_TOOL_ALWAYS_ON117 → 118 (board tools register unconditionally).pnpm docs:driftnamed the ten doc claims that restate them.Verification
pnpm buildpnpm typecheckbiome checkpnpm docs:driftpnpm testEight new tests drive the merge against the real schema (
realSchemaD1+seedTenant): the fields nobody mentioned survive,""clears where omission does not, identity andcreatedAtare pinned, an empty patch is a 400 rather than a cheerful 200, and a field cannot be grown past its create-time cap by editing it.Pre-existing failure, untouched by this PR:
workers/api/src/lib/sql-time-writers.test.ts(#634) fails on cleanorigin/main— verified by stashing this branch and re-running it on the base.Branch base
Local
mainhad diverged (5 behind, plus two commits that duplicate ones already upstream), so this branch is cut fromorigin/mainrather than localHEAD. The files this PR touches are byte-identical between the two, so the counts and the surface hash were computed against the right base.🤖 Generated with Claude Code