fix(workspace): harden omo migration review issues
This commit is contained in:
@@ -34,7 +34,7 @@ interface BoulderState {
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `types.ts` | `BoulderState`, `BoulderWorkState`, `TaskSessionState`, status enums |
|
||||
| `storage.ts` | Atomic CRUD on `.omo/boulder-state.json`. Writes via temp file + rename; file lock per work_id |
|
||||
| `storage.ts` | Atomic CRUD on `.omo/boulder.json`. Writes via temp file + rename; file lock per work_id |
|
||||
| `constants.ts` | Path resolution + schema version constant |
|
||||
| `top-level-task.ts` | Helpers to identify the current top-level plan task and resolve its reusable subagent session |
|
||||
| `format-duration.ts` | `formatDurationHuman(ms)` — "1h 23m 5s" formatting for boulder duration |
|
||||
@@ -67,7 +67,7 @@ session.completed
|
||||
## STORAGE
|
||||
|
||||
```
|
||||
<worktree-root>/.omo/boulder-state.json # gitignored; one file per worktree
|
||||
<worktree-root>/.omo/boulder.json # gitignored; one file per worktree
|
||||
```
|
||||
|
||||
Atomic writes: temp file → fsync (where supported) → rename. File lock prevents concurrent corruption. Schema migrations between versions handled inline in `storage.ts`.
|
||||
|
||||
@@ -288,6 +288,42 @@ describe("prometheus-md-only", () => {
|
||||
).rejects.toThrow("File operations restricted to .omo/*.md plan files only")
|
||||
})
|
||||
|
||||
test("should block Prometheus from writing .md files when .omo is only part of a path segment", async () => {
|
||||
// given
|
||||
const hook = createPrometheusMdOnlyHook(createMockPluginInput())
|
||||
const input = {
|
||||
tool: "Write",
|
||||
sessionID: TEST_SESSION_ID,
|
||||
callID: "call-1",
|
||||
}
|
||||
const output = {
|
||||
args: { filePath: "/tmp/test/work.omo/plans/work-plan.md" },
|
||||
}
|
||||
|
||||
// when / #then
|
||||
await expect(
|
||||
hook["tool.execute.before"](input, output)
|
||||
).rejects.toThrow("File operations restricted to .omo/*.md plan files only")
|
||||
})
|
||||
|
||||
test("should block Prometheus from writing .md files under .omo-backup", async () => {
|
||||
// given
|
||||
const hook = createPrometheusMdOnlyHook(createMockPluginInput())
|
||||
const input = {
|
||||
tool: "Write",
|
||||
sessionID: TEST_SESSION_ID,
|
||||
callID: "call-1",
|
||||
}
|
||||
const output = {
|
||||
args: { filePath: "/tmp/test/.omo-backup/plans/work-plan.md" },
|
||||
}
|
||||
|
||||
// when / #then
|
||||
await expect(
|
||||
hook["tool.execute.before"](input, output)
|
||||
).rejects.toThrow("File operations restricted to .omo/*.md plan files only")
|
||||
})
|
||||
|
||||
test("should block Edit tool for non-.md files", async () => {
|
||||
// given
|
||||
const hook = createPrometheusMdOnlyHook(createMockPluginInput())
|
||||
|
||||
@@ -23,9 +23,7 @@ export function isAllowedFile(filePath: string, workspaceRoot: string): boolean
|
||||
return false
|
||||
}
|
||||
|
||||
// 4. Check if .omo/ or .omo\ exists anywhere in the path (case-insensitive)
|
||||
// This handles both direct paths (.omo/x.md) and nested paths (project/.omo/x.md)
|
||||
if (!/\.omo[/\\]/i.test(rel)) {
|
||||
if (!/(^|[/\\])\.omo([/\\]|$)/i.test(rel)) {
|
||||
return false
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import type { GitFileStat } from "./types"
|
||||
|
||||
function normalizePath(path: string): string {
|
||||
return path.replaceAll("\\", "/")
|
||||
}
|
||||
|
||||
export function formatFileChanges(stats: GitFileStat[], notepadPath?: string): string {
|
||||
if (stats.length === 0) return "[FILE CHANGES SUMMARY]\nNo file changes detected.\n"
|
||||
|
||||
@@ -34,7 +38,11 @@ export function formatFileChanges(stats: GitFileStat[], notepadPath?: string): s
|
||||
}
|
||||
|
||||
if (notepadPath) {
|
||||
const notepadStat = stats.find((s) => s.path.includes("notepad") || s.path.includes(".omo"))
|
||||
const normalizedNotepadPath = normalizePath(notepadPath)
|
||||
const notepadStat = stats.find((s) => {
|
||||
const normalizedPath = normalizePath(s.path)
|
||||
return normalizedPath === normalizedNotepadPath || normalizedPath.includes(".omo/notepads/")
|
||||
})
|
||||
if (notepadStat) {
|
||||
lines.push("[NOTEPAD UPDATED]")
|
||||
lines.push(` ${notepadStat.path} (+${notepadStat.added})`)
|
||||
|
||||
@@ -48,4 +48,21 @@ describe("git-worktree", () => {
|
||||
expect(summary).toContain("src/b.ts")
|
||||
expect(summary).toContain("src/c.ts")
|
||||
})
|
||||
|
||||
test("#given notepad path #when formatting omo plan changes #then does not report notepad updated", () => {
|
||||
const summary = formatFileChanges([
|
||||
{ path: ".omo/plans/work.md", added: 1, removed: 0, status: "modified" },
|
||||
], ".omo/notepads/work/notes.md")
|
||||
|
||||
expect(summary).not.toContain("[NOTEPAD UPDATED]")
|
||||
})
|
||||
|
||||
test("#given notepad path #when formatting omo notepad changes #then reports notepad updated", () => {
|
||||
const summary = formatFileChanges([
|
||||
{ path: ".omo/notepads/work/notes.md", added: 1, removed: 0, status: "modified" },
|
||||
], ".omo/notepads/work/notes.md")
|
||||
|
||||
expect(summary).toContain("[NOTEPAD UPDATED]")
|
||||
expect(summary).toContain(".omo/notepads/work/notes.md")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, test } from "bun:test"
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { migrateLegacyWorkspaceDirectory } from "./legacy-workspace-migration"
|
||||
@@ -72,6 +72,23 @@ describe("migrateLegacyWorkspaceDirectory", () => {
|
||||
expect(readFileSync(targetNotepadPath, "utf-8")).toBe("existing note")
|
||||
})
|
||||
|
||||
test("#given legacy workspace contains symlinks #when migrating #then skips symlinks without copying target contents", () => {
|
||||
// given
|
||||
const externalFilePath = join(testDirectory, "external-secret.md")
|
||||
const legacyLinkPath = join(testDirectory, ".sisyphus", "plans", "linked.md")
|
||||
mkdirSync(join(testDirectory, ".sisyphus", "plans"), { recursive: true })
|
||||
writeFileSync(externalFilePath, "secret", "utf-8")
|
||||
symlinkSync(externalFilePath, legacyLinkPath)
|
||||
|
||||
// when
|
||||
const result = migrateLegacyWorkspaceDirectory(testDirectory)
|
||||
|
||||
// then
|
||||
expect(result.migrated).toBe(false)
|
||||
expect(result.skipped).toContain(join(".omo", "plans", "linked.md"))
|
||||
expect(existsSync(join(testDirectory, ".omo", "plans", "linked.md"))).toBe(false)
|
||||
})
|
||||
|
||||
test("#given no legacy workspace #when migrating #then reports no migration", () => {
|
||||
// when
|
||||
const result = migrateLegacyWorkspaceDirectory(testDirectory)
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { copyFileSync, existsSync, mkdirSync, readdirSync, statSync } from "node:fs"
|
||||
import { copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync } from "node:fs"
|
||||
import { dirname, join, relative } from "node:path"
|
||||
|
||||
import { log } from "./logger"
|
||||
@@ -12,10 +12,15 @@ export type LegacyWorkspaceMigrationResult = {
|
||||
}
|
||||
|
||||
function copyMissingEntries(legacyPath: string, targetPath: string, targetRoot: string, skipped: string[]): boolean {
|
||||
const legacyStat = statSync(legacyPath)
|
||||
const legacyStat = lstatSync(legacyPath)
|
||||
|
||||
if (legacyStat.isSymbolicLink()) {
|
||||
skipped.push(join(WORKSPACE_DIR, relative(targetRoot, targetPath)))
|
||||
return false
|
||||
}
|
||||
|
||||
if (existsSync(targetPath)) {
|
||||
if (legacyStat.isDirectory() && statSync(targetPath).isDirectory()) {
|
||||
if (legacyStat.isDirectory() && lstatSync(targetPath).isDirectory()) {
|
||||
let copiedChild = false
|
||||
for (const entry of readdirSync(legacyPath)) {
|
||||
copiedChild = copyMissingEntries(join(legacyPath, entry), join(targetPath, entry), targetRoot, skipped) || copiedChild
|
||||
|
||||
Reference in New Issue
Block a user