mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 20:46:28 +01:00
fix: address Greptile P2 review on PR #6969
1. scripts/check-no-git-push.mjs: stripLineComment's `prev !== "\\"` guard mis-handles double-escaped backslashes. For input like `"foo\\\\" // git push`, the closing `"` is preceded by a single `\` in the string but that `\` is itself the second half of an escaped `\\`, so the quote is actually unescaped. Count consecutive backslashes and treat the char as escaped only when the count is odd. Adds a regression test covering a Windows-style path literal ending in `\\` followed by a `// git push` comment. 2. server/src/services/issues.ts: in the workspace-finalize barrier branch of computeBlockedIssueReadinessMap, clear allBlockersDone alongside isDependencyReady. Route code today gates on unresolvedBlockerCount so nothing breaks, but future callers using allBlockersDone as a proxy for "this dependent can proceed" would silently skip the finalize gate. Cleared together with a comment explaining why. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
b1c704ea83
commit
04f0d63b0b
3 files changed
+22
-4
No files matched your search
@@ -69,11 +69,18 @@ function stripLineComment(line) {
|
||||
|
||||
for (let index = 0; index < line.length; index += 1) {
|
||||
const char = line[index];
|
||||
const prev = index > 0 ? line[index - 1] : "";
|
||||
// A character is escaped only if it's preceded by an odd number of
|
||||
// backslashes; e.g. `"foo\\"` ends a string because the trailing `\\`
|
||||
// is a single escaped backslash, leaving the closing `"` unescaped.
|
||||
let backslashes = 0;
|
||||
for (let scan = index - 1; scan >= 0 && line[scan] === "\\"; scan -= 1) {
|
||||
backslashes += 1;
|
||||
}
|
||||
const isEscaped = backslashes % 2 === 1;
|
||||
|
||||
if (!inDouble && !inBacktick && char === "'" && prev !== "\\") inSingle = !inSingle;
|
||||
else if (!inSingle && !inBacktick && char === '"' && prev !== "\\") inDouble = !inDouble;
|
||||
else if (!inSingle && !inDouble && char === "`" && prev !== "\\") inBacktick = !inBacktick;
|
||||
if (!inDouble && !inBacktick && char === "'" && !isEscaped) inSingle = !inSingle;
|
||||
else if (!inSingle && !inBacktick && char === '"' && !isEscaped) inDouble = !inDouble;
|
||||
else if (!inSingle && !inDouble && char === "`" && !isEscaped) inBacktick = !inBacktick;
|
||||
else if (!inSingle && !inDouble && !inBacktick && char === "/" && line[index + 1] === "/") {
|
||||
return line.slice(0, index);
|
||||
}
|
||||
|
||||
@@ -67,6 +67,14 @@ test("findGitPushOffenses flags args-array form passed to spawn/execFile", () =>
|
||||
}
|
||||
});
|
||||
|
||||
test("findGitPushOffenses ignores `git push` in a comment after a string ending with a literal backslash", () => {
|
||||
// The closing `"` after `\\` should end the string (even literal count of
|
||||
// backslashes leaves the quote unescaped), so the `// git push` that
|
||||
// follows is comment text and must be stripped.
|
||||
const text = 'const path = "C:\\\\"; // git push origin master\nconst y = 2;\n';
|
||||
assert.deepEqual(findGitPushOffenses(text), []);
|
||||
});
|
||||
|
||||
test("findGitPushOffenses does not flag args-array form when allow marker is present", () => {
|
||||
const text = `// ${ALLOW_MARKER}: release tooling adapter\nspawn("git", ["push", "origin", "main"]);\n`;
|
||||
assert.deepEqual(findGitPushOffenses(text), []);
|
||||
|
||||
@@ -711,9 +711,12 @@ async function listIssueDependencyReadinessMap(
|
||||
// execution workspace hasn't recorded a successful workspace_finalize.
|
||||
// Treat the dependent as not-ready until sync-back lands (or the run
|
||||
// finalizes); a subsequent finalize wake will re-evaluate readiness.
|
||||
// `allBlockersDone` is cleared too so that callers using it as a
|
||||
// proxy for "this dependent can proceed" still see the gate.
|
||||
current.unresolvedBlockerIssueIds.push(row.blockerIssueId);
|
||||
current.unresolvedBlockerCount += 1;
|
||||
current.pendingFinalizeBlockerIssueIds.push(row.blockerIssueId);
|
||||
current.allBlockersDone = false;
|
||||
current.isDependencyReady = false;
|
||||
}
|
||||
readinessMap.set(row.issueId, current);
|
||||
|
||||
Reference in new issue
Block a user