Repository navigation
feat: add a local MCP server and an Agent Skill - #130
renato-umeton wants to merge 36 commits into
Conversation
The manuscript names local MCP exposure as the way an agent reaches a dataset directory without leaving the secure environment (p.9), and it is the precondition for an enrichment flow where an agent proposes semantic fields and a human reviews them. This is a second front door to the same MetadataGenerator, not a second pipeline: an agent can supply only what a person could type, and the ScanReport comes back verbatim so every refusal and its reason are visible. The tool surface is deliberately narrow rather than a mirror of the CLI's flags: the point is an auditable skill. bake takes the semantic fields plus --detect-references and --include/--exclude, and writes through the same _save_dict path, so validation and traceability are unchanged. stdio is the only transport and there is no fetch, search or upload tool, so the local-first, no upload invariant holds. The mcp SDK goes in an optional PEP 735 group so the wheel and the default install stay dependency-light; the subcommand imports it lazily and says how to install it when it is absent. The three tools are plain functions independent of any transport, which is what the tests call. Two helpers come out of main() so both front doors share them rather than diverging: _dry_run_entries (the scan-and-select loop --dry-run already ran) and _parse_creators (the "Name,email,url" parser). Neither changes CLI behaviour.
The CLI reference is generated from typer introspection, so it needs a regeneration for the new subcommand to appear at all. The DEVELOPMENT.md note sits under "Running" because the optional dependency group is the part a reader cannot infer: without `uv sync --group mcp` the subcommand only tells them what to install.
tests/test_mcp_server.py opens with importorskip("mcp"), which is what keeps the
suite green for anyone who has not installed the optional group. On CI that same
guard meant the file skipped silently on both matrix jobs, so the server had no
coverage at all where it matters most.
ScanReport's summary counters are written for a completed bake, where every file is either in the document or accounted for by a reason. A dry run reads nothing, so no entry ever reaches DESCRIBED and the whole scan landed in undescribed: spect_demo, which bakes six of its seven files, reported "described: 0, undescribed: 7". An agent reading that would conclude the directory cannot be described at all, which is the opposite of what the run found. The tool now builds its own summary from the outcomes the dry run does produce, counting claimed and unclaimed the way the CLI's --dry-run already prints them, with by_reason accounting for the unclaimed alone. The per-file list is unchanged, so nothing that was correct moves. Also relocks with the uv revision main uses, rather than the older local one that had downgraded the lock format, and drops the PEP 604 unions for the Optional/List spelling the rest of the package uses.
Closes MIT-LCP#129. The MCP server already gives an agent the three verbs, but not the judgement to use them, so every integration had to re-teach the same things in its own prompt: that the semantic layer (name, description, license, creator, citation, url) is never inferred and must come from the user, that a dry run comes before a bake, that a file refused with a reason is the designed behaviour rather than a gap, and that the fix for bad output is to correct the input and re-run, because the output is deterministic. That knowledge belongs next to the tools. The canonical skill lives inside the package at src/croissant_baker/skills/croissant-baker/, so `pip install croissant-baker` delivers it along with an RAI config template under assets/. The repo-level .agents/skills/croissant-baker is a relative symlink to that one directory, the cross-client discovery convention, so a checkout is discoverable without a second copy to keep in step. The server publishes the same file as the resource croissant-baker://skill, read through importlib.resources, so a client that has the tools can fetch the instructions without knowing anything about the filesystem. Two build details follow from the symlink. Hatchling walks it and keeps one path per file, so whichever path it reaches first wins; `exclude` alone filters after the link has already claimed the files, and only `skip-excluded-dirs` stops the walk, so without it the skill shipped under a dot-directory instead of inside the package. And the blanket .agents/ ignore is narrowed to .agents/* plus a negation, because the link is a product artifact rather than one contributor's local context.
aa946ee to
39a25db
Compare
|
Rebased on main at 0.6.0. Only uv.lock conflicted; it was relocked with the mcp group on top of main's lock. Suite green at 1100. |
Relock with uv 0.12 on top of main's uv.lock to add the mcp group; CI syncs with --locked --group test --group mcp.
# Conflicts: # src/croissant_baker/__main__.py
The SDK hides the text of any exception other than its own ToolError, so a client only learned that bake or dry_run failed. A ValueError (a refusal of the input, such as a creator with no name) or an OSError (a path that cannot be read or written) now reaches the client as a ToolError with its message.
|
@slobentanzer gentle ping here too, main is merged in and its green. happy to split it if smaller is easier to review.. |
Conflicts: uv.lock Took main's uv.lock and relocked with uv 0.12.0 to add the mcp group back.
The skill's description listed the formats from before the spreadsheet, HDF5 and genomic handlers, so an agent matching on them would not pick it for a VCF or BAM directory. The genomic handlers also add --genomic-sample-ids, which publishes a cohort manifest when set, and leave index files unclaimed; the skill now says both.
|
merged main in (ce521f5), only uv.lock clashed, taken from main and relocked so it only adds the mcp deps (+ idna 3.20 which mcp needs). passes --locked on the latest uv. also updated SKILL.md for the formats that landed since: spreadsheet, hdf5 and the genomic ones, w/ a note on --genomic-sample-ids (966bbb3). 1638 tests green |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rafiattrach
left a comment
rafiattrach
left a comment
There was a problem hiding this comment.
Thanks, this is a clean piece of work. I built the wheel, installed it in a fresh environment, drove the server over stdio with a real MCP client, and baked the same fixtures through the CLI and through the server. The outputs are byte for byte identical, the helper extraction in main.py is a true move and not a copy, and the hatch build block is needed (without it the wheel ships no skill at all). Every flag the skill mentions exists.
One thing has to change before this can ship: the mcp extra does not exist for anyone who installs from PyPI, so the feature is unreachable for normal users. Details on the line.
The rest, roughly in order of weight:
validate reports a missing file as an invalid document, so an agent tries to repair nothing
bake returns the full per file list, which floods the agent on a large dataset
validate fetches URLs, which contradicts the "nothing leaves the machine" promise
an empty creator list publishes a placeholder person, where the CLI refuses
a few things in the skill that would send an agent the wrong way
nits on tests and packaging
| ] | ||
| # Optional: the local stdio MCP server (`croissant-baker mcp`). | ||
| # Install with: uv sync --group mcp | ||
| mcp = ["mcp>=2.2"] |
There was a problem hiding this comment.
This is the one blocker.
[dependency-groups] is for people working inside this repo. It is not written into the wheel, so nobody who installs from PyPI can get it. I built the wheel from this branch and checked:
Provides-Extra: none
Requires-Dist mentioning mcp: none
$ pip install "croissant_baker-*.whl[mcp]" # succeeds, installs nothing
$ python -c "import mcp" # ModuleNotFoundError
So pip install croissant-baker[mcp] looks like it worked and the server still will not start. That is the worst kind of failure because the user has no idea what went wrong.
Fix is one move:
[project.optional-dependencies]
mcp = ["mcp>=2.2"]The dependency group can stay as an alias for contributors if you like. Default installs are unchanged either way, the extra stays opt in.
Same gap, second half: README.md never mentions MCP, the skill, or agents, and there is no page for it under docs/. The README is the only thing a PyPI user reads. A short section under "Installation" or "Quick start" saying how to install the extra and how to point a client at croissant-baker mcp would close it. The docs/reference/cli.md edit can be dropped, that file is regenerated on deploy (its first line says so).
There was a problem hiding this comment.
good catch, thx. mcp is now a real extra in [project.optional-dependencies], so pip install croissant-baker[mcp] works. i dropped the group and CI uses --extra mcp, one less thing to keep in sync. new tests/test_packaging.py builds the wheel through hatchling's build_wheel hook and checks Provides-Extra and Requires-Dist, so this can't come back quietly. 39a802b, 701e0c0, 9089aaf
README now has a short "Use with an agent" section with the install and the client config. the docs/reference/cli.md edit is dropped. b49140f
| return {"output": output, "report": generator.scan_report.to_dict()} | ||
|
|
||
|
|
||
| def validate(path: str) -> dict: |
There was a problem hiding this comment.
The new _reported_to_client wrapper is the right fix for the blank errors, thanks. One case it does not cover is here in validate: a missing file comes back as {"valid": False, "error": "File … does not exist."}, the same shape as a real validation failure. An agent branching on valid will then try to repair a document that was never there.
Raising ToolError for a path that does not exist or cannot be read, and keeping valid: False for "mlcroissant looked at it and refused", keeps the two apart.
There was a problem hiding this comment.
agreed. a missing or unreadable path is now a ToolError. valid: False is only for a real mlcroissant refusal. 6259c82
| url=url, | ||
| license=license, | ||
| citation=citation, | ||
| creators=_parse_creators(creators) or None, |
There was a problem hiding this comment.
An empty creators list produces a document naming a person who does not exist. The CLI refuses this outright:
CLI: croissant-baker -i data --name T
Error: At least one '--creator' option is required …
MCP: bake(input_dir="data", name="T", creators=[], …)
succeeds, writes:
"creator": {"@type": "sc:Person", "name": "Dataset Creator",
"email": "creator@example.com"}
creators being required in the schema does not help, [] satisfies it. So the path with no human watching is the one that publishes fake data, which is backwards.
Best fixed in the shared code both paths call, so the two can never disagree again. That is what the extraction was for.
There was a problem hiding this comment.
thx, that one was bad. parse_creators in the shared code now refuses an empty list, and the CLI calls the same function for its check, so both refuse the same input. 1b4515c
over MCP the message names the creators argument, not the --creator flag. same for a bad date_published. CLI text is unchanged. 03061b6. the cli and the server now use one date helper. 3897e80
| ) | ||
| metadata_dict = generator.generate_metadata() | ||
| _save_dict(metadata_dict, output, validate=True) | ||
| return {"output": output, "report": generator.scan_report.to_dict()} |
There was a problem hiding this comment.
Two things about what bake hands back.
Size. This returns the full per file report. On a 5000 file tree that is about 300 KB of JSON, around 77k tokens, in one tool result. The CLI deliberately prints a fixed size summary for bake and makes the per file list opt in (_echo_scan_coverage, verbose). The server has no cap and no switch, so a PhysioNet scale bake floods the agent's context in one call.
Options: (a) return only the counters plus by_reason, and let the agent ask dry_run for details, (b) add a verbose flag defaulting to off, (c) write the report to a file next to the output and return its path. I would do (a) plus (c): it matches the CLI, and the agent still has everything on disk if it wants it.
Path. output is echoed back exactly as given. Relative paths resolve against the server's working directory, which the agent does not control and often cannot see. I launched the server in a temp directory and called bake(output="out.jsonld"): the file landed in that temp directory and the response said "output": "out.jsonld". Return str(Path(output).resolve()) and have the skill tell the agent to pass absolute paths.
There was a problem hiding this comment.
did (a) plus (c) as u suggested. bake returns the counters and by_reason, and writes the full report next to the output (out.jsonld gets out.report.json). it returns report_path. both paths come back resolved. 90cf4c8
the README and the skill tell agents to pass absolute paths. b49140f, 2404de8
two follow ups. a report written inside the dataset dir broke the next bake (the JSON handler claimed it), so the output and report are now left out of the scan when they sit under input_dir. b11c3ee. the skip is by exact relative path, so data with the same name deeper in the tree is still described. 42e9dda. dry_run takes the same optional output and skips the same files, so its counts match the bake. 98f4bb9. and dry_run had the same flood at step one, so it now counts every file but lists only the unclaimed ones, the ones the agent has to decide about. e330710
| import mlcroissant as mlc | ||
|
|
||
| try: | ||
| mlc.Dataset(path) |
There was a problem hiding this comment.
mlc.Dataset(path) accepts a URL and fetches it. So validate("https://…/x.jsonld") makes an outbound HTTP request from the user's machine, while the help text for the mcp command promises "no outbound requests, so a bake still never leaves the local environment", and the skill repeats it.
The CLI has the same behaviour, so this is not new. But the server is the surface an agent drives from untrusted text, and a URL smuggled into a prompt becomes a GET from the user's machine. One check before calling mlcroissant, Path(path).is_file() else ToolError, keeps the promise true. Or drop the promise. Keeping it is better.
|
|
||
| ### 2. Resolve or accept every refusal | ||
|
|
||
| Each unclaimed file carries one of a fixed set of reasons. Decide, do not skip: |
There was a problem hiding this comment.
The table lists the human labels the terminal prints (no handler, archive, not opened, unreadable while selecting a handler). But the skill then tells the agent to read report.json and the MCP by_reason map, and those carry the machine keys (no_handler, archive, claim_failed, unsupported_input, extract_failed, duplicate_by_name, …). A live dry_run returned by_reason: {"archive": 1}. An agent reading JSON cannot match claim_failed to any row. Add the key to each row.
Also, line 48 and line 190 say dry_run "reads no file contents". It reads the first 4 KiB of .json files to tell FHIR from plain JSON, which is why the "unreadable while selecting a handler" reason exists. "Reads at most a small header per file" is accurate.
There was a problem hiding this comment.
the table now has a key column (no_handler, archive, claim_failed, ...) next to the label. a test checks every Reason has a row with both its key and its REASON_LABELS label. the skill now says "reads at most a small header per file". 2404de8
fixed the same wording in the dry_run and dry_run_entries docstrings. cce3525
| lineage, agents, platforms. | ||
|
|
||
| Use flags for two or three fields; switch to the YAML as soon as the user has | ||
| lineage or activities to record. When you need the YAML, read |
There was a problem hiding this comment.
An MCP client cannot follow this. The server publishes only SKILL.md as a resource, so an agent that got here through MCP is sent to a file it has no way to read. Either publish the template as a second resource or inline the accepted key list here. Publishing it is cleaner.
There was a problem hiding this comment.
published it as a second resource, croissant-baker://rai-template, and the skill points at it. a test checks that every resource URI the skill names is one the server serves. 5107d9b
| @@ -0,0 +1,86 @@ | |||
| # RAI config template for `croissant-baker --rai-config <file>`. | |||
There was a problem hiding this comment.
Two things.
Sync. There are now two RAI templates: rai-example.yaml at the repo root and this one. Both have to match rai/schema.py by hand. That is how the social_impact bug lived unnoticed until #134 (merged today, main now has the fixed root example). A test that loads this file through load_rai_config and asserts every accepted key appears in it would catch the next drift on both sides. Fifteen lines.
Leftovers. I filled nothing in and loaded it: it passes. So a forgotten placeholder ships as rai:dataBiases: "REPLACE: who and what is over…" in a published file. The loader will not catch that, it is valid text. One sentence in the skill's RAI section, "check for any remaining REPLACE before baking", covers it. Or the test above can assert the word does not survive a fill.
There was a problem hiding this comment.
sync: the schema test that guards rai-example.yaml now runs on both templates, and both must load through load_rai_config. it found two missing keys in the skill copy (source_datasets.id, platforms.url), now added.
leftovers: the skill says to check for any remaining REPLACE before baking. every blank in the template now has REPLACE in it, example URLs and dates too, and a test keeps it that way. also fixed a stale comment that said a misspelled key is ignored. 3e374e1, 701e0c0
| assert link.resolve() == SKILL_DIR.resolve() | ||
|
|
||
|
|
||
| def test_the_installed_package_carries_the_skill() -> None: |
There was a problem hiding this comment.
nit: test gaps, all cheap, all guarding claims this PR makes.
- This test cannot fail. Under the editable install
uv synccreates,importlib.resources.files("croissant_baker")resolves back tosrc/, so it compares the file with itself. It passes whether or not the wheel ships anything. - Nothing compares CLI and MCP output. I checked they match today, but
test_mcp_server.pycalls the functions directly and never diffs against the CLI. One fixture baked both ways, asserted byte identical, pins the parity the whole extraction exists for. test_bake_writes_a_file_validate_acceptspasses only required arguments. Deletingincludes,excludes,detect_references,urlorcitationfrom theMetadataGeneratorcall fails no test.- Nothing checks the 27 flags in
SKILL.mdagainst the real CLI. A rename would leave the skill telling agents to pass a flag that no longer exists, with no CI signal. Pull--[a-z0-9-]+out of the body and assert each is a registered option.
There was a problem hiding this comment.
all four done, thx.
- self compare test is gone. tests/test_packaging.py builds a real wheel and checks the skill files and the extra in it. 39a802b, 701e0c0
- CLI vs MCP parity: one fixture baked both ways, document and report asserted byte identical. 1d9313f
- bake test now passes includes, excludes, detect_references, url, citation and date_published, and checks each one in the document. 90cf4c8
- flags: a test pulls every
--flagout of SKILL.md and checks it is a registered option on the CLI or a subcommand. 2404de8
| @@ -0,0 +1,229 @@ | |||
| --- | |||
| name: croissant-baker | |||
| description: Generate and validate Croissant 1.1 JSON-LD dataset metadata with the croissant-baker CLI, which walks a directory and infers FileObjects and RecordSets from CSV, TSV, Parquet, FHIR, JSON, JSONL, WFDB, DICOM, NIfTI, image and GEO SOFT files, and refuses to guess the semantic fields. Use this skill whenever the user wants dataset metadata, an mlcroissant or Croissant file, a NeurIPS Datasets and Benchmarks submission, a PhysioNet or other controlled-access clinical or biomedical release, RAI (Responsible AI) dataset documentation, or an answer about FileObject, RecordSet, distribution or conformsTo entries. Use it also when they only say "describe this data directory", "document these files", "make my dataset machine-readable" or "generate a data card", and when they invoke the croissant-baker MCP tools dry_run, bake or validate, even if nobody says the word Croissant. Not for documenting source code or APIs; only for dataset directories. | |||
There was a problem hiding this comment.
This is the same shape of problem as the reason-table comment below, and the rai-template.yaml comment further down: information that's correct in one place, hand-copied into another, with nothing that catches it drifting.
The repo already has the right pattern for this: docs/generate.py builds docs/_generated/formats-table.md straight from the handler registry, so that table can never go stale. This description isn't wired into that generator at all, it's just typed by hand, which is how spreadsheets/HDF5/BigTIFF fell out of sync here in the first place.
Rather than just adding the missing format to this sentence, worth fixing the pattern itself: either generate this description the same way the docs table is generated, or add a cheap test that fails when the handler registry and this description disagree. Same idea applies to the RAI template vs rai/schema.py. Otherwise this is a format (or RAI field) that goes stale again the next time one is added.
There was a problem hiding this comment.
agreed, went with the cheap test, it fits the repo better than generating frontmatter. tests/test_skill.py now fails when the handler registry and the description disagree (format names and the sub-formats they hide), when a scan reason is missing from the table, or when the skill names a flag the CLI lacks. same for the RAI side: both templates are checked against rai/schema.py. 2404de8, a853ef2, 3e374e1
A dependency group is never written into the wheel, so `pip install croissant-baker[mcp]` succeeded, installed nothing, and the server still would not start. The SDK is now the `mcp` extra; CI, DEVELOPMENT.md, the skill and the `mcp` command's error point at it. The old skill packaging test compared the source file with itself under the editable install, so it could not fail. tests/test_packaging.py builds the wheel with hatchling, the project's build backend, and checks the extra, the skill files and the missing symlink in what pip installs. hatchling joins the test group for that.
The README is all a PyPI user reads, and it never mentioned the server, the extra or the skill. The docs/reference/cli.md edit is dropped: that file is regenerated from the CLI on deploy.
…ne.py The server imported its helpers from __main__, and `python -m croissant_baker mcp` runs that file as a script first, so the import loaded a second copy of it with its own Typer app. Harmless now, but any module level state added there later would split in two. parse_creators, dry_run_entries, save_dict, write_jsonld and get_version now live in their own module, which both front doors import. The bodies are moved unchanged.
The CLI refused a bake with no --creator, but the server's bake took `creators=[]`, which the schema allows, and wrote the generator's placeholder person into the document. parse_creators now refuses an empty list itself, and the CLI calls it where it used to check, so both front doors refuse the same input with the same message.
validate returned a missing file as `valid: False`, the same shape as a document mlcroissant refused, so an agent would try to repair a file that was never there. It also handed URLs to mlcroissant, which fetches them, while the server promises no outbound request. A path that is not a readable local file is now a tool error, and `valid: False` is kept for a real refusal.
bake returned the full per-file report, about 77k tokens for a 5000 file tree in one tool result. It now returns the counters and by_reason, as the CLI's bounded summary does, and writes the full report next to the output, returning its path. Both paths come back resolved, since a relative output lands in the server's working directory, which the agent cannot see. The skill told agents to give a publication date, but bake had no way to take one, so every document it wrote carried the datePublished warning. The bake test passed only the required arguments, so dropping any optional one from the generator call failed nothing. A new test passes them all and checks each one reaches the document.
The guard only tried `import mcp`, so an mcp 1.x installed for another tool passed it and the command then failed with a bare traceback. It now tries mcp.server.mcpserver, which only the 2.x SDK has, and prints the install hint otherwise.
No tool carried annotations, so a client had to prompt for all three or approve all three, and bake writes files and creates directories. dry_run and validate now say they only read, and bake says it can replace files. The server also told clients its version was an empty string; it now reports the installed package version.
The pipeline.py extraction exists so the two front doors cannot drift apart, but no test compared their output. This one bakes the same fixture with the same fields both ways and asserts the documents and the per-file reports are byte identical.
The skill tells the agent to start a RAI config from assets/rai-template.yaml, but the server published only SKILL.md, so an agent that came in over MCP was sent to a file it could not read. The template is now croissant-baker://rai-template, the skill names it, and a test checks that every resource URI the skill names is one the server serves.
The skill description, the reason table and the flags in SKILL.md were copied from the code by hand, with nothing to catch them drifting, which is how three registered formats fell out of the description. Like docs/generate.py does for the formats table, the code is now the source: tests fail when a registered format is missing from the description, when a scan reason has no row carrying both its terminal label and its JSON key, or when the skill names a flag the CLI lacks. The reason table gains the machine keys that report.json and by_reason carry, so an agent reading JSON can find its row. The skill no longer says dry_run reads no file contents: it reads a small header to pick a handler. The MCP section follows the new bake result, the date_published argument, the validate errors and the template resource, and asks for absolute paths. The RAI section asks the agent to check for any REPLACE left before baking.
…lank There are two RAI templates, rai-example.yaml and the skill's copy, and both had to match rai/schema.py by hand. The schema check that guards rai-example.yaml now runs on both, and both must load through load_rai_config. It found the skill copy missing source_datasets.id and platforms.url, which are added. Every value to fill in now contains REPLACE, including the example URLs and dates, and a test keeps it so, so the skill's search for REPLACE before baking catches every leftover. The comment saying a misspelled key is silently ignored was stale: the loader now refuses it.
The docstrings still said a dry run reads no file, while some handlers read a small header to decide whether to claim a file, and bake's did not list the empty creator list or a bad date among its refusals. A test pins the date refusal over MCP.
bake writes its report next to the output, so a bake written into the dataset directory found croissant.report.json on the next run. The JSON handler claimed it and mlcroissant refused the record set built from it, so the second bake failed. The output and the report are now excluded from the scan whenever they fall under the input directory.
bake stopped returning one entry per file, but dry_run still did, so the same flood moved to the first step of the loop. dry_run now counts every file and lists only the unclaimed ones, each with its reason, since those are the files the agent has to decide about.
The registry check only looked for each handler's FORMAT_NAME, so BigTIFF and AnnData could drop out of the description under "Images" and "HDF5" without a failure. A short list of such sub-formats, each tied to an extension that must still be registered, now has to appear in the description too. It caught the spreadsheet extensions, which go back in.
An agent that passed creators=[] or a bad date_published was told about --creator and --date-published, flags it never typed. parse_creators now takes cli=False to name the creators argument, and bake checks the date itself before the generator words it for the CLI. The CLI's messages are unchanged.
open_world_hint defaults to true, so a client had to assume each tool could touch the outside world. None does: they read and write local files only.
validate checked the resolved file but handed mlcroissant the raw string, so the two could in principle disagree. It now passes the path it checked. The install hint from the mcp command says the server needs mcp>=2.2, since an older mcp is exactly the case it catches. mcp_server.py also imports ScanReport from scan, as pipeline.py does.
…emption The packaging test skipped itself if hatchling was missing, though the test group installs it, so a broken install would have passed in silence. It now imports hatchling plainly and builds through build_wheel, the hook pip calls, rather than an internal builder class. The REPLACE check exempted every key named id at any depth, which would have let a source dataset id slip through. It now exempts only the activity keys it meant to, by dotted path.
The own-file skip went through the exclude globs, which match from the right, so ds/sub/metadata.json vanished from a bake written to ds/metadata.json, with no reason given. discover_files, scan_directory and MetadataGenerator now take skip_paths, compared with the whole relative path, and bake passes its output and report through it. The CLI passes none, so its scans are unchanged.
After a bake into the dataset directory, dry_run counted the output and its report while the next bake skipped them, so the two disagreed. dry_run takes an optional output and skips the same exact paths through the same helper, and the skill tells the agent to pass it.
The server had its own copy of the CLI's date check, worded for its argument. The CLI helper moves to pipeline.py as check_iso_dates, taking the label to name, and both front doors call it. The CLI's messages are unchanged.
…ture pytest's own context manager replaces the hand-written try and finally around the chdir, the way the rest of the suite changes directory.
|
thx for the deep pass, and for driving it with a real client. all points are in, one commit per fix so u can read them one by one. main ones: 39a802b (mcp extra), 6259c82 (validate), 90cf4c8 and e330710 (result size), 1b4515c (empty creators), 2404de8 and a853ef2 (skill drift tests). also fixed a re-bake bug my own report file caused, b11c3ee and 42e9dda. pls take another look. |
This branch has not been deployed
2 participants
Closes #129.
Two halves, both local-first and both leaving the deterministic structural pipeline untouched.
MCP:
croissant-baker mcpMetadataGenerator:dry_run(input_dir, include?, exclude?)returns a summary (total,would_process,unclaimed,by_reason) plus the per-file list with reasons;bake(...)accepts the semantic fields a human would type on the CLI, runs generation, validates before writing, and returns the output path and report;validate(path)constructsmlcroissant.Datasetand returns success or the error text.mcpSDK lives in an optional dependency group (uv sync --group mcp); runtime dependencies are unchanged and the subcommand fails with an install hint when the group is absent. Imports are lazy, socroissant-baker --helpand a normal bake never loadmcp.mcp2.x SDK renamedFastMCPtoMCPServer; the code uses the latter.Skill:
croissant-bakerSKILL.mdwithname,description,license,compatibility,metadataandallowed-toolsfrontmatter, a procedural body (dry run, resolve refusals, gather semantic fields from the user, bake, validate, iterate), a Gotchas section, and one asset (assets/rai-template.yaml).croissant_baker/skills/croissant-baker/) sopip install croissant-bakerusers receive it;.agents/skills/croissant-bakeris a symlink to it for repository-level discovery by clients that scan that path.SKILL.mdas the resourcecroissant-baker://skill, so an MCP-only client can fetch the instructions without the repository.--dry-run,--reportJSON,validate, documented exit codes).skills-ref validateand guarded bytests/test_skill.py(frontmatter constraints, name matches directory, body length, referenced paths exist, symlink resolves, packaged copy equals the repository copy).Testing
tests/test_mcp_server.py(tool functions called directly, no transport;build_server()lists exactly the three tools and the skill resource;dry_runcounters match per-file outcomes),tests/test_skill.py, and a CLI test thatcroissant-baker mcp --helpmentions stdio.uv run pytest -vwith themcpgroup: 860 passed; without it the MCP and skill-resource tests skip and the rest is unchanged.skills-ref validate: valid skill, no warnings.uv run pre-commit run --all-files: all hooks pass.Design notes
_dry_run_entriesand_parse_creatorswere extracted frommain()so the tools share the CLI's exact behaviour;--dry-runand--creatorparsing are unchanged.uv.lockwas relocked with a currentuvso the lock revision matchesmain; the diff is confined to the optional group plus the pinsmcpitself forces.