|
1 | 1 | ## Session Plan |
2 | 2 |
|
3 | | -Session Title: General self-improvement |
| 3 | +Session Title: Persist safety settings to config file |
4 | 4 |
|
5 | | -### Task 1: Self-assessment and improvement |
6 | | -Files: cmd/iterate/, internal/evolution/ |
7 | | -Description: Read the source code, find one thing to improve (a bug, missing test, or UX gap), implement it, test it, and commit it. |
8 | | -Issue: none |
| 5 | +### Task 1: Add config persistence to safety commands |
| 6 | +Files: |
| 7 | +- internal/commands/safety.go (modify cmdSafe, cmdTrust, cmdAllow, cmdDeny) |
| 8 | +- internal/commands/safety_test.go (add tests for persistence) |
| 9 | + |
| 10 | +Description: |
| 11 | +The `/safe`, `/trust`, `/allow`, and `/deny` commands currently only modify in-memory state. They need to persist changes to the config file so settings survive REPL restarts. |
| 12 | + |
| 13 | +The `/notify` command in `internal/commands/config.go` (lines 206-221) already demonstrates the correct pattern: |
| 14 | +1. Check if `ctx.Config.LoadConfig` and `ctx.Config.SaveConfig` are available |
| 15 | +2. Load the current config using `ctx.Config.LoadConfig()` |
| 16 | +3. Modify the appropriate field |
| 17 | +4. Save with `ctx.Config.SaveConfig(cfg)` |
| 18 | + |
| 19 | +For safety commands: |
| 20 | +- `cmdSafe`: Set `cfg.SafeMode = true`, save to `DeniedTools` if needed |
| 21 | +- `cmdTrust`: Set `cfg.SafeMode = false` |
| 22 | +- `cmdAllow`: Remove tool from `cfg.DeniedTools` slice |
| 23 | +- `cmdDeny`: Add tool to `cfg.DeniedTools` slice |
| 24 | + |
| 25 | +The config struct in `cmd/iterate/config.go` already has `SafeMode bool` and `DeniedTools []string` fields. |
| 26 | + |
| 27 | +Implementation details: |
| 28 | +- Type assert the loaded config to `iterConfig` (or use the concrete type if accessible) |
| 29 | +- Handle case where LoadConfig/SaveConfig callbacks are nil (skip persistence, don't error) |
| 30 | +- For DeniedTools, avoid duplicates when adding, properly remove when allowing |
| 31 | +- Keep existing in-memory updates (they affect immediate behavior) |
| 32 | + |
| 33 | +Testing: |
| 34 | +- Add tests that verify Config.LoadConfig/SaveConfig are called with correct values |
| 35 | +- Test that persistence is skipped gracefully when callbacks are nil |
| 36 | +- Test duplicate prevention in DeniedTools |
9 | 37 |
|
10 | 38 | ### Issue Responses |
| 39 | + |
| 40 | +- none: This is a self-identified improvement from codebase TODOs (lines 54, 63, 77, 91 in internal/commands/safety.go) |
| 41 | + |
| 42 | +### Success Criteria |
| 43 | + |
| 44 | +- [ ] `/safe` persists SafeMode=true to config |
| 45 | +- [ ] `/trust` persists SafeMode=false to config |
| 46 | +- [ ] `/deny <tool>` persists the denied tool to config |
| 47 | +- [ ] `/allow <tool>` removes the tool from denied list in config |
| 48 | +- [ ] All existing tests still pass |
| 49 | +- [ ] New tests cover persistence behavior |
| 50 | +- [ ] The four TODO comments are removed |
0 commit comments