add agent host with skills and code generation and run ability - #181
add agent host with skills and code generation and run ability#181zhongkaifu wants to merge 5 commits into
Conversation
![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: 726388bd4b
ℹ️ 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".
| continue; | ||
| if (existing.Exists && existing.LastWriteTimeUtc >= source.LastWriteTimeUtc) | ||
| continue; | ||
| File.Copy(input.SourcePath, destination, overwrite: true); |
There was a problem hiding this comment.
When code execution uses a persistent workspace, an earlier sandboxed command can replace an attachment name with a symlink to a host path; the next tool call stages the attachment with this unsandboxed File.Copy, which follows the destination symlink and can create or overwrite any path writable by the server account. The lexical IsUnder check does not detect this, so staging must use the workspace's component-by-component symlink validation immediately before writing.
Useful? React with 👍 / 👎.
| TryDeleteDirectory(skill.RootDirectory); | ||
| _index = Build(); | ||
| _logger.LogInformation(LogEventIds.SkillRemoved, "skills.removed id={SkillId}", skill.Id); | ||
| return true; |
There was a problem hiding this comment.
If deletion fails—for example because a file is locked on Windows or permissions changed—TryDeleteDirectory swallows the exception, Build() re-discovers the skill, and this method still returns true; consequently DELETE /api/skills/{name} responds that the skill was removed even though it remains installed. Make deletion failure observable, or verify that the directory and rebuilt index no longer contain the skill before returning success.
Useful? React with 👍 / 👎.
| catch (ArgumentException ex) | ||
| { | ||
| _log.LogError(LogEventIds.CliFailed, "cli.skills.invalid {Error}", ex.Message); | ||
| Console.Error.WriteLine(ex.Message); | ||
| return; |
There was a problem hiding this comment.
When skill parsing or root validation throws—for example for an invalid --skills-max-rounds value or a nonexistent explicitly configured --skills-dir—this catch prints an error and returns without setting Environment.ExitCode, so the CLI exits successfully. Scripts and service wrappers therefore treat the rejected configuration as a successful invocation; set a nonzero exit code before returning, as the neighboring code-exec/configuration error paths do.
Useful? React with 👍 / 👎.
Engine comparison — TensorSharp vs llama.cpp (PR smoke)No report artifact was produced — the benchmark failed before generating results (see the workflow logs). |
1 participant
No description provided.