tick: make the skills-disable opt-out survive ForceTick (pilotctl update / skills check) - #31
Merged
Merged
Conversation
`pilotctl skills disable` set mode=disabled, but ForceTick bypassed the gate entirely (it took a `force` flag whose only effect was to skip the disabled check). ForceTick backs `pilotctl skills check`, `pilotctl update`'s post-update reconcile, and the installer's first-pass — so any of those re-injected skills a user had explicitly turned off. The opt-out did not survive an update. Gate on !dryRun instead of !force: disabled is now a hard opt-out for every write path, while the read-only Plan() (behind `pilotctl skills` status) still previews. The vestigial `force` parameter is removed; ForceTick stays as the immediate-reconcile entry point (enable persists mode=auto before calling in, so re-enable still works). Rewrites the test that pinned the old bypass and adds one for Plan previewing while disabled.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
matthew-pilot
pushed a commit
to pilot-protocol/pilotprotocol
that referenced
this pull request
Jul 19, 2026
Bumps skillinject to the commit merged in pilot-protocol/skillinject#31, so the `pilotctl skills disable` opt-out survives `pilotctl update` / `skills check` / installer re-runs. skillinject is compiled into the daemon + pilotctl, so this bump is what ships that fix in a pilotprotocol release.
Alexgodoroja
added a commit
to pilot-protocol/pilotprotocol
that referenced
this pull request
Jul 20, 2026
…sable opt-out fix (#400) * go.mod: pin skillinject with the disabled-opt-out fix Bumps skillinject to the commit merged in pilot-protocol/skillinject#31, so the `pilotctl skills disable` opt-out survives `pilotctl update` / `skills check` / installer re-runs. skillinject is compiled into the daemon + pilotctl, so this bump is what ships that fix in a pilotprotocol release. * updater: add APP_UPDATE_OPT_OUT to opt out of app auto-updates The pilot-updater keeps installed apps current by periodically running `pilotctl appstore upgrade --all`. Add APP_UPDATE_OPT_OUT=true as the documented way to turn that off — installed apps then stay at the version the operator installed, while the pilot daemon/CLI binaries keep updating. Unset or false (the default) keeps app auto-updates on. Extracts the gate into a testable appAutoUpgradeEnabled() helper following the existing envBool pattern, and keeps PILOT_UPDATER_NO_APP_UPGRADE working as a back-compat alias. Documents the var in the README env-var table and CHANGELOG [1.12.9]. * updater: rename opt-out var to PILOT_APP_UPDATE_OPT_OUT + drop version from changelog Follow the repo's PILOT_-prefixed env-var convention: the app-update opt-out is now PILOT_APP_UPDATE_OPT_OUT (global boolean, default false). PILOT_UPDATER_NO_APP_UPGRADE stays as the back-compat alias. Moves the changelog entry under [Unreleased] so the release version isn't pinned in the PR. --------- Co-authored-by: Alexgodoroja <alex@vulturelabs.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make the
skills disableopt-out surviveForceTickpilotctl skills disablesetsmode=disabledand removes the injected skill,but
ForceTickbypassed the disabled gate entirely — it carried aforceflagwhose only effect was to skip the
GetMode() == ModeDisabledcheck.ForceTickis the entry point for
pilotctl skills check,pilotctl update's post-updatereconcile, and the installer's first-pass, so any of those re-injected skills a
user had explicitly turned off. The opt-out did not survive an update.
Fix
Gate on
!dryRuninstead of!force.disabledis now a hard opt-out for everywrite path (
TickandForceTick), while the read-onlyPlan()behindpilotctl skillsstatus still previews what a re-enable would do. The vestigialforceparameter is removed;ForceTickremains the immediate-reconcile entrypoint (it's equivalent to
Ticknow — manual mode was never gated here, andskills enablepersistsmode=autobefore reconciling, so re-enable still runs).Behavior
skills check/update/ installer re-run, disabledskills enable allpilotctl skillsstatus (Plan), disabledTests
TestForceTick_SkipsDisabledGate→TestForceTick_HonorsDisabledGate(it had pinned the bypass): a disabled config now short-circuits with no fetch
and a
Disabledreport.TestPlan_PreviewsEvenWhenDisabled— the gate is write-only, not astatus blackout.
End-to-end (container, patched pilotctl over a real install)
install →
disable all(removed,mode:disabled) →skills checkstaysremoved (released v1.12.8 re-injects here) → daemon restart stays removed →
enable allrestores.Downstream: needs a skillinject release + a core
go.modbump to ship in apilotprotocol patch (1.12.9).