fix(reports): preserve header metadata across all readers - #1137
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
Contributor
|
Codex review: needs maintainer review before merge. Reviewed August 31, 2026, 4:29 PM ET / 20:29 UTC. ClawSweeper reviewWhat this changesThe PR centralizes parsing of report leading metadata so quoted prose and fenced examples do not override report headers used by review, repair, workflow selection, and decision packets. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open: the base revision still treats quoted body keys as conflicting metadata, while this PR replaces those scans across the affected readers and supplies exact-head consumer and final-effect proof. No introduced correctness or security blocker was found. Priority: P2 Review scores
Verification
How this fits togetherClawSweeper reads persisted Markdown reports to decide whether to publish reviews, enqueue repairs, or close GitHub items. This change affects the shared header parser that supplies metadata to those downstream decision and apply paths. flowchart LR
A[Persisted report] --> B[Leading metadata parser]
B --> C[Review and packet readers]
B --> D[Repair intake]
B --> E[Workflow selection]
C --> F[Apply decision guards]
D --> G[Repair job]
E --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Land the shared leading-header parser with its focused regression coverage and retain the documented fail-closed behavior for duplicate or delimiter-bounded competing metadata. Do we have a high-confidence way to reproduce the issue? Yes. The base source directly reproduces the multiline body scan, and the supplied exact-base versus exact-head Crabbox observations exercise the affected consumers and report the before/after result. Is this the best way to solve the issue? Yes. One shared structural reader removes duplicated whole-body scans while preserving per-consumer decoding and legacy promotion behavior instead of adding a new report format or migration. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against aba9826ab8c0. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🦞 diamond lobster
Contributor
Author
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Contributor
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
status: 📣 needs proof
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Replace duplicated body scans with shared leading-header structure across report, decision, and repair readers. Preserve adapter decoding, literal field handling, and legacy promotion guards while distinguishing ordinary quoted prose from ambiguous metadata. Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Static generated Node programs and explicit path data replace avoidable source interpolation. The network-denial preload is independently self-tested.
steipete
changed the title
Contributor
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Contributor
Controlled persisted-record final-effect repro sourceThis is the complete external harness used for the additional authority-chain proof on head Use a fresh Linux environment with Node24.20.0, pnpm11.10.0, the exact full source checkout, locked dependencies installed, and fresh PROOF_SOURCE_ROOT="$PWD" INPUT_DIR=/tmp/final-effect-inputs OUTPUT_DIR=/tmp/final-effect-results node /tmp/final-effect-inputs/metadata-final-effect-proof.mjsThe driver reuses the committed report fixture and pure source-identity helper. It replaces only the external GitHub transport and network/process boundary, never the parser, decision guard, or close executor. Two positives must reach the final close PATCH; adding only two delimiters around the same body field must reject before any transport call. Independent network/unknown-target self-checks live in separate directories from application counters. One selected item and one close are allowed; Scope is the supported retained-report issue apply path. Matching updated-at plus computed source/timeline identities exercise its normal freshness checks. The fixture retains the committed metadata-final-effect-proof.mjsSHA256: // Controlled production apply code on synthetic persisted state; no live GitHub transport.
import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import { createHash } from "node:crypto";
import { cpSync, existsSync, mkdirSync, readFileSync, readdirSync, realpathSync,
symlinkSync, writeFileSync } from "node:fs";
import { join, resolve, sep } from "node:path";
import { pathToFileURL } from "node:url";
assert.equal(process.versions.node, "24.20.0");
assert.equal(process.platform, "linux", "Prepared for the parent secretless Linux runner only");
for (const name of ["PROOF_SOURCE_ROOT", "INPUT_DIR", "OUTPUT_DIR"])
assert.ok(process.env[name], "Required environment: " + name);
const source = realpathSync(process.env.PROOF_SOURCE_ROOT);
const inputDir = realpathSync(process.env.INPUT_DIR);
const outputArg = resolve(process.env.OUTPUT_DIR);
const out = join(realpathSync(resolve(outputArg, "..")), outputArg.split(sep).at(-1));
assert.ok(!existsSync(out), "OUTPUT_DIR must be new");
assert.ok(out !== source && inputDir !== source);
assert.ok(!out.startsWith(source + sep) && !inputDir.startsWith(source + sep));
const inputs = JSON.parse(readFileSync(join(inputDir, "metadata-final-effect-inputs.json")));
assert.equal(inputs.head, "5087c40b3e223bfa97ac14749073381077666928");
assert.equal(inputs.tree, "def4e53ddc68bf699a571e4d8302334e210c3002");
assert.deepEqual(inputs.cases.map((c) => [c.name, c.expected]), [
["positive-clean", "closed"], ["positive-quoted", "closed"],
["negative-competing", "rejected-before-github"],
]);
assert.equal(JSON.parse(readFileSync(join(source, "package.json"))).packageManager, "pnpm@11.10.0");
const hash = (value) => createHash("sha256").update(value).digest("hex");
function inventory(root, prefix = "", excludeBuild = false) {
return readdirSync(root, { withFileTypes: true }).sort((a,b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)
.filter((entry) => !(excludeBuild && [".git", "node_modules", "dist", ".artifacts", ".crabbox"].includes(entry.name)))
.flatMap((entry) => {
assert.ok(!entry.isSymbolicLink(), "unexpected inventory link: " + entry.name);
const path = prefix + entry.name;
return entry.isDirectory() ? inventory(join(root, entry.name), path + "/", excludeBuild)
: [{ path, sha256: hash(readFileSync(join(root, entry.name))) }];
}).sort((a,b) => a.path < b.path ? -1 : a.path > b.path ? 1 : 0);
}
const sourceBefore = inventory(source, "", true);
assert.equal(sourceBefore.length, inputs.sourceFileCount);
assert.equal(hash(JSON.stringify(sourceBefore)), inputs.sourceInventorySha256);
const distBefore = inventory(join(source, "dist"));
mkdirSync(out, { recursive: true });
const tooling = join(out, "tooling"); mkdirSync(tooling);
for (const name of ["metadata-final-effect-gh.cjs", "metadata-final-effect-guard.cjs"])
cpSync(join(inputDir, name), join(tooling, name));
const transport = join(tooling, "metadata-final-effect-gh.cjs");
const guard = join(tooling, "metadata-final-effect-guard.cjs");
const results = [];
const manifest = { claim: "Production apply code on isolated synthetic persisted state; no live production apply",
executionStatus: "started", sourceHead: inputs.head, sourceTree: inputs.tree,
behaviorBaseline: inputs.behaviorBaseline, baselineExecuted: false,
sourceBefore, distBefore, inputsSha256: hash(readFileSync(join(inputDir, "metadata-final-effect-inputs.json"))),
scriptSha256: hash(readFileSync(new URL(import.meta.url))),
tooling: inventory(tooling), node: process.version, platform: process.platform, cases: results };
const save = () => writeFileSync(join(out, "manifest.json"), JSON.stringify(manifest, null, 2) + "\n");
function environment(root, cli = "") {
for (const name of ["home", "tmp", "config-home", "cache-home", "data-home", "state-home", "empty-bin"])
mkdirSync(join(root, name), { recursive: true });
return { PATH: join(root, "empty-bin"), HOME: join(root, "home"), TMPDIR: join(root, "tmp"),
XDG_CONFIG_HOME: join(root, "config-home"), XDG_CACHE_HOME: join(root, "cache-home"),
XDG_DATA_HOME: join(root, "data-home"), XDG_STATE_HOME: join(root, "state-home"), CI: "1",
GH_BIN: process.execPath, GH_BIN_ARGS: JSON.stringify([transport]),
FINAL_EFFECT_CASE: root, FINAL_EFFECT_CLI: cli, FINAL_EFFECT_TRANSPORT: transport,
FINAL_EFFECT_DENIED_LOG: join(root, "denied.jsonl") };
}
function run(root, env, name, args) {
const argv = [process.execPath, "--require", guard, ...args];
const result = spawnSync(argv[0], argv.slice(1), { cwd: root, env, encoding: "utf8",
timeout: 120000, maxBuffer: 10 * 1024 * 1024 });
writeFileSync(join(root, name + ".stdout"), result.stdout ?? "");
writeFileSync(join(root, name + ".stderr"), result.stderr ?? "");
writeFileSync(join(root, name + "-command.json"), JSON.stringify({ argv, argvSha256: hash(JSON.stringify(argv)), env,
cwd: root, exit: result.status, signal: result.signal, error: result.error?.message }, null, 2));
assert.equal(result.error, undefined);
assert.equal(result.signal, null);
return result;
}
const lines = (file) => existsSync(file) ? readFileSync(file, "utf8").trim().split("\n").filter(Boolean).map(JSON.parse) : [];
try {
const self = join(out, "self-checks"); mkdirSync(self);
const selfEnv = environment(self);
writeFileSync(join(self, "github-state.json"), JSON.stringify({ issue: inputs.issue, comments: [], nextComment: 9321 }));
const network = run(self, selfEnv, "network", ["--eval", 'fetch("https://example.invalid")']);
assert.equal(network.status, 1);
assert.match(network.stderr, /FINAL_EFFECT_DENIED: network/);
assert.deepEqual(lines(join(self, "denied.jsonl")), [{ kind: "network", detail: "fetch" }]);
const rejected = run(self, selfEnv, "transport", [transport, "api", "repos/example/forbidden/issues/999", "--method", "PATCH"]);
assert.equal(rejected.status, 97);
assert.match(rejected.stderr, /FINAL_EFFECT_TRANSPORT_REJECTED/);
assert.deepEqual(lines(join(self, "transport.jsonl")).map((r) => [r.kind, r.accepted]), [["rejected", false]]);
manifest.selfChecks = { network: "denied", unknownTransport: "rejected", separateFromApplicationCounters: true };
// Isolate fixture-module initialization too; never retain caller credentials or state roots.
const generatorEnv = environment(join(out, "fixture-generation"));
for (const key of Object.keys(process.env)) delete process.env[key];
Object.assign(process.env, generatorEnv);
process.chdir(out);
// Existing committed fixtures and pure source identity helper, not replacement guards.
const { implementedCloseReport } = await import(pathToFileURL(join(source, "test/helpers.ts")).href);
const { itemSourceRevisionSha256ForTest } = await import(pathToFileURL(join(source, "dist/clawsweeper.js")).href);
// Keep the committed legacy fixture sentinel; timestamp + actual source/timeline
// identities govern freshness here. This does not claim snapshot-fallback coverage.
const common = implementedCloseReport({ ...inputs.reportOverrides,
item_source_revision: itemSourceRevisionSha256ForTest(inputs.issue, []),
review_timeline_revision: hash("[]") });
writeFileSync(join(out, "common-report.md"), common);
manifest.commonReportSha256 = hash(common);
for (const fixture of inputs.cases) {
const root = join(out, fixture.name); mkdirSync(root);
const runtime = join(root, "runtime"); mkdirSync(runtime);
for (const name of ["dist", "config", "schema", "prompts", "package.json"])
cpSync(join(source, name), join(runtime, name), { recursive: true });
symlinkSync(join(source, "node_modules"), join(runtime, "node_modules"), "dir");
const cli = join(runtime, "dist/clawsweeper.js");
const env = environment(root, cli);
const items = join(root, "records/items"), closed = join(root, "records/closed");
for (const dir of [items, closed, join(runtime, ".artifacts")]) mkdirSync(dir, { recursive: true });
const report = common + fixture.suffix;
const reportFile = join(items, "321.md"); writeFileSync(reportFile, report);
writeFileSync(join(root, "before.md"), report);
writeFileSync(join(root, "github-state.json"), JSON.stringify({ issue: inputs.issue, comments: [], nextComment: 9321 }, null, 2));
const outcomePath = join(root, "apply-report.json");
const args = [cli, "apply-decisions", "--target-repo", "openclaw/openclaw", "--item-number", "321",
"--skip-dashboard", "--apply-kind", "issue", "--record-root", root, "--items-dir", items,
"--closed-dir", closed, "--plans-dir", join(root, "records/plans"),
"--decision-packets-dir", join(root, "records/decision-packets"), "--report-path", outcomePath,
"--artifact-dir", join(root, "artifacts"), "--canonical-record-baseline-dir", join(root, "baselines"),
"--cursor-trace", join(root, "cursor.json"), "--limit", "1", "--processed-limit", "2"];
const result = run(root, env, "apply", args);
const calls = lines(join(root, "transport.jsonl"));
const outcome = existsSync(outcomePath) ? JSON.parse(readFileSync(outcomePath)) : null;
const state = JSON.parse(readFileSync(join(root, "github-state.json")));
const afterPath = existsSync(reportFile) ? reportFile : join(closed, "321.md");
const after = readFileSync(afterPath); writeFileSync(join(root, "after.md"), after);
const row = { name: fixture.name, expected: fixture.expected, exit: result.status, outcome,
beforeSha256: hash(report), afterSha256: hash(after), persistedAfterPath: afterPath,
reads: calls.filter((r) => r.kind === "read").length,
writes: calls.filter((r) => r.kind === "write").length,
closeCalls: calls.filter((r) => r.effect === "close"), rejectedCalls: calls.filter((r) => !r.accepted),
denied: lines(join(root, "denied.jsonl")), finalIssueState: state.issue.state,
transportSha256: hash(JSON.stringify(calls)),
outcomeSha256: existsSync(outcomePath) ? hash(readFileSync(outcomePath)) : null,
githubStateSha256: hash(readFileSync(join(root, "github-state.json"))) };
results.push(row); save();
assert.equal(result.status, 0, result.stderr);
assert.deepEqual(row.denied, []);
assert.deepEqual(row.rejectedCalls, []);
if (fixture.expected === "closed") {
assert.equal(row.closeCalls.length, 1);
assert.deepEqual(row.closeCalls[0].payload, { state: "closed", state_reason: "completed" });
assert.equal(state.issue.state, "closed");
assert.ok(outcome.some((item) => item.number === 321 && item.action === "closed"));
assert.equal(existsSync(reportFile), false);
assert.match(after.toString(), /^action_taken: closed$/m);
} else {
assert.equal(row.reads, 0); assert.equal(row.writes, 0); assert.equal(calls.length, 0);
assert.equal(state.issue.state, "open"); assert.equal(existsSync(join(closed, "321.md")), false);
assert.equal(outcome.length, 1); assert.equal(outcome[0].action, "kept_open");
assert.equal(outcome[0].reason, "invalid maintainer_decision: report front matter is ambiguous");
}
}
assert.deepEqual(inventory(source, "", true), sourceBefore);
assert.deepEqual(inventory(join(source, "dist")), distBefore);
manifest.executionStatus = "passed";
save(); console.log(JSON.stringify({ status: "passed", manifest: join(out, "manifest.json"), cases: results }, null, 2));
} catch (error) {
manifest.executionStatus = "failed"; manifest.error = String(error); save(); throw error;
} finally {
manifest.sourceAfter = inventory(source, "", true);
manifest.distAfter = inventory(join(source, "dist"));
manifest.sourceUnchanged = JSON.stringify(manifest.sourceAfter) === JSON.stringify(sourceBefore);
manifest.distUnchanged = JSON.stringify(manifest.distAfter) === JSON.stringify(distBefore);
save();
}metadata-final-effect-gh.cjsSHA256: // The only GitHub transport: local synthetic state, with no forwarding fallback.
require("./metadata-final-effect-guard.cjs");
const fs = require("node:fs");
const path = require("node:path");
const assert = require("node:assert/strict");
const { createHash } = require("node:crypto");
const root = process.env.FINAL_EFFECT_CASE;
const statePath = path.join(root, "github-state.json");
const state = JSON.parse(fs.readFileSync(statePath, "utf8"));
const raw = process.argv.slice(2);
const args = raw[0] === "--repo" ? raw.slice(2) : raw;
const endpoint = args.find((arg) => /^(repos\/|search\/issues\?|graphql$)/.test(arg)) || "";
const method = args.includes("--method") ? args[args.indexOf("--method") + 1] : "GET";
const argv = [process.execPath, ...process.execArgv, ...process.argv.slice(1)];
const trace = { argv, argvSha256: createHash("sha256").update(JSON.stringify(argv)).digest("hex"),
args, method, endpoint, kind: "read", accepted: false };
const record = () => fs.appendFileSync(path.join(root, "transport.jsonl"), JSON.stringify(trace) + "\n");
const output = (value, paged = false) => {
trace.accepted = true;
record();
fs.writeFileSync(statePath, JSON.stringify(state, null, 2) + "\n");
if (args.includes("-i")) process.stdout.write("HTTP/2 200\n\n");
console.log(JSON.stringify(paged && args.includes("--slurp") ? [value] : value));
};
const input = () => {
const file = args[args.indexOf("--input") + 1];
assert.ok(args.includes("--input") && path.resolve(file).startsWith(root + path.sep));
const value = JSON.parse(fs.readFileSync(file, "utf8"));
trace.payload = value;
return value;
};
const mutate = () => {
trace.kind = "write";
state.issue.updated_at = new Date().toISOString();
};
try {
for (let i = 0; i < raw.length; i++)
if (raw[i] === "--repo") assert.equal(raw[i + 1], "openclaw/openclaw");
if (args[0] === "api" && endpoint === "repos/openclaw/openclaw/issues/321") {
if (method === "PATCH") {
const payload = input();
assert.deepEqual(payload, { state: "closed", state_reason: "completed" });
mutate();
trace.effect = "close";
state.issue.state = "closed";
state.issue.state_reason = payload.state_reason;
state.issue.closed_at = state.issue.updated_at;
} else assert.equal(method, "GET");
output({ ...state.issue, comments: state.comments.length });
} else if (args[0] === "api" && /^repos\/openclaw\/openclaw\/issues\/321\/comments(?:\?|$)/.test(endpoint)) {
if (method === "POST") {
const payload = input();
assert.equal(typeof payload.body, "string");
mutate();
const id = ++state.nextComment;
const comment = { id, user: { login: "clawsweeper[bot]" }, body: payload.body,
html_url: "https://github.com/openclaw/openclaw/issues/321#issuecomment-" + id,
created_at: state.issue.updated_at, updated_at: state.issue.updated_at };
state.comments.push(comment);
output(comment);
} else {
assert.equal(method, "GET");
output(state.comments, true);
}
} else if (args[0] === "api" && /^repos\/openclaw\/openclaw\/issues\/comments\/\d+$/.test(endpoint)) {
const id = Number(endpoint.split("/").at(-1));
const comment = state.comments.find((c) => c.id === id);
assert.ok(comment, "unknown fixture comment");
if (method === "PATCH") {
const payload = input();
mutate();
comment.body = payload.body;
comment.updated_at = state.issue.updated_at;
} else if (method === "DELETE") {
mutate();
state.comments = state.comments.filter((c) => c.id !== id);
} else assert.equal(method, "GET");
output(comment);
} else if (args[0] === "api" && /^repos\/openclaw\/openclaw\/issues\/321\/timeline(?:\?|$)/.test(endpoint)) {
assert.equal(method, "GET"); output([], true);
} else if (args[0] === "issue" && args[1] === "view" && args[2] === "321") {
output({ closedByPullRequestsReferences: [] });
} else if (args[0] === "api" && endpoint.startsWith("search/issues?")) {
assert.equal(method, "GET"); output({ total_count: 0, incomplete_results: false, items: [] });
} else if (args[0] === "label" && args[1] === "list") {
output(state.labelDefinitions ?? []);
} else if (args[0] === "label" && args[1] === "create") {
trace.kind = "write";
const name = args[2];
state.labelDefinitions ??= [];
state.labelDefinitions = state.labelDefinitions.filter((label) => label.name !== name);
state.labelDefinitions.push({ name, color: args[args.indexOf("--color") + 1],
description: args[args.indexOf("--description") + 1] });
output({});
} else if (args[0] === "issue" && args[1] === "edit" && args[2] === "321") {
mutate();
for (const [flag, add] of [["--add-label", true], ["--remove-label", false]]) {
for (let i = 0; i < args.length; i++) if (args[i] === flag) {
for (const name of args[i + 1].split(",")) {
state.issue.labels = state.issue.labels.filter((l) => l.name !== name);
if (add) state.issue.labels.push({ name });
}
}
}
output({});
} else throw new Error("unknown fixture request");
} catch (error) {
trace.kind = "rejected";
trace.error = String(error);
record();
console.error("FINAL_EFFECT_TRANSPORT_REJECTED (HTTP 422)");
process.exitCode = 97;
}metadata-final-effect-guard.cjsSHA256: // Transport/process containment only; no parser or apply guard is replaced.
const fs = require("node:fs");
const cp = require("node:child_process");
const { syncBuiltinESMExports } = require("node:module");
const deny = (kind, detail) => {
fs.appendFileSync(process.env.FINAL_EFFECT_DENIED_LOG, JSON.stringify({ kind, detail }) + "\n");
throw new Error("FINAL_EFFECT_DENIED: " + kind);
};
globalThis.fetch = () => deny("network", "fetch");
require("node:net").Socket.prototype.connect = () => deny("network", "socket");
for (const name of ["node:http", "node:https"]) {
const mod = require(name);
mod.request = () => deny("network", name + ".request");
mod.get = () => deny("network", name + ".get");
}
require("node:tls").connect = () => deny("network", "tls");
require("node:dgram").createSocket = () => deny("network", "datagram");
for (const name of ["node:dns", "node:dns/promises"]) {
const dns = require(name);
for (const key of Object.keys(dns))
if (/^(lookup|resolve|reverse)/.test(key)) dns[key] = () => deny("network", name + "." + key);
}
for (const method of ["spawn", "spawnSync", "execFile", "execFileSync"]) {
const original = cp[method];
cp[method] = (command, args, ...rest) => {
const allowed = process.argv[1] === process.env.FINAL_EFFECT_CLI &&
command === process.execPath && Array.isArray(args) &&
args[0] === process.env.FINAL_EFFECT_TRANSPORT;
if (!allowed) return deny("subprocess", { command, args });
return original(command, args, ...rest);
};
}
for (const method of ["exec", "execSync", "fork"])
cp[method] = () => deny("subprocess", method);
syncBuiltinESMExports();metadata-final-effect-inputs.jsonSHA256: {
"head": "5087c40b3e223bfa97ac14749073381077666928",
"tree": "def4e53ddc68bf699a571e4d8302334e210c3002",
"behaviorBaseline": "aba9826ab8c010a8f5a2b4411484dc4cb7e94f51",
"sourceFileCount": 1324,
"sourceInventorySha256": "ad8cce3c7a68b07d6a6c94c663bee8f5c6eeb8e6ec48eb6303f8866e2da2acc1",
"issue": {
"number": 321,
"title": "Synthetic retained metadata close proposal",
"body": "Synthetic issue body: behavior supplied by the recorded implementation.",
"html_url": "https://github.com/openclaw/openclaw/issues/321",
"created_at": "2026-05-01T00:00:00Z",
"updated_at": "2026-05-01T00:00:00Z",
"closed_at": null,
"state": "open",
"locked": false,
"active_lock_reason": null,
"author_association": "CONTRIBUTOR",
"user": {
"login": "synthetic-contributor"
},
"labels": [],
"comments": 0,
"pull_request": null
},
"reportOverrides": {
"repository": "openclaw/openclaw",
"number": 321,
"type": "issue",
"title": "Synthetic retained metadata close proposal",
"url": "https://github.com/openclaw/openclaw/issues/321",
"author": "synthetic-contributor",
"author_association": "CONTRIBUTOR",
"labels": "[]",
"reviewed_at": "2026-08-31T19:00:00Z",
"maintainer_decision": "none"
},
"cases": [
{
"name": "positive-clean",
"suffix": "\n\n## Quoted metadata example\n\nOrdinary explanatory prose.\n",
"expected": "closed"
},
{
"name": "positive-quoted",
"suffix": "\n\n## Quoted metadata example\n\nOrdinary explanatory prose.\nmaintainer_decision: unknown\n",
"expected": "closed"
},
{
"name": "negative-competing",
"suffix": "\n\n## Quoted metadata example\n\nOrdinary explanatory prose.\n---\nmaintainer_decision: unknown\n---\n",
"expected": "rejected-before-github"
}
],
"scope": "Production retained-report issue apply path; timestamp/source/timeline freshness guards active. No PR promotion or exact-event mutation-lease protocol claim."
} |
clawsweeper
Bot
added
P2
Contributor
|
Merged as ced376c. Thanks @dwin-gharibi for the report and original fix; contributor history and co-author credit were preserved. The four readers now share leading-header structure without unifying their distinct value decoding or weakening legacy promotion guards. Exact-head validation passed 169 focused tests, the full check with 4,248 passes and eight skips, and 13/13 changed-coverage tests. Consumer proof passed 13 baseline + 13 candidate scenarios. The additional real apply-CLI proof reached the close boundary for both valid controls and rejected the conflicting persisted record before any transport call. All effects were confined to synthetic state; no live GitHub item was touched by the repro. CI, CodeQL, and the current ClawSweeper review completed. Proof was accepted as sufficient with no remaining findings or rank-up requests; conditional skips were not counted as passes. The main body and complete final-effect harness retain the execution identities, preparation corrections, and scope limits. No suppression or proof override was used. |
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
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.
Closes #1134.
What this fixes
A valid report could become unreadable when ordinary review prose or a fenced example quoted a leading-header key such as
title:,repository:, ornumber:. Four runtime readers independently scanned the whole body and treated any matching line as competing metadata. That could reject repair intake, invent a maintainer-decision blocker, or suppress a real decision packet.This updates the existing contributor PR rather than replacing it. Thanks @dwin-gharibi for the original report and proposed fix. The normal fast-forward update preserves all five contributor commits and their five tests; the integration commit and eventual squash retain co-author credit.
Fix and ownership boundary
One small structural reader now owns the anchored leading header, literal raw fields, body lookalikes, and competing-record ambiguity. The primary report reader, workflow selectors, repair intake, and decision packets retain thin adapters for their intentionally different quote, empty-value, default, and JSON-decoding behavior.
The old partial
front-matter-blockshelper and its one-reader proof were removed from the unmerged branch, not kept as parallel implementations. There is no YAML migration, new dependency, compatibility shim, parser-mode framework, or change to close policy. The broader Markdown parser changes only the export of its existing fence-transition helper.Previous review disposition: the remaining-reader finding and proof mismatch are addressed together. Both repair readers and decision packets now use the shared structure owner, and the evidence below describes the exact current head rather than the older five/seven-test snapshot. Previous partial-reader proof claims and earlier-head results are superseded as final landing evidence; no proof override is requested.
CodeQL follow-up: the generated-code alert prompted removal of unnecessary path-to-program interpolation from the proof harness. Generated preload, tripwire, and decision programs are now static; paths travel as argv or a child-only non-secret log value. Node receives the preload through explicit
--requirearguments. Independent command and network-denial self-checks run before application measurements, and local preparation also passed with spaces and quotes in the output path. CodeQL reports the associated instance as fixed; no suppression or manual alert dismissal was added. The old preload was a Node CommonJS file, not an HTML script element, so this does not claim a demonstrated script-tag exploit. Product code did not change during this follow-up; the final proof was rerun on the new head.Consumer-path real-behavior proof
Claim: valid leading metadata remains usable by the real repair-intake and decision consumers despite quoted body keys, without turning ambiguous metadata into legacy close eligibility.
Exercised surface: freshly built
create-job --from-report --dry-run --no-check-existing, the actualmaintainerDecisionBlocksCloseandbuildDecisionPacketFromReportexports, and the actualworkflow-utils proposed-item-numbersCLI. Fixtures are synthetic reports in isolated flat record directories. This is consumer-path proof, not merely a direct call to the shared parser.5087c40b3e223bfa97ac14749073381077666928def4e53ddc68bf699a571e4d8302334e210c3002aba9826ab8c010a8f5a2b4411484dc4cb7e94f51clawsweeper-checkprofileami-0461d919be7deb53c/c7a.8xlarge/eu-west-1cbx_63838f59e7ca(quick-crab-a91a)The portal may require authentication. The normalized observations here and the immutable runnable source below are the inspectable review evidence; the portal is not the sole proof artifact.
Observed before/after results
Both runs passed their explicit expectations: 13 baseline + 13 candidate scenarios, with 138 baseline / 152 candidate assertions, 22 actual application invocations per mode, and two separately cleared self-checks per mode. The command self-check requires exit 97 and its denial log; the network self-check requires exit 1 with the known preload-denial error and log. Those expected rejections are not application/test failures.
noneincorrectly blocks; required packet suppressed#321;nonedoes not block; required packet retained322322Required packets preserve the exact original repository, number, title, question, rationale, options, and owner. No quoted body repository or item number was adopted. The six benign structural controls already work on the baseline; they are not misreported as original defects.
Every application run used a credential-free environment, command tripwires, and a Node network-denial preload. After clearing the self-check logs, the proof observed zero application GitHub reads, zero network attempts, zero model commands, and zero jobs created. The positive legacy selector demonstrates that negative selections are caused by ambiguity rather than a globally disabled selector. F/missing legacy ratings remain valid promotion criteria.
The outer execution used no normal repository sync, no Actions hydration, no instance role (IMDS credential endpoint returned 404), and a CI-only ambient allowlist. It fetched one exact public candidate commit with shallow fresh Git metadata; no local Git history, credentials, or unrelated artifacts were uploaded. Two complete explicit source archives were built fresh with the same installed compiler, not one old module mixed with new siblings. Source and shared-dependency fingerprints stayed unchanged.
Crabbox emitted transient broker heartbeat/event-append timeout warnings. The remote command, final source verification, and both explicit evidence downloads nevertheless completed successfully. The recorded results above come from those downloaded manifests and logs, not from assuming portal completeness.
Inspectable source and reproduction
The complete proof folder at the executed head contains methodology and readable source only: README, archive/build launcher, and actual-consumer driver. It contains no generated execution receipt, raw logs, archives, media, or agent transcripts.
Create both archives using the README's explicit closure:
src config schema package.json pnpm-lock.yaml pnpm-workspace.yaml tsconfig.json tsconfig.repair.json. Archive identities independently checked against the executed source are:b4e0968d35f70c2470196522feaff64b7f48ed36406053c0a3459062d7a13567.a31103affe9a82d20d0e7e3538ffee76ca267ff8b769e8eb59be97436cb2b1ad.ca240fcfc3c02e37f4ec2d0c85ce6036240d68264dfb42dae55cce9cd8933c6d.With the complete proof folder and matching installed dependencies, the executed invocation is reproduced below with temporary paths normalized:
bash docs/proof/report-front-matter/run-proof.sh \ --baseline-archive /tmp/report-metadata-baseline.tar \ --baseline-id commit:aba9826ab8c010a8f5a2b4411484dc4cb7e94f51 \ --baseline-sha256 b4e0968d35f70c2470196522feaff64b7f48ed36406053c0a3459062d7a13567 \ --candidate-archive /tmp/report-metadata-candidate.tar \ --candidate-id commit:5087c40b3e223bfa97ac14749073381077666928 \ --candidate-sha256 a31103affe9a82d20d0e7e3538ffee76ca267ff8b769e8eb59be97436cb2b1ad \ --deps-root "$PWD" --out /tmp/report-metadata-proof-resultsThe launcher verifies archive digests, records the caller-verified source IDs, captures fresh source/dist/fixture/complete-argv hashes, and refuses reused output directories. It validates the pinned build commands and invokes their installed compiler directly, avoiding pnpm's automatic dependency relinking in temporary source trees. Earlier local preparation attempts that exposed that relinking were superseded; the final local and AWS runs verify unchanged dependency fingerprints.
Persisted-record final-effect proof
This additional run addresses the 19:17 UTC review's authority-chain request, without a source change or proof override. The preceding 13+13 consumer driver retains its own documented limits; this separate run executes the unmodified production
apply-decisionsCLI through the retained-report issue-close path. Only the external GitHub transport is synthetic. No parser, decision guard, freshness check, or close executor is replaced.5087c40b3e223bfa97ac14749073381077666928/def4e53ddc68bf699a571e4d8302334e210c3002ami-0461d919be7deb53c/eu-west-1c7a.8xlarge/cbx_c48af1a1a949(jade-prawn-1956)ca240fcfc3c02e37f4ec2d0c85ce6036240d68264dfb42dae55cce9cd8933c6dEach case begins with the same persisted leading header, including
maintainer_decision: none, the same synthetic non-maintainer issue and deterministic source/timeline identities, and separate temporary records, closed-record, HOME/XDG, and transport-state directories. The positive-quoted and negative-competing records differ only by two---delimiter lines around the samemaintainer_decision: unknownbody example.Actual final-effect observations
review_comment_synced, thenclosed; synthetic issue closed and report archivedkept_open:invalid maintainer_decision: report front matter is ambiguous; issue remains open, report stays in items and is not archivedEach positive's two writes were its durable-comment write and the actual final close request serialized by the production executor:
These calls changed only the local synthetic JSON state. They prove that the positive controls reached the GitHub mutation boundary, not that a live issue was closed. The negative made no transport call at all, rather than being rejected by the fake transport: the real apply guard rejected the persisted record before
fetchApplyItem. Positives continued through the normal source/freshness/comment guards to the realcloseItemserializer and observed mutation call.All three application invocations exited 0, with no unexpected subprocess, network attempt, or rejected adapter request. Separate self-checks proved that the same preload denies network access and that the same adapter rejects an unknown target PATCH. Their counters were isolated from the application cases. The actual CLI uses no dry-run or sync-only flag; it selects one item and permits one close.
--processed-limit 2allows the ordinary comment-sync operation followed by close. The first preparation attempt used a limit of one and correctly stopped after comment synchronization; that failed receipt is retained and is not counted as final proof. No product guard, test deadline, or validation threshold was changed.Complete reproducible harness and limits
The complete driver, local transport, containment preload, and synthetic inputs are published as readable source, with exact file hashes and the invocation. They reuse the committed report fixture and pure identity helper, copy freshly built runtime files, and call the real CLI through its existing
GH_BIN/GH_BIN_ARGSinterface. Driver SHA256:74ff52fe36be2fa818978956094d24a73ddaaa4d7a1f45bda6adda1b85095b18. Input JSON SHA256:bdd8eab79f721803ab596e4c85015ec21764fc5d5622db1e52f61a5f3c3c4a3a. Complete argv hashes, before/after persisted files, outcomes and transport counters were captured and checked. The portal link is supplementary; the results and executable source are inspectable directly here.The child environment has no credentials or real
gh/model binary path. The preload denies network and every subprocess except the exact local adapter, and the adapter has no outbound fallback. The lease had no instance role (IMDS credential endpoint 404), no hydration or normal repository sync, and was released afterward. Broker heartbeat/event-append warnings did not prevent execution, source verification, or explicit evidence downloads.Scope is deliberately the supported retained-report issue apply path. Matching
item_updated_atand computed source/timeline identities exercise its normal freshness route; the committed fixture'sreviewed-snapshotsentinel is not claimed as timestamp-absent snapshot-fallback proof. This run does not claim PR promotion, exact-event mutation-lease coverage, a deployed Worker, real GitHub authorization/server behavior, or live production closure. It establishes the requested persisted-body rejection before the final GitHub boundary, with a real positive effect path so a globally disabled close cannot satisfy the test.Supporting validation and limits
Fresh AWS
pnpm run build:alland all 169 focused tests passed. Freshpnpm run checkpassed 4,248 tests, zero failures, eight skips, plus 13/13 changed-coverage tests. Coverage was 82.80% lines, 75.86% branches, and 88.08% functions. No test deadlines, dependency versions, coverage thresholds, or validation policies changed. Managed precommit reviews and the final committed-branch Codex review were scoped-clean at the default P0 priority.Neither proof contacts live GitHub or production state, creates real repair jobs, or invokes a model. The additional run exercises production retained-report issue apply code on synthetic state; it does not claim a deployed Worker, live server-side authorization, a complete hosted workflow, PR promotion, exact-event mutation leases, or timestamp-absent snapshot fallback. Hosted CI and the current-head/current-body ClawSweeper review remain separate landing gates.
OpenClaw Bay: no schema, UI, or projection change is needed. Report shape and observer-only navigation are unchanged. Queue behavior, scanner/provenance/statistics, close policy, and the intentionally broader advisory metadata-spoofing inventory are unchanged. State-storage documentation and the changelog describe the shared reader and its guardrails. This proof folder follows the existing documentation lifecycle convention.