From 559e759b6c32d20e1b45560ec10094bc7ef79c97 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Fri, 19 Jun 2026 09:30:31 +0300 Subject: [PATCH 1/3] =?UTF-8?q?chore:=20update=20repo=20references=20TeoSl?= =?UTF-8?q?ayer/pilotprotocol=20=E2=86=92=20pilot-protocol/pilotprotocol?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae76006..24b86bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: - name: Checkout web4 (sibling protocol repo) uses: actions/checkout@v6 with: - repository: TeoSlayer/pilotprotocol + repository: pilot-protocol/pilotprotocol path: web4 - uses: actions/setup-go@v6 From 374cc141eeda051f9ec0d57399903a198f2c2ca1 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Sun, 21 Jun 2026 19:39:50 +0300 Subject: [PATCH 2/3] fix: mode constant, README API docs, SOUL.md Hermes warning, error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - config.go: GetMode() now falls back to defaultMode (ModeManual) instead of always returning ModeAuto when no config exists; the constant was declared but never used as a fallback. - README.md: replace non-existent Reconcile() in usage example with the real public API: Run, Tick, and ForceTick. - skillinject.go: add slog.Warn at SOUL.md write path noting that gateway-mode Hermes ignores the file (issue #26596). - manifest.go: add TODO comments above DefaultManifestURL and DefaultRepoBaseURL pointing to the future pilot-protocol/pilot-skills repo (values unchanged — repo does not exist yet). - plugin_allowlist.go: classifyPluginAllowList now distinguishes os.IsNotExist (StateDrifted — create the file) from permission-denied and other I/O errors (StateIdentical — skip merge, don't overwrite an unreadable config). Co-Authored-By: Claude Sonnet 4.6 --- README.md | 14 ++++++++++---- config.go | 16 ++++++++-------- manifest.go | 4 ++++ plugin_allowlist.go | 16 ++++++++++------ skillinject.go | 6 ++++++ 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index a8e5e9c..b455ff5 100644 --- a/README.md +++ b/README.md @@ -49,12 +49,18 @@ import "github.com/pilot-protocol/skillinject" ## Usage ```go -// Daemon registration: +// Daemon registration (runs the ticker loop until ctx is cancelled): rt.Register(skillinject.NewService(skillinject.Config{ /* ... */ })) -// Standalone (e.g. from a CLI): -report := skillinject.Reconcile(skillinject.Config{ /* ... */ }) -_ = report +// Blocking loop (first tick fires immediately; subsequent ticks on cfg.Interval +// unless mode is ModeManual, in which case only the startup tick runs): +skillinject.Run(ctx, skillinject.Config{ /* ... */ }) + +// Single scan+reconcile pass (respects ModeDisabled): +report, err := skillinject.Tick(ctx, skillinject.Config{ /* ... */ }) + +// Single scan+reconcile pass, ignoring ModeDisabled (e.g. post-update): +report, err = skillinject.ForceTick(ctx, skillinject.Config{ /* ... */ }) ``` ## Layout diff --git a/config.go b/config.go index 792e950..d8ae9e9 100644 --- a/config.go +++ b/config.go @@ -48,23 +48,23 @@ func configFilePath(home string) string { return filepath.Join(home, ".pilot", configFileName) } -// GetMode returns the current skill_inject mode. Defaults to ModeAuto -// when the flag isn't present, so existing installs keep their current -// live-ticker behaviour. New installs will be set to ModeManual on -// the first call to SetMode (configured by the daemon's startup path). +// GetMode returns the current skill_inject mode. Falls back to +// defaultMode (ModeManual) when the flag is absent or unreadable, +// so fresh installs get the one-shot startup behaviour without a +// periodic ticker until the user explicitly opts in to ModeAuto. func GetMode(home string) string { f, err := os.Open(configFilePath(home)) if err != nil { - return ModeAuto // existing behaviour — live ticker + return defaultMode // no config file — use compiled-in default } defer f.Close() var raw map[string]json.RawMessage if err := json.NewDecoder(f).Decode(&raw); err != nil { - return ModeAuto + return defaultMode } sub, ok := raw[configKey] if !ok { - return ModeAuto + return defaultMode } // Try the new mode-based format first. var modeFlag ModeFlag @@ -83,7 +83,7 @@ func GetMode(home string) string { } return ModeDisabled } - return ModeAuto + return defaultMode } // SetMode persists the skill_inject mode. Reads the existing config (if diff --git a/manifest.go b/manifest.go index d74e6d5..14d687d 100644 --- a/manifest.go +++ b/manifest.go @@ -16,10 +16,14 @@ import ( "time" ) +// TODO: update to pilot-protocol/pilot-skills once that repo is transferred from TeoSlayer. + // DefaultManifestURL is the canonical raw GitHub URL for the inject // manifest. Overridable via Config.ManifestURL (test hook). const DefaultManifestURL = "https://raw.githubusercontent.com/TeoSlayer/pilot-skills/main/inject-manifest.json" +// TODO: update to pilot-protocol/pilot-skills once that repo is transferred from TeoSlayer. + // DefaultRepoBaseURL is the prefix used to fetch any path the manifest // references (skills//SKILL.md, heartbeats/.md). Overridable // via Config.RepoBaseURL. diff --git a/plugin_allowlist.go b/plugin_allowlist.go index 4f33c21..72c3cbf 100644 --- a/plugin_allowlist.go +++ b/plugin_allowlist.go @@ -27,15 +27,19 @@ import ( func classifyPluginAllowList(configPath, allowJsonPath, entriesJsonPath, pluginID string) State { raw, err := os.ReadFile(configPath) if err != nil { - // Don't conflate "config file missing" with "needs writing" — - // if the tool isn't installed at all, the caller's dirExists - // check on rootDir already skipped this whole tool. A missing - // config file at this point means the tool installed but - // hasn't run yet; treat as drifted so the daemon creates it. if os.IsNotExist(err) { + // Config file missing: tool installed but hasn't written its + // config yet (first run). Treat as drifted so the daemon + // creates it. The caller's dirExists check on rootDir already + // skipped tools that aren't installed at all. return StateDrifted } - return StateDrifted + // Permission-denied or other I/O error: we cannot read the file + // and should not attempt to overwrite it. Return StateIdentical + // so the caller skips the merge and surfaces an error on the next + // tick (when the permission situation may have changed) rather + // than silently treating an unreadable config as needing a write. + return StateIdentical } var obj map[string]any if err := json.Unmarshal(raw, &obj); err != nil { diff --git a/skillinject.go b/skillinject.go index dcde805..07f07d8 100644 --- a/skillinject.go +++ b/skillinject.go @@ -274,6 +274,12 @@ func tick(ctx context.Context, cfg Config, force bool) (*Report, error) { } hbPath := expandHome(mt.HeartbeatPath, home) + // NOTE: gateway-mode Hermes ignores SOUL.md (#26596) -- this write + // is a no-op for gateway users. Only local-daemon Hermes instances + // pick up the file. See project_harness_plugin_models.md for context. + if strings.HasSuffix(hbPath, "SOUL.md") { + slog.Warn("skillinject: writing SOUL.md for Hermes; gateway-mode Hermes ignores this file (issue #26596) -- write is a no-op for gateway users", "path", hbPath) + } mState := classifyMarker(hbPath, skillShort) mAction := actionFor(mState) mo := Outcome{ From e7cd8de7c90e2954acff8e198cf2fc1746eae118 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 24 Jun 2026 12:28:18 +0300 Subject: [PATCH 3/3] fix: keep live ticker default and surface unreadable plugin configs Revert two backward-incompatible behaviour changes that broke CI: - config.go: GetMode again defaults to ModeAuto when no skill_inject flag is present (or the config is unreadable/unparseable). The previous change to defaultMode (ModeManual) silently disabled the 15-minute reconcile ticker for every existing install with no flag set, since Run() returns early for manual/disabled mode. New installs still get manual mode via the daemon's explicit SetMode startup path. Fixes TestGetMode_EmptyDefaultsAuto. - plugin_allowlist.go: classifyPluginAllowList returns StateDrifted for any non-IsNotExist read error (permission denied, path is a directory) instead of StateIdentical. StateDrifted routes through mergePluginAllowList, which re-reads and surfaces the failure as an explicit ActionError; StateIdentical silently swallowed it as a no-op. mergePluginAllowList never overwrites an unreadable file, so the config is not at risk. Fixes TestClassifyPluginAllowList_PathIsDirectory. --- config.go | 19 +++++++++++-------- plugin_allowlist.go | 22 ++++++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/config.go b/config.go index d8ae9e9..5ceec14 100644 --- a/config.go +++ b/config.go @@ -48,23 +48,26 @@ func configFilePath(home string) string { return filepath.Join(home, ".pilot", configFileName) } -// GetMode returns the current skill_inject mode. Falls back to -// defaultMode (ModeManual) when the flag is absent or unreadable, -// so fresh installs get the one-shot startup behaviour without a -// periodic ticker until the user explicitly opts in to ModeAuto. +// GetMode returns the current skill_inject mode. Defaults to ModeAuto +// when the flag isn't present (or the config is unreadable/unparseable), +// so existing installs keep their current live-ticker behaviour. New +// installs are set to ModeManual explicitly on the first call to SetMode +// (configured by the daemon's startup path) — we never flip an existing +// install off the live ticker by silently changing the absent-config +// default. func GetMode(home string) string { f, err := os.Open(configFilePath(home)) if err != nil { - return defaultMode // no config file — use compiled-in default + return ModeAuto // existing behaviour — live ticker } defer f.Close() var raw map[string]json.RawMessage if err := json.NewDecoder(f).Decode(&raw); err != nil { - return defaultMode + return ModeAuto } sub, ok := raw[configKey] if !ok { - return defaultMode + return ModeAuto } // Try the new mode-based format first. var modeFlag ModeFlag @@ -83,7 +86,7 @@ func GetMode(home string) string { } return ModeDisabled } - return defaultMode + return ModeAuto } // SetMode persists the skill_inject mode. Reads the existing config (if diff --git a/plugin_allowlist.go b/plugin_allowlist.go index 72c3cbf..d874918 100644 --- a/plugin_allowlist.go +++ b/plugin_allowlist.go @@ -27,19 +27,21 @@ import ( func classifyPluginAllowList(configPath, allowJsonPath, entriesJsonPath, pluginID string) State { raw, err := os.ReadFile(configPath) if err != nil { + // Don't conflate "config file missing" with "needs writing" — + // if the tool isn't installed at all, the caller's dirExists + // check on rootDir already skipped this whole tool. A missing + // config file at this point means the tool installed but + // hasn't run yet; treat as drifted so the daemon creates it. + // + // For any other read error (permission denied, path is a + // directory) we also return StateDrifted so the caller proceeds + // to mergePluginAllowList, which re-reads the file and surfaces + // the error as an explicit ActionError. Returning StateIdentical + // here would silently swallow an unreadable config as a no-op. if os.IsNotExist(err) { - // Config file missing: tool installed but hasn't written its - // config yet (first run). Treat as drifted so the daemon - // creates it. The caller's dirExists check on rootDir already - // skipped tools that aren't installed at all. return StateDrifted } - // Permission-denied or other I/O error: we cannot read the file - // and should not attempt to overwrite it. Return StateIdentical - // so the caller skips the merge and surfaces an error on the next - // tick (when the permission situation may have changed) rather - // than silently treating an unreadable config as needing a write. - return StateIdentical + return StateDrifted } var obj map[string]any if err := json.Unmarshal(raw, &obj); err != nil {