Skip to content

docs: add dummy marker note to task completion requirements - #155

Closed
juliusmarminge wants to merge 2 commits into
mainfrom
feature/docs/dummy-marker-note
Closed

docs: add dummy marker note to task completion requirements#155
juliusmarminge wants to merge 2 commits into
mainfrom
feature/docs/dummy-marker-note

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Mar 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a no-op documentation marker under Task Completion Requirements in AGENTS.md.
  • Keep existing completion requirements (bun lint and bun typecheck) unchanged.

Testing

  • Not run (documentation-only change).

Note

Low Risk
Low risk documentation-only change that adds two no-op bullets; no runtime or build behavior is affected.

Overview
Adds two no-op “dummy marker” bullets under Task Completion Requirements in AGENTS.md, leaving the existing bun lint/bun typecheck completion requirements unchanged.

Written by Cursor Bugbot for commit 0c246e7. This will update automatically on new commits. Configure here.

Note

Add dummy marker notes to AGENTS.md Task Completion Requirements to record a no-op update dated 2026-03-03

Add two bullet points to the Task Completion Requirements section in AGENTS.md: a no-op documentation marker and the date 2026-03-03.

📍Where to Start

Start with the Task Completion Requirements section in AGENTS.md.

Macroscope summarized 0c246e7.

- Document no-op update marker in AGENTS.md
@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/docs/dummy-marker-note

Comment @coderabbitai help to get the list of available commands and usage tips.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: No-op dummy marker committed to agent instructions
    • Removed the meaningless "Dummy marker: touched for a no-op documentation update" line from AGENTS.md (CLAUDE.md was already clean).

Create PR

Or push these changes by commenting:

@cursor push 2ad885a22c
Preview (2ad885a22c)
diff --git a/AGENTS.md b/AGENTS.md
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -3,7 +3,6 @@
 ## Task Completion Requirements
 
 - Both `bun lint` and `bun typecheck` must pass before considering tasks completed.
-- Dummy marker: touched for a no-op documentation update.
 
 ## Project Snapshot

Comment thread AGENTS.md
## Task Completion Requirements

- Both `bun lint` and `bun typecheck` must pass before considering tasks completed.
- Dummy marker: touched for a no-op documentation update.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No-op dummy marker committed to agent instructions

Low Severity

A "Dummy marker" line with no functional or informational purpose has been added to AGENTS.md under Task Completion Requirements. This file serves as instructions for AI coding agents, and placing a self-described "no-op" marker alongside real requirements (like bun lint and bun typecheck) adds noise that could confuse agents or future contributors about what the actual task completion criteria are.

Fix in Cursor Fix in Web

- Document a no-op update marker with explicit date `2026-03-03`

Co-authored-by: codex <codex@users.noreply.github.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…t self-providing McpServer.toolkit

Root-cause (confirmed at effect source): McpServer.toolkit(x) =
  Layer.effectDiscard(registerToolkit(x)).pipe(Layer.provide(McpServer.layer))
(effect McpServer.js:951) — it SELF-PROVIDES its own McpServer.layer, so each
declarative toolkit registration (preview + generation, incl. generate_3d)
registers into a throwaway McpServer the HTTP transport (layerHttp) never
serves. The fork's /mcp then exposes an empty tools/list and no harness tool
reaches agents. Switch both declarative registrations to
`Layer.effectDiscard(McpServer.registerToolkit(x))` — which leaves McpServer an
OUTER requirement, discharged from the transport's served instance via the
existing `Layer.provideMerge(McpTransportLive)` — matching the manual
registerPreviewSnapshot/registerInspectGeneration registrations.

STATUS: necessary + correct (all 79 MCP unit tests still pass, tsgo clean), but
NOT yet confirmed sufficient LIVE — the one desktop re-test was a flaky
no-harness-connection turn and a direct /mcp tools/list probe couldn't catch a
live agent. A deeper memo-fork (ancestor-provide of a single McpServer across
transport+registrations) may still be required, plus a PRODUCTION-topology test
(build the real McpHttpServer.layer, assert the served server lists generate_3d)
— the existing McpHttpServer.test.ts builds an ancestor-McpServer topology, NOT
the production provideMerge(layerHttp) one, which is exactly why this shipped
undetected. Tracked in task pingdotgg#155.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AJELkERZpuwwe4zS4iXj27
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…mcp serves generate_3d + preview tools

