Simplify Relay test accounting - #1202
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Walkthrough생성된 런타임·테스트 원장과 인벤토리를 제거했습니다. Relay 계약 검증을 현재 경로로 전환했습니다. CI 매트릭스와 테스트 지시문 검사를 추가하고, 테스트 픽스처와 명칭을 Relay 기준으로 정리했습니다. ChangesRelay accounting and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24121e5943
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const alternateSkipOption = /(?:\{|,)\s*skip\s*(?=[,}])/; | ||
| if (alternateSkipOption.test(optionObject.source)) violations.push(`${relative}: shorthand skip is forbidden`); | ||
| for (const match of optionObject.source.matchAll(/(?:\{|,)\s*skip\s*:/g)) { |
There was a problem hiding this comment.
When a test supplies directives through an object spread, such as const hidden = { skip: true }; test("case", { ...hidden }, fn), these regexes find no explicit skip, only, or todo property, so the directive guard passes even though Node marks the test skipped. This bypasses the exact skip allowlist and permits ordinary Relay tests to disappear from CI; reject top-level option spreads or resolve them before validating directives.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 85cba6f. Top-level inline option spreads now fail closed; variable and literal spread regressions are covered while nested and callback-body spreads remain allowed. Focused skills-lint passed and an independent adversarial review found no remaining P1/P2.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
tests/skills-lint/scripts/test-directives.test.js (2)
216-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
it과describe의 옵션 객체 skip 케이스를 주입 목록에 추가하세요.Line 229부터의 주입 목록은
.skip멤버 형태에서만it과describe를 다룹니다. 옵션 객체 형태(it('x', { skip: true }, ...))는 없습니다. 이 형태가literalNameBefore의test(전용 검색과 만나는 경로가 검증되지 않습니다. 해당 케이스를 추가하세요.♻️ 제안 변경
"test('hidden', { [`skip`]: true }, () => {});", + `it('hidden', { ${skipProperty} }, () => {});`, + `describe('hidden', { ${skipProperty} }, () => {});`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/test-directives.test.js` around lines 216 - 275, Extend the injections array in the “directive guard rejects injected unauthorized directives” test with options-object skip cases for both it and describe, using the same skipProperty pattern as the existing test case. Keep the cases unauthorized and ensure they exercise the literalNameBefore handling for non-test callers.
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value수동 스캐너에 짧은 설명 주석을 추가하세요.
regexStartsAt,withoutComments,stringMask는 문자열, 템플릿 리터럴, 정규식 리터럴, 문자 클래스를 직접 추적합니다. 각 함수의 목적과 처리 대상을 한두 줄로 명시하세요. 복잡한 로직에는 의미 있는 주석을 사용해야 합니다.특히 Line 51의
"([{:;,=!&|?+-*%~"는 문자 클래스가 아니라 일반 문자열입니다. 이 점을 주석으로 밝히면 오해를 막을 수 있습니다.Also applies to: 54-87, 89-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/test-directives.test.js` around lines 49 - 52, Add concise one- or two-line comments above regexStartsAt, withoutComments, and stringMask describing each scanner’s purpose and the strings, template literals, regex literals, and character classes it tracks. In particular, document that the character sequence "([{:;,=!&|?+-*%"~" in regexStartsAt is an ordinary string used for delimiter membership checks, not a regular-expression character class.Source: Coding guidelines
tests/skills-lint/scripts/ci-relay-matrix.test.js (2)
105-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
runnerStep의 주석 제거 동작을 문서화하세요.
runnerStep은run블록 안의 주석 줄을 본문에서 제외합니다. 이 동작 때문에 주석이 삽입되어도EXPECTED_RUNNER_STEP비교는 통과합니다. 반대로 실행 줄을 주석으로 바꾸면 해당 줄이 사라져서 비교가 실패합니다. 이 의도를 짧은 주석으로 명시하세요. 복잡한 로직에는 의미 있는 주석을 사용해야 합니다.또한 Line 115의
Math.min은 불필요합니다.String.prototype.slice는 인덱스를 길이로 자동 제한합니다.♻️ 제안 변경
const stepIndent = indent(steps[0].line); const body = []; + // Comment lines are dropped from the body. Commenting out an executable + // line therefore removes it and fails the exact-match comparison. for (let index = steps[0].index + 1; index < lines.length; index += 1) { if (lines[index].trim() && indent(lines[index]) <= stepIndent) break; if (lines[index].trim() && !lines[index].trimStart().startsWith("#")) { - body.push(lines[index].slice(Math.min(lines[index].length, stepIndent))); + body.push(lines[index].slice(stepIndent)); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/ci-relay-matrix.test.js` around lines 105 - 124, Update runnerStep to document that comment-only lines inside the run block are excluded from the extracted body, while commenting out an executable line causes the exact EXPECTED_RUNNER_STEP comparison to fail; retain the existing behavior. Also simplify the body-line slicing by removing the unnecessary Math.min wrapper and pass stepIndent directly to slice.Source: Coding guidelines
198-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win약화 케이스의 오류 메시지 검증을 좁히세요.
Line 216의 정규식
/runner|step|exact|array|zero|serialize|guarded/i는 이 파일의 거의 모든 단언 메시지와 일치합니다. 예를 들어 잡 봉투 실패 메시지와 러너 실패 메시지가 서로 구분되지 않습니다. 그래서 케이스가 의도와 다른 이유로 실패해도 테스트는 통과합니다. 각 약화 케이스에 기대 메시지를 함께 지정하세요.♻️ 제안 변경
- for (const weakened of [ - value.workflow.replace("files=()", ""), - ... - ]) assert.throws(() => assertCiContract({ testsDir: value.root, workflow: weakened }), /runner|step|exact|array|zero|serialize|guarded/i); + for (const [weakened, expected] of [ + [value.workflow.replace("files=()", ""), /fail-closed shell program/], + [value.workflow.replace(" strategy:", " if: always()\n strategy:"), /job-level policy envelope/], + // ... 각 케이스에 해당 메시지를 지정 + ]) assert.throws(() => assertCiContract({ testsDir: value.root, workflow: weakened }), expected);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/ci-relay-matrix.test.js` around lines 198 - 217, Update the weakened-workflow cases in the assert.throws test around assertCiContract to associate each mutation with its specific expected error-message pattern. Replace the broad shared /runner|step|exact|array|zero|serialize|guarded/i matcher with per-case assertions that distinguish failures such as runner, step, exact, array, zero, serialize, and guarded contract violations.tests/relay-dispatch/fixtures/fake-codex.js (1)
7-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winfixture 제어 프로토콜을 주석으로 설명하세요.
-C,-o, stdin JSON,delay_ms,empty가 각각 작업 디렉터리, 결과 경로, 지연, 결과 파일 생성을 제어합니다. 이 의미가 호출부만으로는 명확하지 않습니다. 특히empty가 작업 디렉터리 변경은 남기고 결과 artifact만 생략한다는 의도를 기록하세요.주석 추가 예시
const args = process.argv.slice(2); +// The harness passes the working directory and result path as argv values. const value = (flag) => { ... const output = value("-o"); +// stdin controls delay injection and omission of the result artifact. let controls = {}; ... if (controls.empty === true) process.exit(0);As per coding guidelines, 복잡한 로직에는 의미 있는 주석을 추가해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/fake-codex.js` around lines 7 - 21, fake-codex fixture의 제어 프로토콜을 코드에 의미 있는 주석으로 설명하세요. `-C`와 `-o`가 작업 디렉터리와 결과 경로를 지정하고, stdin JSON의 `delay_ms`가 지연을 제어하며 `empty`가 작업 디렉터리 변경은 유지한 채 결과 artifact 생성만 생략한다는 내용을 해당 파싱 및 분기 로직 주변에 명시하세요.Source: Coding guidelines
tests/relay-dispatch/fixtures/write-containment-executor.js (1)
60-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
proof_in_result가 참이고-o가 없으면 증거가 사라집니다.63-64행은
controls.proof_in_result === true일 때output이 있을 때만 기록합니다.output이 null이면proof가 어디에도 저장되지 않습니다. 테스트는 증거 없이 실패하고 원인을 알기 어렵습니다. 이 조합에서는 worktree 내containment-proof.json으로 되돌리거나 명시적으로 종료 코드를 설정하십시오.♻️ 제안 수정
if (controls.proof_in_result === true) { - if (output) fs.writeFileSync(output, bytes); + if (output) fs.writeFileSync(output, bytes); + else fs.writeFileSync(path.join(worktree, "containment-proof.json"), bytes); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/write-containment-executor.js` around lines 60 - 68, Update finish() so that when controls.proof_in_result === true and output is absent, the proof is still persisted to worktree/containment-proof.json; retain writing proof to output when output is provided and preserve the existing executor status message for the non-proof-in-result path.tests/relay-dispatch/scripts/facts.test.js (1)
481-514: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win주입된 fs 실패가 의도한 경계보다 먼저 발생합니다.
appendFact와repairTornTail은 모두 본 작업 전에readFacts({ eventsPath, fsModule })를 호출합니다. 두 테스트의 스텁은 해당 fs 메서드의 모든 호출에서 예외를 던집니다. 그러므로 예외는 항상 사전 판독 단계에서 발생하고, 테스트 이름이 지목하는 append open 경계와 repair journal-read 경계는 실행되지 않습니다. 두 테스트는 통과하지만 대상 코드를 검사하지 않습니다.
tests/relay-dispatch/scripts/facts.test.js#L481-L514:openSync호출 횟수를 세어 두 번째(append 경로) 호출에서만 예외를 던지십시오.tests/relay-dispatch/scripts/facts.test.js#L542-L565:fsModule스텁 대신repairTornTail이 제공하는fault콜백을 사용하고journal_read단계에서만 예외를 던지십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/scripts/facts.test.js` around lines 481 - 514, Update tests/relay-dispatch/scripts/facts.test.js lines 481-514 to count openSync calls and inject the failure only on the second call, so the initial readFacts preflight succeeds and the append open boundary is exercised. Update lines 542-565 to use repairTornTail’s fault callback instead of an fsModule stub, throwing only for the journal_read stage; no other fault stages should be affected.tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js (1)
8-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
topLevelTestNames의 상태 전이를 설명하는 주석을 추가하세요.이 함수는 문자열과 주석을 건너뛰고, 중괄호 깊이와 이전 최상위 토큰으로 직접 테스트 등록을 판별합니다. 이 보수적 판별 규칙을 설명하지 않으면 이후 변경에서 가짜 anchor를 허용하거나 유효한 anchor를 거부할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js` around lines 8 - 44, Add comments within topLevelTestNames explaining how string/comment skipping, braceDepth tracking, and lastTopLevelToken transitions conservatively identify valid top-level test registrations. Document the anchor requirements and state transitions so future changes preserve rejection of false anchors and acceptance of valid ones.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/relay-dispatch/fixtures/fake-codex.js`:
- Around line 14-15: Update the control-input parsing in the fake Codex fixture
so empty or whitespace-only stdin continues to use the default controls, but
non-empty invalid JSON writes an error to stderr and exits with a non-zero
status instead of continuing with {}. Preserve valid JSON handling and the
existing delay_ms/empty behavior.
In `@tests/relay-dispatch/fixtures/runtime-contract-noop.js`:
- Around line 3-18: Update runtime-contract.test.js to compare the exact export
keys of productionRuntime and the injected runtime-contract-noop fixture, using
exactKeys(...) or equivalent. Ensure the test fails when either runtime adds,
removes, or renames an export, including keeping the no-op fixture synchronized
with the runtime contract.
In `@tests/relay-dispatch/fixtures/write-containment-executor.js`:
- Line 23: Update the temp-directory initialization in the proof setup to use
os.tmpdir() when process.env.TMPDIR is unset, ensuring path.join receives a
valid directory and the temp-write probe reports the actual containment result.
Import or reuse the existing os module in the fixture.
In `@tests/relay-dispatch/scripts/facts.test.js`:
- Around line 349-352: Replace the forged-capability assertion around appendFact
with a test that reuses the capability returned by acquireFactLock while
substituting run_dir with the value from another execution. Keep the shared
capability identity intact and assert that appendFact rejects this mismatched
execution context, so the “forged capabilities” test validates run_dir checking
rather than rejection of a cloned lock object.
In `@tests/relay-dispatch/scripts/inspect-recover-blackbox.test.js`:
- Line 609: Add explicit coverage near the “verification refuses dirty
reviewable bytes” test for special worktree entries: create Unix socket and FIFO
entries, run the verification flow, and assert each is rejected with
unsafe_worktree_entry. Reuse the existing socket/FIFO setup and assertion
helpers from inspect-recover-blackbox.test.js, preserving the expected behavior
for regular files.
In `@tests/skills-lint/scripts/test-directives.test.js`:
- Around line 39-41: Update literalNameBefore to locate the nearest call for
test, it, or describe rather than searching only for test(. Allow optional
whitespace before the opening parenthesis, and ensure the selected call is the
closest preceding direct call so literalNameBefore reports the correct name or
preserves the existing fail-closed error.
- Around line 37-47: Update literalNameBefore so escape restoration matches the
source literal encodings: make the newline pattern match one backslash followed
by n and the backslash pattern match two backslashes. Perform backslash
restoration once at the end, after quote and newline decoding, while preserving
the existing quote handling.
---
Nitpick comments:
In `@tests/relay-dispatch/fixtures/fake-codex.js`:
- Around line 7-21: fake-codex fixture의 제어 프로토콜을 코드에 의미 있는 주석으로 설명하세요. `-C`와
`-o`가 작업 디렉터리와 결과 경로를 지정하고, stdin JSON의 `delay_ms`가 지연을 제어하며 `empty`가 작업 디렉터리
변경은 유지한 채 결과 artifact 생성만 생략한다는 내용을 해당 파싱 및 분기 로직 주변에 명시하세요.
In `@tests/relay-dispatch/fixtures/write-containment-executor.js`:
- Around line 60-68: Update finish() so that when controls.proof_in_result ===
true and output is absent, the proof is still persisted to
worktree/containment-proof.json; retain writing proof to output when output is
provided and preserve the existing executor status message for the
non-proof-in-result path.
In `@tests/relay-dispatch/scripts/facts.test.js`:
- Around line 481-514: Update tests/relay-dispatch/scripts/facts.test.js lines
481-514 to count openSync calls and inject the failure only on the second call,
so the initial readFacts preflight succeeds and the append open boundary is
exercised. Update lines 542-565 to use repairTornTail’s fault callback instead
of an fsModule stub, throwing only for the journal_read stage; no other fault
stages should be affected.
In `@tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js`:
- Around line 8-44: Add comments within topLevelTestNames explaining how
string/comment skipping, braceDepth tracking, and lastTopLevelToken transitions
conservatively identify valid top-level test registrations. Document the anchor
requirements and state transitions so future changes preserve rejection of false
anchors and acceptance of valid ones.
In `@tests/skills-lint/scripts/ci-relay-matrix.test.js`:
- Around line 105-124: Update runnerStep to document that comment-only lines
inside the run block are excluded from the extracted body, while commenting out
an executable line causes the exact EXPECTED_RUNNER_STEP comparison to fail;
retain the existing behavior. Also simplify the body-line slicing by removing
the unnecessary Math.min wrapper and pass stepIndent directly to slice.
- Around line 198-217: Update the weakened-workflow cases in the assert.throws
test around assertCiContract to associate each mutation with its specific
expected error-message pattern. Replace the broad shared
/runner|step|exact|array|zero|serialize|guarded/i matcher with per-case
assertions that distinguish failures such as runner, step, exact, array, zero,
serialize, and guarded contract violations.
In `@tests/skills-lint/scripts/test-directives.test.js`:
- Around line 216-275: Extend the injections array in the “directive guard
rejects injected unauthorized directives” test with options-object skip cases
for both it and describe, using the same skipProperty pattern as the existing
test case. Keep the cases unauthorized and ensure they exercise the
literalNameBefore handling for non-test callers.
- Around line 49-52: Add concise one- or two-line comments above regexStartsAt,
withoutComments, and stringMask describing each scanner’s purpose and the
strings, template literals, regex literals, and character classes it tracks. In
particular, document that the character sequence "([{:;,=!&|?+-*%"~" in
regexStartsAt is an ordinary string used for delimiter membership checks, not a
regular-expression character class.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36723021-52ee-40cd-b376-d51147bcf733
⛔ Files ignored due to path filters (2)
tests/ledger/vnext-baseline.generated.jsonis excluded by!**/*.generated.*tests/ledger/vnext-test-sites.generated.jsonis excluded by!**/*.generated.*
📒 Files selected for processing (53)
CLAUDE.mdREADME.mdbacklog/sprints/2026-08-relay-runtime-slimming.mddocs/contracts/relay-runtime-contracts.v1.jsondocs/contracts/relay-runtime-inventory.v1.jsondocs/ranked-deletion-inventory.mddocs/relay-operator-guide.mddocs/script-inventory-and-cleanup.mddocs/test-accounting-retirement-1196.mdreferences/architecture.mdskills/relay-dispatch/scripts/recover.jstests/ledger/README.mdtests/ledger/vnext-baseline-measurements.jsontests/ledger/vnext-test-ledger.jsontests/relay-dispatch/fixtures/adapter-runtime-preload.jstests/relay-dispatch/fixtures/fake-cline.jstests/relay-dispatch/fixtures/fake-codex.jstests/relay-dispatch/fixtures/fake-cursor.jstests/relay-dispatch/fixtures/json-observer.jstests/relay-dispatch/fixtures/runtime-contract-noop.jstests/relay-dispatch/fixtures/vnext-shadow-parity-corpus.jsontests/relay-dispatch/fixtures/write-containment-executor.jstests/relay-dispatch/scripts/dispatch.test.jstests/relay-dispatch/scripts/docs-defaults.test.jstests/relay-dispatch/scripts/facts.test.jstests/relay-dispatch/scripts/host-concurrency.test.jstests/relay-dispatch/scripts/inspect-recover-blackbox.test.jstests/relay-dispatch/scripts/recover-safe-staging.test.jstests/relay-dispatch/scripts/relay-recover-cli.test.jstests/relay-dispatch/scripts/run-fold.test.jstests/relay-dispatch/scripts/run-store.test.jstests/relay-dispatch/scripts/runtime-contract-blackbox.test.jstests/relay-dispatch/scripts/runtime-contract-noop.test.jstests/relay-dispatch/scripts/runtime-contract.test.jstests/relay-dispatch/scripts/toolset-mismatch.test.jstests/relay-fleet/scripts/fleet-derived.test.jstests/relay-merge/fixtures/merge-observer.jstests/relay-merge/scripts/finalize-run.test.jstests/relay-merge/scripts/gate-check.test.jstests/relay-review/fixtures/fake-gh.jstests/relay-review/scripts/review-runner.test.jstests/relay/scripts/relay-status-recover.test.jstests/relay/scripts/run-preflight.test.jstests/skills-lint/scripts/ci-matrix-completeness.test.jstests/skills-lint/scripts/ci-relay-matrix.test.jstests/skills-lint/scripts/ci-test-coverage.test.jstests/skills-lint/scripts/pr-view-json-contract.test.jstests/skills-lint/scripts/skill-inputs-drift.test.jstests/skills-lint/scripts/test-directives.test.jstests/skills-lint/scripts/vnext-runtime-inventory.jstests/skills-lint/scripts/vnext-runtime-inventory.test.jstests/skills-lint/scripts/vnext-test-ledger.jstests/skills-lint/scripts/vnext-test-ledger.test.js
💤 Files with no reviewable changes (12)
- tests/ledger/README.md
- tests/skills-lint/scripts/ci-test-coverage.test.js
- tests/skills-lint/scripts/ci-matrix-completeness.test.js
- tests/ledger/vnext-test-ledger.json
- tests/skills-lint/scripts/vnext-runtime-inventory.test.js
- tests/relay-dispatch/fixtures/vnext-shadow-parity-corpus.json
- tests/skills-lint/scripts/vnext-test-ledger.js
- tests/skills-lint/scripts/vnext-test-ledger.test.js
- tests/skills-lint/scripts/vnext-runtime-inventory.js
- skills/relay-dispatch/scripts/recover.js
- docs/contracts/relay-runtime-inventory.v1.json
- tests/ledger/vnext-baseline-measurements.json
| }); | ||
|
|
||
| test("verification refuses dirty reviewable bytes and special entries classify without reading", () => { | ||
| test("verification refuses dirty reviewable bytes", () => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 특수 파일 분류 로직이 프로덕션에 남아 있는지, 남은 테스트가 있는지 확인
set -uo pipefail
echo "== assertCleanVerificationObservation 구현"
rg -n -C 20 'assertCleanVerificationObservation' skills || true
echo "== 특수 파일 / socket 분류 참조"
rg -n --type=js -C 4 'isSocket|isFIFO|isBlockDevice|isCharacterDevice|special' skills || true
echo "== 남은 socket 관련 테스트"
rg -n --type=js -C 3 'isSocket|unix socket|mkfifo' tests || trueRepository: sungjunlee/dev-relay
Length of output: 19976
🏁 Script executed:
#!/bin/bash
set -uo pipefail
echo "== recover.js unsafeWorktreeEntries implementation"
sed -n '520,590p' skills/relay-dispatch/scripts/recover.js | cat -n
echo "== inspect-recover-blackblack relevant tests around line 609"
sed -n '560,635p' tests/relay-dispatch/scripts/inspect-recover-blackbox.test.js | cat -n
echo "== diff summary"
git diff --stat || trueRepository: sungjunlee/dev-relay
Length of output: 7083
🏁 Script executed:
#!/bin/bash
set -uo pipefail
echo "== locate and inspect unsafeWorktreeEntries definition"
rg -n 'function unsafeWorktreeEntries|const unsafeWorktreeEntries|unsafeWorktreeEntries' skills/relay-dispatch/scripts/recover.js -C 80
echo "== tests around removed section"
rg -n -C 12 'unix socket|special entry|ignored|ignored entry|assertCleanVerificationObservation|special entries' tests/relay-dispatch/scripts/inspect-recover-blackbox.test.js || trueRepository: sungjunlee/dev-relay
Length of output: 28216
특수 파일 거부 경로에 대한 새 테스트를 추가하십시오.
unsafeWorktreeEntries()가 fs.lstatSync()에서 isSymbolicLink() || !entry.isFile()로 reject하므로, Unix socket, FIFO, device 파일 같은 특수 파일은 아직 unsafe_worktree_entry로 막힙니다. 하지만 inspect-recover-blackbox.test.js의 socket/FIFO 관련 테스트가 제거되면 이 분기는 새로운 테스트로 보호되지 않습니다. 삭제된 테스트 대신 socket과 FIFO case를 명시적으로 커버하는 테스트를 추가하십시오.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/relay-dispatch/scripts/inspect-recover-blackbox.test.js` at line 609,
Add explicit coverage near the “verification refuses dirty reviewable bytes”
test for special worktree entries: create Unix socket and FIFO entries, run the
verification flow, and assert each is rejected with unsafe_worktree_entry. Reuse
the existing socket/FIFO setup and assertion helpers from
inspect-recover-blackbox.test.js, preserving the expected behavior for regular
files.
| function literalNameBefore(source, offset) { | ||
| const prefix = source.slice(0, offset); | ||
| const call = prefix.lastIndexOf("test("); | ||
| if (call < 0) throw new Error("skip directive is not attached to a literal test call"); | ||
| const argument = prefix.slice(call + 5).trimStart(); | ||
| const match = /^("(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|`(?:\\.|[^`\\$]|\$(?!\{))*`)/.exec(argument); | ||
| if (!match) throw new Error("skip directive test name must be a normalized literal"); | ||
| const quote = match[1][0]; | ||
| const body = match[1].slice(1, -1); | ||
| return body.replace(new RegExp(`\\\\${quote}`, "g"), quote).replace(/\\\\n/g, "\n").replace(/\\\\\\\\/g, "\\"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
리터럴 이스케이프 해제 규칙이 한 단계 과도하게 이스케이프되었습니다.
Line 46의 첫 번째 replace는 정규식 패턴을 문자열로 만들기 때문에 \" 한 개의 백슬래시를 정확히 처리합니다. 그러나 뒤의 두 정규식 리터럴은 백슬래시가 두 배입니다.
/\\\\n/는\\n(백슬래시 2개 +n)과 일치합니다. 소스 리터럴에 실제로 들어 있는\n(백슬래시 1개 +n)과는 일치하지 않습니다./\\\\\\\\/는 백슬래시 4개와 일치합니다. 소스 리터럴의\\(백슬래시 2개)와는 일치하지 않습니다.
그래서 테스트 이름에 \n 또는 \\ 이스케이프가 들어가면 복원된 이름이 ALLOWED_SKIPS 항목과 달라집니다. 현재 허용 목록에는 이스케이프가 없어 즉시 실패하지는 않습니다. 향후 이름 추가 시 잘못된 불일치가 발생합니다.
또한 백슬래시 복원은 마지막에 한 번만 수행하는 순서가 안전합니다.
🐛 제안 수정
- return body.replace(new RegExp(`\\\\${quote}`, "g"), quote).replace(/\\\\n/g, "\n").replace(/\\\\\\\\/g, "\\");
+ return body.replace(/\\(.)/g, (_, character) => {
+ if (character === "n") return "\n";
+ if (character === "t") return "\t";
+ return character;
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function literalNameBefore(source, offset) { | |
| const prefix = source.slice(0, offset); | |
| const call = prefix.lastIndexOf("test("); | |
| if (call < 0) throw new Error("skip directive is not attached to a literal test call"); | |
| const argument = prefix.slice(call + 5).trimStart(); | |
| const match = /^("(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|`(?:\\.|[^`\\$]|\$(?!\{))*`)/.exec(argument); | |
| if (!match) throw new Error("skip directive test name must be a normalized literal"); | |
| const quote = match[1][0]; | |
| const body = match[1].slice(1, -1); | |
| return body.replace(new RegExp(`\\\\${quote}`, "g"), quote).replace(/\\\\n/g, "\n").replace(/\\\\\\\\/g, "\\"); | |
| } | |
| function literalNameBefore(source, offset) { | |
| const prefix = source.slice(0, offset); | |
| const call = prefix.lastIndexOf("test("); | |
| if (call < 0) throw new Error("skip directive is not attached to a literal test call"); | |
| const argument = prefix.slice(call + 5).trimStart(); | |
| const match = /^("(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|`(?:\\.|[^`\\$]|\$(?!\{))*`)/.exec(argument); | |
| if (!match) throw new Error("skip directive test name must be a normalized literal"); | |
| const quote = match[1][0]; | |
| const body = match[1].slice(1, -1); | |
| return body.replace(/\\(.)/g, (_, character) => { | |
| if (character === "n") return "\n"; | |
| if (character === "t") return "\t"; | |
| return character; | |
| }); | |
| } |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 45-45: Detects non-literal values in regular expressions
Context: new RegExp(\\\\${quote}, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
🪛 OpenGrep (1.26.0)
[ERROR] 42-42: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/skills-lint/scripts/test-directives.test.js` around lines 37 - 47,
Update literalNameBefore so escape restoration matches the source literal
encodings: make the newline pattern match one backslash followed by n and the
backslash pattern match two backslashes. Perform backslash restoration once at
the end, after quote and newline decoding, while preserving the existing quote
handling.
| const call = prefix.lastIndexOf("test("); | ||
| if (call < 0) throw new Error("skip directive is not attached to a literal test call"); | ||
| const argument = prefix.slice(call + 5).trimStart(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
literalNameBefore는 it과 describe 호출을 인식하지 못합니다.
Line 39는 test(만 역방향 검색합니다. directCallOptions는 test, it, describe를 모두 수집합니다. 따라서 it('x', { skip: true }, ...) 형태에서는 앞쪽의 다른 test( 호출 이름이 잘못 추출되거나 예외가 발생합니다. 또한 test ( 처럼 공백이 있는 호출도 놓칩니다.
현재 동작은 대체로 fail-closed입니다. 그래도 진단 메시지가 잘못된 이름을 가리킬 수 있습니다. 세 이름과 공백을 모두 인식하도록 검색을 수정하세요.
🐛 제안 수정
function literalNameBefore(source, offset) {
const prefix = source.slice(0, offset);
- const call = prefix.lastIndexOf("test(");
- if (call < 0) throw new Error("skip directive is not attached to a literal test call");
- const argument = prefix.slice(call + 5).trimStart();
+ const calls = [...prefix.matchAll(/\b(?:test|it|describe)\s*\(/g)];
+ const call = calls.at(-1);
+ if (!call) throw new Error("skip directive is not attached to a literal test call");
+ const argument = prefix.slice(call.index + call[0].length).trimStart();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const call = prefix.lastIndexOf("test("); | |
| if (call < 0) throw new Error("skip directive is not attached to a literal test call"); | |
| const argument = prefix.slice(call + 5).trimStart(); | |
| function literalNameBefore(source, offset) { | |
| const prefix = source.slice(0, offset); | |
| const calls = [...prefix.matchAll(/\b(?:test|it|describe)\s*\(/g)]; | |
| const call = calls.at(-1); | |
| if (!call) throw new Error("skip directive is not attached to a literal test call"); | |
| const argument = prefix.slice(call.index + call[0].length).trimStart(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/skills-lint/scripts/test-directives.test.js` around lines 39 - 41,
Update literalNameBefore to locate the nearest call for test, it, or describe
rather than searching only for test(. Allow optional whitespace before the
opening parenthesis, and ensure the selected call is the closest preceding
direct call so literalNameBefore reports the correct name or preserves the
existing fail-closed error.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (8)
tests/skills-lint/scripts/test-directives.test.js (2)
216-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
it과describe의 옵션 객체 skip 케이스를 주입 목록에 추가하세요.Line 229부터의 주입 목록은
.skip멤버 형태에서만it과describe를 다룹니다. 옵션 객체 형태(it('x', { skip: true }, ...))는 없습니다. 이 형태가literalNameBefore의test(전용 검색과 만나는 경로가 검증되지 않습니다. 해당 케이스를 추가하세요.♻️ 제안 변경
"test('hidden', { [`skip`]: true }, () => {});", + `it('hidden', { ${skipProperty} }, () => {});`, + `describe('hidden', { ${skipProperty} }, () => {});`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/test-directives.test.js` around lines 216 - 275, Extend the injections array in the “directive guard rejects injected unauthorized directives” test with options-object skip cases for both it and describe, using the same skipProperty pattern as the existing test case. Keep the cases unauthorized and ensure they exercise the literalNameBefore handling for non-test callers.
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value수동 스캐너에 짧은 설명 주석을 추가하세요.
regexStartsAt,withoutComments,stringMask는 문자열, 템플릿 리터럴, 정규식 리터럴, 문자 클래스를 직접 추적합니다. 각 함수의 목적과 처리 대상을 한두 줄로 명시하세요. 복잡한 로직에는 의미 있는 주석을 사용해야 합니다.특히 Line 51의
"([{:;,=!&|?+-*%~"는 문자 클래스가 아니라 일반 문자열입니다. 이 점을 주석으로 밝히면 오해를 막을 수 있습니다.Also applies to: 54-87, 89-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/test-directives.test.js` around lines 49 - 52, Add concise one- or two-line comments above regexStartsAt, withoutComments, and stringMask describing each scanner’s purpose and the strings, template literals, regex literals, and character classes it tracks. In particular, document that the character sequence "([{:;,=!&|?+-*%"~" in regexStartsAt is an ordinary string used for delimiter membership checks, not a regular-expression character class.Source: Coding guidelines
tests/skills-lint/scripts/ci-relay-matrix.test.js (2)
105-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
runnerStep의 주석 제거 동작을 문서화하세요.
runnerStep은run블록 안의 주석 줄을 본문에서 제외합니다. 이 동작 때문에 주석이 삽입되어도EXPECTED_RUNNER_STEP비교는 통과합니다. 반대로 실행 줄을 주석으로 바꾸면 해당 줄이 사라져서 비교가 실패합니다. 이 의도를 짧은 주석으로 명시하세요. 복잡한 로직에는 의미 있는 주석을 사용해야 합니다.또한 Line 115의
Math.min은 불필요합니다.String.prototype.slice는 인덱스를 길이로 자동 제한합니다.♻️ 제안 변경
const stepIndent = indent(steps[0].line); const body = []; + // Comment lines are dropped from the body. Commenting out an executable + // line therefore removes it and fails the exact-match comparison. for (let index = steps[0].index + 1; index < lines.length; index += 1) { if (lines[index].trim() && indent(lines[index]) <= stepIndent) break; if (lines[index].trim() && !lines[index].trimStart().startsWith("#")) { - body.push(lines[index].slice(Math.min(lines[index].length, stepIndent))); + body.push(lines[index].slice(stepIndent)); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/ci-relay-matrix.test.js` around lines 105 - 124, Update runnerStep to document that comment-only lines inside the run block are excluded from the extracted body, while commenting out an executable line causes the exact EXPECTED_RUNNER_STEP comparison to fail; retain the existing behavior. Also simplify the body-line slicing by removing the unnecessary Math.min wrapper and pass stepIndent directly to slice.Source: Coding guidelines
198-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win약화 케이스의 오류 메시지 검증을 좁히세요.
Line 216의 정규식
/runner|step|exact|array|zero|serialize|guarded/i는 이 파일의 거의 모든 단언 메시지와 일치합니다. 예를 들어 잡 봉투 실패 메시지와 러너 실패 메시지가 서로 구분되지 않습니다. 그래서 케이스가 의도와 다른 이유로 실패해도 테스트는 통과합니다. 각 약화 케이스에 기대 메시지를 함께 지정하세요.♻️ 제안 변경
- for (const weakened of [ - value.workflow.replace("files=()", ""), - ... - ]) assert.throws(() => assertCiContract({ testsDir: value.root, workflow: weakened }), /runner|step|exact|array|zero|serialize|guarded/i); + for (const [weakened, expected] of [ + [value.workflow.replace("files=()", ""), /fail-closed shell program/], + [value.workflow.replace(" strategy:", " if: always()\n strategy:"), /job-level policy envelope/], + // ... 각 케이스에 해당 메시지를 지정 + ]) assert.throws(() => assertCiContract({ testsDir: value.root, workflow: weakened }), expected);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/skills-lint/scripts/ci-relay-matrix.test.js` around lines 198 - 217, Update the weakened-workflow cases in the assert.throws test around assertCiContract to associate each mutation with its specific expected error-message pattern. Replace the broad shared /runner|step|exact|array|zero|serialize|guarded/i matcher with per-case assertions that distinguish failures such as runner, step, exact, array, zero, serialize, and guarded contract violations.tests/relay-dispatch/fixtures/fake-codex.js (1)
7-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winfixture 제어 프로토콜을 주석으로 설명하세요.
-C,-o, stdin JSON,delay_ms,empty가 각각 작업 디렉터리, 결과 경로, 지연, 결과 파일 생성을 제어합니다. 이 의미가 호출부만으로는 명확하지 않습니다. 특히empty가 작업 디렉터리 변경은 남기고 결과 artifact만 생략한다는 의도를 기록하세요.주석 추가 예시
const args = process.argv.slice(2); +// The harness passes the working directory and result path as argv values. const value = (flag) => { ... const output = value("-o"); +// stdin controls delay injection and omission of the result artifact. let controls = {}; ... if (controls.empty === true) process.exit(0);As per coding guidelines, 복잡한 로직에는 의미 있는 주석을 추가해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/fake-codex.js` around lines 7 - 21, fake-codex fixture의 제어 프로토콜을 코드에 의미 있는 주석으로 설명하세요. `-C`와 `-o`가 작업 디렉터리와 결과 경로를 지정하고, stdin JSON의 `delay_ms`가 지연을 제어하며 `empty`가 작업 디렉터리 변경은 유지한 채 결과 artifact 생성만 생략한다는 내용을 해당 파싱 및 분기 로직 주변에 명시하세요.Source: Coding guidelines
tests/relay-dispatch/fixtures/write-containment-executor.js (1)
60-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
proof_in_result가 참이고-o가 없으면 증거가 사라집니다.63-64행은
controls.proof_in_result === true일 때output이 있을 때만 기록합니다.output이 null이면proof가 어디에도 저장되지 않습니다. 테스트는 증거 없이 실패하고 원인을 알기 어렵습니다. 이 조합에서는 worktree 내containment-proof.json으로 되돌리거나 명시적으로 종료 코드를 설정하십시오.♻️ 제안 수정
if (controls.proof_in_result === true) { - if (output) fs.writeFileSync(output, bytes); + if (output) fs.writeFileSync(output, bytes); + else fs.writeFileSync(path.join(worktree, "containment-proof.json"), bytes); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/write-containment-executor.js` around lines 60 - 68, Update finish() so that when controls.proof_in_result === true and output is absent, the proof is still persisted to worktree/containment-proof.json; retain writing proof to output when output is provided and preserve the existing executor status message for the non-proof-in-result path.tests/relay-dispatch/scripts/facts.test.js (1)
481-514: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win주입된 fs 실패가 의도한 경계보다 먼저 발생합니다.
appendFact와repairTornTail은 모두 본 작업 전에readFacts({ eventsPath, fsModule })를 호출합니다. 두 테스트의 스텁은 해당 fs 메서드의 모든 호출에서 예외를 던집니다. 그러므로 예외는 항상 사전 판독 단계에서 발생하고, 테스트 이름이 지목하는 append open 경계와 repair journal-read 경계는 실행되지 않습니다. 두 테스트는 통과하지만 대상 코드를 검사하지 않습니다.
tests/relay-dispatch/scripts/facts.test.js#L481-L514:openSync호출 횟수를 세어 두 번째(append 경로) 호출에서만 예외를 던지십시오.tests/relay-dispatch/scripts/facts.test.js#L542-L565:fsModule스텁 대신repairTornTail이 제공하는fault콜백을 사용하고journal_read단계에서만 예외를 던지십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/scripts/facts.test.js` around lines 481 - 514, Update tests/relay-dispatch/scripts/facts.test.js lines 481-514 to count openSync calls and inject the failure only on the second call, so the initial readFacts preflight succeeds and the append open boundary is exercised. Update lines 542-565 to use repairTornTail’s fault callback instead of an fsModule stub, throwing only for the journal_read stage; no other fault stages should be affected.tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js (1)
8-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
topLevelTestNames의 상태 전이를 설명하는 주석을 추가하세요.이 함수는 문자열과 주석을 건너뛰고, 중괄호 깊이와 이전 최상위 토큰으로 직접 테스트 등록을 판별합니다. 이 보수적 판별 규칙을 설명하지 않으면 이후 변경에서 가짜 anchor를 허용하거나 유효한 anchor를 거부할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js` around lines 8 - 44, Add comments within topLevelTestNames explaining how string/comment skipping, braceDepth tracking, and lastTopLevelToken transitions conservatively identify valid top-level test registrations. Document the anchor requirements and state transitions so future changes preserve rejection of false anchors and acceptance of valid ones.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/relay-dispatch/fixtures/fake-codex.js`:
- Around line 14-15: Update the control-input parsing in the fake Codex fixture
so empty or whitespace-only stdin continues to use the default controls, but
non-empty invalid JSON writes an error to stderr and exits with a non-zero
status instead of continuing with {}. Preserve valid JSON handling and the
existing delay_ms/empty behavior.
In `@tests/relay-dispatch/fixtures/runtime-contract-noop.js`:
- Around line 3-18: Update runtime-contract.test.js to compare the exact export
keys of productionRuntime and the injected runtime-contract-noop fixture, using
exactKeys(...) or equivalent. Ensure the test fails when either runtime adds,
removes, or renames an export, including keeping the no-op fixture synchronized
with the runtime contract.
In `@tests/relay-dispatch/fixtures/write-containment-executor.js`:
- Line 23: Update the temp-directory initialization in the proof setup to use
os.tmpdir() when process.env.TMPDIR is unset, ensuring path.join receives a
valid directory and the temp-write probe reports the actual containment result.
Import or reuse the existing os module in the fixture.
In `@tests/relay-dispatch/scripts/facts.test.js`:
- Around line 349-352: Replace the forged-capability assertion around appendFact
with a test that reuses the capability returned by acquireFactLock while
substituting run_dir with the value from another execution. Keep the shared
capability identity intact and assert that appendFact rejects this mismatched
execution context, so the “forged capabilities” test validates run_dir checking
rather than rejection of a cloned lock object.
In `@tests/relay-dispatch/scripts/inspect-recover-blackbox.test.js`:
- Line 609: Add explicit coverage near the “verification refuses dirty
reviewable bytes” test for special worktree entries: create Unix socket and FIFO
entries, run the verification flow, and assert each is rejected with
unsafe_worktree_entry. Reuse the existing socket/FIFO setup and assertion
helpers from inspect-recover-blackbox.test.js, preserving the expected behavior
for regular files.
In `@tests/skills-lint/scripts/test-directives.test.js`:
- Around line 39-41: Update literalNameBefore to locate the nearest call for
test, it, or describe rather than searching only for test(. Allow optional
whitespace before the opening parenthesis, and ensure the selected call is the
closest preceding direct call so literalNameBefore reports the correct name or
preserves the existing fail-closed error.
- Around line 37-47: Update literalNameBefore so escape restoration matches the
source literal encodings: make the newline pattern match one backslash followed
by n and the backslash pattern match two backslashes. Perform backslash
restoration once at the end, after quote and newline decoding, while preserving
the existing quote handling.
---
Nitpick comments:
In `@tests/relay-dispatch/fixtures/fake-codex.js`:
- Around line 7-21: fake-codex fixture의 제어 프로토콜을 코드에 의미 있는 주석으로 설명하세요. `-C`와
`-o`가 작업 디렉터리와 결과 경로를 지정하고, stdin JSON의 `delay_ms`가 지연을 제어하며 `empty`가 작업 디렉터리
변경은 유지한 채 결과 artifact 생성만 생략한다는 내용을 해당 파싱 및 분기 로직 주변에 명시하세요.
In `@tests/relay-dispatch/fixtures/write-containment-executor.js`:
- Around line 60-68: Update finish() so that when controls.proof_in_result ===
true and output is absent, the proof is still persisted to
worktree/containment-proof.json; retain writing proof to output when output is
provided and preserve the existing executor status message for the
non-proof-in-result path.
In `@tests/relay-dispatch/scripts/facts.test.js`:
- Around line 481-514: Update tests/relay-dispatch/scripts/facts.test.js lines
481-514 to count openSync calls and inject the failure only on the second call,
so the initial readFacts preflight succeeds and the append open boundary is
exercised. Update lines 542-565 to use repairTornTail’s fault callback instead
of an fsModule stub, throwing only for the journal_read stage; no other fault
stages should be affected.
In `@tests/relay-dispatch/scripts/runtime-contract-blackbox.test.js`:
- Around line 8-44: Add comments within topLevelTestNames explaining how
string/comment skipping, braceDepth tracking, and lastTopLevelToken transitions
conservatively identify valid top-level test registrations. Document the anchor
requirements and state transitions so future changes preserve rejection of false
anchors and acceptance of valid ones.
In `@tests/skills-lint/scripts/ci-relay-matrix.test.js`:
- Around line 105-124: Update runnerStep to document that comment-only lines
inside the run block are excluded from the extracted body, while commenting out
an executable line causes the exact EXPECTED_RUNNER_STEP comparison to fail;
retain the existing behavior. Also simplify the body-line slicing by removing
the unnecessary Math.min wrapper and pass stepIndent directly to slice.
- Around line 198-217: Update the weakened-workflow cases in the assert.throws
test around assertCiContract to associate each mutation with its specific
expected error-message pattern. Replace the broad shared
/runner|step|exact|array|zero|serialize|guarded/i matcher with per-case
assertions that distinguish failures such as runner, step, exact, array, zero,
serialize, and guarded contract violations.
In `@tests/skills-lint/scripts/test-directives.test.js`:
- Around line 216-275: Extend the injections array in the “directive guard
rejects injected unauthorized directives” test with options-object skip cases
for both it and describe, using the same skipProperty pattern as the existing
test case. Keep the cases unauthorized and ensure they exercise the
literalNameBefore handling for non-test callers.
- Around line 49-52: Add concise one- or two-line comments above regexStartsAt,
withoutComments, and stringMask describing each scanner’s purpose and the
strings, template literals, regex literals, and character classes it tracks. In
particular, document that the character sequence "([{:;,=!&|?+-*%"~" in
regexStartsAt is an ordinary string used for delimiter membership checks, not a
regular-expression character class.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36723021-52ee-40cd-b376-d51147bcf733
⛔ Files ignored due to path filters (2)
tests/ledger/vnext-baseline.generated.jsonis excluded by!**/*.generated.*tests/ledger/vnext-test-sites.generated.jsonis excluded by!**/*.generated.*
📒 Files selected for processing (53)
CLAUDE.mdREADME.mdbacklog/sprints/2026-08-relay-runtime-slimming.mddocs/contracts/relay-runtime-contracts.v1.jsondocs/contracts/relay-runtime-inventory.v1.jsondocs/ranked-deletion-inventory.mddocs/relay-operator-guide.mddocs/script-inventory-and-cleanup.mddocs/test-accounting-retirement-1196.mdreferences/architecture.mdskills/relay-dispatch/scripts/recover.jstests/ledger/README.mdtests/ledger/vnext-baseline-measurements.jsontests/ledger/vnext-test-ledger.jsontests/relay-dispatch/fixtures/adapter-runtime-preload.jstests/relay-dispatch/fixtures/fake-cline.jstests/relay-dispatch/fixtures/fake-codex.jstests/relay-dispatch/fixtures/fake-cursor.jstests/relay-dispatch/fixtures/json-observer.jstests/relay-dispatch/fixtures/runtime-contract-noop.jstests/relay-dispatch/fixtures/vnext-shadow-parity-corpus.jsontests/relay-dispatch/fixtures/write-containment-executor.jstests/relay-dispatch/scripts/dispatch.test.jstests/relay-dispatch/scripts/docs-defaults.test.jstests/relay-dispatch/scripts/facts.test.jstests/relay-dispatch/scripts/host-concurrency.test.jstests/relay-dispatch/scripts/inspect-recover-blackbox.test.jstests/relay-dispatch/scripts/recover-safe-staging.test.jstests/relay-dispatch/scripts/relay-recover-cli.test.jstests/relay-dispatch/scripts/run-fold.test.jstests/relay-dispatch/scripts/run-store.test.jstests/relay-dispatch/scripts/runtime-contract-blackbox.test.jstests/relay-dispatch/scripts/runtime-contract-noop.test.jstests/relay-dispatch/scripts/runtime-contract.test.jstests/relay-dispatch/scripts/toolset-mismatch.test.jstests/relay-fleet/scripts/fleet-derived.test.jstests/relay-merge/fixtures/merge-observer.jstests/relay-merge/scripts/finalize-run.test.jstests/relay-merge/scripts/gate-check.test.jstests/relay-review/fixtures/fake-gh.jstests/relay-review/scripts/review-runner.test.jstests/relay/scripts/relay-status-recover.test.jstests/relay/scripts/run-preflight.test.jstests/skills-lint/scripts/ci-matrix-completeness.test.jstests/skills-lint/scripts/ci-relay-matrix.test.jstests/skills-lint/scripts/ci-test-coverage.test.jstests/skills-lint/scripts/pr-view-json-contract.test.jstests/skills-lint/scripts/skill-inputs-drift.test.jstests/skills-lint/scripts/test-directives.test.jstests/skills-lint/scripts/vnext-runtime-inventory.jstests/skills-lint/scripts/vnext-runtime-inventory.test.jstests/skills-lint/scripts/vnext-test-ledger.jstests/skills-lint/scripts/vnext-test-ledger.test.js
💤 Files with no reviewable changes (12)
- tests/ledger/README.md
- tests/skills-lint/scripts/ci-test-coverage.test.js
- tests/skills-lint/scripts/ci-matrix-completeness.test.js
- tests/ledger/vnext-test-ledger.json
- tests/skills-lint/scripts/vnext-runtime-inventory.test.js
- tests/relay-dispatch/fixtures/vnext-shadow-parity-corpus.json
- tests/skills-lint/scripts/vnext-test-ledger.js
- tests/skills-lint/scripts/vnext-test-ledger.test.js
- tests/skills-lint/scripts/vnext-runtime-inventory.js
- skills/relay-dispatch/scripts/recover.js
- docs/contracts/relay-runtime-inventory.v1.json
- tests/ledger/vnext-baseline-measurements.json
🛑 Comments failed to post (4)
tests/relay-dispatch/fixtures/fake-codex.js (1)
14-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
잘못된 제어 JSON을 조용히 기본값으로 처리하지 마세요.
비어 있지 않은 stdin이 잘못된 JSON이면
catch {}가controls = {}상태로 계속 실행합니다. 그러면 fixture가delay_ms또는empty를 무시하고 정상 완료하여 테스트가 잘못 통과할 수 있습니다. 빈 stdin만 기본값으로 허용하고, 비어 있지 않은 파싱 오류는 stderr와 비정상 종료로 처리하세요.수정 예시
-let controls = {}; -try { controls = JSON.parse(fs.readFileSync(0, "utf8")); } catch {} +const controlText = fs.readFileSync(0, "utf8").trim(); +let controls = {}; +if (controlText) { + try { + controls = JSON.parse(controlText); + } catch (error) { + process.stderr.write(`invalid control JSON: ${error.message}\n`); + process.exit(65); + } +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const controlText = fs.readFileSync(0, "utf8").trim(); let controls = {}; if (controlText) { try { controls = JSON.parse(controlText); } catch (error) { process.stderr.write(`invalid control JSON: ${error.message}\n`); process.exit(65); } }🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 14-14: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(0, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/fake-codex.js` around lines 14 - 15, Update the control-input parsing in the fake Codex fixture so empty or whitespace-only stdin continues to use the default controls, but non-empty invalid JSON writes an error to stderr and exits with a non-zero status instead of continuing with {}. Preserve valid JSON handling and the existing delay_ms/empty behavior.tests/relay-dispatch/fixtures/runtime-contract-noop.js (1)
3-18: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # no-op 픽스처 키 집합이 실제 런타임 계약 export와 일치하는지 확인 set -uo pipefail echo "== 픽스처 소비 지점" rg -n -C 6 'runtime-contract-noop' tests skills || true echo "== 런타임 계약 모듈 후보 탐색" rg -n --type=js -C 2 '\brevalidateExternalFacts\b' skills || true echo "== 후보 모듈의 export 구조" fd -t f -e js . skills/relay-dispatch/scripts --exec sh -c 'rg -lq "revalidateExternalFacts" "$1" && ast-grep outline "$1" --items all' _ {}Repository: sungjunlee/dev-relay
Length of output: 11857
🏁 Script executed:
#!/bin/bash set -uo pipefail echo "== scripts/facts.js exports and exactKeys contract tests ==" sed -n '700,760p' skills/relay-dispatch/scripts/facts.js echo rg -n 'exactKeys\(factsModule|runtime-contract-noop|runtime-contract.test.js|runtime-contract-noop.test.js' tests/relay-dispatch/scripts -C 4 || true echo sed -n '1,140p' tests/relay-dispatch/scripts/runtime-contract-noop.test.js echo sed -n '1,200p' tests/relay-dispatch/scripts/runtime-contract.test.js echo "== JavaScript structural verifier ==" node - <<'JS' const fs = require('fs'); const path = require('path'); const root = process.cwd(); const noopPath = path.join(root, 'tests/relay-dispatch/fixtures/runtime-contract-noop.js'); const factsPath = path.join(root, 'skills/relay-dispatch/scripts/facts.js'); const testPath = path.join(root, 'tests/relay-dispatch/scripts/runtime-contract-noop.test.js'); const runnerPath = path.join('/tmp/coderabbit-shell-logs/shell-output-az11Qv'); // not useful here; use above. function trimComments(script) { return script .replace(/\/\/[^\n]*/g, '') .replace(/\/\*[\s\S]*?\*\//g, ''); } const noop = require(noopPath); const noopKeys = new Set(Object.keys(noop)); const factsSource = fs.readFileSync(factsPath, 'utf8'); const trimmed = trimComments(factsSource); const lastExportMatch = [...trimmed.matchAll(/\bmodule\.exports\s*=\s*\{([^{]*?)\}/g)].pop(); if (!lastExportMatch) throw new Error('facts.js last module.exports object not found'); const exportsObjectText = '{' + lastExportMatch[1] + '}'; let exported; function evalSafe() { const sandbox = {}; eval('sandbox.exports = ' + exportsObjectText); return sandbox.exports; } try { exported = evalSafe(); } catch (e) { console.error('eval parse failed:', e.message); const keys = exportsObjectText.match(/\b([A-Za-z_$][\w$]*)\s*:/g) || []; exported = Object.fromEntries(keys.map(k => [k.match(/\b([A-Za-z_$][\w$]*)/)[1], true])); } const exportedKeys = new Set(Object.keys(exported)); function parseRequireExactKeys() { const text = fs.readFileSync(testPath, 'utf8'); const parts = text.split('replaceModuleWithRequire'); if (!['runtime-contract-noop'].every(k => parts.includes(k))) { return null; } const replacePart = parts[1].split('\nmodule.exports')[0].split('function replaceModuleWithRequire')[1] || ''; const all = replacePart + parts[parts.length - 1]; return all; } const requireText = parseRequireExactKeys(); console.log(JSON.stringify({ noopKeys: Array.from(noopKeys).sort().join(','), exportedKeys: Array.from(exportedKeys).sort().join(','), noopMissingFromExported: Array.from(noopKeys.difference(exportedKeys)).sort(), exportedMissingFromNoop: Array.from(exportedKeys.difference(noopKeys)).sort(), exactKeysDetected: /exactKeys\s*\([^)]*factsModule/.test(testPath.includes?) || (function(){ try { return requireText && /exactKeys\s*\([^)*?factsModule[^)]*\)?:[\s\S]{1,1000}/.test(requireText); } catch { return false; } })(), }, null, 2)); function tryEvalRequire() { const vm = require('vm'); const vmPath = fs.readFileSync('/tmp/vm-wrapper-1.js', 'utf-8').trim(); fs.writeFileSync(vmPath, ''); vm.runInNewContext(requireText, { exports: noop, exactKeys: () => true }); } const vmPath = '/tmp/vm-wrapper-1.js'; fs.writeFileSync(vmPath, ''); try{tryEvalRequire()}catch(e){console.error('requireText eval error:', e.message)} JSRepository: sungjunlee/dev-relay
Length of output: 14723
🏁 Script executed:
#!/bin/bash set -uo pipefail echo "== runtime-contract-noop test relevant source ==" cat -n tests/relay-dispatch/scripts/runtime-contract-noop.test.js | sed -n '1,180p' echo "== runtime-contract test exactKeys usages ==" rg -n -C 8 'exactKeys|createModuleWithRequires|replaceModuleWithRequires|runtime-contract-noop' tests/relay-dispatch/scripts/runtime-contract.test.js || true echo "== facts.js export block ==" cat -n skills/relay-dispatch/scripts/facts.js | sed -n '724,740p' echo "== deterministic key equality check from source text ==" python3 - <<'PY' from pathlib import Path import re test = Path('tests/relay-dispatch/scripts/runtime-contract-noop.test.js').read_text() facts = Path('skills/relay-dispatch/scripts/facts.js').read_text() m = re.search(r'exactKeys\s*\(\s*([A-Za-z_$][\w$]*)\s*,\s*\[([^\]]+)\]\s*\)\s*:",\s*"([A-Za-z_$][\w$]*)"', test) if not m: print('exactKeys call pattern not found, printing exactKeys occurrences') for i,line in enumerate(test.splitlines(),1): if 'exactKeys' in line: print(f'line {i}: {line.strip()}') else: obj_name = m.group(1) keys = sorted(k.strip().strip('"\'') for k in m.group(2).split(',')) label = m.group(3) or '<no label>' print('exactKeys object:', obj_name) print('keys count:', len(keys)) print('keys:', keys) # Compare no-op fixture exported keys and facts.js final exports. noop_export = next(re.finditer(r'\bmodule\.exports\s*=\s*\{(?P<body>(?:[^{}]|\{(?P>n)\}[^{}\n]*)*?)\}\s*;', facts), None) export_expr = next(re.finditer(r'\bmodule\.exports\s*=\s*\{(?P<body>.{0,150}?)\}\s*;', test) or re.finditer(r'\bmodule\.exports\s*=\s*\{(.*?);$', test)) # Actually, final line should be at end; get last object assignment. last_obj = re.search(r'\bmodule\.exports\s*=\s*\{([^}]*)\}\s*;', facts, flags=re.S) if last_obj: keys_from_export = sorted(re.findall(r'\b([A-Za-z_$][\w$]*)\s*:', last_obj.group(1))) print('facts.js module.exports count:', len(keys_from_export)) print('facts.js module.exports keys:', keys_from_export) print('no_op_fixture_keys_differ_from_exports:', keys_from_export != sorted(re.findall(r'\b([A-Za-z_$][\w$]*)\s*:', Path('tests/relay-dispatch/fixtures/runtime-contract-noop.js').read_text()))) PYRepository: sungjunlee/dev-relay
Length of output: 3394
no-op 픽스처와 런타임 계약 export 집합을 단말에서 맞춤으로 검증하세요.
runtime-contract-noop.js는 현재RELAY_RUNTIME_PATH로 주입되고,runtime-contract.test.js에는runtimeexport 키의 정확한 일치 검증이 없습니다. 팩트가 추가되거나 이름이 바뀌면 no-op 목록이 낡아도 contract test는 해당 함수가 없는 경우createRunRecord is not a function만 보고 통과할 수 있습니다.productionRuntime과 custom runtime export 집합의 동기화를exactKeys(...)또는 동등한 검증에서 강제하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/runtime-contract-noop.js` around lines 3 - 18, Update runtime-contract.test.js to compare the exact export keys of productionRuntime and the injected runtime-contract-noop fixture, using exactKeys(...) or equivalent. Ensure the test fails when either runtime adds, removes, or renames an export, including keeping the no-op fixture synchronized with the runtime contract.tests/relay-dispatch/fixtures/write-containment-executor.js (1)
23-23: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
TMPDIR이 없으면 temp 프로브가 거짓denied를 기록합니다.23행은
process.env.TMPDIR || null을 사용합니다. Linux에서는TMPDIR이 보통 설정되지 않습니다. 그러면 37행의path.join(null, ...)이TypeError를 던집니다. catch는error.code를 읽지만TypeError에는code가 없으므로proof.temp가denied:unknown이 됩니다.결과적으로 샌드박스가 temp 쓰기를 실제로 차단하지 않아도 프로브가 차단된 것처럼 보입니다. containment 증거가 거짓 음성이 됩니다.
os.tmpdir()을 fallback으로 사용하십시오.🐛 제안 수정
const fs = require("fs"); const net = require("net"); +const os = require("os"); const path = require("path"); const { spawnSync } = require("child_process");-const proof = { tempdir: process.env.TMPDIR || null }; +const proof = { tempdir: process.env.TMPDIR || os.tmpdir() };Also applies to: 36-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/fixtures/write-containment-executor.js` at line 23, Update the temp-directory initialization in the proof setup to use os.tmpdir() when process.env.TMPDIR is unset, ensuring path.join receives a valid directory and the temp-write probe reports the actual containment result. Import or reuse the existing os module in the fixture.tests/relay-dispatch/scripts/facts.test.js (1)
349-352: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # capability 구조와 lock 식별 방식 확인 set -uo pipefail echo "== capability 생성 및 등록" rg -n -C 6 'issuedLocks|lockStates|Object\.freeze\(\{ lock_id' skills/relay-dispatch/scripts/host.js echo "== requireLockHeld / assertRunLockHeld / stateFor 구현" ast-grep run --pattern 'function $NAME($$$) { $$$ }' --lang javascript skills/relay-dispatch/scripts/host.js \ | rg -n -A 12 'function (stateFor|assertRunLockHeld)\b' || true rg -n -C 10 'function requireLockHeld' skills/relay-dispatch/scripts/facts.js echo "== capability 에 owner 속성이 있는지" rg -n -C 3 '\bowner\b' skills/relay-dispatch/scripts/host.js | rg -n 'capability|return' || trueRepository: sungjunlee/dev-relay
Length of output: 14747
🏁 Script executed:
#!/bin/bash set -uo pipefail echo "== facts.test.js lock tests around reported lines and capability helpers/setup" sed -n '300,375p' tests/relay-dispatch/scripts/facts.test.js | cat -n echo echo "== lockContext usage and capability mocking in tests" rg -n -C 5 'lockContext|acquireRunLock|lock_id|run_dir|owner:.*token|token' tests/relay-dispatch/scripts/facts.test.js echo echo "== facts.js appendFact / requireLockHeld implementation" rg -n -C 8 'function appendFact|function releaseFact|requireLockHeld' tests/relay-dispatch/scripts/facts.jsRepository: sungjunlee/dev-relay
Length of output: 13083
🏁 Script executed:
#!/bin/bash set -uo pipefail echo "== fact lock functions in host.js" sed -n '645,665p' skills/relay-dispatch/scripts/host.js | cat -n echo echo "== stateFor in facts.js with full path" sed -n '80,110p' skills/relay-dispatch/scripts/facts.js | cat -n sed -n '375,392p' skills/relay-dispatch/scripts/facts.js | cat -n echo echo "== host capability shape behavioral probe" node - <<'JS' const capability = Object.freeze({ lock_id: "l1", attempt_id: "a1", operation: "dispatch", run_dir: "/tmp/run", host_kind: "local_supervisor", host_handle: "dispatch:42:abc" }); const original = { ...capability, owner: { ...capability.owner, token: "forged" } }; console.log("capability has owner:", Object.prototype.hasOwnProperty.call(capability, "owner"), "owner value:", capability.owner); console.log("original lock_id:", original.lock_id, "run_dir:", original.run_dir, "owner:", original.owner); console.log("same object:", original === capability); const issuedLocks = new WeakSet(); issuedLocks.add(capability); console.log("WeakSet.has(original):", issuedLocks.has(original)); JSRepository: sungjunlee/dev-relay
Length of output: 3999
스프레드 복제 capability 테스트를 교체합니다.
350행의
{ ...lock, owner: { ...lock.owner, token: "forged" } }는acquireFactLock이 반환한 frozen capability와 다른 객체 참조가 됩니다.stateFor()는 registered object 동일성으로 lock을 확인하므로 이 단언은 위조 token 검증을 하지 않고 복제된 lock context 객체를 거절하는 동작만 검증합니다. 테스트 이름의 “forged capabilities”를 실제 검증하려면, 발급된 capability를 공유한 상태 중run_dir을 다른 실행의 값으로 바꾼 사례를 추가하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/relay-dispatch/scripts/facts.test.js` around lines 349 - 352, Replace the forged-capability assertion around appendFact with a test that reuses the capability returned by acquireFactLock while substituting run_dir with the value from another execution. Keep the shared capability identity intact and assert that appendFact rejects this mismatched execution context, so the “forged capabilities” test validates run_dir checking rather than rejection of a cloned lock object.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85cba6fed3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const member = (name) => new RegExp(`\\b(?:test|it|describe)\\s*(?:\\.\\s*${name}|\\[\\s*(["'])${name}\\1\\s*\\])\\s*\\(`); | ||
| const option = (name) => new RegExp(`(?:\\{|,)\\s*(?:${name}\\s*:|\\[\\s*(["'])${name}\\1\\s*\\]\\s*:|${name}\\s*(?=[,}]))`); | ||
| if (member("only").test(executable)) violations.push(`${relative}: only registration is forbidden`); | ||
| if (member("todo").test(executable)) violations.push(`${relative}: todo is forbidden`); | ||
| if (member("skip").test(executable)) violations.push(`${relative}: skip registration is forbidden`); | ||
| if (/\b(?:test|it|describe)\s*\[/.test(maskedExecutable)) { |
There was a problem hiding this comment.
An optional chain bypasses all of these patterns: test?.skip("hidden", fn) does not match the member regex, computed-registration check, or direct-call parser, but Node accepts it and reports the test as skipped. Because this registration is never added to skips, the exact allowlist still matches and CI passes while an ordinary Relay test disappears; reject optional chaining on test registrations or parse these calls structurally.
Useful? React with 👍 / 👎.
1 participant
Summary
Measurements
Verification
Fixes #1196
Parent #1197
Summary by CodeRabbit
문서
테스트 및 안정성