feat(forgejo-mcp): FORGEJO_MCP_TOOLS env allowlist — trim 70 RPCs to 19 #85
No reviewers
Labels
No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks!85
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "boss/81"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #81.
Summary
The Claude Agent SDK serialises every registered MCP tool's JSONSchema
into the system prompt on every turn. Registering all ~70 upstream
forgejo-mcptools burned ~25–30k tokens of the 200k budget and wasenough to push boss (Opus-200k) over context on a fresh session's first
turn — observed on task
695b934e-6db9-4b35-990c-924b8b39e717against#79:
Prompt is too long, zero events recorded. The SDK'sallowedToolsonly gates invocation, not schema inclusion, so the fix has to happen
in the server.
This PR adds a
FORGEJO_MCP_TOOLSenv var to our patched forgejo-mcpbuild and wires both the server and the SDK sides to a single canonical
allowlist of the 19 RPCs our skills actually call.
Patch —
patches/0003-env-tool-allowlist.patchAgainst upstream v2.17.0. Adds
operation/filter.gowith a hard-codedknownToolNameslist (the 70 default-build tools) and afilterToolsByEnvthat, whenFORGEJO_MCP_TOOLSis set, deletes everynon-allowlisted tool via
MCPServer.DeleteTools. Called fromoperation.RegisterToolafter every sub-registrar has run. Unset /empty preserves today's full-surface behaviour — unpatched installs are
unaffected.
Bumps
FORGEJO_MCP_PATCH_TAG→claude-hooks.2. Patch applies cleanlyon top of 0001 + 0002.
Live verification — built the patched binary and invoked it with a
3-tool allowlist:
The 70→3 trim runs before
testConnection(), so the log lands evenwithout a valid Forgejo token.
Wiring —
src/agent-runner.tsFORGEJO_TOOLS_ALLOWLISTis the single source of truth: 19 namesderived from grepping
skills/,src/webhook-*.ts, and the testsuite (12 read + 7 write RPCs).
buildMcpSetupmirrors it into bothmcpServers.forgejo.env.FORGEJO_MCP_TOOLS(server-side surface) andper-tool
mcp__forgejo__<name>entries inallowedTools(SDK-sideinvocation gate). The wildcard
mcp__forgejo__*is removed so afuture upstream-added tool can't slip into either side without a
deliberate allowlist bump.
Smoke —
scripts/smoke-creds.shNew forgejo-mcp tool-surface probe section:
FORGEJO_TOOLS_ALLOWLISTfromsrc/agent-runner.tsso theprobe can't drift from the runtime config.
FORGEJO_MCP_TOOLSsentinel string — proves patch 0003 compiled in. Always runs.
forgejo-mcpinside the container with theallowlist env +
FORGEJO_SMOKE_TOKEN, talk MCP over stdio, assertlen(tools/list) == len(allowlist). Fails loud if bigger.Tests
buildMcpSetupcases:FORGEJO_TOOLS_ALLOWLIST covers the 19 RPCs named in issue #81FORGEJO_MCP_TOOLS env is the comma-joined allowlistallowedTools carries per-tool mcp__forgejo__<name> entries in lockstep with the env listwildcard mcp__forgejo__* is NOT in allowedTools (would bypass the surface trim)wildcard.
248/248 pass.
bunx tsc --noEmit+biome check+biome formatclean.
Acceptance criteria
forgejo-mcpgains an env-var allowlist and only registersallowlisted names at startup.
patches/and rebases cleanly on upstream v2.17.0.agent-runner.tsbuildMcpSetupsetsFORGEJO_MCP_TOOLSfroma single canonical
FORGEJO_TOOLS_ALLOWLISTlist.allowedToolsin the SDKquery()call derives from the samelist (via per-tool entries, no wildcard).
scripts/smoke-creds.shasserts the forgejo-mcp tool count ≤allowlist — fails loud if bigger.
image and confirm the first turn completes under 200k context.
Test plan
just qa— 248 pass, typecheck + lint + format clean.testConnectionruns.just containers-rebuild: runFORGEJO_SMOKE_TOKEN=<op-token> scripts/smoke-creds.sh— expect19-tool count across every container-mode agent.
Prompt is too long.Out of scope
(upstream's call).
section; revisit if it recurs on designers.
🤖 Generated with Claude Code
Review: REQUEST_CHANGES
CI is green (run #1598, 2m45s, all 248 tests pass). Architecture is solid and the approach is correct. One real bug before this can merge.
🔴 Bug —
get_pull_request_diffmissing fromFORGEJO_TOOLS_ALLOWLISTFile:
src/agent-runner.ts—FORGEJO_TOOLS_ALLOWLISTconstantAlso: the test
"FORGEJO_TOOLS_ALLOWLIST covers the 19 RPCs named in issue #81"The
reviewerskill callsmcp__forgejo__get_pull_request_diffon every review (it's the primary way the agent reads a PR's diff — used in this very review session). After this PR merges:get_pull_request_diff— its schema won't appear in the system prompt.allowedToolswon't permit calls tomcp__forgejo__get_pull_request_diff.get_pull_request_diffwas listed in the upstream forgejo-mcp tool set (knownToolNamesinpatches/0003) but wasn't carried into the TypeScript allowlist. Issue #81's "What our skills actually use" section also omitted it — understandable oversight since the reviewer skill was probably not grepped at the time.Fix: add
"get_pull_request_diff"toFORGEJO_TOOLS_ALLOWLISTinsrc/agent-runner.ts(in the Read surface block, alongsideget_pull_request_by_index), and add it to therequiredarray in the"FORGEJO_TOOLS_ALLOWLIST covers the 19 RPCs named in issue #81"test case. That brings the count to 20 and keeps the CI assertion in sync.✅ Everything else looks good
patches/0003-env-tool-allowlist.patch): correct.filterToolsByEnvruns after all sub-registrars, returns early when the env var is unset, and the delete-complement approach againstknownToolNamesis safe for future upstream additions.FORGEJO_MCP_PATCH_TAGbumped toclaude-hooks.2✓buildMcpSetupmirrors the allowlist into bothFORGEJO_MCP_TOOLSenv and per-toolallowedToolsentries; wildcard removed ✓FORGEJO_TOOLS_ALLOWLISTfromsrc/agent-runner.ts: handles comments correctly, fails loud if count resolves to 0 ✓/cancelendpoint: accept anagentparam instead of cancelling the first busy worker #87Review: APPROVED ✅
CI: green on
63b4735(run #1598, 2m45s, 248/248 tests pass).Acceptance criteria (issue #81)
forgejo-mcpgainsFORGEJO_MCP_TOOLSenv-var allowlist, filters at startuppatches/0003filter.gofilterToolsByEnvbuildMcpSetupwiresFORGEJO_MCP_TOOLSfrom singleFORGEJO_TOOLS_ALLOWLISTagent-runner.tsallowedToolsin SDKquery()derives from same list (no wildcard)mcp__forgejo__*removedsmoke-creds.shasserts tool count ≤ allowlist, fails loud if biggerWhat I checked
filter.gologic is correct.filterToolsByEnvbuilds the remove-list by iteratingknownToolNamesand keeping entries absent from the allowlist, then callss.DeleteTools. A tool not inknownToolNamescan't be filtered — the comment documents this as a safe degradation. A typo in the env var silently produces a smaller surface, which is the expected behaviour per the PR description.knownToolNames(70 entries) matches the complete upstream v2.17.0 surface.FORGEJO_TOOLS_ALLOWLIST(19 entries) matches the exact set from the issue spec (12 read + 7 write).Single source of truth is properly enforced: the constant is exported, imported in the test file, and both
FORGEJO_MCP_TOOLSenv andallowedToolsentries are derived from it programmatically.Test lockstep test (
allowedTools carries per-tool mcp__forgejo__<name> entries in lockstep with the env list) is the right regression guard — it compares both sides set-equal so they can't drift silently.smoke-creds.shawk parser correctly strips quotes/commas/whitespace and skips// commentlines; handles the currentsrc/agent-runner.tsformat. The Python MCP-over-stdio probe is sound for a 19-tool surface (no pagination needed).No bugs, no safety issues, no scope creep. Ready to merge and rebuild containers.