Builds the REAL exported McpHttpServer.layer(fakeGen) wrapped in HttpRouter.serve
(the way server.ts wraps it) + NodeHttpServer.layerTest, then asserts the SERVED
McpServer.tools includes generate_3d/generation_status/list_generations/
import_generated_asset/inspect_generation/preview_status/preview_snapshot. Unlike
the existing McpHttpServer tests — which build an ANCESTOR McpServer.McpServer.layer
topology (a false-green that never exercises provideMerge(layerHttp)) — this drives
the production composition. Control mutation proven: deleting a registration from the
merge makes it go RED, so it genuinely guards the served /mcp tool surface.

HONEST CAVEAT (pingdotgg#155): this test does NOT distinguish McpServer.toolkit (self-provide)
from registerToolkit (outer) — a rigorous source-level check of effect Layer.js
memoization shows both reference the SAME McpServer.layer object, memoized by
reference and built once in this plain-Layer.provide composition (no
HttpRouter.provideRequest fork near McpHttpServer.layer), so they are semantically
equivalent here. So the committed registerToolkit fix (8681e57) is better-practice
but NOT proven to be the live empty-tools/list root cause — that cause is either a
memo-fork in the FULL server.ts graph or the per-turn harness-connection flakiness,
pending a live /mcp tools/list probe. Tracked in pingdotgg#155.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AJELkERZpuwwe4zS4iXj27
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…pingdotgg#155 first)

Charter for the autonomous overnight loop: keystone-first (fix pingdotgg#155 so agents
can call generate_3d in the app), then close 2b.1's E2E visual, then 2b.2 if
credits/time allow. Encodes the pipeline doctrine (implementers → fresh critics
→ code review → ultracode workflows → Codex computer-use QA), gates, pacing/stop
conditions, and the owner constraints verbatim.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AJELkERZpuwwe4zS4iXj27
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…ls/list measurement

Add three redacted [mcp-diag] INFO logs to classify why the harness /mcp
tools (generate_3d, preview...) don't reach in-app agents:

1. Served McpServer tool count + names at build (THE decisive log). Reads the
   SERVED instance via the same Layer.provideMerge(McpTransportLive) graph, and
   is sequenced AFTER toolkit registration (Layer.provide, not a concurrent
   mergeAll sibling) so it cannot log a truncated list — a false negative for
   the exact question it exists to answer.
2. Per-/mcp-request auth outcome in the auth middleware (bearer redacted:
   presence + length only, never the value).
3. Injection lifecycle set/clear in ProviderService (authorization header
   redacted: presence + length only).

No behavior change beyond two behavior-preserving dedup hoists. tsgo clean;
pnpm vitest run src/mcp/ = 80 passed | 1 skipped (incl. the production-topology
guard test, which builds the exact modified layer graph and asserts the served
instance still carries every generation + preview tool).

Spec: docs/v2/specs/increment-155-A1-instrumentation.md

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…o harness tools reach them

Root cause (evidence-grade, captured from the real `claude` CLI invocation): the
per-turn ClaudeAdapter query loaded settingSources [user,project,local] with no
strictMcpConfig and no explicit mcpServers base, so the SDK ALSO merged the
operator's ambient MCP config — filesystem (~/.claude.json + project .mcp.json)
AND connected claude.ai servers (Gmail/Drive/Strava/...) — alongside the injected
devgame /mcp entry. Those ambient servers surfaced to in-app agents (a privacy
leak) AND crowded out the harness's own generate_3d / preview tools, so no harness
tool reached the agent (pingdotgg#155). The capability probe already isolates itself this
way (ClaudeProvider.buildClaudeCapabilitiesProbeQueryOptions); commit aa5ec80
(pingdotgg#4015) wired that isolation only to the probe, never to the real per-turn path.

Fix — mirror the probe's isolation on the real per-turn query:
- strictMcpConfig: true + env ENABLE_CLAUDEAI_MCP_SERVERS=false, UNCONDITIONAL for
  every in-app Claude turn (privacy must not depend on whether devgame is injected).
- keep mcpServers:{devgame} additive when a harness session exists; add
  alwaysLoad:true so its tools land in the turn-1 prompt instead of being deferred
  behind tool-search.
- settingSources unchanged, so workspace commands / CLAUDE.md still work.

This is also a real privacy/security fix: the operator's personal MCP servers no
longer leak into in-app agents.

Guard (ClaudeAdapter.test.ts): two red-green tests — (1) with an mcpSession, the
query isolates + injects only devgame with alwaysLoad; (2) with NO mcpSession,
isolation still holds and no server is injected (the privacy property is
independent of harness injection). Both proven to go red when their lever is
removed. tsgo clean; ClaudeAdapter suite 70/70.

Verified structurally live: the real claude CLI invocation now carries
--strict-mcp-config (absent before the fix). The in-app generate_3d E2E on the
real desktop app remains the closing gate (standalone dev rig is auth-blocked by
OAuth contention, not fix-blocked).

Reviewed by an independent Opus pass (correct + mergeable); its two actionable
findings (unconditional privacy levers; alwaysLoad) are folded in. Sibling
adapters (Grok/Cursor/Codex/OpenCode) inject devgame without this isolation but
use array-style mcpServers — unverified, tracked separately.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…e owner action to close A4/B

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…e CLI reject ALL harness tools

The in-app Claude agent could never see the harness's generation/preview tools
(generate_3d etc.) — the true, deterministic root cause, pinned via the spawned
`claude` CLI's own debug log:

  MCP server "devgame": tools/list failed (Invalid result for tools/list:
    tools.16.inputSchema.type: Invalid input: expected "object")
  [ERROR] Failed to fetch tools ... Dynamic tool loading: 0/18 ... included

The CLI's client-side validator rejects the ENTIRE tools/list array if even one
tool's inputSchema lacks a top-level `type:"object"` — so ONE malformed schema
took all 19 devgame tools dark for every provider and session. `tools.16` =
`list_generations`, whose input was a bare `Schema.Struct({})`
(packages/contracts/src/generation/index.ts) — the only zero-field struct among
the 19. Effect's JSON Schema converter emits `{anyOf:[{type:"object"},
{type:"array"}]}` (no top-level `type`) for a fields-less struct, which the MCP
SDK validator rejects.

Fix: `Schema.StructWithRest(Schema.Struct({}), [Schema.Record(String, Never)])`
— a public-API form that gives the AST a vacuous index signature so the converter
emits a real `{type:"object", additionalProperties:false}`, while keeping the tool
genuinely no-arg (no fake user-facing param; `{}` decodes, stray keys rejected).

Defensive guard (McpHttpServer.ts): at server startup, after every registration,
fail LOUD (naming offenders) if any served tool's inputSchema lacks
`type:"object"` — the all-or-nothing blast radius (one bad schema kills the whole
toolkit) means this must never silently recur.

Red-green test (McpHttpServer.test.ts): asserts all 19 served tools have
inputSchema.type==="object"; proven RED against the pre-fix struct (names
list_generations), GREEN after. Removed a now-stale list_generations exemption in
tools.test.ts. tsgo clean (contracts + server); vitest src/mcp/ 81 passed.

LIVE-PROVEN end-to-end (clean authed backend, source-linked fix, no build): an
in-app agent turn now emits a real tool_use for mcp__devgame__list_generations —
CLI debug: `Calling MCP tool: list_generations ... completed successfully`,
`tool_dispatch_end outcome=ok`; the pre-fix `tools/list failed` error is gone.
This closes the pingdotgg#155 "agent generates" gate. Separate from and downstream of the
ambient-MCP isolation fix (374d3ed), which stays as a real privacy/correctness
improvement.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
Inert unless T3CODE_CLAUDE_DEBUG_FILE is set; then the spawned claude CLI writes its verbose MCP debug (incl. tools/list handling) to that path. This is the diagnostic that pinned pingdotgg#155's root cause (the CLI's tools/list validation rejecting the whole array). No behavior change when unset.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
piero-dev25 added a commit to piero-dev25/devgame that referenced this pull request Aug 15, 2026
…ent calls a devgame tool)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant