Add safe embedded manage-tink refresh - #59
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: 9678fe1a57
ℹ️ 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".
| } | ||
|
|
||
| pub(crate) fn is_current(installed: &Skill) -> Result<bool, Error> { | ||
| let (_staging, embedded) = prepare_manage_tink()?; |
There was a problem hiding this comment.
Whenever a receipt-free manage-tink is installed, tink skill check reaches this call through require_current, and prepare_manage_tink creates a temporary directory and writes three files into it. This violates the C4 contract that skill check performs no filesystem writes and also makes an otherwise read-only check fail when the project is readable but the temporary filesystem is unavailable or unwritable; compare against an in-memory representation or precomputed digest instead.
Useful? React with 👍 / 👎.
| output::stdout_line(format_args!( | ||
| "{}", | ||
| style.muted("Next: run `tink skill refresh manage-tink` in each Tink project") | ||
| ))?; |
There was a problem hiding this comment.
When an output consumer accepts the first two update-report lines and then closes stdout before this new Next: line, the binary has already been replaced but the propagated write error makes tink update exit nonzero. That reports a successful irreversible mutation as a failure and creates retry ambiguity solely because an advisory could not be delivered; this final guidance should use best-effort output that cannot retroactively fail the update.
Useful? React with 👍 / 👎.
| if skill.name == "manage-tink" && provenance.is_none() { | ||
| crate::manage_tink::require_current(&skill)?; |
There was a problem hiding this comment.
After upgrading the binary while a project still has the previous receipt-free manage-tink, putting this validation in load_project_skills blocks every caller of the generic loader, not just the C8 check/lock boundaries. In particular, project listing, verification, synchronization, and refreshing an unrelated imported skill all fail on manage-tink drift before performing their requested operation; keep the currentness gate in the check and lock command paths so unrelated project operations remain usable while the explicit repair is pending.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
Merge and release remain separately approval-gated.