Repository navigation
feat(body-field-contract): a shared body-field parser contract, a Groovy implementation, Git/SVN storage, and custom formats at setup - #1698
Merged
Conversation
github-actions
Bot
added
family:setup
The bridge-write tests run bridge.groovy against a mock JIRA server on 127.0.0.1 with a clean environment: PATH, HOME, the tracker variables and the token. That environment drops JAVA_TOOL_OPTIONS, so a shell that sets -Djava.net.preferIPv4Stack=true loses it and the JVM opens an IPv6 socket to the loopback address. Under the agent sandbox on macOS that connect is refused (`SocketException: Operation not permitted`), and every test that reaches the server fails, so the workspace pytest hook failed on every commit that touched a Python tool. The test environment now passes JAVA_TOOL_OPTIONS through with -Djava.net.preferIPv4Stack=true added when it is not there already. IPv4 loopback is harmless everywhere else. All 28 bridge-write tests pass inside the sandbox. No other test starts a JVM with a clean environment. Part of #1672 Generated-by: Claude Opus 5
…h a shared conformance suite How a tracker body's `### <Name>` fields are found was stated only in prose spread over the GitHub tool's README, the issue-template doc and the parser's docstring, so a second implementation (another language, a local parser for a custom report format) had nothing to check itself against. tools/body-field-contract/ states the rules once, language-neutral, in RFC 2119 terms, each with a `BF-*` identifier: line endings, field boundaries (every column-0 `###` line ends a field, code block or not), names, closed code blocks, which copy of a heading is the field (the one outside every closed block; a field quoted only inside closed blocks is absent), the container-heading and mid-line Jira macro (`macro-ambiguous`) decisions, the four operations with their inputs, outputs and errors, and what a writer of untrusted text must escape (escape text, escape body). conformance/cases.jsonl holds 98 cases, each naming the clause it tests; a test checks every clause has one and every case names a real clause. tools/github-body-field/tests/test_conformance.py runs every case against the reference parser, the vetted-ops copy, the local-files tracker and the Jira bridge (reads and writes through their own entry points, lenient name lookup opting out of the exact-name cases), and the writer cases against the local-files tracker, the Jira bridge and the import's inert block. The byte-identical copy pins stay; the copies are re-synced for the parser docstring's new pointer. The taxonomy gains `contract:body-field` (validator capability set, labels doc, generated labeler config and vendor-neutrality score), the implementing tools declare it, and the issue-template doc, READMEs and specs point at the contract. Part of #1672 Generated-by: Claude Opus 5
…contract The contract is meant to be language-neutral. A second implementation, in the language the Jira bridge is written in, shows that it is and gives Groovy scripts a parser of their own. tools/body-field-contract/groovy/BodyFieldParser.groovy implements every operation of the contract: list fields, extract, replace and normalise, with the not-found, duplicate and ambiguous refusals, the container-heading and macro-ambiguous decisions, whole-body preservation on replace, and the two writer operations (escape text, escape body). It follows the conventions of tools/jira/bridge.groovy and needs nothing beyond the Groovy runtime; it pulls no dependency with @grab. conformance.groovy beside it reads cases.jsonl and prints one {"id", "result"} line per case. tools/github-body-field/tests/test_conformance.py runs it once per session when `groovy` is on PATH and compares every case with its expected result. Without groovy, the check is skipped and says why. It was executed here with Groovy 6.0.0 on JVM 27: all 98 cases pass. No CI workflow installs Groovy, so in CI this check is skipped; this commit adds no workflow for it. The contract's implementations table, its README and the spec list the Groovy implementation. Part of #1672 Generated-by: Claude Opus 5
…Git and Subversion storage The local-files tracker read report files in Python's universal-newline mode and wrote them back with LF. On a Git checkout with core.autocrlf, or an SVN one with svn:eol-style=native, a report that reached the working copy with CRLF line endings was rewritten in full by any field-set: every line of it changed, not just the field. A lone CR was turned into a line break as well. Reads now take the file as stored, remember its convention (LF, CRLF or CR, and whether it ends with one) and hand the parser LF lines; writes put that convention back. A field write therefore changes no byte outside the field, whatever the storage did to the file. A file that mixes line endings is refused (exit 4) until it is normalised to one. Expanded svn:keywords ($Id: ... $) are ordinary text to the parser. The store keeps the parser's duplicate or ambiguous error as the cause of its refusal, so callers can tell the two apart. tests/test_storage_conformance.py runs the body-field conformance cases (extract and replace) through the store. Each case body becomes a report in a temporary Git repository with core.autocrlf=true and in a temporary SVN working copy with svn:eol-style=native and svn:keywords. The cases are committed, the field reads and writes run, and the result is committed and read back from a fresh checkout. The field values must match the contract's and the untouched content must be byte-identical. The SVN half skips with a reason when svn or svnadmin is missing; both ran here. Unit tests cover CRLF, CR and LF files, a missing final newline and a file with mixed endings. Part of #1672 Generated-by: Claude Opus 5
…detected report format
A team whose existing reports keep their fields some other way than `###`
sections (`Name: value` lines, say) could adopt the local-files tracker,
but no field in those reports could be read: the default parser finds
none, and nothing said so.
detect now reports `field_layout`: `default` or `custom`, a one-line
description, and as evidence the field names that two or more reports
share. It never reports report text. A custom layout is noted, and the
descriptor records it.
The contract gains a format adapter interface (BF-FMT-1 to BF-FMT-3): the
contract's operations, refusals and decisions for one layout's own boundary
rule, standard library only, with synthetic-value fixtures beside the
adapter. tools/body-field-contract/formats/ is the upstream home for
adapters; its _template.py is a working `Name: value` adapter and the
skeleton to copy. The conformance runner runs every adapter there, and any
named by BODY_FIELD_FORMAT_ADAPTERS: the generic properties every layout
shares, and the adapter's own fixtures.
`setup config` gains a local-files sub-step, kept in
local-files/custom-parser.md so the skill's headings and surface hash stay
as they are. When detect says custom, the sub-step:
- writes a parser from the template to
.apache-magpie-overrides/tools/local-tracker/parsers/<name>.py, with
synthetic fixtures and never report text;
- runs the conformance suite on it;
- enables it only on the user's explicit confirmation of its SHA-256.
The new `trust-parser` records `body_parser: {path, sha256}` in the
descriptor, and the same hash in the working copy's personal layer, which
is never committed.
The parser is code that vetted operations run outside the sandbox, so every
command loads it only when all of these hold:
- the descriptor names it;
- its path is a .py file directly in the parser directory, with no `..`
and no symlink on the way;
- its hash matches both the descriptor and the personal layer.
Otherwise the command stops with exit 2, saying to review the change and
re-confirm it. A custom layout is never read with the default parser.
field-get, field-set and body go through the parser; create and edit with
a whole `###` body are refused on a custom layout. A test covers every
refusal.
setup-override-upstream lists generated parsers and upstreams them as
format adapters (cases/parser-adapters.md). A new setup eval suite,
config-local-custom-parser, covers four cases: a custom layout,
the default layout, a declined parser, and a confirmed one.
Self-eval passed 4/4.
Part of #1672
Generated-by: Claude Opus 5
A write keeps the file's own line-ending convention and refuses a file that mixes several. Report text comes from outside the team, though, and a report or field value carrying its own \r\n or lone \r was written as it came: the file then mixed conventions, and every later write to that tracker was refused until someone fixed the file by hand. Untrusted text now has its line endings turned into \n before it is escaped, and every write normalises its text before putting the file's convention back, so no write can leave a file mixing them. A file that already mixed them before the tracker touched it is still refused. Generated-by: Claude Opus 5
A custom body parser is code from the tracker's repository, and vetted operations run it outside the sandbox, so it loads only when its hash matches one the user confirmed. That confirmation was not the user's: - On Subversion it lived in <top>/.svn/apache-magpie/. In a repository hosted on Git a .svn/ directory is ordinary committed content, and the working-copy probe takes the nearest .svn/, so one commit carrying a parser, its descriptor entry and a .svn/ trust record got the parser run. - trust-parser only compared the hash it was handed. The confirmation lived in the skill's prose, and the agent ran the command itself, writing the record into the git directory, which a sandboxed command may write. The record now lives in ~/.config/apache-magpie/local-tracker-parsers.json, under the working copy's real path: outside every working copy, so nothing a commit carries can stand in for it, and a path the secure setup never lets a sandboxed command write. trust-parser asks on the controlling terminal and refuses without one, and setup hands the user the command to run in their own terminal instead of running it. Generated-by: Claude Opus 5
The user's parser confirmations live in ~/.config/apache-magpie, beside the Gmail token, and the secure setup's sandbox reads nothing there. A sandboxed command on a custom-layout tracker therefore could not see the record, and said the parser had to be reviewed and re-confirmed, which cannot help: a new confirmation lands in the same unreadable place. A record the sandbox cannot read now says the parser cannot be verified here and to run the command through vetted operations, which run outside the sandbox; doctor marks that check unverifiable_here. trust-parser, which the user runs in their own terminal, loads the parser after recording it and prints parser_loads, so setup checks that instead of a sandboxed doctor. A default-layout tracker never reads the record. The agent's own file tools are not sandboxed, so the floor now denies Edit(~/.config/apache-magpie/local-tracker-parsers.json), and sandbox-lint requires that deny. Generated-by: Claude Opus 5
… cannot annotate Generated-by: Claude Opus 5
potiuk
force-pushed
the
feat/body-field-contract
branch
from
October 18, 2026 17:05
d01234b to
20617cb
Compare
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
1 participant
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.
Follow-up to #1680 and #1686 (#1672, umbrella #1342).
What this adds
The security trackers' body fields are read and written through one parser since #1680.
This PR turns that parser's behaviour into a contract other implementations can follow, and puts it to work in three places.
tools/body-field-contract/(contract:body-field).contract.mdstates the rules normatively: every column-0###line ends a field, closed code blocks (CommonMark fences, Jira{code}/{noformat}) only decide which heading starts a field, a field quoted only inside a closed block is absent, names are whitespace-lenient and case-sensitive, and ambiguous bodies come back asneeds_decision(duplicate,container-heading,macro-ambiguous) instead of a guess.conformance/cases.jsonlholds 98 cases; a runner checks every implementation against them, including the byte-identical Python copies in vetted-ops, local-tracker and the Jira bridge.tools/body-field-contract/groovy/BodyFieldParser.groovy, for JVM-side tooling such as the Jira bridge.It passes all 98 cases (Groovy 6.0.0). No CI workflow installs Groovy yet, so its pytest adapter skips there.
detectreports a field layout the stock parser does not cover, setup generates a parser for it in.apache-magpie-overrides/tools/local-tracker/parsers/from theformats/template, following the contract's format adapter interface, runs the conformance suite on it, and suggests upstreaming it to Magpie throughsetup-override-upstream.How a custom parser is trusted
The parser is code from the tracker's repository, and vetted operations run it outside the sandbox.
.., no symlink) and its SHA-256 matches both the committed descriptor and the user's own confirmation; anything else stops with exit 2, never falling back to the default parser.~/.config/apache-magpie/local-tracker-parsers.json, under the working copy's real path: outside every working copy, so nothing a commit carries can stand in for it (a committed.svn/directory included).trust-parserasks on the controlling terminal and refuses without one; setup hands the user the command to run in their own terminal and never runs it. It then loads the parser and printsparser_loads.Edit(~/.config/apache-magpie/local-tracker-parsers.json)inpermissions.deny, since the agent's file tools are not sandboxed, and sandbox-lint requires it.~/.config/apache-magpie, so a custom-layout tracker is read through vetted operations; a sandboxed command says the parser cannot be verified there (anddoctormarks the checkunverifiable_here) instead of asking for a re-confirmation. Default-layout trackers never read the record.Fixes from the security review
\r\nor lone\rwas written as it came, leaving the tracker file mixing line endings, which every later write refuses. Untrusted text is now normalised before it is escaped, and every write normalises before putting the file's convention back.<git-common-dir>/.svn/apache-magpie/, and the agent rantrust-parser: a committed.svn/record could confirm a parser, and the agent could confirm one itself. Both are closed as described above.Also: the Jira bridge tests keep the JVM on IPv4 loopback (
-Djava.net.preferIPv4Stack=true), so they pass inside the macOS sandbox, where the IPv6 loopback socket is denied.Verification
prek run --all-filespasses.mainbeyond CR removal.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines
🤖 Generated with Claude Code