feat: upgrade ink for performance, and use stdio not http#8004
Conversation
michaelneale
changed the title
![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: 89eec86cc1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "ink": "^6.8.0", | ||
| "ink-text-input": "^6.0.0", | ||
| "marked": "^15.0.12", | ||
| "marked-terminal": "^7.3.0", | ||
| "meow": "^13.2.0", | ||
| "react": "^18.3.1" | ||
| "react": "^19.2.4" |
There was a problem hiding this comment.
These version bumps were only recorded in the workspace lockfile; ui/text/package-lock.json:15-31 still pins ink@^5.1.0, react@^18.3.1, and does not include the new marked packages. That leaves the package's documented cd ui/text && npm install path on a different dependency set, and npm ci in ui/text will now fail because npm expects package.json and package-lock.json to stay in sync (see the npm install/ci docs: https://docs.npmjs.com/cli/v11/commands/npm-install/).
Useful? React with 👍 / 👎.
worth thinking through what the limitations might be for the http version in this area, but I agree using stdio for tui makes sense |
| ] | ||
| ], | ||
| "overrides": { | ||
| "react": "19.2.4" |
There was a problem hiding this comment.
why do we need this? Ink not depending on a version we like?
could this be in ui/text/package.json instead?
|
architecturally stdio makes sense. I think this began life as a PoC for the http transport |
|
@jamadeo or should we stick with http as GUI will use it, and keeps thing ready for having one daemon running ? (I can patch the http one so if launches free port and secret so it works like it is one biinary for now?) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bc2dd316d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3260018ec7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
No big deal either way what transport the TUI uses for now. If there isn't a performance issue with using HTTP I think it's good to use it as a reference implementation, but not required. I do think it would be good if So this PR looks good to go to me |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64f7038434
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c93a7a0efd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| serverProcess = spawn(binary, ["acp"], { | ||
| stdio: ["pipe", "pipe", "ignore"], | ||
| detached: false, | ||
| }); | ||
|
|
||
| try { | ||
| await waitForServer(DEFAULT_URL); | ||
| } catch (err) { | ||
| console.error((err as Error).message); | ||
| serverProcess.kill(); | ||
| process.exit(1); | ||
| } | ||
| serverProcess.on("error", (err) => { | ||
| console.error(`Failed to start goose acp: ${err.message}`); | ||
| process.exit(1); |
There was a problem hiding this comment.
If the bundled goose binary rejects acp or aborts during startup, Node will emit an exit/close event rather than error. This path only listens for error and also discards stderr, so the TUI still renders and then fails later with an opaque transport error instead of surfacing the real startup problem. The old HTTP path waited for readiness before entering the UI; the stdio path needs an equivalent exit/readiness check.
Useful? React with 👍 / 👎.
…#8004) - CLI parts only Cherry-picked leaf/leaf-acp related changes: - Add /status route to ACP transport
…#8004) - CLI parts only Cherry-picked leaf/leaf-acp related changes: - Add /status route to ACP transport
…#8004) Signed-off-by: esnyder <elijah.snyder1@gmail.com>
…#8004) Signed-off-by: esnyder <elijah.snyder1@gmail.com>
3 participants
Don't use http, use studio as better, more secure (and means can have many running)