feat(shell): sync agent plans to pull requests#371
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughPR интегрирует инструмент plan-to-git для синхронизации планов в PR/ветки: добавляет установку CLI в Docker-образы с pinned ревизией, расширяет пост-push логику для вызова синхронизации, генерирует managed Codex hooks, исключает ChangesИнтеграция plan-to-git для синхронизации планов
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app/tests/docker-git/core-templates.test.ts (1)
66-90: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winПокройте новые инварианты через property-based тесты (
fast-check).Сейчас проверки завязаны на один конфиг и не доказывают инварианты для всего допустимого пространства
TemplateConfig.Пример минимального усиления теста
+import * as fc from "fast-check" ... - it("keeps plan-to-git state out of generated git and docker contexts", () => { - const files = planFiles(makeTemplateConfig()) - const gitignore = getGeneratedFile(files, ".gitignore") - const dockerignore = getGeneratedFile(files, ".dockerignore") - - expect(gitignore.contents).toContain(".agent-plan.json") - expect(dockerignore.contents).toContain(".agent-plan.json") - }) + it("keeps plan-to-git state out of generated git and docker contexts", () => { + fc.assert( + fc.property(fc.boolean(), fc.constantFrom<TemplateConfig["gpu"]>("none", "all"), (enableMcpPlaywright, gpu) => { + const files = planFiles(makeTemplateConfig({ enableMcpPlaywright, gpu })) + const gitignore = getGeneratedFile(files, ".gitignore") + const dockerignore = getGeneratedFile(files, ".dockerignore") + expect(gitignore.contents).toContain(".agent-plan.json") + expect(dockerignore.contents).toContain(".agent-plan.json") + }) + ) + })As per coding guidelines:
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants.Also applies to: 92-99
🤖 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 `@packages/app/tests/docker-git/core-templates.test.ts` around lines 66 - 90, Add a property-based test using fast-check that generates random TemplateConfig instances and for each run renders the Dockerfile and entrypoint (the existing variables checked as dockerfile.contents and entrypoint.contents) and asserts the invariants currently hard-coded in the test (e.g., presence of "cargo install ... plan-to-git", "/usr/local/bin/plan-to-git --help >/dev/null", "make build-essential docker.io", "docker_git_stop_playwright_browser()", "docker-git-browser-connection", "plan-to-git sync", "plan-to-git hook --source codex", CODEX_REQUIREMENTS_FILE, managed_dir and hooks entries, and the absence checks like "docker-git-playwright-mcp" and "docker_git_start_rust_browser_connection" etc.); import fast-check (fc), create an Arbitrary for TemplateConfig (or reuse existing factory), call the template rendering function used by this test to produce dockerfile.contents and entrypoint.contents, and replace or augment the one-off assertions with fc.assert(fc.property(...)) so these invariants hold across the generated TemplateConfig space.
🤖 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 `@packages/lib/tests/core/git-post-push-wrapper.test.ts`:
- Around line 113-125: The fakePlanToGitScript test helper currently logs any
arguments passed without verifying the command; update the fakePlanToGitScript
to validate that the first positional parameter is "sync" and fail the script
(non-zero exit or explicit error log) if it is not, so tests catch incorrect
invocations. Locate the fakePlanToGitScript string used in
git-post-push-wrapper.test.ts and add a conditional after the existing logging
that checks "$1" (the first arg) equals "sync" and exits with an error code
(and/or appends a clear error message to FAKE_PLAN_TO_GIT_LOG_PATH) when the
check fails.
In `@packages/lib/tests/core/templates.test.ts`:
- Around line 513-528: Replace the single-case test with a fast-check property
test that runs for all valid TemplateConfig instances: use the existing
arbitrary for TemplateConfig (e.g., templateConfigArbitrary) and write an
fc.assert(fc.property(templateConfigArbitrary, cfg => { const files =
planFiles(cfg); const gitignore = files.find(f => f._tag === "File" &&
f.relativePath === ".gitignore"); const dockerignore = files.find(f => f._tag
=== "File" && f.relativePath === ".dockerignore"); return
gitignore?.contents.includes(".agent-plan.json") &&
dockerignore?.contents.includes(".agent-plan.json"); })), ensuring you import
fast-check and the TemplateConfig arbitrary and keep references to planFiles and
the file-match logic (._tag and relativePath) intact so the test verifies the
ignore invariant for all generated configs.
---
Outside diff comments:
In `@packages/app/tests/docker-git/core-templates.test.ts`:
- Around line 66-90: Add a property-based test using fast-check that generates
random TemplateConfig instances and for each run renders the Dockerfile and
entrypoint (the existing variables checked as dockerfile.contents and
entrypoint.contents) and asserts the invariants currently hard-coded in the test
(e.g., presence of "cargo install ... plan-to-git", "/usr/local/bin/plan-to-git
--help >/dev/null", "make build-essential docker.io",
"docker_git_stop_playwright_browser()", "docker-git-browser-connection",
"plan-to-git sync", "plan-to-git hook --source codex", CODEX_REQUIREMENTS_FILE,
managed_dir and hooks entries, and the absence checks like
"docker-git-playwright-mcp" and "docker_git_start_rust_browser_connection"
etc.); import fast-check (fc), create an Arbitrary for TemplateConfig (or reuse
existing factory), call the template rendering function used by this test to
produce dockerfile.contents and entrypoint.contents, and replace or augment the
one-off assertions with fc.assert(fc.property(...)) so these invariants hold
across the generated TemplateConfig space.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 24b19a0d-7c7e-4df3-819b-10cb1b9f5b8e
📒 Files selected for processing (9)
packages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/lib/tests/core/git-post-push-wrapper.test.tspackages/lib/tests/core/templates.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Login context)
- GitHub Check: Test
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Lint
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile-prelude.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile-prelude.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates-entrypoint/git-hooks.tspackages/app/src/lib/core/templates-entrypoint/git-hooks.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/core/templates.test.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/core/templates.test.tspackages/lib/tests/core/git-post-push-wrapper.test.ts
🧠 Learnings (3)
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore} : Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Applied to files:
packages/app/src/lib/core/templates.tspackages/lib/src/core/templates.ts
📚 Learning: 2026-05-13T07:10:13.213Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-13T07:10:13.213Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Applied to files:
packages/lib/tests/core/git-post-push-wrapper.test.ts
📚 Learning: 2026-05-13T07:09:47.992Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-13T07:09:47.992Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: `Effect.provide(MockService), Effect.runPromise`.
Applied to files:
packages/lib/tests/core/git-post-push-wrapper.test.ts
🔇 Additional comments (13)
packages/lib/tests/core/git-post-push-wrapper.test.ts (6)
28-28: LGTM!
221-221: LGTM!
263-263: LGTM!Also applies to: 274-274, 300-300
315-320: LGTM!Also applies to: 333-338, 352-357, 379-386, 434-437
388-403: LGTM!
405-421: LGTM!packages/lib/src/core/templates/dockerfile-prelude.ts (1)
86-102: LGTM!Also applies to: 104-115
packages/app/src/lib/core/templates/dockerfile-prelude.ts (1)
86-102: LGTM!Also applies to: 104-115
packages/app/src/lib/core/templates.ts (1)
42-42: LGTM!Also applies to: 54-54
packages/lib/src/core/templates.ts (1)
41-41: LGTM!Also applies to: 53-53
packages/app/src/lib/core/templates-entrypoint/git-hooks.ts (1)
8-9: LGTM!Also applies to: 141-157, 177-217
packages/lib/src/core/templates-entrypoint/git-hooks.ts (1)
8-9: LGTM!Also applies to: 141-157, 177-217
packages/lib/tests/core/templates.test.ts (1)
210-213: LGTM!Also applies to: 467-492
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-06-04T14:34:40Z Current container plan-to-git smoke test
|
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-06-04T09:21:11.600Z Plan: Upload Agent Plans To PRs With plan-to-gitSummary
Key Changes
Public Interface
Test Plan
Assumptions
2. PlanSource: codex - Captured: 2026-06-04T10:03:57.021Z Keep plan-to-git Installed via CargoSummary
Key Changes
Test Plan
Assumptions
|
Summary
plan-to-gitin generated project images via pinned Cargo git revision, matching the existingrust-browser-connectiondelivery model.plan-to-git hook --source codex.plan-to-git syncfrom the generated global git post-push wrapper before session backup..agent-plan.jsonout of generated git and Docker contexts.Closes #369.
E2E proof
I ran a live generated docker-git project container from this branch against
https://github.com/octocat/Hello-World/issues/1, then executed the generated hook inside the project container with a fakeghthat only records the PR comment request. This proves the real compiledplan-to-gitbinary, generated hook config, local state write, and PR-comment sync path without posting to octocat.Checks
bun run --cwd packages/app lintwas attempted, but this environment killed the full app source ESLint auto-fix process with SIGKILL/code 137. The narrower ESLint check on the changed app source files passed, and the app test lint path ran as part of the focused app test command.