[PF-26] Fixing planforge doctor errors on Mac - #44
Conversation
…ser would be likely to ask.
![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: 18ef5afbe6
ℹ️ 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".
| workflow_content | ||
| + "\n\nAnswer in one sentence only: what command or action you will take for the user request. Do not run anything." | ||
| ) | ||
| system_prompt = _load_workflow_mdc(project_root) |
There was a problem hiding this comment.
This line has one extra leading space compared to the surrounding block, which makes the module fail to parse with IndentationError (unindent does not match any outer indentation level). As a result, importing or running the Python CLI command path that loads doctor.py breaks immediately, so planforge doctor cannot run in the Python implementation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec54381ad
ℹ️ 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".
| for (const filePath of REQUIRED_FILES) { | ||
| if (!fs.existsSync(filePath)) { | ||
| fail(`missing required file: ${filePath}`); | ||
| } | ||
| return fs.readFileSync(filePath, "utf8"); | ||
| } | ||
|
|
||
| function requireContains(filePath, text, required) { | ||
| for (const needle of required) { | ||
| if (!text.includes(needle)) { | ||
| fail(`${filePath} is missing required text: ${JSON.stringify(needle)}`); | ||
| } | ||
| console.error(`[validate_cursor_assets] ERROR: missing required file: ${filePath}`); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
This script is the CI gate run by pr-build.yml/release-js.yml, but it now only verifies that files exist and no longer checks frontmatter, required guardrail text, or template/runtime equality, so mismatched or stale .cursor assets will pass validation and can be shipped unnoticed. That regresses the integrity guarantee this check previously provided and makes workflow-behavior drift much harder to catch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60764d93d3
ℹ️ 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".
| #!/usr/bin/env bash | ||
| # Run plan command (provider from planforge.json). Called from Cursor /p with user goal as arguments. | ||
| # Context dir is taken from planforge.json (contextDir) or CLI default; do not hard-code it here. | ||
| set -e |
There was a problem hiding this comment.
This file was converted to CRLF, which breaks the mac/Linux execution path the skill relies on: running it via bash now emits set: -^M: invalid option at set -e and appends a carriage return to the final argument (e.g. goal becomes goal\r), so /p can pass malformed input to planforge plan and lose fail-fast behavior; the same regression is present in .cursor/skills/i/scripts/run_implement.sh. This only appears on non-Windows environments, but that is exactly where these .sh scripts are used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f2de7ce18
ℹ️ 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".
| stdio: ["pipe", "pipe", "pipe"], | ||
| }); | ||
| const clearTimeoutRef = scheduleTimeout(child); | ||
| child.stdin.write(fullPrompt, "utf-8"); |
There was a problem hiding this comment.
In the non-Windows streaming path, the code writes to child.stdin without handling stdin stream errors. If codex exec - exits before consuming stdin (for example due to startup/auth/usage failures), Node can emit write EPIPE on that stream and terminate the process with an unhandled error, so runPlan/runImplement/doctor streaming crash instead of surfacing the Codex stderr message.
Useful? React with 👍 / 👎.
| proc.stdin.write(full_prompt) | ||
| proc.stdin.close() |
There was a problem hiding this comment.
The non-Windows Python streaming path writes the entire prompt to proc.stdin synchronously before entering the normal wait/error flow. If codex exits early without reading stdin, this write/close can raise BrokenPipeError (especially with larger prompts), which bypasses the intended process exit handling and returns a low-level pipe failure instead of the CLI error context.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
* [PF-24] Configurable stream timeout and config merge from template JSON [PF-24] Configurable stream timeout and config merge from template JSON * [PF-24] Advisory messages, plan loading spinner, and spinner module (#43) * fix : high effort plan mode timeout (300 => 360) * refactor : print information message before run cursor sandbox * feat : add spinner in plan mode * refactor : spinner moduleration * [PF-26] Fixing planforge doctor errors on Mac (#44) * fix : planforge run in mac os * chore : Change the Planforge Doctor TC1 question to one that a real user would be likely to ask. * chore : improve CI Build error * fix : python error * chore : improve CI GuardRail * chore : CRLF => LF * fix : add codex error process (Authentication/Usage/Execution Failure) * fix : add codex error process (Authentication/Usage/Execution Failure)(python) * [PF-15] Doctor first screen simplified; doctor ai uses planforge.json and planforge model flow (#45) * feat : sync doctor ai config model * fix : improve plan mode rule * refactor : integration model selector and doctor ai * refactor : remove no use file * chore : PR writing rule change lang(ko=>en) * fix : build error * release : v0.1.8 (#46)
1 participant
Purpose
Mac에서
planforge doctorAI 모드 실행 시 발생하던 TC2(implement request)·TC3(/p with implementation-style) 실패를 제거하고, Windows와 동일하게 동작하도록 수정합니다.Description
원인
codex exec에 프롬프트를 인자로 넘기면, 프롬프트 내---(workflow 규칙 YAML frontmatter)가 codex CLI에서 별도 인자로 파싱되어unexpected argument '---'발생.수정 내용
runCodexExec는spawnSync(exe, ["exec", "-"], { input: fullPrompt, ... }),runCodexExecStreaming은spawn(exe, ["exec", "-"], { stdio: ["pipe", "pipe", "pipe"] })후child.stdin.write(fullPrompt); child.stdin.end()._run_codex_exec는subprocess.run([exe, "exec", "-"], input=full_prompt, ...),_run_codex_exec_streaming은Popen([exe, "exec", "-"], stdin=PIPE, ...)후proc.stdin.write(full_prompt); proc.stdin.close().tc3SlashPWithImplementationStyleContent: "/p"를 맨 앞 리터럴이 아닌 문맥 서술로 변경 (예: "The user invoked the plan command (/p) with this request: ...").유지 사항: Windows는 기존처럼 임시 파일 + PowerShell 파이프(
codex exec -) 방식 유지. cross-platform / node-python-symmetry 규칙에 맞춰 cli-js·cli-py 동작을 맞춤.How to test
planforge doctor실행.planforge doctor→ ai 실행해 기존처럼 통과하는지 회귀 확인.Review Requirement
codex exec -+ stdin 전달 시, 기존 Windows 전제(임시 파일·PowerShell)와 동작이 일치하는지.Additional Info