fix(omp): review follow-ups — OSC tails, argument range, cancelled generation, cache freshness, docs

Author: pedro

README.md

@@ -19,8 +19,8 @@ We wanted something performant, remote-ready, and truly open. If we ever go the
 > - Claude: install [Claude Code](https://claude.com/product/claude-code) and run `claude auth login`
 > - Cursor: install [Cursor CLI](https://cursor.com/cli) and run `agent login`
 > - Grok Build: install [Grok Build CLI](https://x.ai/cli) and run `grok login`
+> - Oh My Pi (omp): install [Oh My Pi](https://github.com/can1357/oh-my-pi) and run `omp setup`
 > - OpenCode: install [OpenCode](https://opencode.ai) and run `opencode auth login`
-> - Oh My Pi (omp): install [Oh My Pi](https://github.com/can1357/oh-my-pi) and run `omp`
 > - Antigravity: enable it in Settings, then use **Install Antigravity** and **Sign in with Google**. No CLI is required.
 
 ### Try it out (install-free)

apps/server/src/provider/Drivers/OmpDriver.test.ts

@@ -350,6 +350,50 @@ it.layer(testLayer)("OmpDriver", (it) => {
     }).pipe(Effect.scoped),
   );
 
+  // A cache hit re-records the cwd so the LRU keeps it, which must not also
+  // restart its freshness window: a polled cwd would then never re-probe and
+  // a skill installed out of band would stay invisible for the session.
+  it.effect("re-probes after the freshness window even while the cwd is polled", () =>
+    Effect.gen(function* () {
+      const fs = yield* FileSystem.FileSystem;
+      const path = yield* Path.Path;
+      const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-omp-driver-stale-" });
+      const probeLogPath = path.join(root, "probes.log");
+      const fakePath = yield* makeFakeOmp({
+        prefix: "t3-omp-driver-stale-bin-",
+        checkOutput: "Current version: 18.1.18\n",
+        probeLogPath,
+      });
+      const instance = yield* createTestInstance("omp-catalog-stale", {
+        binaryPath: fakePath,
+        enabled: true,
+      });
+      const snapshotForCwd = instance.snapshotForCwd;
+      if (!snapshotForCwd)
+        return yield* Effect.die("OmpDriver does not expose workspace snapshots.");
+      const workspace = yield* fs.makeTempDirectoryScoped({ prefix: "t3-omp-stale-ws-" });
+
+      const realNow = Date.now;
+      let clockOffsetMillis = 0;
+      const nowSpy = vi.spyOn(Date, "now").mockImplementation(() => realNow() + clockOffsetMillis);
+      try {
+        yield* snapshotForCwd(workspace);
+        // Polls inside the window are served from cache.
+        clockOffsetMillis = 20_000;
+        yield* snapshotForCwd(workspace);
+        clockOffsetMillis = 29_000;
+        yield* snapshotForCwd(workspace);
+        expect(yield* readProbeCount(probeLogPath, workspace)).toBe(1);
+
+        clockOffsetMillis = 31_000;
+        yield* snapshotForCwd(workspace);
+        expect(yield* readProbeCount(probeLogPath, workspace)).toBe(2);
+      } finally {
+        nowSpy.mockRestore();
+      }
+    }).pipe(Effect.scoped),
+  );
+
   it.effect("applies a live available_commands_update without a second probe", () =>
     Effect.gen(function* () {
       const fs = yield* FileSystem.FileSystem;

apps/server/src/provider/Drivers/OmpDriver.ts

@@ -180,10 +180,16 @@ export const OmpDriver: ProviderDriver<OmpSettings, OmpDriverEnv> = {
         workspaceCwd: string,
         catalog: OmpCommandCatalog,
         checkedAt: string,
+        probedAtMillis?: number,
       ): void => {
         skillNamesByCwd.set(workspaceCwd, new Set(catalog.skills.map((skill) => skill.name)));
+        // The stamp is the probe's own time: re-recording a cached catalog
+        // keeps the cwd hot in the LRU without extending its freshness
+        // window, or a cwd polled faster than the window would never
+        // re-probe and an out-of-band skill install would stay invisible.
         // @effect-diagnostics-next-line globalDate:off - cache stamp shares Date.now with the freshness read below; Effect Clock is unavailable in the sync callback path.
-        catalogCacheByCwd.set(workspaceCwd, { catalog, cachedAt: Date.now() });
+        const cachedAt = probedAtMillis ?? Date.now();
+        catalogCacheByCwd.set(workspaceCwd, { catalog, cachedAt });
         retainedWorkspaceSnapshots = appendOmpWorkspaceSnapshot(retainedWorkspaceSnapshots, {
           cwd: workspaceCwd,
           checkedAt,
@@ -264,8 +270,14 @@ export const OmpDriver: ProviderDriver<OmpSettings, OmpDriverEnv> = {
             Effect.flatMap((machineSnapshot) =>
               Effect.map(DateTime.now, (now) => {
                 // Re-record so a revisited cwd moves last and an evicted one
-                // comes back; the timestamp slides while the cwd stays hot.
-                rememberCatalog(workspaceCwd, cached.catalog, DateTime.formatIso(now));
+                // comes back; the probe's own timestamp carries over so the
+                // freshness window still expires on schedule.
+                rememberCatalog(
+                  workspaceCwd,
+                  cached.catalog,
+                  DateTime.formatIso(now),
+                  cached.cachedAt,
+                );
                 return {
                   ...machineSnapshot,
                   skills: cached.catalog.skills,

apps/server/src/provider/Drivers/OmpMaintenance.ts

@@ -2,13 +2,14 @@
  * OmpMaintenance — update capabilities and workspace-snapshot helpers for the
  * Oh My Pi (`omp`) driver.
  *
- * omp is its own updater: it ships outside npm/homebrew, so no package
- * manager owns its install. `omp update --check` prints the installed
- * version (`Current version: X`) and, when behind, the version it would
- * install (`New version available: Y`); `omp update` performs the install.
- * The maintenance resolver therefore advertises the resolved `omp` binary
- * itself as the updater and bakes the `--check` output into `latestVersion`,
- * instead of guessing a registry the way npm/homebrew-backed drivers do.
+ * omp is its own updater: `omp update` detects how this copy was installed
+ * (Homebrew, mise, Bun, npm, or a direct binary) and delegates to it, so no
+ * single registry describes the install. `omp update --check` prints the
+ * installed version (`Current version: X`) and, when behind, the version it
+ * would install (`New version available: Y`). The maintenance resolver
+ * therefore advertises the resolved `omp` binary itself as the updater and
+ * bakes the `--check` output into `latestVersion`, instead of guessing a
+ * registry the way npm/homebrew-backed drivers do.
  *
  * @module provider/Drivers/OmpMaintenance
  */

apps/server/src/provider/acp/OmpAnsi.test.ts

@@ -40,6 +40,20 @@ describe("makeAnsiFilter", () => {
     expect(filter.flush()).toBe("");
   });
 
+  it("keeps text printed after a terminated hyperlink in the same chunk", () => {
+    const filter = makeAnsiFilter();
+
+    expect(filter.push("\u001B]8;;http://x\u0007label after")).toBe("label after");
+    expect(filter.flush()).toBe("");
+  });
+
+  it("still holds an unterminated hyperlink until it closes", () => {
+    const filter = makeAnsiFilter();
+
+    expect(filter.push("start \u001B]8;;http://x")).toBe("start ");
+    expect(filter.push("\u0007label")).toBe("label");
+  });
+
   it("discards a partial escape that the stream never completed", () => {
     const filter = makeAnsiFilter();
 

apps/server/src/provider/acp/OmpAnsi.ts

@@ -21,10 +21,15 @@ const ANSI_PATTERN =
   // eslint-disable-next-line no-control-regex
   /\u001B\[[0-9;:?]*[ -/]*[@-~]|\u001B\][\s\S]*?(?:\u0007|\u001B\\)|\u001B[ -/]+[0-~]|\u001B[@-Z\\-_]/g;
 
-/** A tail that could still become a complete sequence once more text arrives. */
+/**
+ * A tail that could still become a complete sequence once more text arrives.
+ * The OSC branch requires the sequence to be unterminated: a `ESC ]…BEL`
+ * that already closed is a complete escape, and treating it as a tail would
+ * withhold every character printed after it.
+ */
 const PARTIAL_ANSI_TAIL_PATTERN =
   // eslint-disable-next-line no-control-regex
-  /\u001B(?:\[[0-9;:?]*[ -/]*|\][\s\S]*)?$/;
+  /\u001B(?:\[[0-9;:?]*[ -/]*|\](?:(?!\u0007|\u001B\\)[\s\S])*)?$/;
 
 /** Remove every terminal escape sequence from a complete string. */
 export function stripAnsi(text: string): string {

apps/server/src/textGeneration/OmpTextGeneration.ts

@@ -133,14 +133,20 @@ export const makeOmpTextGeneration = Effect.fn("makeOmpTextGeneration")(function
         ),
       );
 
+      // A cancelled turn is a failure even when omp already streamed
+      // parseable output: the text is a fragment of an answer nobody
+      // finished, not a result.
+      if (promptResult.stopReason === "cancelled") {
+        return yield* new TextGenerationError({
+          operation,
+          detail: "Oh My Pi ACP request was cancelled.",
+        });
+      }
       const rawResult = (yield* Ref.get(outputRef)).trim();
       if (!rawResult) {
         return yield* new TextGenerationError({
           operation,
-          detail:
-            promptResult.stopReason === "cancelled"
-              ? "Oh My Pi ACP request was cancelled."
-              : "Oh My Pi returned empty output.",
+          detail: "Oh My Pi returned empty output.",
         });
       }
 

apps/web/src/components/ComposerPromptEditor.test.ts

@@ -873,9 +873,7 @@ describe("isOpenableSkillPath", () => {
   });
 
   it("refuses internal URLs and blank paths", () => {
-    expect(
-      isOpenableSkillPath("C:\Storage\AI\Agents\.omp\curated-skills\matt\matt-tdd\SKILL.md"),
-    ).toBe(false);
+    expect(isOpenableSkillPath("skill://tdd/SKILL.md")).toBe(false);
     expect(isOpenableSkillPath("https://example.com/SKILL.md")).toBe(false);
     expect(isOpenableSkillPath("   ")).toBe(false);
   });

apps/web/src/composer-logic.test.ts

@@ -167,6 +167,18 @@ describe("detectComposerTrigger", () => {
     });
   });
 
+  it("covers the whole argument token when the caret sits inside it", () => {
+    const text = "/compact remx";
+
+    expect(detectComposerTrigger(text, "/compact rem".length)).toEqual({
+      kind: "slash-argument",
+      command: "compact",
+      query: "rem",
+      rangeStart: "/compact ".length,
+      rangeEnd: text.length,
+    });
+  });
+
   it("stops triggering once a second argument word is typed", () => {
     const text = "/compact remote focus here";
 

apps/web/src/composer-logic.ts

@@ -235,12 +235,18 @@ export function detectComposerTrigger(text: string, cursorInput: number): Compos
     const argumentMatch = /^\/(\S+)[ \t]+(\S*)$/.exec(linePrefix);
     if (argumentMatch) {
       const query = argumentMatch[2] ?? "";
+      // The caret may sit inside the argument (`/compact rem|x`). Selecting a
+      // choice replaces the whole token, not just the part before the caret,
+      // or the leftover would trail the inserted value.
+      const lineEnd = text.indexOf("\n", cursor);
+      const restOfLine = text.slice(cursor, lineEnd === -1 ? text.length : lineEnd);
+      const tokenRest = /^\S*/.exec(restOfLine)?.[0] ?? "";
       return {
         kind: "slash-argument",
         command: argumentMatch[1] ?? "",
         query,
         rangeStart: cursor - query.length,
-        rangeEnd: cursor,
+        rangeEnd: cursor + tokenRest.length,
       };
     }
   }