mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-04 20:31:33 +00:00
refactor(tooling): round-trip the repinned review JSON
Preserving byte-exact formatting required a hand-rolled JSON scanner: string escape handling, container nesting, and span splicing, for a file that lives in gitignored .local/ and is only ever read back through JSON.parse. repin already parses the artifact to validate it, so assign the two identity fields and re-serialize. JSON.parse/stringify keeps insertion order, which is the only formatting property worth holding. Drops four helpers.
This commit is contained in:
@@ -534,81 +534,6 @@ function readJson(filePath) {
|
||||
}
|
||||
}
|
||||
|
||||
function skipJsonWhitespace(source, start) {
|
||||
let index = start;
|
||||
while (/\s/u.test(source[index] ?? "")) {
|
||||
index += 1;
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
function jsonStringEnd(source, start) {
|
||||
for (let index = start + 1; index < source.length; index += 1) {
|
||||
if (source[index] === "\\") {
|
||||
index += 1;
|
||||
} else if (source[index] === '"') {
|
||||
return index + 1;
|
||||
}
|
||||
}
|
||||
throw new Error("Invalid JSON string while locating review identity");
|
||||
}
|
||||
|
||||
function jsonValueEnd(source, start) {
|
||||
const first = source[start];
|
||||
if (first === '"') {
|
||||
return jsonStringEnd(source, start);
|
||||
}
|
||||
if (first === "{" || first === "[") {
|
||||
const openings = [first];
|
||||
for (let index = start + 1; index < source.length; index += 1) {
|
||||
const character = source[index];
|
||||
if (character === '"') {
|
||||
index = jsonStringEnd(source, index) - 1;
|
||||
} else if (character === "{" || character === "[") {
|
||||
openings.push(character);
|
||||
} else if (character === "}" || character === "]") {
|
||||
openings.pop();
|
||||
if (openings.length === 0) {
|
||||
return index + 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
throw new Error("Invalid JSON container while locating review identity");
|
||||
}
|
||||
let index = start;
|
||||
while (index < source.length && !/[\s,}\]]/u.test(source[index])) {
|
||||
index += 1;
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
function findJsonPropertyValue(source, objectStart, property) {
|
||||
let index = skipJsonWhitespace(source, objectStart + 1);
|
||||
let found;
|
||||
while (source[index] !== "}") {
|
||||
const keyStart = index;
|
||||
const keyEnd = jsonStringEnd(source, keyStart);
|
||||
const key = JSON.parse(source.slice(keyStart, keyEnd));
|
||||
index = skipJsonWhitespace(source, keyEnd);
|
||||
if (source[index] !== ":") {
|
||||
throw new Error("Invalid JSON object while locating review identity");
|
||||
}
|
||||
const valueStart = skipJsonWhitespace(source, index + 1);
|
||||
const valueEnd = jsonValueEnd(source, valueStart);
|
||||
if (key === property) {
|
||||
found = { start: valueStart, end: valueEnd };
|
||||
}
|
||||
index = skipJsonWhitespace(source, valueEnd);
|
||||
if (source[index] === ",") {
|
||||
index = skipJsonWhitespace(source, index + 1);
|
||||
}
|
||||
}
|
||||
if (found) {
|
||||
return found;
|
||||
}
|
||||
throw new Error(`Missing ${property} while locating review identity`);
|
||||
}
|
||||
|
||||
function repinReviewArtifacts({ expectedPrNumber, reviewPath, reviewMarkdownPath, prMetaPath }) {
|
||||
if (!existsSync(prMetaPath)) {
|
||||
throw new Error(`Missing ${prMetaPath}; run scripts/pr review-init <PR>.`);
|
||||
@@ -631,28 +556,18 @@ function repinReviewArtifacts({ expectedPrNumber, reviewPath, reviewMarkdownPath
|
||||
);
|
||||
}
|
||||
|
||||
const reviewSource = readFileSync(reviewPath, "utf8");
|
||||
const review = readJson(reviewPath);
|
||||
if (!isObject(review) || !isObject(review.pr)) {
|
||||
throw new Error(
|
||||
`Invalid PR identity in ${reviewPath}; run scripts/pr review-artifacts-init ${expectedPrNumber}.`,
|
||||
);
|
||||
}
|
||||
const objectStart = skipJsonWhitespace(reviewSource, 0);
|
||||
const prSpan = findJsonPropertyValue(reviewSource, objectStart, "pr");
|
||||
const numberSpan = findJsonPropertyValue(reviewSource, prSpan.start, "number");
|
||||
const headShaSpan = findJsonPropertyValue(reviewSource, prSpan.start, "headSha");
|
||||
const replacements = [
|
||||
{ ...numberSpan, value: String(prMeta.number) },
|
||||
{ ...headShaSpan, value: JSON.stringify(prMeta.headRefOid) },
|
||||
].toSorted((left, right) => right.start - left.start);
|
||||
let nextReviewSource = reviewSource;
|
||||
for (const replacement of replacements) {
|
||||
nextReviewSource =
|
||||
nextReviewSource.slice(0, replacement.start) +
|
||||
replacement.value +
|
||||
nextReviewSource.slice(replacement.end);
|
||||
}
|
||||
// Round-trip rather than splice the source: JSON.parse/stringify keeps insertion
|
||||
// order, and `.local/` artifacts are gitignored scratch that only ever gets read
|
||||
// back through JSON.parse, so byte-exact formatting buys nothing.
|
||||
review.pr.number = prMeta.number;
|
||||
review.pr.headSha = prMeta.headRefOid;
|
||||
const nextReviewSource = `${JSON.stringify(review, null, 2)}\n`;
|
||||
|
||||
const markdownSource = readFileSync(reviewMarkdownPath, "utf8");
|
||||
const firstNewline = markdownSource.indexOf("\n");
|
||||
|
||||
@@ -437,11 +437,14 @@ describePosix("scripts/pr review artifact validation", () => {
|
||||
const { result, localDir, reviewSource, markdownBody } = runArtifactsRepin();
|
||||
|
||||
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
|
||||
const repinnedReview = readFileSync(join(localDir, "review.json"), "utf8");
|
||||
const expectedReview = reviewSource
|
||||
.replace(`"number": ${SUPERSEDED_PR}`, `"number": ${REVIEWED_PR}`)
|
||||
.replace(`"headSha": "${SUPERSEDED_HEAD}"`, `"headSha": "${REVIEWED_HEAD}"`);
|
||||
expect(repinnedReview).toBe(expectedReview);
|
||||
const repinned = JSON.parse(readFileSync(join(localDir, "review.json"), "utf8"));
|
||||
const expected = JSON.parse(reviewSource);
|
||||
expected.pr.number = REVIEWED_PR;
|
||||
expected.pr.headSha = REVIEWED_HEAD;
|
||||
// Deep-equal, so every authored field outside `pr` must survive verbatim.
|
||||
expect(repinned).toEqual(expected);
|
||||
// Key order matters for readability of an artifact humans hand-edit.
|
||||
expect(Object.keys(repinned)).toEqual(Object.keys(expected));
|
||||
expect(readFileSync(join(localDir, "review.md"), "utf8")).toBe(
|
||||
`${REVIEWED_IDENTITY_LINE}\n${markdownBody}`,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user