mirror of
https://bitbucket.org/siakitem/my-pi.git
synced 2026-08-28 08:35:57 +00:00
37 lines
2.0 KiB
Markdown
37 lines
2.0 KiB
Markdown
---
|
|
issue: 57
|
|
issue_title: "Replace SessionApprovalCache with session Ruleset"
|
|
---
|
|
|
|
# Retro: #57 — Replace SessionApprovalCache with session Ruleset
|
|
|
|
## Final Retrospective (2026-05-03T21:10:00-04:00)
|
|
|
|
### Session summary
|
|
|
|
Replaced `SessionApprovalCache` (directory-prefix matching via `isPathWithinDirectory()`) with `SessionRules` (a plain `Ruleset` evaluated via `evaluate()` / `wildcardMatch()`).
|
|
Five commits landed matching the five TDD steps in the plan, plus the docs commit.
|
|
Released as v3.10.0 with no user-visible behavior change.
|
|
This unblocks #51 (generalize session approvals to all permission surfaces).
|
|
|
|
### Observations
|
|
|
|
#### What went well
|
|
|
|
- The plan-to-implementation mapping was 1:1 — every TDD step produced exactly one commit with the suggested message.
|
|
- The wildcard semantics concern (sibling directory false positive) was validated immediately by the `session-rules.test.ts` integration tests.
|
|
`wildcardMatch("/other/project/*", "/other/project-b/foo.ts")` correctly returns false because the regex is anchored.
|
|
- The `evaluate()` integration approach (checking `sessionRuleset.includes(sessionMatch)` to distinguish a real match from a synthetic default) was clean and required no special-casing.
|
|
|
|
#### What caused friction (agent side)
|
|
|
|
1. `missing-context` — The plan identified `tests/handlers/lifecycle.test.ts` and `tests/handlers/tool-call.test.ts` as the test files needing mock updates, but missed `tests/handlers/before-agent-start.test.ts` and `tests/handlers/input.test.ts`, which also construct `makeRuntime()` helpers with `sessionApprovalCache`.
|
|
Vitest (esbuild) does not typecheck, so the stale mocks compiled and ran without error.
|
|
The mismatch was only caught by `npm run build` (`tsc`) during the final verification step.
|
|
Impact: required amending the "remove SessionApprovalCache" commit to include two additional test file updates.
|
|
Self-identified during the `tsc` step.
|
|
|
|
#### What caused friction (user side)
|
|
|
|
- No friction observed — the session required no user intervention beyond the standard autoformat hooks.
|