-
Notifications
You must be signed in to change notification settings - Fork 98
fix(update-bulletin-job): shell-quote workspace path in cleanup hint #26496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ const EMPTY_HASH = "empty"; | |
|
|
||
| function updateBulletinHint(): string { | ||
| const workspace = getWorkspaceDirDisplay(); | ||
| return `Check ${workspace}/UPDATES.md — new release notes are present. Apply any assistant-facing behavior changes (new tools, deprecations, memory updates). If the user would benefit from knowing about a user-facing change, surface it only when the next topic makes it relevant — do not interrupt them with a proactive message. When you're done processing, delete the file by running \`cd ${workspace} && rm UPDATES.md\` (the bare-filename \`rm UPDATES.md\` is auto-allowed; path-qualified deletes are not). A silent no-op is preferable to low-signal chatter.`; | ||
| return `Check ${workspace}/UPDATES.md — new release notes are present. Apply any assistant-facing behavior changes (new tools, deprecations, memory updates). If the user would benefit from knowing about a user-facing change, surface it only when the next topic makes it relevant — do not interrupt them with a proactive message. When you're done processing, delete the file by running \`cd "${workspace}" && rm UPDATES.md\` (the bare-filename \`rm UPDATES.md\` is auto-allowed; path-qualified deletes are not). A silent no-op is preferable to low-signal chatter.`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| type ReadResult = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Quoting workspace path prevents tilde expansion in shell command hint
The PR wraps
${workspace}in double quotes in the shell command hint (cd "${workspace}" && rm UPDATES.md). However,getWorkspaceDirDisplay()atassistant/src/util/platform.ts:330-337replaces the home directory prefix with~(e.g.~/.vellum/workspace). In bash, tilde expansion does not occur inside double quotes —cd "~/.vellum/workspace"fails with "No such file or directory" because bash treats the literal character~as part of the path rather than expanding it to the home directory. This is the common case for all users whose workspace is under their home directory. The agent receiving this hint would attempt to execute the failingcdcommand and be unable to deleteUPDATES.md, causing the bulletin job to retry on every startup without ever completing.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.