diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 000000000..13c8be784 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,27 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "uv run python scripts/openspec-gate.py pre-tool-use", + "timeout": 30 + } + ] + } + ], + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "uv run python scripts/openspec-gate.py remind", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/.codex/hooks.json b/.codex/hooks.json new file mode 100644 index 000000000..8956f56e5 --- /dev/null +++ b/.codex/hooks.json @@ -0,0 +1,26 @@ +{ + "hooks": { + "PreToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": "uv run python scripts/openspec-gate.py pre-tool-use", + "timeout": 30 + } + ] + } + ], + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "uv run python scripts/openspec-gate.py remind", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/.kiro/hooks/openspec-gate.json b/.kiro/hooks/openspec-gate.json new file mode 100644 index 000000000..0e95663c3 --- /dev/null +++ b/.kiro/hooks/openspec-gate.json @@ -0,0 +1,22 @@ +{ + "version": "v1", + "hooks": [ + { + "name": "openspec-gate (early warning, not enforcement)", + "trigger": "PreToolUse", + "matcher": "execute_bash", + "action": { + "type": "command", + "command": "uv run python scripts/openspec-gate.py pre-tool-use" + } + }, + { + "name": "openspec-remind", + "trigger": "UserPromptSubmit", + "action": { + "type": "command", + "command": "uv run python scripts/openspec-gate.py remind" + } + } + ] +} diff --git a/CHANGELOG.md b/CHANGELOG.md index f5bfb43cb..d762e99a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,60 @@ experience may change before `1.0.0`. ## [Unreleased] +### Added + +- `studyloop plan record --title [--body … | --body-file …]` + appends a learning record to a plan document, with a matching MCP tool + (`record_plan_learning`) so a mentor can record what was learned at + wind-down. Before this, a learning record existed only if you typed it into + the plan by hand — and an xTiles wind-down's record lived only in xTiles. + The write goes through the plan renderer (never appended as raw text), + numbering continues from the highest existing record, and re-running with + the same title and body changes nothing. The xTiles wind-down now records + into the plan first, so the xTiles page is a projection of a record the plan + already has. ADR-0010 was amended to state the rule the code obeys: + `studyloop plan …` is the plan document's only writer. +- `studyloop brain wind-down --json [--connector NAME]...` answers the one + question the end-of-session protocol needs: which second-brain offer to make, + if any. It returns the channel, whether to offer, the exact sentence to say, + and why — so an agent no longer derives the decision from flags, and the + offer sentence cannot drift between the CLI, the protocol, the skill and the + guide (they are pinned byte-identical by tests). + +### Changed + +- The xTiles guidance now matches what a person actually saw running the three + prompts end to end (Kiro CLI 2.21.0, 2026-09-04). The planner prompt creates a + planner tile and reports its URL — the one shape the live UI check validates. + The project prompt no longer promises a Kanban board or refreshing collection + pages: the connector cannot create board views and refuses to patch collection + pages on any tier. The wind-down prompt now says to skip the Review task when + `get_due_cards` returns nothing due, rather than inventing a date. The guide + also now says that the next action and due reviews sent to xTiles reflect your + whole study history (they are not plan-scoped), that per-write permission + prompts are xTiles' statement about its connector and what you see depends on + your assistant, that the xTiles learning record is not written back into the + plan document, and that planner tiles can be removed through the connector + while pages and projects must be deleted in the xTiles interface by hand. +- `studyloop install agents` no longer links the xTiles wind-down skill into + `~/.config/opencode/skills`: OpenCode already lists the shared skills hub as a + global search path, so the extra link risked a duplicate listing. + +### Fixed + +- The Obsidian publisher closed three review residuals: containment is now + checked before any directory is created (a hostile ancestor symlink can no + longer cause directories outside the vault) and once more immediately before + the atomic replace; `--dry-run` now says "would replace your edits in …" — + with the same warning a real publish prints — instead of a plain "would + write"; and a note whose permissions cannot be read is refused rather than + silently rewritten with default permissions. +- `get_study_history` (the MCP tool) and `studyloop plan evaluate` no longer + fail when searching session history: the full-text query joined two tables + that both carry a `content` column without qualifying it, and OR'd multiple + `MATCH` constraints, which FTS5 refuses. Multi-word topics such as + "window functions" now also match as phrases rather than scattered terms. + ## [0.2.0] - 2026-09-04 ### Added diff --git a/Justfile b/Justfile index d686e7378..844bf8354 100644 --- a/Justfile +++ b/Justfile @@ -148,9 +148,31 @@ smoke-extras: build-release: ./scripts/build-release.sh +# WD-5/WD-6: the live wind-down gate checks, captured through Claude Code +# headless against the LiteLLM gateway (no vendor credential; the key is read +# from the proxy's own config at runtime). Opt-in — burns gateway spend +# (estimate: reviews/2026-09-04-gate-checks/ESTIMATE.md). Writes transcripts +# and the pass/fail summary under reviews/…/evidence/gate-checks/. +gate-checks: + STUDYLOOP_EVIDENCE_DIR={{justfile_directory()}}/reviews/2026-09-04-gate-checks/evidence/gate-checks \ + uv run --group dev pytest packages/studyloop/tests/live/test_wind_down_transcripts.py -m live_provider -q + release-consistency: uv run python scripts/check-release-consistency.py --skip-wheel +# The release-mode superset: everything above PLUS the openspec guards — a +# change with commits since the last tag must be archived or carry a +# `deferred: <reason>`, and archive entries ADDED since the last tag must pass +# `openspec validate` (soft-skipped when the CLI is absent, same convention as +# spec-check; not `--archived --all`, because a July archive predating this +# guard has unticked tasks nobody has evidence to reconcile, and re-failing +# every future release on it would teach people to ignore the gate). +# Deliberately NOT part of preflight: open changes are legal during a cycle; +# only shipping one is not. Both guards would have fired on the 0.2.0 cut +# (2026-09-04 review, Q5). +release-consistency-shipped: + uv run python scripts/check-release-consistency.py --skip-wheel --release + prepare-release version: uv run python scripts/prepare-release.py {{version}} @@ -193,7 +215,7 @@ xtiles-auth: preflight: lint typecheck test test-js docs release-consistency spec-check -release-check: test test-js lint typecheck shellcheck docs audit audit-full release-consistency smoke-installed smoke-extras +release-check: test test-js lint typecheck shellcheck docs audit audit-full release-consistency-shipped smoke-installed smoke-extras # "Would GitHub Actions pass?" locally, before pushing. `check` runs the # host-answerable gates (lint, typecheck, test, sast, audit, docs, ...); `lint` diff --git a/agents/manifest.json b/agents/manifest.json index 256e2898b..3cc7b2ce9 100644 --- a/agents/manifest.json +++ b/agents/manifest.json @@ -3,63 +3,63 @@ "agents": { "claude/socratic-mentor.md": { "hash": "42880ffa80484ec9", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "codex/AGENTS.md": { "hash": "23dc3d4fbdea89c3", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "kiro/study-mentor.json": { "hash": "c9f2302ebc8d3c39", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "opencode/study-mentor.md": { "hash": "0ff72c058de9f7d2", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "pi/AGENTS.md": { "hash": "8706855f619c71b5", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/audhd-framework.md": { "hash": "8b694064b100741f", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/break-science.md": { "hash": "74541a44431f7f6a", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/knowledge-bridging.md": { "hash": "adb4221cda83498f", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/network-bridges.md": { "hash": "8af4732b77cc15ae", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/session-protocol.md": { "hash": "b58178df20cb9008", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/socratic-engine.md": { "hash": "42738fba9479ec14", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/teach-back-protocol.md": { "hash": "9bbe8831f1c74837", - "updated": "2026-09-03" + "updated": "2026-09-04" }, "shared/wind-down-protocol.md": { - "hash": "d9d0dde390e03b4c", - "updated": "2026-09-03" + "hash": "5b1ec3303b8d1086", + "updated": "2026-09-04" }, "skills/studyloop-xtiles-wind-down/SKILL.md": { - "hash": "aa31c59092ffe929", - "updated": "2026-09-03" + "hash": "a2e84febb865f24d", + "updated": "2026-09-04" }, "skills/studyloop-xtiles-wind-down/references/harnesses.md": { - "hash": "e3f0ebc00ee792a8", - "updated": "2026-09-03" + "hash": "d2beeed44e1c0154", + "updated": "2026-09-04" } } } diff --git a/agents/shared/wind-down-protocol.md b/agents/shared/wind-down-protocol.md index 982d45177..aca937c8c 100644 --- a/agents/shared/wind-down-protocol.md +++ b/agents/shared/wind-down-protocol.md @@ -48,14 +48,29 @@ Standard end-of-session tasks from session-protocol.md: 4. Set next review dates via spaced repetition schedule. -5. Offer the second brain — **only if one is configured, and only once**: +5. Offer the second brain — **only if the decision command says to, and only + once**. Run this step silently: never announce, name or narrate it in any + state — the learner hears either the offer sentence or nothing at all. ```bash - studyloop brain status --json + studyloop brain wind-down --json --connector <name> ``` - If **both** `configured` and `supports_publish` are `true`, offer exactly once, - in one sentence: + ```bash + studyloop brain wind-down --json + ``` + + That is the whole command when no MCP tools are visible to you. Add + `--connector <server>` for each MCP server whose tools you can actually + see in this session — MCP tools are named `mcp__<server>__…`, so the + server name is read off the tools themselves, and only `xtiles` affects + the decision. The flag is a statement of fact about this session, not part + of the command's syntax: **naming a connector whose tools you cannot see + fabricates an offer for a service the session cannot reach.** The command + answers with `channel`, `offer`, `sentence` and `reason`. + + If `offer` is `true`, say `sentence` **verbatim, exactly once**. For the + `publish` channel that sentence is: <!-- wind-down-offer --> Want me to publish today's study record and this plan to your Obsidian vault (Study/Today.md and Study/Plans/<plan-id>.md)? Yes or no — I'll only ask once. @@ -67,13 +82,23 @@ Standard end-of-session tasks from session-protocol.md: studyloop brain publish --today --plan <plan-id> ``` - On **no**, or in any other case, **say nothing about second brains at all** and - continue the wind-down. Do not repeat the offer later in the session. - - Both flags are required, not just `configured`. A learner on xTiles *is* - configured but has no programmatic backend (`supports_publish: false`), so - offering the publish command would name something that cannot work — and would - do it at the end of every session. + For the `xtiles` channel, follow the `studyloop-xtiles-wind-down` skill, + which carries its own pinned sentence. + + If `offer` is `false`, or on **no**, **say nothing about second brains at + all** and continue the wind-down. Do not repeat the offer later in the + session. "No second brain is configured, so nothing to offer", "running + the second brain check now", and "the learner declined, so no xTiles + write happens" are all violations, not courtesies — a declined offer is + acknowledged by moving on, never by naming what was declined. + + The command computes two separate rules — not one conjunction. `configured` + plus `supports_publish` (both from `studyloop brain status --json`) selects + the publish offer; provider `xtiles` plus a connected `xtiles` connector + selects the skill's offer. A learner on xTiles *is* configured but has no + programmatic backend (`supports_publish: false`), so the publish sentence + would name something that cannot work — which is why the decision lives in + the command rather than in this prose. ### Phase 2: Consolidation Guidance (spoken if voice mode is active) diff --git a/agents/skills/studyloop-xtiles-wind-down/SKILL.md b/agents/skills/studyloop-xtiles-wind-down/SKILL.md index 77fe0e973..4a33bea6c 100644 --- a/agents/skills/studyloop-xtiles-wind-down/SKILL.md +++ b/agents/skills/studyloop-xtiles-wind-down/SKILL.md @@ -17,35 +17,53 @@ you, not a feature of the CLI. Use only during Phase 1 of `~/.agents/shared/wind-down-protocol.md`, after progress has been recorded with `studyloop progress "<concept>" -t <topic> -c <confidence>`. -## Gate — both halves, checked every session +## Gate — decided by the CLI, checked every session + +Run the decision command, naming each MCP server connected in this session: + +```bash +studyloop brain wind-down --json --connector xtiles +``` + +Offer only when it answers `"channel": "xtiles"` with `"offer": true`. That +happens exactly when both halves of the gate hold: 1. `studyloop brain status --json` reports `provider: xtiles`. Any other provider means this file does not apply: an Obsidian learner has already been offered the publish command in Phase 1, and a learner on `none` has chosen neither. 2. An MCP server named `xtiles` is connected in this session — its tools are - visible to you. If it is not, do nothing and say nothing about xTiles. Do not - suggest they connect one; an offer to set up a service they never asked for is - the thing this gate exists to prevent. + visible to you, and you passed `--connector xtiles` to say so. If it is not, + do nothing and say nothing about xTiles. Do not suggest they connect one; an + offer to set up a service they never asked for is the thing this gate exists + to prevent. -If either half is false, continue the wind-down without mentioning xTiles at all. +If `offer` is false, continue the wind-down without mentioning xTiles at all. ## The offer -Offer once, in one line: +Offer once, in one line — the `sentence` the command returned, verbatim: +<!-- xtiles-wind-down-offer --> > Want me to add today's learning record and the next review to your xTiles project? Yes or no — I'll only ask once. +<!-- /xtiles-wind-down-offer --> -On **yes**, follow prompt P3 from the Second Brain guide: one learning-record page -under the plan's project, one dated planner task for the next review, then say what -you wrote. Ask before each write, and if xTiles refuses a write, report what it -said rather than retrying. +On **yes**, record into the plan **first**: `studyloop plan record <plan-id> +--title "<topic>" --body "<the summary>"` (or the `record_plan_learning` MCP +tool). The plan document is the source of truth, and the xTiles page must be a +projection of a record it already has — never the only copy. Then follow prompt +P3 from the Second Brain guide: one learning-record page under the plan's +project, one dated planner task for the next review, then say what you wrote. +Ask before each write, and if xTiles refuses a write, report what it said +rather than retrying. On **no**, continue the wind-down and do not raise it again this session. ## What this sends, and where The summary, the plan title and the next review date go to the model service -backing this session and, through the connector, to xTiles' cloud. xTiles asks +backing this session and, through the connector, to xTiles' cloud. The due +reviews come from the learner's whole study state, not one plan's — nothing in +these tools is plan-scoped. xTiles asks permission per request, and your assistant reaches only what the learner's own xTiles account can already see — nothing is shared with other users. StudyLoop stores no xTiles credential and keeps no copy of what was written: the sign-in diff --git a/agents/skills/studyloop-xtiles-wind-down/references/harnesses.md b/agents/skills/studyloop-xtiles-wind-down/references/harnesses.md index c3339215f..a908bb63f 100644 --- a/agents/skills/studyloop-xtiles-wind-down/references/harnesses.md +++ b/agents/skills/studyloop-xtiles-wind-down/references/harnesses.md @@ -18,13 +18,14 @@ one place. | Codex | `~/.agents/skills/` — the hub itself, read natively; no extra link needed | <https://developers.openai.com/codex/skills>, 2026-09-03 | | Kiro CLI | `~/.kiro/skills/` | this repository already installs four skills there | | Claude Code | `~/.claude/skills/` | the Agent Skills convention | -| OpenCode | `~/.config/opencode/skills/` | <https://opencode.ai/docs/skills/>, 2026-09-03 | +| OpenCode | `~/.agents/skills/` — the hub itself, listed as a global search path; no extra link installed | <https://opencode.ai/docs/skills/>, 2026-09-03 | | pi | not verified — no skills-directory documentation found | — | `~/.agents/skills/` is not a StudyLoop invention: Codex reads it as its USER scope, -and OpenCode lists it as a global search path. Using it as the hub means Codex is -served by the hub alone, and it is why the hub is that directory rather than -somewhere under `~/.studyloop/`. +and OpenCode lists it as a global search path. Using it as the hub means Codex and +OpenCode are served by the hub alone (a second OpenCode link would risk a duplicate +listing nothing has verified it de-duplicates), and it is why the hub is that +directory rather than somewhere under `~/.studyloop/`. pi gets a self-gated paragraph in its `AGENTS.md` instead. When a pi skills directory is documented, that paragraph is replaced by a link like the others. diff --git a/docs/adr/0010-second-brains-are-projections.md b/docs/adr/0010-second-brains-are-projections.md index b6dd615af..1238ec7d9 100644 --- a/docs/adr/0010-second-brains-are-projections.md +++ b/docs/adr/0010-second-brains-are-projections.md @@ -1,6 +1,7 @@ # ADR-0010 — Second brains are projections; plan Markdown is the source of truth -**Status:** Proposed, 2026-09-03. Motivated by change `openspec/changes/second-brain/` +**Status:** Accepted, 2026-09-04 (proposed 2026-09-03; shipped as `v0.2.0`; clause 1 +amended 2026-09-04). Motivated by change `openspec/changes/archive/2026-09-04-second-brain/` (R-84, 0.2.0). Contract: `docs/architecture/second-brain.md`. ## Context @@ -17,10 +18,17 @@ notes come back only when the learner asks). ## Decision 1. The plan Markdown under `STUDYLOOP_PLANS_DIR` is the only source of truth. - Backends render **projections** of it; nothing in a backend, the CLI or an - agent protocol writes the plan file. Pulling notes is an explicit command - that returns text for the learner and agent to fold in through - `studyloop plan …`. + Backends render **projections** of it; **nothing in the second-brain layer + writes it** — no backend, no `brain` command, no agent protocol. + `studyloop plan …` is its only writer, and it writes through `render_plan` + (parse → mutate the model → `save_plan`), so the document's shape stays the + renderer's business. Pulling notes is an explicit command that returns text + for the learner and agent to fold in through `studyloop plan …`. + *(Amended 2026-09-04: the original clause said "nothing in a backend, the + CLI or an agent protocol writes the plan file", which was untrue on the day + it shipped — `plan new`, `plan milestone`, `plan status`, `plan evaluate + --record` and `plan reindex` all write it, by design. The rule the code + obeys, stated above, is what the clause always meant.)* 2. Providers hide behind a small runtime-checkable `SecondBrain` protocol (six methods), selected only from configuration (`second_brain.provider`, default `none`); no environment variable selects a provider. With `none` diff --git a/docs/adr/README.md b/docs/adr/README.md index 2440a962c..60d01ef19 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -39,4 +39,4 @@ the code and ask "why on earth is it done this way?" — it gets an ADR, and | [0007](0007-dev-only-vendored-assets.md) | Dev-only vendored assets live in git, not in the wheel | Accepted | `vendor/dev/` | | [0008](0008-retire-ttyd-entirely.md) | Retire ttyd entirely; the Web UI owns interactive sessions | Accepted | ttyd retirement stages 2-7 | | [0009](0009-one-session-authority.md) | The session-state file is the claim; the in-process slot is a cache | Proposed | M2 session-authority remediation | -| [0010](0010-second-brains-are-projections.md) | Second brains are projections; plan Markdown is the source of truth | Proposed | `second-brain` | +| [0010](0010-second-brains-are-projections.md) | Second brains are projections; plan Markdown is the source of truth | Accepted | `second-brain` | diff --git a/docs/second-brain.md b/docs/second-brain.md index 49583ce80..01e2f57a6 100644 --- a/docs/second-brain.md +++ b/docs/second-brain.md @@ -296,11 +296,15 @@ copy or store, and xTiles also publishes a connector in Claude's Connectors Directory if you would rather not add the URL by hand. Tool names appear only after you have signed in. -This is written for Claude Code, which is where it was tested. StudyLoop installs -the wind-down skill into every harness it detects, but whether Kiro, Codex, -OpenCode or pi can complete xTiles' browser authorisation is not something this -page has verified — the skill stays silent unless an `xtiles` server is actually -connected, so an untested harness costs you nothing. +This is written for Claude Code. Exercised end to end by a person on 2026-09-04 +with an xTiles Plus account and Kiro CLI 2.21.0 — not yet run in Claude Code. The +planner and wind-down prompts wrote what they describe; the project prompt created +the project but not the board or the visible page structure it promises (see that +prompt's section). StudyLoop installs the wind-down skill into every harness it +detects: installation paths are verified against each vendor's documentation, but +whether Kiro, Codex, OpenCode or pi can complete xTiles' browser authorisation is +not something this page has verified — the skill stays silent unless an `xtiles` +server is actually connected, so an untested harness costs you nothing. ### Checking it yourself @@ -315,6 +319,8 @@ just live-xtiles "<url>" "<title prefix>" # is it actually visible in xTiles? The xTiles one answers the question its connector cannot answer about itself: whether what your assistant wrote is *visible in the interface*. A write can succeed at the API and still render nowhere, and the interface is where you live. +Of the three prompts below, only the planner one returns a URL to check — a +planner tile is the one shape this live check has actually validated. It needs a session captured in a real browser window rather than a stored password, because `xtiles.app` sign-in is behind reCAPTCHA and fails silently @@ -326,30 +332,46 @@ match your own content. When you run one of these prompts, the plan text, today's next action, the due reviews and the session's learning record go to your assistant's model service — -Anthropic, for Claude Code — and through the connector to xTiles' cloud. Nothing -is sent unless you run a prompt. xTiles asks permission per request, and states -that your assistant only gets what your own account can already see and that -nothing is shared with other users. Your sign-in lives in your assistant's own MCP +Anthropic, for Claude Code — and through the connector to xTiles' cloud. The next +action and the due reviews are your **whole study state**, not the named plan's: +`get_next_action` and `get_due_cards` are not plan-scoped, so the title and reason +sent to xTiles come from your entire study history even when the prompt names one +plan. Nothing is sent unless you run a prompt. xTiles states that its connector +asks permission per request, that your assistant only gets what your own account +can already see, and that nothing is shared with other users — what you are +actually shown per request depends on your assistant, and in a recorded Kiro CLI +run no per-write prompt surfaced. Your sign-in lives in your assistant's own MCP settings rather than in StudyLoop, and that is also where you end it: remove the `xtiles` server there. +One more boundary worth knowing before you write anything: cleanup is uneven. +Planner tiles can be removed through the connector (by patching the planner +page); pages and projects have no delete tool and have to be deleted in the +xTiles interface by hand. + ### Today into your planner -Creates one task, so it works on any plan including Free. +Creates one planner tile, so it works on any plan including Free. This is the one +prompt that returns a URL you can check, and the shape the live UI check has +validated. ```text -Using the StudyLoop tools, call get_next_action with energy "medium", time_minutes 25 and modality "recall", and get_due_cards with limit 20. Then, in xTiles, add ONE task to today's planner titled "Study: <primary concept>" with the recommendation's reason and estimated minutes in the body, and a checklist of the due reviews, one line per card and at most 20. Do not create a project. Ask me before writing if the planner already has a "Study:" task today. +Using the StudyLoop tools, call get_next_action with energy "medium", time_minutes 25 and modality "recall", and get_due_cards with limit 20. Then, in xTiles, add ONE item to today's planner as a tile built from Markdown, titled "Study: <primary concept>", with the recommendation's reason and estimated minutes, and a checklist of the due reviews, one line per card and at most 20. Do not create a project. Ask me before writing if the planner already has a "Study:" tile today, and tell me the page URL when you are done. ``` ### Your plan as a project -Creating the project works on Free. Refreshing one — editing pages that are -already there — is the paid case: Plus for a personal space, Pro for a shared one. -No MCP tool returns plan Markdown, so paste it in from +Creating the project works on Free, but know the connector's own limits before +tier limits: it cannot create a board view, and pages that are collections +(tables, boards) cannot be refreshed through it at all — they return an error +regardless of what you pay. Normal text pages refresh in place. Expect the +project URL to open on the home page; the other pages may not be visible from +there in the interface even when the connector reports them created. No MCP tool +returns plan Markdown, so paste it in from `studyloop plan show <plan-id> --markdown`. ```text -Here is my StudyLoop study plan as Markdown, pasted from the CLI. In xTiles, create or refresh a project named "<plan title>" with pages Mission, Milestones, Learning Records, Resources, Checkpoints and Today. Put the Mission text on the home page, the milestones on a Kanban board (done/not done), the learning records as one page each, the resources in a table, and checkpoints as dated tasks. Update existing pages in place; do not delete anything. Tell me what you changed. +Here is my StudyLoop study plan as Markdown, pasted from the CLI. In xTiles, create or refresh a project named "<plan title>" with pages Mission, Milestones, Learning Records, Resources, Checkpoints and Today. Put the Mission text on the home page, the milestones in a table with a Status column (done/not done), the learning records as one page each, the resources in a table, and checkpoints as dated tasks. Refresh normal pages in place; do not try to refresh tables or boards, and do not delete anything. Tell me what you changed, and say which pages could not be refreshed. ``` Name the project exactly, every time. A renamed project is a project this prompt @@ -360,9 +382,16 @@ cannot find, so it creates a second one. Adds a page and a task, so this too works on Free. ```text -We are finishing a study session. Summarise what I covered in three bullets and one insight, then: (1) in xTiles, add that summary as a new learning record page under my "<plan title>" project, titled "LR — <date> — <topic>"; (2) add ONE planner task titled "Review: <concept>" on the next review date from get_due_cards. If we reviewed cards, record each one in StudyLoop with record_study_progress, passing the card_hash that get_due_cards returned. Ask before writing to xTiles; do not repeat the offer if I decline. +We are finishing a study session. Summarise what I covered in three bullets and one insight, then: (1) record that summary in StudyLoop first, with record_plan_learning for plan "<plan-id>", titled "<topic>" — the plan document is the source of truth; (2) in xTiles, add the same summary as a new learning record page under my "<plan title>" project, titled "LR — <date> — <topic>"; (3) add ONE planner task titled "Review: <concept>" on the next review date from get_due_cards. If get_due_cards returns no cards, do not invent a date; skip the Review task and the progress writes and say why. If we reviewed cards, record each one in StudyLoop with record_study_progress, passing the card_hash that get_due_cards returned. Ask before writing to xTiles; do not repeat the offer if I decline. ``` +A limit worth knowing: `get_due_cards` returns cards that are **already due** — +it is not a source of the next review date. With nothing due there is no date to +schedule, which is why the prompt tells your assistant to skip rather than +invent one. The prompt records into the plan **first** (via +`record_plan_learning`, or `studyloop plan record` at a shell) so the xTiles +page is a projection of a record the plan already has — never the only copy. + Your mentor offers this last one for you. `studyloop install agents` installs an opt-in wind-down skill into every harness it finds, and it stays silent unless `studyloop brain status --json` reports `provider: xtiles` **and** an `xtiles` @@ -372,6 +401,7 @@ server is connected in that session. ```bash studyloop brain status --json # provider, whether it can publish, where notes land +studyloop brain wind-down --json --connector xtiles # the one offer to make at wind-down, if any studyloop brain enable obsidian --vault ~/Obsidian/Personal studyloop brain publish # today's note plus every active plan studyloop brain publish --all # every plan, whatever its status @@ -382,8 +412,10 @@ studyloop brain template --install studyloop brain enable none # turn it off again ``` -`studyloop brain status --json` is what an agent reads. It publishes only when both -`configured` and `supports_publish` are true, which is why a learner on xTiles is +`studyloop brain wind-down --json` is what an agent reads at the end of a session: +it answers with the channel, whether to offer, the exact sentence to say and why. +The publish offer is made only when both `configured` and `supports_publish` are +true (both visible in `brain status --json`), which is why a learner on xTiles is never offered a command that cannot work. At the end of a session your mentor offers this once, and only when a provider that @@ -393,7 +425,8 @@ can publish is configured: Want me to publish today's study record and this plan to your Obsidian vault (Study/Today.md and Study/Plans/<plan-id>.md)? Yes or no — I'll only ask once. <!-- /wind-down-offer --> -Say no and it will not ask again. +The skill instructs your assistant to offer once and to drop the subject on a no; +that behaviour has not yet been observed in a recorded session. ## Configuration reference diff --git a/openspec/changes/second-brain/.openspec.yaml b/openspec/changes/archive/2026-09-04-second-brain/.openspec.yaml similarity index 100% rename from openspec/changes/second-brain/.openspec.yaml rename to openspec/changes/archive/2026-09-04-second-brain/.openspec.yaml diff --git a/openspec/changes/second-brain/design.md b/openspec/changes/archive/2026-09-04-second-brain/design.md similarity index 100% rename from openspec/changes/second-brain/design.md rename to openspec/changes/archive/2026-09-04-second-brain/design.md diff --git a/openspec/changes/second-brain/proposal.md b/openspec/changes/archive/2026-09-04-second-brain/proposal.md similarity index 90% rename from openspec/changes/second-brain/proposal.md rename to openspec/changes/archive/2026-09-04-second-brain/proposal.md index 6e3d97349..707dbcd1d 100644 --- a/openspec/changes/second-brain/proposal.md +++ b/openspec/changes/archive/2026-09-04-second-brain/proposal.md @@ -29,8 +29,10 @@ Three constraints shape the answer: - **An Obsidian backend** writing projections into `<vault>/<folder>/` (`Study/Plans/<plan_id>.md`, `Study/Today.md`): atomic, idempotent, refusing any target outside the vault or lacking StudyLoop's `studyloop:` frontmatter - marker. The official Obsidian CLI is an opt-in adapter that degrades to the - file writer whenever the app is not answering. + marker. Plain files and nothing else: an official-Obsidian-CLI adapter was + built and withdrawn before release (design D4), and the four config keys it + used (`use_cli`, `vault_name`, `template`, `daily_note`) are refused with an + error naming them rather than silently ignored (design D12). - **A `studyloop brain` command group** — `status`, `publish`, `pull`, `enable`, `template` — lazily registered, each with `--json`. - **A once-only wind-down offer**: the protocol offers a publish exactly once, @@ -88,9 +90,10 @@ Three constraints shape the answer: - **Writes into a learner's real files.** Mitigated by the ownership marker, the vault-boundary refusal, atomic replace, and a test suite that cannot resolve the real vault at all. -- **The Obsidian CLI grammar is unversioned.** Mitigated by keeping it an - optional adapter behind a probe: a grammar change costs one file and degrades - to plain files meanwhile. +- **The Obsidian CLI grammar is unversioned.** Resolved by withdrawal: the + opt-in adapter was built, reviewed and removed before release (design D4); + plain files carry the whole feature, and its four config keys are refused + with an error naming them. - **A learner edits a projection and loses the edit.** Accepted and documented: edits are replaced on the next publish with a warning, and personal notes belong in the sibling `.notes.md` file that StudyLoop only ever reads. diff --git a/openspec/changes/second-brain/specs/cli-surface/spec.md b/openspec/changes/archive/2026-09-04-second-brain/specs/cli-surface/spec.md similarity index 100% rename from openspec/changes/second-brain/specs/cli-surface/spec.md rename to openspec/changes/archive/2026-09-04-second-brain/specs/cli-surface/spec.md diff --git a/openspec/changes/second-brain/specs/configuration-and-secrets/spec.md b/openspec/changes/archive/2026-09-04-second-brain/specs/configuration-and-secrets/spec.md similarity index 100% rename from openspec/changes/second-brain/specs/configuration-and-secrets/spec.md rename to openspec/changes/archive/2026-09-04-second-brain/specs/configuration-and-secrets/spec.md diff --git a/openspec/changes/second-brain/specs/second-brain/spec.md b/openspec/changes/archive/2026-09-04-second-brain/specs/second-brain/spec.md similarity index 100% rename from openspec/changes/second-brain/specs/second-brain/spec.md rename to openspec/changes/archive/2026-09-04-second-brain/specs/second-brain/spec.md diff --git a/openspec/changes/archive/2026-09-04-second-brain/tasks.md b/openspec/changes/archive/2026-09-04-second-brain/tasks.md new file mode 100644 index 000000000..8cf2d610a --- /dev/null +++ b/openspec/changes/archive/2026-09-04-second-brain/tasks.md @@ -0,0 +1,110 @@ +# Implementation Tasks + +Three lanes, each on its own branch and worktree, each with a disjoint file set +enforced by `packages/studyloop/tests/fixtures/lane_ownership.yaml`. Lane **m7** +(core) and lane **m9** (process artefacts) ran in parallel; lane **m8** (xTiles +stage 1) started once both had merged into the integration branch. + +Every item was red-first: a failing test on concrete data, then the code, then +`env -u VIRTUAL_ENV just preflight` as the per-item gate, with docs and the +changelog landing in the same commit as the behaviour they describe. Each item +names the evidence subdirectory that holds its `00-dod.md`, `01-red.txt`, +`02-green.txt`, `03-gate.txt` and `05-docs.diff`; the roots are +`reviews/2026-09-03-second-brain/evidence/m7/`, `…/m9/` and `…/m8/`. + +Reconciled 2026-09-04 before archiving, per the review ruling +(`reviews/2026-09-04-second-brain-review/ARBITRATION.md` Q5): ticks record what +shipped in 0.2.0, strikes record what was cut, and the per-lane verifier items +are replaced by the review that actually happened. + +## Lane m7 — core (`lane/m7-second-brain-core`) + +- [x] **Foundation commit.** Repoint the lane-ownership guard's merge base to an + ordered tuple of integration branches with an env override, map lanes + m7/m8/m9, add the `live_obsidian` marker and deselect it by default in + both `pyproject.toml` files, and add the vault-isolation fixture plus the + session-finish hook that fails the run if the real vault changed. + _Evidence: `00-foundation/`._ +- [x] **Protocol, config and the null path.** `SecondBrain` protocol with its + exact-method guard, `SecondBrainConfig`, `NullBackend`, the xTiles + stage-1 object, `brain status` and `brain publish --plan`, and the + optionality tests (`sys.modules`, directory-tree snapshot, CLI output). + _Evidence: `T1/`._ +- [x] **Heading constants.** Extract the plan-Markdown heading constants in + `planning/markdown.py` so the projection renderer reads them instead of + re-deriving the same strings a second time. _Evidence: `T3a/`._ +- [x] **Obsidian backend.** Plan and Today projections, the atomic + vault-boundary writer with the ownership marker and content hash, + backlinks behind a lazy import with a warn-once fallback, and due-card + extraction shared with the review service. ~~The opt-in CLI adapter with + its probe and fallback~~ — built, reviewed and **withdrawn before + release** (design D4); its four config keys (`use_cli`, `vault_name`, + `template`, `daily_note`) are refused with an error naming them (D12). + _Evidence: `T2/`._ +- [x] **Templates as package data.** Ship the Obsidian templates under + `studyloop/data/templates/obsidian/`, add the drift guard that keeps them + in step with the renderer, assert they carry no ownership marker, and + implement `brain template`. _Evidence: `T3/`._ +- [x] **Full command group and integration points.** The rest of the `brain` + group (`pull`, `enable`), the `config init` follow-up, the doctor check, + the once-only wind-down offer, and the regenerated agent manifest. + ~~`daily_note`~~ — cut with the adapter; never shipped. _Evidence: `T4/`._ +- [x] **Obsidian half of the guide.** `docs/second-brain.md`, the touched pages, + the mkdocs entry, and the docs-drift guards that make a stale sentence a + red test. _Evidence: `T6a/`._ +- [x] **Verification.** ~~Independent per-lane verifier in a clean worktree~~ — + replaced by the P2 review council on the merged diff (SIGNOFF-P2): a + four-family independent review of the shipped layer, arbitrated in + `reviews/2026-09-04-second-brain-review/ARBITRATION.md`, with the static + checks (no module-level provider import, no `Path.home()` in the package, + vault untouched by the suite) carried by always-on tests instead of a + one-off verifier. +- [x] **Sign-off and merge** into the integration branch; shipped as `v0.2.0`. + +## Lane m9 — process artefacts (`lane/m9-second-brain-spec`) + +- [x] **ADR-0010.** Record that second brains are projections and that the plan + Markdown is the source of truth, with the rejected alternatives + (two-way sync, an xTiles client now, writing into `AgentMemory/`, an + environment-variable provider override, a web-UI button), and add the + index row in `docs/adr/README.md`. Clause 1 amended 2026-09-04 to the + rule the code obeys (`studyloop plan …` is the plan's only writer). + _Evidence: `T7/`._ +- [x] **Contract page.** `docs/architecture/second-brain.md`: the ten clauses + with the check that proves each one, plus the `.gitignore` exception that + makes the page trackable under the `docs/architecture/*` deny rule. + _Evidence: `T7/`._ +- [x] **OpenSpec change.** This directory: proposal, design with the D1–D13 + decision table and the alternatives, the new `second-brain` capability + spec, and the deltas to `configuration-and-secrets` and `cli-surface`. + _Evidence: `T7/`._ +- [x] **Gates.** `just spec-check`, `just docs`, `just lint`, `just typecheck`, + the lane-ownership guard on this branch, and the hygiene grep proving no + absolute path, account name or commit hash reached a public file. + _Evidence: `T7/`._ +- [x] **Sign-off and merge** into the integration branch — via the P2 council + on the merged diff (SIGNOFF-P2), not a per-lane verifier. + +## Lane m8 — xTiles stage 1 (`lane/m8-xtiles-stage1`) + +- [x] **The shared wind-down skill.** One skill body, self-gated on + `provider: xtiles` plus a connected `xtiles` MCP server, installed into + every detected harness by `studyloop install agents`, with the harness + wrappers, the agent-instruction paragraphs and a regenerated manifest. + _Evidence: `T5/`._ +- [x] **xTiles half of the guide.** The provider section of + `docs/second-brain.md`, the three prompts, and the sources rows. + Reworded post-run per ARBITRATION Q2/N1–N4/N6 (planner tile not task; + no board-view promise; skip the Review task when nothing is due). + _Evidence: `T6b/`._ +- [x] **Owner prompt run.** Run 2026-09-04 by the owner in Kiro CLI 2.21.0 + against a real xTiles Plus account — not Claude Code, and the docs say + so. P1/P1b and P3 wrote what they describe; P2 created the project but + not the board or the visible page structure it promised. Filled + checklist and redacted transcript: + `reviews/2026-09-03-second-brain/evidence/m8/xtiles-prompts/`. +- [x] **Verification.** ~~Preflight, docs-drift guards and hygiene grep rerun + by an independent verifier~~ — replaced by the P2 council on the merged + diff (SIGNOFF-P2). +- [x] **Sign-off**, integration gate, review council, and owner merge and tag + (`v0.2.0`, 2026-09-04). diff --git a/openspec/changes/second-brain/tasks.md b/openspec/changes/second-brain/tasks.md deleted file mode 100644 index 86d85c47b..000000000 --- a/openspec/changes/second-brain/tasks.md +++ /dev/null @@ -1,91 +0,0 @@ -# Implementation Tasks - -Three lanes, each on its own branch and worktree, each with a disjoint file set -enforced by `packages/studyloop/tests/fixtures/lane_ownership.yaml`. Lane **m7** -(core) and lane **m9** (process artefacts) run in parallel; lane **m8** (xTiles -stage 1) starts once both have merged into the integration branch. - -Every item is red-first: a failing test on concrete data, then the code, then -`env -u VIRTUAL_ENV just preflight` as the per-item gate, with docs and the -changelog landing in the same commit as the behaviour they describe. Each item -names the evidence subdirectory that holds its `00-dod.md`, `01-red.txt`, -`02-green.txt`, `03-gate.txt` and `05-docs.diff`; the roots are -`reviews/2026-09-03-second-brain/evidence/m7/`, `…/m9/` and `…/m8/`. - -## Lane m7 — core (`lane/m7-second-brain-core`) - -- [ ] **Foundation commit.** Repoint the lane-ownership guard's merge base to an - ordered tuple of integration branches with an env override, map lanes - m7/m8/m9, add the `live_obsidian` marker and deselect it by default in - both `pyproject.toml` files, and add the vault-isolation fixture plus the - session-finish hook that fails the run if the real vault changed. - _Evidence: `00-foundation/`._ -- [ ] **Protocol, config and the null path.** `SecondBrain` protocol with its - exact-method guard, `SecondBrainConfig`, `NullBackend`, the xTiles - stage-1 object, `brain status` and `brain publish --plan`, and the - optionality tests (`sys.modules`, directory-tree snapshot, CLI output). - _Evidence: `T1/`._ -- [ ] **Heading constants.** Extract the plan-Markdown heading constants in - `planning/markdown.py` so the projection renderer reads them instead of - re-deriving the same strings a second time. _Evidence: `T3a/`._ -- [ ] **Obsidian backend.** Plan and Today projections, the atomic - vault-boundary writer with the ownership marker and content hash, - backlinks behind a lazy import with a warn-once fallback, the opt-in CLI - adapter with its probe and fallback, and due-card extraction shared with - the review service. _Evidence: `T2/`._ -- [ ] **Templates as package data.** Ship the Obsidian templates under - `studyloop/data/templates/obsidian/`, add the drift guard that keeps them - in step with the renderer, assert they carry no ownership marker, and - implement `brain template`. _Evidence: `T3/`._ -- [ ] **Full command group and integration points.** The rest of the `brain` - group (`pull`, `enable`), the `config init` follow-up, the doctor check, - the once-only wind-down offer, and the regenerated agent manifest. - _Evidence: `T4/`._ -- [ ] **Obsidian half of the guide.** `docs/second-brain.md`, the touched pages, - the mkdocs entry, and the docs-drift guards that make a stale sentence a - red test. _Evidence: `T6a/`._ -- [ ] **Lane verification.** Independent verifier in a clean worktree: every - gate rerun plus the lane-specific static checks (no module-level provider - import, no `Path.home()` in the package, no MCP import, no publish call - site outside the CLI, real vault and config directory byte-identical - before and after the suite). _Evidence: `SIGNOFF-M7/`._ -- [ ] **Sign-off and merge** into the integration branch. - -## Lane m9 — process artefacts (`lane/m9-second-brain-spec`) - -- [ ] **ADR-0010.** Record that second brains are projections and that the plan - Markdown is the source of truth, with the rejected alternatives - (two-way sync, an xTiles client now, writing into `AgentMemory/`, an - environment-variable provider override, a web-UI button), and add the - index row in `docs/adr/README.md`. _Evidence: `T7/`._ -- [ ] **Contract page.** `docs/architecture/second-brain.md`: the ten clauses - with the check that proves each one, plus the `.gitignore` exception that - makes the page trackable under the `docs/architecture/*` deny rule. - _Evidence: `T7/`._ -- [ ] **OpenSpec change.** This directory: proposal, design with the D1–D13 - decision table and the alternatives, the new `second-brain` capability - spec, and the deltas to `configuration-and-secrets` and `cli-surface`. - _Evidence: `T7/`._ -- [ ] **Gates.** `just spec-check`, `just docs`, `just lint`, `just typecheck`, - the lane-ownership guard on this branch, and the hygiene grep proving no - absolute path, account name or commit hash reached a public file. - _Evidence: `T7/`._ -- [ ] **Sign-off and merge** into the integration branch. - -## Lane m8 — xTiles stage 1 (`lane/m8-xtiles-stage1`) - -- [ ] **The shared wind-down skill.** One skill body, self-gated on - `provider: xtiles` plus a connected `xtiles` MCP server, installed into - every detected harness by `studyloop install agents` (two installer rows), - with the harness wrappers, the agent-instruction paragraphs and a - regenerated manifest. _Evidence: `T5/`._ -- [ ] **xTiles half of the guide.** The provider section of - `docs/second-brain.md`, the three prompts, and the sources rows. - _Evidence: `T6b/`._ -- [ ] **Owner prompt run.** The owner runs the three prompts end to end against - a real xTiles board and records what came back, because stage 1 is a - prompt contract and only a human can judge whether it reads well. - _Evidence: `T5/`._ -- [ ] **Lane verification.** Preflight, the docs-drift guards and the hygiene - grep, rerun by an independent verifier. _Evidence: `SIGNOFF-M8/`._ -- [ ] **Sign-off**, integration gate, review council, and owner merge and tag. diff --git a/openspec/specs/cli-surface/spec.md b/openspec/specs/cli-surface/spec.md index be42c56c9..c973a0050 100644 --- a/openspec/specs/cli-surface/spec.md +++ b/openspec/specs/cli-surface/spec.md @@ -166,3 +166,13 @@ version. - **WHEN** a user runs `studyloop --version` - **THEN** Click prints a line containing the package name and version (e.g. `studyloop, version 2.5.0`) and exits 0 + +### Requirement: The brain group is lazily registered and every command has --json +`studyloop.cli.__init__` SHALL register `"brain": "studyloop.cli._brain:brain_group"` +in `lazy_subcommands`; `brain status`, `publish`, `pull`, `enable` and +`template` SHALL each accept `--json`; `studyloop.cli._brain` SHALL import +`studyloop.second_brain` only inside command bodies. + +#### Scenario: Help without a backend import +- **WHEN** `studyloop brain --help` runs +- **THEN** it exits 0 and `studyloop.second_brain.obsidian` is not imported diff --git a/openspec/specs/configuration-and-secrets/spec.md b/openspec/specs/configuration-and-secrets/spec.md index e9129130f..590113bc1 100644 --- a/openspec/specs/configuration-and-secrets/spec.md +++ b/openspec/specs/configuration-and-secrets/spec.md @@ -171,3 +171,17 @@ skippable via `STUDYLOOP_SKIP_LEGACY_MIGRATION` environment variable. #### Scenario: Both directories already exist - **WHEN** both `~/.config/studyctl/` and `~/.config/studyloop/` exist - **THEN** no migration occurs — the new directory is authoritative + +### Requirement: The second_brain section is parsed into SecondBrainConfig with one-line errors +`studyloop.settings.load_settings()` SHALL parse an optional top-level +`second_brain` mapping into `SecondBrainConfig(provider, vault_path, folder, +backlinks)`, defaulting `provider` to `none`; a provider outside +`none|obsidian|xtiles`, a non-boolean `backlinks`, or a folder that is absolute or +contains `..` SHALL raise `ConfigError` with a one-line message; any of the retired +keys `use_cli`, `vault_name`, `template` or `daily_note` SHALL also raise, naming +the key; and `second_brain` SHALL count as a known key for the unknown-key report. + +#### Scenario: Misspelled provider +- **WHEN** `config.yaml` contains `second_brain: {provider: obsidan}` +- **THEN** `studyloop brain status` prints one `ConfigError` line naming the + allowed values and exits 1 without a traceback diff --git a/openspec/specs/second-brain/spec.md b/openspec/specs/second-brain/spec.md new file mode 100644 index 000000000..7ed715c17 --- /dev/null +++ b/openspec/specs/second-brain/spec.md @@ -0,0 +1,146 @@ +# second-brain Specification + +## Purpose +Publish read-only projections of study plans and of today's study into an +optional, user-chosen second brain (Obsidian first; xTiles via an assistant), +keeping the plan Markdown under `STUDYLOOP_PLANS_DIR` as the single source of +truth. Nothing here runs unless `second_brain.provider` is set. + +## Requirements + +### Requirement: A disabled or absent second_brain section imports no backend and writes nothing +`studyloop.second_brain.factory.get_backend()` SHALL return +`studyloop.second_brain.core.NullBackend` when `Settings.second_brain.provider` +is `none` or the section is absent, without importing +`studyloop.second_brain.obsidian`, and every operation on it SHALL return a +skipped result without touching the filesystem. + +#### Scenario: Status with no configuration +- **WHEN** `studyloop brain status --json` runs with no `second_brain` section +- **THEN** it prints `configured: false` and `studyloop.second_brain.obsidian` + is not in `sys.modules`, and no file is created anywhere + +### Requirement: The SecondBrain protocol has exactly six methods +`studyloop.second_brain.core.SecondBrain` SHALL be a runtime-checkable +`Protocol` exposing exactly `describe`, `is_available`, `publish_plan`, +`publish_today`, `publish_learning_record` and `pull_notes`; every backend +SHALL satisfy `isinstance(backend, SecondBrain)`. + +#### Scenario: A backend gains a method +- **WHEN** a seventh public method is added to the protocol +- **THEN** `tests/test_second_brain_protocol.py` fails until the spec and the + method-set guard are updated together + +### Requirement: The Obsidian backend writes only StudyLoop-owned files under the configured vault folder +`studyloop.second_brain.obsidian.ObsidianBackend` SHALL write only under +`<vault_path>/<folder>/` (default `Study/`), only to files whose frontmatter +carries a `studyloop:` ownership marker with matching identity, atomically +(temporary file and `os.replace`), and SHALL refuse with `SecondBrainError` +any target that resolves outside the vault or lacks the marker. + +#### Scenario: A user note occupies the target path +- **WHEN** `Study/Plans/<id>.md` exists without a `studyloop:` marker +- **THEN** `publish_plan(<id>)` raises `SecondBrainError` naming the file and + the file's bytes are unchanged + +#### Scenario: A symlinked folder points outside the vault +- **WHEN** `<vault>/Study` is a symlink to a directory outside the vault +- **THEN** every publish operation is refused before any write + +### Requirement: Republishing unchanged content performs no write +`ObsidianBackend` SHALL compare the rendered projection against the existing +file's own contents and, when they are identical, return the path under +`PublishResult.unchanged` without calling `os.replace` or changing `st_mtime_ns`. +The comparison SHALL NOT use the `content_hash` recorded in the existing file's +ownership marker: that value records what StudyLoop last intended to write, so a +projection the learner has edited by hand still carries the hash of the correct +content and would be reported as unchanged, leaving the edit in place and making +the vault a second source of truth. + +#### Scenario: Publish twice +- **WHEN** `publish_plan(<id>)` runs twice with the plan unchanged +- **THEN** the second result lists the path under `unchanged` and the file's + mtime is identical + +#### Scenario: An edited projection is restored +- **WHEN** the learner appends a line to `Study/Plans/<id>.md` and + `publish_plan(<id>)` runs again +- **THEN** the file is rewritten from the plan document and the appended line is + gone + +### Requirement: A symlink at the target path is never replaced +`write_projection` SHALL refuse, using `lstat` rather than `exists`, when the target +is a symbolic link. The link is content the learner created and StudyLoop cannot +recreate; validating the referent's ownership marker and then calling `os.replace` +would destroy the link itself while reporting success. + +#### Scenario: A learner symlinks a projection to somewhere else in the vault +- **WHEN** `Study/Plans/<id>.md` is a symlink to another note inside the vault +- **THEN** `publish_plan(<id>)` raises `SecondBrainError` naming the link, and both + the link and its target are unchanged + +### Requirement: A target exchanged during preparation is refused +`write_projection` SHALL capture the target's device, inode and change time when it +validates ownership, re-check them immediately before `os.replace`, and refuse when +they differ. A vault is written by Obsidian and by sync clients, so a note the +learner owns can appear in the window between the ownership check and the rename, +and `os.replace` would delete it without ever having read its frontmatter. + +#### Scenario: A note appears after the ownership check +- **WHEN** the target is replaced by a different file between validation and rename +- **THEN** the write is refused, the temporary file is removed, and the message says + nothing was written + +### Requirement: Publishing never modifies the plan file +No operation of any `SecondBrain` backend SHALL write to +`STUDYLOOP_PLANS_DIR/<id>.md`; the plan bytes SHALL be identical before and +after `publish_plan`, `publish_today`, `publish_learning_record` and `pull_notes`. + +#### Scenario: Backend contract fixture +- **WHEN** the shared contract in `tests/test_second_brain_backend_contract.py` + runs against every registered backend +- **THEN** the byte snapshot of the plan file matches after every operation + +### Requirement: Pulling notes is explicit and read-only +`ObsidianBackend.pull_notes(plan_id)` SHALL read only +`<vault>/<folder>/Plans/<id>.notes.md`, SHALL never create or modify it, and +SHALL return a `PullNotesResult` with `found: false` when it is absent; +`studyloop brain pull` is the only caller. + +#### Scenario: No user note yet +- **WHEN** `studyloop brain pull <id>` runs and the sibling note does not exist +- **THEN** the command exits 0, reports `found: false`, and creates nothing + +### Requirement: No operation of this feature runs an external program +No module under `studyloop.second_brain` SHALL spawn a subprocess. An adapter for +the official Obsidian CLI was implemented and withdrawn before release: it sent +notes to whichever vault the running desktop app answered for, with no way to bind +that vault to the configured `vault_path`, and it passed the rendered plan as a +command-line argument, where any other local user could read it from the process +table. The guarded file writer SHALL be the only path that produces a note. + +#### Scenario: Publishing with subprocess spawning made to fail +- **WHEN** `subprocess.run` and `subprocess.Popen` are replaced with functions that + raise, and `publish_today()` runs +- **THEN** the note is written and nothing raises + +### Requirement: Retired configuration keys are reported rather than ignored +`load_settings()` SHALL raise `ConfigError` naming any of `use_cli`, `vault_name`, +`template` or `daily_note` found under `second_brain`, because a learner who set +`daily_note: true` authorised a write into a note they own and must be told it no +longer happens. + +#### Scenario: A pre-release config still names daily_note +- **WHEN** `config.yaml` contains `second_brain: {provider: obsidian, daily_note: true}` +- **THEN** `studyloop brain status` exits 1 with one line naming `daily_note` and + stating the adapter was withdrawn + +### Requirement: The wind-down protocol offers publishing once and only when a publishing provider is configured +`agents/shared/wind-down-protocol.md` SHALL instruct the agent to run +`studyloop brain status --json` and offer `studyloop brain publish` exactly once +when `configured` and `supports_publish` are both true, and to say nothing +otherwise; the offer sentence SHALL be identical in `docs/second-brain.md`. + +#### Scenario: xTiles stage 1 configured +- **WHEN** `provider: xtiles` +- **THEN** `supports_publish` is false and the protocol makes no publish offer diff --git a/packages/studyloop/src/studyloop/cli/_brain.py b/packages/studyloop/src/studyloop/cli/_brain.py index d5db822d8..2ef898547 100644 --- a/packages/studyloop/src/studyloop/cli/_brain.py +++ b/packages/studyloop/src/studyloop/cli/_brain.py @@ -113,6 +113,36 @@ def status_cmd(as_json: bool) -> None: console.print(f"{key}: {value}", soft_wrap=True) +@brain_group.command("wind-down") +@click.option( + "--connector", + "connectors", + multiple=True, + metavar="NAME", + help="An MCP server attached in this session. Repeatable; only 'xtiles' matters.", +) +@click.option("--json", "as_json", is_flag=True, help="Machine-readable output.") +def wind_down_cmd(connectors: tuple[str, ...], as_json: bool) -> None: + """Decide the one second-brain offer for this session's wind-down. + + Emits channel/offer/sentence/reason. If offer is true, say the sentence + verbatim, once; otherwise say nothing about second brains. Connector state + is per-session — which is why this is not a field on ``brain status``, and + why the caller must pass --connector for each MCP server it can see. + """ + from studyloop.second_brain import get_backend + from studyloop.second_brain.wind_down import decide_wind_down + + payload = decide_wind_down(get_backend().describe(), connectors).to_json_dict() + if as_json: + # click.echo for the same reason as status: Rich soft-wraps long lines, + # and the sentence is a long line. + click.echo(json.dumps(payload, indent=2)) + return + for key, value in payload.items(): + console.print(f"{key}: {value}", soft_wrap=True) + + @brain_group.command("publish") @click.option( "--plan", diff --git a/packages/studyloop/src/studyloop/cli/_plan.py b/packages/studyloop/src/studyloop/cli/_plan.py index 504e4aca3..ee6c09317 100644 --- a/packages/studyloop/src/studyloop/cli/_plan.py +++ b/packages/studyloop/src/studyloop/cli/_plan.py @@ -11,6 +11,7 @@ from __future__ import annotations import json +from pathlib import Path from typing import NoReturn import click @@ -30,6 +31,7 @@ load_plan_text, plans_dir, readiness, + record_learning, reindex_all, save_plan, seed_from_history, @@ -342,6 +344,60 @@ def plan_status(plan_id: str, status: str) -> None: console.print(f"[green]{plan.plan_id}[/green] → {status}") +@plan_group.command("record") +@click.argument("plan_id") +@click.option("--title", required=True, help="What was learned, in one line.") +@click.option("--body", default="", help="The record's body, as Markdown prose.") +@click.option( + "--body-file", + type=click.Path(exists=True, dir_okay=False), + default=None, + help="Read the body from a file instead of --body.", +) +@click.option( + "--status", + default="active", + show_default=True, + help="Record status (e.g. active, superseded).", +) +@click.option("--json", "as_json", is_flag=True, help="Machine-readable output.") +def plan_record( + plan_id: str, title: str, body: str, body_file: str | None, status: str, as_json: bool +) -> None: + """Append a learning record to a plan — the wind-down's 'record first' step. + + Parses the document, appends the record to the model, and re-renders the + whole file, so the on-disk shape stays the renderer's business (ADR-0010). + Re-running with the same title and body is a no-op, which makes it safe for + an agent to retry. + """ + if body and body_file: + _fail("Pass --body or --body-file, not both.") + if body_file: + body = Path(body_file).read_text(encoding="utf-8") + plan = _load(plan_id) # maps not-found/invalid-id to the friendly failure + try: + record, created = record_learning(plan.plan_id, title, body=body, status=status) + except ValueError as exc: + _fail(str(exc)) + if as_json: + click.echo( + json.dumps( + { + "plan_id": plan.plan_id, + "number": record.number, + "title": record.title, + "status": record.status, + "created": created, + }, + indent=2, + ) + ) + return + verb = "recorded" if created else "already recorded (no change)" + console.print(f"[green]LR-{record.number:04d}[/green] — {record.title}: {verb}") + + @plan_group.command("reindex") def plan_reindex() -> None: """Rebuild the derived plan index in the sessions DB from the documents.""" diff --git a/packages/studyloop/src/studyloop/history/search.py b/packages/studyloop/src/studyloop/history/search.py index 1e6b52814..49e84d220 100644 --- a/packages/studyloop/src/studyloop/history/search.py +++ b/packages/studyloop/src/studyloop/history/search.py @@ -65,23 +65,38 @@ def topic_frequency(topic_keywords: list[str], days: int = 30) -> list[dict]: Returns list of {date, session_id, snippet} for sessions mentioning the topic. """ + if not topic_keywords: + return [] + conn = _connection._connect() if not conn: return [] cutoff = (datetime.now(UTC) - timedelta(days=days)).isoformat() - placeholders = " OR ".join("content MATCH ?" for _ in topic_keywords) - query = f""" + # ONE qualified MATCH, with the keywords OR'd inside the FTS5 query + # string (R-92). Two defects lived in the old + # `" OR ".join("content MATCH ?" ...)` shape: + # * `content` is ambiguous — both messages_fts and messages carry the + # column, so SQLite raised "ambiguous column name: content" on every + # call through get_study_history, and R-22b (correctly) re-raised it; + # * FTS5 refuses more than one MATCH constraint per table in a WHERE + # ("unable to use function MATCH in the requested context"), so the + # multi-keyword path — the normal path — was broken either way. + # Each keyword is double-quoted as an FTS5 phrase, because several study + # terms carry spaces ("window functions", "lake formation") and unquoted + # they would parse as separate AND'd terms. + match_expr = " OR ".join('"' + kw.replace('"', '""') + '"' for kw in topic_keywords) + query = """ SELECT m.session_id, m.timestamp, snippet(messages_fts, 0, '>>>', '<<<', '...', 30) as snippet FROM messages_fts JOIN messages m ON messages_fts.rowid = m.rowid - WHERE ({placeholders}) AND m.timestamp > ? + WHERE messages_fts.content MATCH ? AND m.timestamp > ? ORDER BY m.timestamp DESC LIMIT 50 """ try: - rows = conn.execute(query, [*topic_keywords, cutoff]).fetchall() + rows = conn.execute(query, [match_expr, cutoff]).fetchall() return [dict(r) for r in rows] except sqlite3.OperationalError as exc: # R-22b: a bare `except sqlite3.OperationalError: return []` cannot diff --git a/packages/studyloop/src/studyloop/installers.py b/packages/studyloop/src/studyloop/installers.py index 43248dd98..72dc821b4 100644 --- a/packages/studyloop/src/studyloop/installers.py +++ b/packages/studyloop/src/studyloop/installers.py @@ -94,12 +94,13 @@ class LinkSpec: #: Each harness's own skills directory, for the ones whose location is documented. #: pi is absent on purpose: no pi skills directory is documented, so it gets a #: self-gated paragraph in its AGENTS.md instead of a link to a guessed path. +#: OpenCode is absent too (2026-09-04 review, Q2): it lists ``~/.agents/skills`` +#: — the hub itself — as a global search path, so a second link into +#: ``~/.config/opencode/skills`` was redundant at best, and whether OpenCode +#: de-duplicates two hits by name is unverified. XTILES_SKILL_LINKS: dict[str, LinkSpec] = { "kiro": LinkSpec(str(XTILES_SKILL_HUB), str(_HOME / ".kiro/skills" / XTILES_SKILL_NAME)), "claude": LinkSpec(str(XTILES_SKILL_HUB), str(_HOME / ".claude/skills" / XTILES_SKILL_NAME)), - "opencode": LinkSpec( - str(XTILES_SKILL_HUB), str(_HOME / ".config/opencode/skills" / XTILES_SKILL_NAME) - ), } _AGENT_CHOICES = RELEASE_HARNESSES diff --git a/packages/studyloop/src/studyloop/mcp/tools.py b/packages/studyloop/src/studyloop/mcp/tools.py index 2c093c182..c9af74ff2 100644 --- a/packages/studyloop/src/studyloop/mcp/tools.py +++ b/packages/studyloop/src/studyloop/mcp/tools.py @@ -89,6 +89,41 @@ def record_study_progress(course: str, card_hash: str, correct: bool) -> dict[st ) return {"status": "recorded"} + @mcp.tool() + def record_plan_learning( + plan_id: str, title: str, body: str = "", status: str = "active" + ) -> dict[str, Any]: + """Append a learning record to a study plan (the wind-down's first write). + + Record what was learned into the plan document BEFORE any second-brain + projection is offered: the plan Markdown is the source of truth + (ADR-0010), and a learning record that exists only in a second brain + is a record the plan does not have. + + Idempotent: calling again with the same title and body changes nothing + and reports created=false, so a retry is always safe. + + Args: + plan_id: The study plan id (from `studyloop plan list`). + title: What was learned, in one line. + body: The record's body, as Markdown prose. + status: Record status (default "active"). + """ + from studyloop.planning import record_learning + from studyloop.planning.store import InvalidPlanIdError, PlanNotFoundError + + try: + record, created = record_learning(plan_id, title, body=body, status=status) + except (PlanNotFoundError, InvalidPlanIdError, ValueError) as exc: + raise ToolError(str(exc)) from exc + return { + "plan_id": plan_id, + "number": record.number, + "title": record.title, + "status": record.status, + "created": created, + } + @mcp.tool() def generate_flashcards(course: str, chapter: int, content: str) -> dict[str, Any]: """Save agent-generated flashcards to a course directory. diff --git a/packages/studyloop/src/studyloop/planning/__init__.py b/packages/studyloop/src/studyloop/planning/__init__.py index 77ab7f53b..803874871 100644 --- a/packages/studyloop/src/studyloop/planning/__init__.py +++ b/packages/studyloop/src/studyloop/planning/__init__.py @@ -62,6 +62,7 @@ load_plan_text, plan_path, plans_dir, + record_learning, save_plan, unique_plan_id, ) @@ -105,6 +106,7 @@ "plans_dir", "preferred_backend", "readiness", + "record_learning", "reindex_all", "render_plan", "save_plan", diff --git a/packages/studyloop/src/studyloop/planning/store.py b/packages/studyloop/src/studyloop/planning/store.py index 4f64492e6..37d3a0a8d 100644 --- a/packages/studyloop/src/studyloop/planning/store.py +++ b/packages/studyloop/src/studyloop/planning/store.py @@ -20,7 +20,7 @@ from pathlib import Path from .markdown import parse_plan, render_plan -from .models import StudyPlan, slugify, utc_now_iso +from .models import LearningRecord, StudyPlan, slugify, utc_now_iso logger = logging.getLogger(__name__) @@ -199,3 +199,66 @@ def unique_plan_id(title: str) -> str: candidate = f"{base}-{counter}" counter += 1 return candidate + + +def record_learning( + plan_id: str, + title: str, + *, + body: str = "", + status: str = "active", +) -> tuple[LearningRecord, bool]: + """Append a learning record to ``plan_id``. Returns ``(record, created)``. + + The R-93 writer: before this, :class:`LearningRecord` was constructed in + exactly one place — the Markdown parser — so a record existed only if the + learner typed it into the plan document by hand, and an xTiles wind-down's + learning record lived only in xTiles (inverting ADR-0010). + + Parse → append → :func:`save_plan`, never an append of raw Markdown: + ``save_plan`` re-renders the whole document through ``render_plan``, so the + on-disk shape cannot drift from the renderer that the projection and + template guards already pin (``### LR-0004 — Title`` is the renderer's + business, not this function's). + + Idempotent the same way the vault writer is: re-recording an existing + record (same title and body, case-preserved, whitespace-trimmed the way the + parser trims) is a no-op that returns ``(existing, False)`` and leaves the + file's bytes untouched. Numbering is ``max(existing) + 1`` so records can + cite each other and be superseded rather than renumbered. + + Raises :class:`PlanNotFoundError` / :class:`InvalidPlanIdError` from the + load, and :class:`ValueError` for an empty title. + """ + title = title.strip() + if not title: + msg = "a learning record needs a title" + raise ValueError(msg) + body = body.strip() + # H1-H3 lines in a body would be re-parsed as new sections or new records + # on the next load (_split_sections / _subsection_items split on them, and + # _subsection_items does not honour code fences), silently corrupting the + # document's structure. Refuse rather than mangle; H4+ is safe prose. + for line in body.splitlines(): + if re.match(r"\A#{1,3}\s", line.strip()): + msg = ( + "a learning record body cannot contain #, ## or ### headings " + f"(found {line.strip()!r}); use #### or deeper, or plain prose" + ) + raise ValueError(msg) + status = status.strip() or "active" + + plan = load_plan(plan_id) + for existing in plan.learning_records: + if existing.title == title and existing.body == body: + return existing, False + + record = LearningRecord( + number=max((r.number for r in plan.learning_records), default=0) + 1, + title=title, + body=body, + status=status, + ) + plan.learning_records.append(record) + save_plan(plan) + return record, True diff --git a/packages/studyloop/src/studyloop/second_brain/core.py b/packages/studyloop/src/studyloop/second_brain/core.py index 6a18458b4..ea6787f76 100644 --- a/packages/studyloop/src/studyloop/second_brain/core.py +++ b/packages/studyloop/src/studyloop/second_brain/core.py @@ -256,7 +256,7 @@ class NullBackend(_InertBackend): class XtilesStageOneBackend(_InertBackend): """``provider: xtiles`` — configured, but not programmatically reachable. - xTiles is served in stage 1 by documentation, three tested prompts and an + xTiles is served in stage 1 by documentation, three supplied prompts and an opt-in assistant skill: the learner's assistant talks to xTiles' own MCP connector, StudyLoop does not. Reporting ``configured=True`` with ``supports_publish=False`` is what stops the wind-down protocol offering a diff --git a/packages/studyloop/src/studyloop/second_brain/obsidian.py b/packages/studyloop/src/studyloop/second_brain/obsidian.py index 252a19652..c91361c92 100644 --- a/packages/studyloop/src/studyloop/second_brain/obsidian.py +++ b/packages/studyloop/src/studyloop/second_brain/obsidian.py @@ -237,6 +237,7 @@ def plan_dry_run(self, plan_id: str | None) -> PublishResult: written: list[str] = [] unchanged: list[str] = [] skipped: list[str] = [] + warnings: list[str] = [] if plan_id is None: identity = self._identity("today-projection", None) @@ -273,6 +274,16 @@ def plan_dry_run(self, plan_id: str | None) -> PublishResult: unchanged.append(target.relative) elif verdict.refusal is not None: skipped.append(verdict.refusal) + elif verdict.outcome is WriteOutcome.REPLACED: + # The dry run must preview the warning a real publish prints + # (O4): "would write" alone hid that saying yes replaces the + # learner's own edits. + written.append(target.relative) + warnings.append( + f"would replace your edits in '{target.relative}' — a projection " + "is regenerated from the plan, so write beside it in " + f"'{target.relative.removesuffix('.md')}.notes.md' instead." + ) else: written.append(target.relative) @@ -282,6 +293,7 @@ def plan_dry_run(self, plan_id: str | None) -> PublishResult: written=tuple(written), unchanged=tuple(unchanged), skipped=tuple(skipped), + warnings=tuple(warnings), ) def _backlinks(self, plan: StudyPlan) -> tuple[str, ...]: diff --git a/packages/studyloop/src/studyloop/second_brain/obsidian_writer.py b/packages/studyloop/src/studyloop/second_brain/obsidian_writer.py index 27b5411da..60581e91a 100644 --- a/packages/studyloop/src/studyloop/second_brain/obsidian_writer.py +++ b/packages/studyloop/src/studyloop/second_brain/obsidian_writer.py @@ -242,6 +242,27 @@ def _assert_replaceable( ) +def _assert_parent_contained(target: VaultTarget, *, when: str) -> None: + """Refuse when ``target``'s directory no longer resolves under the vault. + + Resolved on the nearest EXISTING ancestor: ``Path.resolve`` on a path whose + directories do not exist yet cannot follow a symlink that would be followed + once they do, so checking the unbuilt parent directly would pass exactly + when it matters (O1). ``when`` names the call site in the message so a + refusal says which window it caught. + """ + ancestor = target.path.parent + while not ancestor.exists(): + ancestor = ancestor.parent + try: + ancestor.resolve().relative_to(target.root) + except ValueError as exc: + raise SecondBrainError( + f"Refusing to write outside the vault: '{target.relative}' no longer " + f"resolves under the configured vault (checked {when})." + ) from exc + + @dataclass(frozen=True) class WriteVerdict: """What a write WOULD do, decided without doing it. @@ -283,7 +304,11 @@ def classify_write( return WriteVerdict(WriteOutcome.WRITTEN, str(exc)) if existing_text == rendered: return WriteVerdict(WriteOutcome.UNCHANGED) - return WriteVerdict(WriteOutcome.WRITTEN) + # REPLACED, not WRITTEN (O4): the real publish distinguishes creating a + # note from overwriting a learner's edits, and warns on the latter. A dry + # run that said "would write" for both could not preview the one warning + # that was added precisely so a learner is told before losing text. + return WriteVerdict(WriteOutcome.REPLACED) def write_projection( @@ -336,26 +361,38 @@ def write_projection( # StudyLoop wrote it. Recorded so the caller can say so. was_replaced = True + # Containment check BEFORE anything is created (O1, 2026-09-04 review): + # mkdir(parents=True) used to run first, so an ancestor swapped for a + # symlink between projection_path and here created directories OUTSIDE the + # vault before the file write was refused. Checked on the nearest EXISTING + # ancestor, because resolve() on a not-yet-created path cannot see where a + # hostile symlink would send its children. + _assert_parent_contained(target, when="before creating directories") + path.parent.mkdir(parents=True, exist_ok=True) # Re-check containment now that the parents exist: a symlink could have been # created since projection_path ran. The residual TOCTOU window is accepted -- # it cannot be closed with POSIX path APIs alone -- but narrowing it to the # microseconds before the replace is worth the extra call. - try: - path.parent.resolve().relative_to(target.root) - except ValueError as exc: - raise SecondBrainError( - f"Refusing to write outside the vault: '{target.relative}' no longer " - "resolves under the configured vault." - ) from exc + _assert_parent_contained(target, when="after creating directories") # Replace-by-rename creates a NEW inode, so an existing note's mode has to be # copied across or a learner who tightened its permissions silently loses that. + # A file that vanished between the read above and this stat is a new file for + # mode purposes (the inode-identity check below still refuses the replace); + # any OTHER stat failure on an existing note is refused rather than defaulted + # (O7): silently re-opening a note the learner had locked down to 0o600 as + # 0o644 is exactly the loss the copy exists to prevent. try: existing_mode = stat.S_IMODE(path.stat().st_mode) - except OSError: + except FileNotFoundError: existing_mode = 0o644 + except OSError as exc: + raise SecondBrainError( + f"Could not read the permissions of '{target.relative}': {exc}. " + "Refusing to replace it, so its mode cannot be silently widened." + ) from exc # delete=False, and closed by the `with handle:` block below before the # rename: a context manager here would delete the file we are about to @@ -387,6 +424,14 @@ def write_projection( f"Refusing to overwrite '{target.relative}': it changed while StudyLoop " "was preparing the new version. Nothing was written; try again." ) + # And containment one last time, immediately before the replace (O1): + # everything above narrowed the window; this closes it to the rename + # itself, which is as far as POSIX path APIs can take it. + try: + _assert_parent_contained(target, when="before the replace") + except SecondBrainError: + temp_path.unlink(missing_ok=True) + raise os.replace(temp_path, path) except OSError as exc: temp_path.unlink(missing_ok=True) diff --git a/packages/studyloop/src/studyloop/second_brain/wind_down.py b/packages/studyloop/src/studyloop/second_brain/wind_down.py new file mode 100644 index 000000000..d5ac4c26d --- /dev/null +++ b/packages/studyloop/src/studyloop/second_brain/wind_down.py @@ -0,0 +1,137 @@ +"""The wind-down second-brain decision, as data instead of protocol prose. + +Why this exists +--------------- +The wind-down protocol used to ask the agent to derive the one second-brain +offer itself: read ``brain status --json``, combine two flags, remember which +provider means which sentence, and say nothing in every other case. Two of the +three acceptance gate checks for the second-brain layer therefore needed a +human to watch a transcript. This module moves the decision into code so a +deterministic test (and the agent) can read it as one JSON object: +``studyloop brain wind-down --json``. + +Two rules, deliberately NOT one conjunction (the 2026-09-04 design council's +D1): computing everything from ``configured and supports_publish`` would make +the xTiles channel permanently silent, because ``XtilesStageOneBackend`` sets +``supports_publish=False`` on purpose — the *publish* sentence must never be +offered to an xTiles learner. + +* ``publish`` — ``configured and supports_publish``. Vault writability is NOT + part of the rule: ``available`` is a runtime condition the publish itself + reports, so the offer stands for an Obsidian learner whose vault is + currently unmounted. +* ``xtiles`` — ``provider == "xtiles"`` and a connector named ``xtiles`` is + attached in this session. Connector state is per-session and only the caller + knows it, which is why it arrives as an argument (``--connector``) rather + than being probed here. + +The two sentences below are the canonical copies. The agent-facing docs carry +the same bytes between ``<!-- wind-down-offer -->`` / +``<!-- xtiles-wind-down-offer -->`` markers, and +``tests/test_second_brain_docs.py`` pins all copies to these constants, so the +CLI, the protocol, the skill and the guide cannot drift apart. + +Import boundary: nothing here imports a provider module — the decision reads a +:class:`~studyloop.second_brain.core.BrainDescription`, whoever produced it. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable + + from studyloop.second_brain.core import BrainDescription + +#: The one publish offer, verbatim. Byte-identical to the delimited sentence in +#: ``agents/shared/wind-down-protocol.md`` and ``docs/second-brain.md``. +PUBLISH_OFFER_SENTENCE = ( + "Want me to publish today's study record and this plan to your Obsidian vault " + "(Study/Today.md and Study/Plans/<plan-id>.md)? Yes or no — I'll only ask once." +) + +#: The one xTiles offer, verbatim. Byte-identical to the delimited sentence in +#: ``agents/skills/studyloop-xtiles-wind-down/SKILL.md``. +XTILES_OFFER_SENTENCE = ( + "Want me to add today's learning record and the next review to your xTiles " + "project? Yes or no — I'll only ask once." +) + +#: The MCP server name the xTiles channel requires, exactly as the skill and +#: the Second Brain guide spell it. +XTILES_CONNECTOR = "xtiles" + + +@dataclass(frozen=True) +class WindDownDecision: + """One offer (or none), with the exact sentence and why. + + ``sentence`` is the full pinned sentence or ``""`` — never a template the + agent must fill. There is deliberately no ``command`` field: nothing + guarantees a plan id exists at wind-down, and a half-filled command string + is worse than none (council ruling, 2026-09-04). + """ + + channel: str # "none" | "publish" | "xtiles" + offer: bool + sentence: str + reason: str + + def to_json_dict(self) -> dict[str, object]: + return { + "channel": self.channel, + "offer": bool(self.offer), + "sentence": self.sentence, + "reason": self.reason, + } + + +def decide_wind_down( + description: BrainDescription, connectors: Iterable[str] = () +) -> WindDownDecision: + """Which second-brain offer this session's wind-down makes, if any. + + Pure: no config read, no probe, no I/O. ``description`` is what + ``get_backend().describe()`` returned; ``connectors`` is the caller's list + of MCP server names attached in this session (only the ``xtiles`` entry + matters). Unknown providers never reach here — ``get_backend`` raises + ``ConfigError`` first. + """ + if description.configured and description.supports_publish: + return WindDownDecision( + channel="publish", + offer=True, + sentence=PUBLISH_OFFER_SENTENCE, + reason=(f"provider {description.provider!r} is configured and supports publish"), + ) + if description.provider == "xtiles": + if XTILES_CONNECTOR in set(connectors): + return WindDownDecision( + channel="xtiles", + offer=True, + sentence=XTILES_OFFER_SENTENCE, + reason="provider is 'xtiles' and an 'xtiles' connector is attached", + ) + return WindDownDecision( + channel="none", + offer=False, + sentence="", + reason="provider is 'xtiles' but no 'xtiles' connector is attached this session", + ) + return WindDownDecision( + channel="none", + offer=False, + sentence="", + reason="no second brain is configured", + ) + + +__all__ = [ + "PUBLISH_OFFER_SENTENCE", + "XTILES_CONNECTOR", + "XTILES_OFFER_SENTENCE", + "WindDownDecision", + "decide_wind_down", +] diff --git a/packages/studyloop/tests/_xtiles_stub_server.py b/packages/studyloop/tests/_xtiles_stub_server.py new file mode 100644 index 000000000..037fd662f --- /dev/null +++ b/packages/studyloop/tests/_xtiles_stub_server.py @@ -0,0 +1,85 @@ +"""A stub ``xtiles`` MCP server — Layer 2 of the acceptance harness. + +Its only job is to make "an ``xtiles`` connector is attached" REAL for the +transcript acceptance tests (WD-5/WD-6) without a network, an account or an +OAuth flow: a harness pointed at this server sees a server named ``xtiles`` +exposing the three write tools the prompts select between, and every call is +appended to a JSON file the test reads afterwards. + +Deliberately dumb. No validation, no state, no fidelity to xTiles' real +responses beyond the one thing the prompts check for (the planner-tile tool +returns a URL). Anything smarter would be a second implementation of xTiles +for tests to accidentally depend on. + +NO NETWORK — enforced, not aspirational: the transport is stdio, and this +module must import nothing that can open a socket. WD-4 scans this file's +imports; adding ``socket``, ``http``, ``urllib.request``, ``requests`` or +similar here turns that test red. + +Environment: + XTILES_STUB_CALL_LOG Path of the JSON call log (required). Each call is + appended as {"tool": ..., "arguments": {...}}. +""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +from mcp.server.fastmcp import FastMCP + +mcp = FastMCP("xtiles") + + +def _log_path() -> Path: + raw = os.environ.get("XTILES_STUB_CALL_LOG", "").strip() + if not raw: + print("XTILES_STUB_CALL_LOG is not set; refusing to run unlogged", file=sys.stderr) + raise SystemExit(2) + return Path(raw) + + +def _log_call(tool: str, arguments: dict) -> None: + """Append one call. Read-modify-write is fine: one server, one client.""" + path = _log_path() + calls = json.loads(path.read_text(encoding="utf-8")) if path.is_file() else [] + calls.append({"tool": tool, "arguments": arguments}) + path.write_text(json.dumps(calls, indent=2), encoding="utf-8") + + +@mcp.tool() +def xtiles_create_tasks(tasks: list[dict], projectId: str = "") -> dict: # noqa: N803 — xTiles' own casing + """Create one or more tasks (stub: logs and returns fake ids).""" + _log_call("xtiles_create_tasks", {"tasks": tasks, "projectId": projectId}) + return {"tasks": [{"id": f"stub-task-{i}"} for i, _ in enumerate(tasks)]} + + +@mcp.tool() +def xtiles_create_tiles_from_markdown_in_my_planner(period: str, date: str, markdown: str) -> dict: + """Append tiles to the personal planner (stub: logs and returns fake URLs).""" + _log_call( + "xtiles_create_tiles_from_markdown_in_my_planner", + {"period": period, "date": date, "markdown": markdown}, + ) + return { + "view_id": "stub-view-planner", + "tiles": [{"id": "stub-tile-1", "resource_url": "https://xtiles.app/stub-view-planner"}], + "parent_resource_url": "https://xtiles.app/stub-view-planner", + } + + +@mcp.tool() +def xtiles_create_view_from_markdown(projectId: str, markdown: str) -> dict: # noqa: N803 + """Create a new page in a project (stub: logs and returns a fake URL).""" + _log_call("xtiles_create_view_from_markdown", {"projectId": projectId, "markdown": markdown}) + return { + "view_id": "stub-view-page", + "resource_url": "https://xtiles.app/stub-view-page", + } + + +if __name__ == "__main__": + _log_path() # fail before the handshake when unconfigured, not on first call + mcp.run() # stdio transport — the default, and the point diff --git a/packages/studyloop/tests/e2e/_env.py b/packages/studyloop/tests/e2e/_env.py index 869a25fc3..c9b9bbe26 100644 --- a/packages/studyloop/tests/e2e/_env.py +++ b/packages/studyloop/tests/e2e/_env.py @@ -468,3 +468,35 @@ def goto_view(page: Page, view: str) -> None: page.wait_for_function("() => !!window.Alpine && !!window.Alpine.store('nav')", timeout=15000) page.evaluate("(v) => window.Alpine.store('nav').go(v)", view) page.wait_for_timeout(250) + + +def await_async_predicate( + page: Page, + js: str, + *, + arg: object = None, + timeout: float = 15.0, + what: str = "async page predicate", + poll_ms: int = 200, +) -> None: + """Poll an ``async`` JS predicate until truthy, from Python. + + Exists because ``page.wait_for_function`` does NOT await a returned + Promise: the Promise object itself is truthy, so an ``async () => …`` + predicate passes on its first poll no matter what it would resolve to — + the wait is silently a no-op. (Verified: + ``wait_for_function("async () => false")`` returns instantly.) That + no-op is exactly how phase 5 of the body-double journey went red on CI + twice on 2026-09-04: its "wait until the server holds both notes" guard + never waited, and the read raced the second POST. + + ``page.evaluate`` DOES await, so this loop drives it from Python with a + deadline. Sync predicates should keep using ``wait_for_function``, which + polls in-page and is cheaper. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if page.evaluate(js, arg): + return + page.wait_for_timeout(poll_ms) + raise AssertionError(f"timed out after {timeout:.0f}s waiting for {what}") diff --git a/packages/studyloop/tests/e2e/test_body_double_journey.py b/packages/studyloop/tests/e2e/test_body_double_journey.py index 994875e97..f26857357 100644 --- a/packages/studyloop/tests/e2e/test_body_double_journey.py +++ b/packages/studyloop/tests/e2e/test_body_double_journey.py @@ -51,7 +51,12 @@ sys.path.insert(0, _tests_dir) from _playwright_paths import PLAYWRIGHT_ARTIFACTS as RESULTS # noqa: E402 -from e2e._env import RunningServer, build_test_world, start_server # noqa: E402 +from e2e._env import ( # noqa: E402 + RunningServer, + await_async_predicate, + build_test_world, + start_server, +) if TYPE_CHECKING: from collections.abc import Generator @@ -293,8 +298,10 @@ def _park(page: Page, question: str, notes: str = "") -> None: # The invariant the product actually guarantees: one more pending topic # exists server-side. Polled through the page so it shares the browser's - # origin and auth, exactly as _api does. - page.wait_for_function( + # origin and auth, exactly as _api does. await_async_predicate, not + # wait_for_function: the latter does not await async predicates (_env.py). + await_async_predicate( + page, """async (want) => { const r = await fetch('/api/backlog'); if (!r.ok) return false; @@ -302,7 +309,8 @@ def _park(page: Page, question: str, notes: str = "") -> None: return (b.active_count + b.parking_lot_count) >= want; }""", arg=expected, - timeout=15_000, + timeout=15.0, + what=f"backlog to reach {expected} pending topics", ) # Only now is the form guaranteed quiescent: submitPark() clears the # question field in its success path, so returning before that lands would @@ -619,9 +627,16 @@ def test_phase5_notes_are_structured_markdown_and_preview_renders( # then races the second POST. The write itself is durable once it returns # -- add_note commits before responding -- so polling the count is the # honest signal. test_phase8 already polls a count for this reason. - page.wait_for_function( + # + # await_async_predicate, not wait_for_function: the previous de-flake + # used wait_for_function with an async predicate, which never awaits + # the Promise and so never waited at all — red on main twice, + # 2026-09-04. See _env.await_async_predicate. + await_async_predicate( + page, "async () => (await (await fetch('/api/notes')).json()).active_total === 2", - timeout=10_000, + timeout=10.0, + what="both notes to land server-side", ) notes = _api(page, "/api/notes") @@ -1092,8 +1107,10 @@ def test_phase10_park_form_survives_a_draft_typed_while_a_save_is_in_flight( page.locator("#bd-park-submit").click() page.locator("#bd-park-question").fill("Race second tangent") - # Let the first save complete server-side. - page.wait_for_function( + # Let the first save complete server-side. await_async_predicate, not + # wait_for_function, which never awaits an async predicate (_env.py). + await_async_predicate( + page, """async (want) => { const r = await fetch('/api/backlog'); if (!r.ok) return false; @@ -1101,7 +1118,8 @@ def test_phase10_park_form_survives_a_draft_typed_while_a_save_is_in_flight( return (b.active_count + b.parking_lot_count) >= want; }""", arg=total + 1, - timeout=15_000, + timeout=15.0, + what="the first park to land server-side", ) # The draft must still be there. Before the fix this was '' and the next diff --git a/packages/studyloop/tests/e2e/test_ghostty_dev_terminal.py b/packages/studyloop/tests/e2e/test_ghostty_dev_terminal.py index ac54d8bf1..1edb5cb10 100644 --- a/packages/studyloop/tests/e2e/test_ghostty_dev_terminal.py +++ b/packages/studyloop/tests/e2e/test_ghostty_dev_terminal.py @@ -51,7 +51,12 @@ if _tests_dir not in sys.path: sys.path.insert(0, _tests_dir) -from e2e._env import RunningServer, build_test_world, start_server # noqa: E402 +from e2e._env import ( # noqa: E402 + RunningServer, + await_async_predicate, + build_test_world, + start_server, +) if TYPE_CHECKING: from collections.abc import Generator @@ -735,7 +740,10 @@ def _await_server_sees_session(page, session_id: str) -> None: is visible: without this the test asserts on a race and fails with an opaque KeyError when it loses. """ - page.wait_for_function( + # await_async_predicate, not wait_for_function, which never awaits an + # async predicate (_env.py). + await_async_predicate( + page, """async (id) => { const res = await fetch('/api/session/state', { cache: 'no-store' }); if (!res.ok) return false; @@ -743,7 +751,8 @@ def _await_server_sees_session(page, session_id: str) -> None: return state.study_session_id === id; }""", arg=session_id, - timeout=15_000, + timeout=15.0, + what=f"the server to report session {session_id}", ) def test_session_state_survives_reload(self, dev_page) -> None: diff --git a/packages/studyloop/tests/e2e/test_session_recovery_journey.py b/packages/studyloop/tests/e2e/test_session_recovery_journey.py index f850a6892..df003dda3 100644 --- a/packages/studyloop/tests/e2e/test_session_recovery_journey.py +++ b/packages/studyloop/tests/e2e/test_session_recovery_journey.py @@ -49,6 +49,7 @@ sys.path.insert(0, _tests_dir) from _playwright_helpers import start_web_server # noqa: E402 +from e2e._env import await_async_predicate # noqa: E402 _TEST_AGENT_SCRIPT = Path(_tests_dir) / "_fake_agent.py" @@ -283,13 +284,17 @@ def test_end_from_the_picker_releases_the_slot(self, clean_session: str, page: P dialog.wait_for(state="visible", timeout=5_000) page.locator("[data-testid='study-end-confirm-yes']").click() - page.wait_for_function( + # await_async_predicate, not wait_for_function, which never awaits an + # async predicate (_env.py). + await_async_predicate( + page, """async () => { const res = await fetch('/api/session/state'); const s = await res.json(); return !s.study_session_id; }""", - timeout=10_000, + timeout=10.0, + what="the session to be released server-side", ) page.wait_for_function( """() => { @@ -385,13 +390,17 @@ def test_body_double_picker_can_end_a_foreign_session( page.locator("#bd-conflict-end").wait_for(state="visible", timeout=5_000) page.locator("#bd-conflict-end").click() - page.wait_for_function( + # await_async_predicate, not wait_for_function, which never awaits an + # async predicate (_env.py). + await_async_predicate( + page, """async () => { const res = await fetch('/api/session/state'); const state = await res.json(); return !state.study_session_id; }""", - timeout=10_000, + timeout=10.0, + what="the conflicting session to be ended server-side", ) def test_body_double_picker_can_reattach_its_own_session( diff --git a/packages/studyloop/tests/journeys/test_obsidian_learners_week.py b/packages/studyloop/tests/journeys/test_obsidian_learners_week.py index 26cb86604..e8326e047 100644 --- a/packages/studyloop/tests/journeys/test_obsidian_learners_week.py +++ b/packages/studyloop/tests/journeys/test_obsidian_learners_week.py @@ -213,6 +213,19 @@ def remember(beat: str) -> None: projection + "\n\nI typed this into the projection by mistake.\n", encoding="utf-8", ) + # O4 (2026-09-04 review): BEFORE the replace happens, the dry run must + # preview the warning — "would write" alone hid that saying yes replaces + # the learner's own edits, which defeats the one thing a preview is for. + preview = world.run("brain", "publish", "--dry-run") + assert preview.exit_code == 0, preview.output + preview_warnings = [detail for status, detail in preview.results() if status == "warning"] + assert any("replace your edits" in w for w in preview_warnings), ( + "beat 10: the dry run did not warn that the learner's edits would be " + f"replaced; it printed:\n{preview.output}" + ) + assert "I typed this into the projection by mistake." in world.read(PROJECTION), ( + "beat 10: the DRY RUN itself replaced the edit" + ) republished = world.run("brain", "publish") assert republished.exit_code == 0, republished.output assert "I typed this into the projection by mistake." not in world.read(PROJECTION), ( diff --git a/packages/studyloop/tests/live/test_wind_down_transcripts.py b/packages/studyloop/tests/live/test_wind_down_transcripts.py new file mode 100644 index 000000000..7d8a6782a --- /dev/null +++ b/packages/studyloop/tests/live/test_wind_down_transcripts.py @@ -0,0 +1,553 @@ +"""WD-5/WD-6 — transcript acceptance for the wind-down offer, in a real harness. + +Claude Code headless (the one harness with attested flags: ``-p``, +``--mcp-config``, ``--append-system-prompt-file``, ``--output-format +stream-json``, ``--resume``), pointed at the LiteLLM gateway so no vendor +credential is needed. Marked ``live_provider``: burns gateway spend, opt in +with ``just gate-checks``. + +The three gate checks, graded on captured transcripts: + +* **S1** ``provider: none``, no connector — the wind-down says nothing about + second brains. +* **S2** ``provider: xtiles``, no connector — same silence. +* **S3** ``provider: xtiles`` + the stub ``xtiles`` server — the offer is made + exactly once, the learner declines, and the subject never returns. 3/3 runs + required: this is the multi-turn claim a single lucky run flatters. + +Every silent-state assertion carries D4's positive control: the transcript +must show the harness actually ran ``studyloop brain wind-down`` and read its +decision, or a crashed run, a wrong prompt file or an unauthenticated CLI +would all "pass" by producing an absence. + +The weakest honest assertions, and nothing stronger (per the arbitrated plan): +the pinned sentence appears **at most once** across the whole transcript, +never "in turn N"; silence means neither the pinned sentence nor second-brain +vocabulary appears in assistant prose *while the positive control does*; and a +declined offer leaves the stub's call log empty. One retry only for a +pre-response infrastructure failure, and the flake count is written into the +artefact rather than smoothed. + +Secrets discipline: the gateway key is read from the proxy's own config file +at call time, enters the child's environment only, and is never printed, +logged, asserted on, or passed in argv. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +import time +from pathlib import Path + +import pytest +import yaml + +from studyloop.second_brain.wind_down import ( + PUBLISH_OFFER_SENTENCE, + XTILES_OFFER_SENTENCE, +) + +pytestmark = [ + pytest.mark.live_provider, + # Three harness turns at ~15-25s each, plus one permitted infra retry — + # the suite-wide 60s signal timeout would kill a healthy S3 run. + pytest.mark.timeout(900), +] + +REPO_ROOT = Path(__file__).resolve().parents[4] +STUB = Path(__file__).resolve().parents[1] / "_xtiles_stub_server.py" +PROTOCOL = REPO_ROOT / "agents" / "shared" / "wind-down-protocol.md" + +GATEWAY = os.environ.get("STUDYLOOP_GATE_GATEWAY", "http://localhost:4000") +MODEL = os.environ.get("STUDYLOOP_GATE_MODEL", "claude-sonnet-4-6") +PROXY_ENV = Path.home() / ".config" / "litellm-proxy-docker" / ".env" + +#: Words that must not appear in assistant prose during a silent state or +#: after a decline. Lower-cased comparison; both the spaced and hyphenated +#: forms of "second brain" (the first live capture said "second-brain +#: decision" and a space-only needle missed it). +SECOND_BRAIN_VOCAB = ("second brain", "second-brain", "obsidian", "xtiles") + +_EVIDENCE = Path( + os.environ.get( + "STUDYLOOP_EVIDENCE_DIR", + REPO_ROOT / "reviews" / "2026-09-04-gate-checks" / "evidence" / "gate-checks", + ) +) + +#: Collected across tests and flushed by the session fixture, so a partial run +#: still leaves a summary naming what it did and did not capture. +_SUMMARY: list[dict] = [] + + +def _gateway_key() -> str: + """The proxy's real key, from its own config file. NEVER printed.""" + if not PROXY_ENV.is_file(): + pytest.skip(f"gateway config not found: {PROXY_ENV}") + for line in PROXY_ENV.read_text(encoding="utf-8").splitlines(): + for name in ("LITELLM_MASTER_KEY", "LITELLM_API_KEY"): + if line.startswith(f"{name}="): + return line.split("=", 1)[1].strip().strip("'\"") + pytest.skip("no gateway key in the proxy config file") + + +@pytest.fixture(scope="session", autouse=True) +def _preconditions(): + if shutil.which("claude") is None: + pytest.skip("claude CLI not installed") + import urllib.request + + try: + with urllib.request.urlopen(f"{GATEWAY}/health/liveliness", timeout=5) as resp: + assert resp.status == 200 + except Exception: + pytest.skip(f"gateway not answering at {GATEWAY}") + _EVIDENCE.mkdir(parents=True, exist_ok=True) + (_EVIDENCE / "transcripts").mkdir(exist_ok=True) + yield + (_EVIDENCE / "summary.json").write_text(json.dumps(_SUMMARY, indent=2), encoding="utf-8") + + +@pytest.fixture() +def world(tmp_path): + """An isolated home + StudyLoop config dir. The real vault, the real + ~/.claude and the real StudyLoop state are unreachable by construction. + + D2 (the council's second defect): the wind-down skill is resolved from + ``Path.home()`` at install, so a hermetic HOME hides the skill and the + harness drives a default behaviour that cannot see it — run 3 of the first + capture proved it (``Unknown skill: studyloop-xtiles-wind-down`` killed + the offer turn). Fix as ruled: run the REAL installer into the isolated + home, which also exercises ``studyloop install agents`` for free. + """ + home = tmp_path / "home" + home.mkdir() + # Claude Code first-run: mark onboarding done so headless -p does not stall. + (home / ".claude.json").write_text(json.dumps({"hasCompletedOnboarding": True})) + install = subprocess.run( + [ + str(REPO_ROOT / ".venv" / "bin" / "studyloop"), + "install", + "agents", + "--repo-root", + str(REPO_ROOT), + "--tool", + "claude", + ], + capture_output=True, + text=True, + env={"HOME": str(home), "PATH": "/usr/bin:/bin"}, + timeout=120, + ) + assert install.returncode == 0, f"skill install into the isolated home failed: {install.stderr}" + assert (home / ".claude" / "skills" / "studyloop-xtiles-wind-down" / "SKILL.md").exists(), ( + "the isolated home has no wind-down skill; the harness would test its absence (D2)" + ) + return home + + +def _config(home: Path, provider: str | None) -> Path: + mapping: dict = {"topics": []} + if provider: + mapping["second_brain"] = {"provider": provider} + path = home / "config.yaml" + path.write_text(yaml.dump(mapping, default_flow_style=False, sort_keys=False)) + return path + + +def _system_prompt_file(home: Path) -> Path: + """A minimal preamble plus the REAL protocol file. + + Grading a paraphrase of the protocol would test the paraphrase (the + wrong-line-numbers lesson). The preamble must also not NAME the feature + under test: the first S2 capture failed on the phrase "second-brain + decision" that this preamble itself had planted — the assistant merely + echoed its instructions. + """ + text = ( + "You are a StudyLoop study mentor. The learner is finishing a study " + "session. Follow the wind-down protocol below exactly, using the Bash " + "tool to run studyloop commands. Skip steps that need session state " + "this conversation does not have (progress recording, session end, " + "voice); never skip step 5.\n\n---\n\n" + PROTOCOL.read_text(encoding="utf-8") + ) + path = home / "wind-down-system-prompt.md" + path.write_text(text, encoding="utf-8") + return path + + +def _mcp_config(home: Path, log_path: Path) -> Path: + config = { + "mcpServers": { + "xtiles": { + "command": sys.executable, + "args": [str(STUB)], + "env": {"XTILES_STUB_CALL_LOG": str(log_path)}, + } + } + } + path = home / "mcp-stub.json" + path.write_text(json.dumps(config, indent=2)) + return path + + +def _claude_bin() -> str: + path = shutil.which("claude") + if path is None: + pytest.skip("claude CLI not installed") + return path + + +def _child_env(home: Path, config_path: Path) -> dict[str, str]: + claude_dir = str(Path(_claude_bin()).parent) + return { + "HOME": str(home), + "PATH": f"{REPO_ROOT}/.venv/bin:{claude_dir}:/usr/bin:/bin", + "ANTHROPIC_BASE_URL": GATEWAY, + "ANTHROPIC_AUTH_TOKEN": _gateway_key(), + "ANTHROPIC_MODEL": MODEL, + "STUDYLOOP_CONFIG": str(config_path), + "STUDYLOOP_PLANS_DIR": str(home / "plans"), + "DISABLE_AUTOUPDATER": "1", + "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC": "1", + # The gateway's anthropic-passthrough adapter (LiteLLM, Bedrock invoke + # path) dies on extended-thinking blocks: "API Error: Content block is + # not a text block", reproduced deterministically on prompts long + # enough to trigger thinking and absent on short ones (proxy logs, + # 2026-09-04). Thinking adds nothing to a protocol-following check, so + # it is off rather than worked around. Remove once the proxy is + # upgraded past the adapter bug. + "MAX_THINKING_TOKENS": "0", + "TERM": "dumb", + } + + +class Turn: + """One harness turn, parsed from stream-json.""" + + def __init__(self, events: list[dict]): + self.events = events + + @property + def assistant_text(self) -> str: + parts: list[str] = [] + for event in self.events: + if event.get("type") == "assistant": + for block in event.get("message", {}).get("content", []): + if block.get("type") == "text": + parts.append(block.get("text", "")) + return "\n".join(parts) + + @property + def bash_commands(self) -> list[str]: + out: list[str] = [] + for event in self.events: + if event.get("type") == "assistant": + for block in event.get("message", {}).get("content", []): + if block.get("type") == "tool_use" and block.get("name") == "Bash": + out.append(str(block.get("input", {}).get("command", ""))) + return out + + @property + def session_id(self) -> str | None: + for event in self.events: + if event.get("type") == "result": + return event.get("session_id") + return None + + @property + def errored(self) -> bool: + return any(event.get("type") == "result" and event.get("is_error") for event in self.events) + + +def _run_turn( + prompt: str, + *, + env: dict[str, str], + system_prompt: Path, + mcp_config: Path | None = None, + resume: str | None = None, +) -> Turn: + argv = [ + _claude_bin(), + "-p", + prompt, + "--output-format", + "stream-json", + "--verbose", + "--model", + MODEL, + "--max-turns", + "10", + "--allowed-tools", + # The xtiles tools are ALLOWED whenever the connector is attached, so + # "declined, and nothing was written" is a real choice the model made + # — an offer it could not have acted on would pass the no-writes + # assert vacuously (trap #1). + "Bash(studyloop:*),mcp__xtiles__*" if mcp_config is not None else "Bash(studyloop:*)", + "--append-system-prompt-file", + str(system_prompt), + ] + if mcp_config is not None: + argv += ["--mcp-config", str(mcp_config), "--strict-mcp-config"] + if resume is not None: + argv += ["--resume", resume] + result = subprocess.run( + argv, + capture_output=True, + text=True, + env=env, + cwd=env["HOME"], + timeout=420, + ) + events = [] + for line in result.stdout.splitlines(): + line = line.strip() + if line.startswith("{"): + try: + events.append(json.loads(line)) + except json.JSONDecodeError: + continue + turn = Turn(events) + if result.returncode != 0 and not events: + raise RuntimeError(f"claude exited {result.returncode}: {result.stderr[-800:]}") + return turn + + +def _capture_session( + home: Path, + provider: str | None, + *, + with_connector: bool, + decline_turns: bool, +) -> tuple[list[Turn], Path | None, int]: + """Run one wind-down conversation. Returns (turns, stub_log, flakes).""" + config_path = _config(home, provider) + system_prompt = _system_prompt_file(home) + stub_log: Path | None = None + mcp_config: Path | None = None + if with_connector: + stub_log = home / "stub-calls.json" + mcp_config = _mcp_config(home, stub_log) + env = _child_env(home, config_path) + + flakes = 0 + + def once(prompt: str, resume: str | None) -> Turn: + nonlocal flakes + + def attempt() -> Turn: + return _run_turn( + prompt, + env=env, + system_prompt=system_prompt, + mcp_config=mcp_config, + resume=resume, + ) + + try: + turn = attempt() + except (RuntimeError, subprocess.TimeoutExpired): + # One retry, for a PRE-RESPONSE infrastructure failure only, and + # counted rather than smoothed (the arbitrated flake rule). + flakes += 1 + time.sleep(5) + return attempt() + if turn.errored and len(turn.assistant_text.strip()) < 200: + # An is_error result with no real assistant prose is the gateway + # failing before a response (observed: "API Error: Content block + # is not a text block" from the un-restarted proxy). Same rule. + flakes += 1 + time.sleep(5) + return attempt() + return turn + + turns = [once("Let's wrap up today's session, please run the wind-down.", None)] + if decline_turns: + session_id = turns[0].session_id + assert session_id, "no session id in the first turn; cannot resume" + turns.append(once("No thanks.", session_id)) + turns.append( + once("One more thing — what's a good way to consolidate today's topic?", session_id) + ) + return turns, stub_log, flakes + + +def _save_transcript(name: str, turns: list[Turn]) -> None: + (_EVIDENCE / "transcripts" / f"{name}.json").write_text( + json.dumps([t.events for t in turns], indent=2), encoding="utf-8" + ) + + +def _positive_control(turns: list[Turn]) -> bool: + """D4: the wind-down actually consulted the decision command.""" + return any("brain wind-down" in cmd for turn in turns for cmd in turn.bash_commands) + + +def _sentence_count(turns: list[Turn], sentence: str) -> int: + return sum(turn.assistant_text.count(sentence) for turn in turns) + + +def _vocab_hits(turns: list[Turn], *, ignoring: str = "") -> list[str]: + hits: list[str] = [] + for i, turn in enumerate(turns): + text = turn.assistant_text.replace(ignoring, "").lower() + hits.extend(f"turn {i + 1}: {w}" for w in SECOND_BRAIN_VOCAB if w in text) + return hits + + +def _record(state: str, *, expected: str, observed: str, runs: int, flakes: int, ok: bool) -> None: + _SUMMARY.append( + { + "state": state, + "expected": expected, + "observed": observed, + "runs": runs, + "flakes": flakes, + "verdict": "pass" if ok else "FAIL", + } + ) + + +# --------------------------------------------------------------------------- +# WD-5 — the three gate checks +# --------------------------------------------------------------------------- + + +def test_s1_provider_none_is_silent(world) -> None: + turns, _, flakes = _capture_session(world, None, with_connector=False, decline_turns=False) + _save_transcript("s1-provider-none", turns) + + assert _positive_control(turns), ( + "the harness never ran `studyloop brain wind-down` — silence would be vacuous (D4)" + ) + hits = _vocab_hits(turns) + count = _sentence_count(turns, PUBLISH_OFFER_SENTENCE) + _sentence_count( + turns, XTILES_OFFER_SENTENCE + ) + observed = "silent" if not hits and count == 0 else f"spoke: {hits or 'offer sentence'}" + _record( + "s1-provider-none", + expected="silent", + observed=observed, + runs=1, + flakes=flakes, + ok=not hits and count == 0, + ) + assert count == 0, "an offer sentence was made with no provider configured" + assert not hits, f"second-brain vocabulary in a silent state: {hits}" + + +def test_s2_xtiles_without_connector_is_silent(world) -> None: + turns, _, flakes = _capture_session(world, "xtiles", with_connector=False, decline_turns=False) + _save_transcript("s2-xtiles-no-connector", turns) + + assert _positive_control(turns), ( + "the harness never ran `studyloop brain wind-down` — silence would be vacuous (D4)" + ) + hits = _vocab_hits(turns) + count = _sentence_count(turns, PUBLISH_OFFER_SENTENCE) + _sentence_count( + turns, XTILES_OFFER_SENTENCE + ) + observed = "silent" if not hits and count == 0 else f"spoke: {hits or 'offer sentence'}" + _record( + "s2-xtiles-no-connector", + expected="silent", + observed=observed, + runs=1, + flakes=flakes, + ok=not hits and count == 0, + ) + assert count == 0, "an offer sentence was made with no connector attached" + assert not hits, f"second-brain vocabulary in a silent state: {hits}" + + +@pytest.mark.parametrize("run_number", [1, 2, 3]) +def test_s3_offer_once_then_decline_then_silence(world, run_number: int) -> None: + """3/3 required — never a quorum that averages a violation away.""" + turns, stub_log, flakes = _capture_session( + world, "xtiles", with_connector=True, decline_turns=True + ) + _save_transcript(f"s3-offer-decline-run{run_number}", turns) + + assert _positive_control(turns), "the wind-down never consulted the decision command (D4)" + + offer_count = _sentence_count(turns, XTILES_OFFER_SENTENCE) + publish_count = _sentence_count(turns, PUBLISH_OFFER_SENTENCE) + after_decline_hits = _vocab_hits(turns[1:]) + declined_writes = json.loads(stub_log.read_text()) if stub_log and stub_log.is_file() else [] + + ok = offer_count == 1 and publish_count == 0 and not after_decline_hits and not declined_writes + _record( + f"s3-offer-decline-run{run_number}", + expected="offer once, decline honoured, then silence, no writes", + observed="as expected" + if ok + else ( + f"offer x{offer_count}, publish-sentence x{publish_count}, " + f"after-decline {after_decline_hits}, stub writes {len(declined_writes)}" + ), + runs=1, + flakes=flakes, + ok=ok, + ) + assert offer_count == 1, f"the xTiles offer appeared {offer_count} times, not exactly once" + assert publish_count == 0, "the PUBLISH sentence reached an xTiles learner (D1 violation)" + assert not after_decline_hits, ( + f"second brains came back after the decline: {after_decline_hits}" + ) + assert not declined_writes, "the learner declined but the stub logged writes" + + +# --------------------------------------------------------------------------- +# WD-6 — observation only: which tool the planner wording selects +# --------------------------------------------------------------------------- + + +def test_wd6_tool_routing_observation(world) -> None: + """Records, never gates. One sample is not a distribution (ruling D3).""" + stub_log = world / "stub-calls.json" + mcp_config = _mcp_config(world, stub_log) + env = _child_env(world, _config(world, "xtiles")) + prompt = ( + "My next study action is 'Study: Python decorators' — reason: they keep " + "appearing in code review; estimated 25 minutes. There are no due " + "reviews. First tell me in one short sentence what you are about to " + "do, then, in xTiles, add ONE item to today's planner as a tile built " + 'from Markdown, titled "Study: Python decorators", with the reason ' + "and estimated minutes. Do not create a project. Tell me the page URL " + "when you are done." + ) + system_prompt = world / "wd6-system-prompt.md" + system_prompt.write_text( + "You are an assistant with an xtiles MCP server connected. Use its " + "tools directly; do not ask for confirmation.", + encoding="utf-8", + ) + turn = _run_turn(prompt, env=env, system_prompt=system_prompt, mcp_config=mcp_config) + if turn.errored and len(turn.assistant_text.strip()) < 200: + # One retry for a pre-response gateway failure, same rule as WD-5. + time.sleep(5) + turn = _run_turn(prompt, env=env, system_prompt=system_prompt, mcp_config=mcp_config) + + calls = json.loads(stub_log.read_text()) if stub_log.is_file() else [] + (_EVIDENCE / "tool-routing.json").write_text( + json.dumps( + { + "wording": "P1b planner-tile prompt", + "model": MODEL, + "tools_called": [c["tool"] for c in calls], + "note": "observation only — one sample is not a distribution (D3)", + }, + indent=2, + ), + encoding="utf-8", + ) + _save_transcript("wd6-tool-routing", [turn]) + # The only assertion is that the observation was CAPTURED — a run that + # called nothing recorded nothing worth keeping. + assert calls, "the harness made no stub call, so there is no routing to record" diff --git a/packages/studyloop/tests/test_history.py b/packages/studyloop/tests/test_history.py index 7df724061..b3c21d363 100644 --- a/packages/studyloop/tests/test_history.py +++ b/packages/studyloop/tests/test_history.py @@ -979,3 +979,108 @@ def test_returns_no_db(self, monkeypatch): monkeypatch.setattr(_conn, "_connect", lambda: None) assert hist.list_concepts() == [] + + +def _make_messages_db(tmp_path: Path) -> Path: + """The REAL export schema — messages + messages_fts + sync triggers. + + Built from agent-session-tools' schema.sql rather than a hand-rolled + subset, because the R-92 bug only exists in the real shape: both + ``messages`` and ``messages_fts`` carry a ``content`` column, so an + unqualified ``content MATCH ?`` in their join is "ambiguous column + name: content". + """ + schema = ( + Path(__file__).resolve().parents[2] + / "agent-session-tools" + / "src" + / "agent_session_tools" + / "schema.sql" + ).read_text() + db_path = tmp_path / "sessions.db" + conn = sqlite3.connect(db_path) + conn.executescript(schema) + conn.execute("INSERT INTO sessions (id, source) VALUES ('s1', 'kiro_cli')") + conn.execute( + "INSERT INTO messages (id, session_id, role, content, timestamp) " + "VALUES ('m1', 's1', 'user', 'I keep mixing up sql window functions', ?)", + ((datetime.now(UTC) - timedelta(days=1)).isoformat(),), + ) + conn.commit() + conn.close() + return db_path + + +class TestTopicFrequencyR92: + """R-92: the FTS join must qualify ``messages_fts.content``. + + ``get_study_history`` (the MCP tool) goes through ``topic_frequency``; + before the fix every call raised ``sqlite3.OperationalError: ambiguous + column name: content``, which R-22b then (correctly) re-raised rather + than masking as "topic never mentioned". + """ + + def _mock(self, db_path: Path, monkeypatch) -> None: + def mock_connect(): + conn = sqlite3.connect(db_path, timeout=5) + conn.row_factory = sqlite3.Row + return conn + + import studyloop.history._connection as _conn + + monkeypatch.setattr(_conn, "_connect", mock_connect) + + def test_matches_against_the_real_join_without_ambiguity(self, tmp_path, monkeypatch): + self._mock(_make_messages_db(tmp_path), monkeypatch) + + import studyloop.history as hist + + rows = hist.topic_frequency(["sql"], days=30) + + assert len(rows) == 1 + assert rows[0]["session_id"] == "s1" + assert "sql" in rows[0]["snippet"].lower() + + def test_multiple_keywords_or_together(self, tmp_path, monkeypatch): + """Every placeholder in the OR chain must be qualified, not just one.""" + self._mock(_make_messages_db(tmp_path), monkeypatch) + + import studyloop.history as hist + + rows = hist.topic_frequency(["spark", "window functions"], days=30) + + assert len(rows) == 1, "second keyword in the OR chain did not match" + + def test_phrase_keywords_do_not_match_scattered_terms(self, tmp_path, monkeypatch): + """Keywords with spaces are FTS5 phrases, not AND'd loose terms.""" + db_path = _make_messages_db(tmp_path) + conn = sqlite3.connect(db_path) + conn.execute( + "INSERT INTO messages (id, session_id, role, content, timestamp) " + "VALUES ('m2', 's1', 'user', " + "'the functions of a window manager', ?)", + ((datetime.now(UTC) - timedelta(days=1)).isoformat(),), + ) + conn.commit() + conn.close() + self._mock(db_path, monkeypatch) + + import studyloop.history as hist + + rows = hist.topic_frequency(["window functions"], days=30) + + assert [r["session_id"] for r in rows] == ["s1"] + assert "window" in rows[0]["snippet"].lower() + # m2 has both words but not the phrase; matching it would mean the + # keyword was parsed as two loose terms. + assert len(rows) == 1 + + def test_no_keywords_returns_empty_without_touching_the_db(self, monkeypatch): + import studyloop.history as hist + import studyloop.history._connection as _conn + + def explode(): # pragma: no cover - reaching this IS the failure + raise AssertionError("an empty keyword list must not open the DB") + + monkeypatch.setattr(_conn, "_connect", explode) + assert hist.topic_frequency([], days=30) == [] diff --git a/packages/studyloop/tests/test_install_agent_contracts.py b/packages/studyloop/tests/test_install_agent_contracts.py index 9ff342403..8d98cbcba 100644 --- a/packages/studyloop/tests/test_install_agent_contracts.py +++ b/packages/studyloop/tests/test_install_agent_contracts.py @@ -40,10 +40,13 @@ def _repo_root() -> Path: #: Harnesses whose skills directory is DOCUMENTED, and therefore linked. #: pi is absent deliberately: no pi skills directory is documented anywhere, so it #: gets a self-gated paragraph rather than a link to a guessed path. -_XTILES_LINKED_HARNESSES = ("kiro", "claude", "opencode") +#: opencode moved to the hub-served set (2026-09-04 review, Q2): the hub is on its +#: global search path, so a per-harness link was redundant at best. +_XTILES_LINKED_HARNESSES = ("kiro", "claude") #: Codex needs no link of its own -- the hub IS its user-scope skills directory. -_XTILES_HUB_SERVED_HARNESSES = ("codex",) +#: OpenCode likewise lists the hub as a global search path. +_XTILES_HUB_SERVED_HARNESSES = ("codex", "opencode") #: Harnesses whose definition file carries a self-gated paragraph as well. _XTILES_PARAGRAPH_FILES = ( @@ -374,11 +377,15 @@ def test_xtiles_skill_installed_for_each_detected_tool(tmp_path: Path, monkeypat installed = { "kiro": tmp_path / ".kiro/skills/studyloop-xtiles-wind-down", "claude": tmp_path / ".claude/skills/studyloop-xtiles-wind-down", - "opencode": tmp_path / ".config/opencode/skills/studyloop-xtiles-wind-down", } missing = sorted(tool for tool, path in installed.items() if not (path / "SKILL.md").is_file()) assert missing == [], f"no skill installed for: {missing}" + # OpenCode gets NO per-harness link (2026-09-04 review, Q2): the hub itself + # is on its global search path, and a second link would risk a duplicate + # listing that nothing has verified OpenCode de-duplicates. + assert not (tmp_path / ".config/opencode/skills/studyloop-xtiles-wind-down").exists() + # Every harness reads the SAME bytes. Asserted through the link, not by # comparing content: two copies that happen to match today are still two copies. for tool, path in installed.items(): diff --git a/packages/studyloop/tests/test_obsidian_writer.py b/packages/studyloop/tests/test_obsidian_writer.py index 4d8bf9676..4316d55fa 100644 --- a/packages/studyloop/tests/test_obsidian_writer.py +++ b/packages/studyloop/tests/test_obsidian_writer.py @@ -290,3 +290,79 @@ def test_create_only_write_refuses_an_existing_file(vault) -> None: write_projection(target, "# Template\n", _identity(), create_only=True) with pytest.raises(SecondBrainError, match="already exists"): write_projection(target, "# Template\n", _identity(), create_only=True) + + +# --------------------------------------------------------------------------- +# O1/O4/O7 — the 2026-09-04 review's residuals +# --------------------------------------------------------------------------- + + +def test_no_directory_is_created_outside_the_vault(vault, tmp_path) -> None: + """O1: containment is checked BEFORE mkdir, on the nearest existing ancestor. + + mkdir(parents=True) used to run first, so an ancestor swapped for a symlink + between projection_path and the write created directories OUTSIDE the vault + before the file write was refused. The refusal is not enough on its own — + the test's point is that nothing appears on the far side of the link. + """ + outside = tmp_path / "elsewhere" + outside.mkdir() + target = projection_path(vault, "Study", "Plans/python-decorators.md") + # The ancestor swap, after path validation, before the write. + (vault / "Study").symlink_to(outside, target_is_directory=True) + + with pytest.raises(SecondBrainError, match="outside the vault"): + write_projection(target, _rendered(), _identity()) + + assert list(outside.iterdir()) == [], "directories were created outside the vault" + + +def test_classify_write_previews_the_replace_warning(vault) -> None: + """O4: a dry run must say 'would replace', not 'would write'. + + classify_write returned WRITTEN for an owned-and-changed note, so --dry-run + could not preview the warning that was added precisely so a learner is told + before losing text. + """ + from studyloop.second_brain.obsidian_writer import classify_write + + target = projection_path(vault, "Study", "Plans/python-decorators.md") + write_projection(target, _rendered(), _identity()) + + plan = full_plan() + plan.mission.why = "Edited by hand since the last publish." + changed = render_plan_projection(plan, _identity()) + + assert classify_write(target, changed, _identity()).outcome is WriteOutcome.REPLACED + # And the counterpart: a first write must still be WRITTEN, or the warning + # appears on every publish and the learner learns to ignore it. + fresh = projection_path(vault, "Study", "Plans/other.md") + assert classify_write(fresh, changed, _identity()).outcome is WriteOutcome.WRITTEN + + +def test_unreadable_mode_on_an_existing_note_refuses_the_replace(vault, monkeypatch) -> None: + """O7: never default a real note's mode to 0o644. + + A learner who chmod-ed a note to 0o600 must not have it silently reopened + as 0o644 because one stat failed. New files keep the 0o644 default; an + existing note whose mode cannot be read refuses instead. + """ + target = projection_path(vault, "Study", "Plans/python-decorators.md") + write_projection(target, _rendered(), _identity()) + + real_stat = Path.stat + + def failing_stat(self, *args, **kwargs): + if self == target.path: + raise PermissionError(13, "Permission denied", str(self)) + return real_stat(self, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", failing_stat) + + plan = full_plan() + plan.mission.why = "Changed." + with pytest.raises(SecondBrainError, match="permissions"): + write_projection(target, render_plan_projection(plan, _identity()), _identity()) + + monkeypatch.undo() + assert "Changed." not in target.path.read_text(), "the note was replaced anyway" diff --git a/packages/studyloop/tests/test_openspec_gate_hook.py b/packages/studyloop/tests/test_openspec_gate_hook.py new file mode 100644 index 000000000..11b5bd4d4 --- /dev/null +++ b/packages/studyloop/tests/test_openspec_gate_hook.py @@ -0,0 +1,141 @@ +"""The openspec early-warning hook: block release actions, warn on commits. + +Drives ``scripts/openspec-gate.py`` in a subprocess with the same stdin JSON +shape every harness sends (``tool_input.command``), against fixture git repos +where the release guard genuinely fails and genuinely passes — so both the +exit-2 branch and the fail-open contract are proven, not assumed. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +GATE = Path(__file__).resolve().parents[3] / "scripts" / "openspec-gate.py" +CONSISTENCY = GATE.parent / "check-release-consistency.py" + + +def _git(cwd: Path, *args: str) -> None: + subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + env={ + "PATH": "/usr/bin:/bin", + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", + "HOME": str(cwd), + }, + timeout=60, + ) + + +@pytest.fixture() +def repo(tmp_path: Path) -> Path: + """A repo with an UNARCHIVED, UNDEFERRED change committed after the tag — + the exact state the 0.2.0 cut shipped in.""" + root = tmp_path / "repo" + (root / "scripts").mkdir(parents=True) + # The gate imports the guard from the repo under test, so the fixture + # carries the REAL script — a stub here would test the stub. + (root / "scripts" / "check-release-consistency.py").write_text( + CONSISTENCY.read_text(encoding="utf-8"), encoding="utf-8" + ) + (root / "seed.txt").write_text("seed") + _git(root, "init", "-q") + _git(root, "add", ".") + _git(root, "commit", "-qm", "seed") + _git(root, "tag", "v0.0.1") + change = root / "openspec" / "changes" / "unshipped-thing" + change.mkdir(parents=True) + (change / "proposal.md").write_text("## Why\n") + _git(root, "add", "openspec") + _git(root, "commit", "-qm", "work on unshipped-thing") + return root + + +def _run_gate(repo_root: Path, command: str, mode: str = "pre-tool-use"): + payload = json.dumps({"tool_name": "Bash", "tool_input": {"command": command}}) + return subprocess.run( + [sys.executable, str(GATE), mode, "--repo-root", str(repo_root)], + input=payload, + capture_output=True, + text=True, + timeout=60, + ) + + +def test_release_actions_are_blocked_when_the_guard_fails(repo: Path) -> None: + for command in ("git tag v0.0.2", "uv run python scripts/prepare-release.py 0.0.2"): + result = _run_gate(repo, command) + assert result.returncode == 2, f"{command!r} was not blocked: {result.stdout}" + assert "unshipped-thing" in result.stderr + assert "release-check" in result.stderr, "the block must name the hard gate" + + +def test_commits_are_warned_never_blocked(repo: Path) -> None: + result = _run_gate(repo, "git commit -m 'normal cycle work'") + assert result.returncode == 0, "a commit was blocked — open changes are legal in a cycle" + assert "unshipped-thing" in result.stdout + + +def test_unrelated_commands_pass_silently(repo: Path) -> None: + result = _run_gate(repo, "ls -la && pytest -q") + assert result.returncode == 0 + assert result.stdout == "" + + +def test_deferred_change_unblocks_the_release_action(repo: Path) -> None: + meta = repo / "openspec" / "changes" / "unshipped-thing" / ".openspec.yaml" + meta.write_text("deferred: waiting on the transport decision\n") + _git(repo, "add", str(meta.relative_to(repo))) + _git(repo, "commit", "-qm", "defer it") + + result = _run_gate(repo, "git tag v0.0.2") + assert result.returncode == 0, result.stderr + + +def test_remind_mode_is_terse_and_never_blocks(repo: Path) -> None: + result = _run_gate(repo, "", mode="remind") + assert result.returncode == 0 + assert "unshipped-thing" in result.stdout + assert len(result.stdout.splitlines()) == 1 + + +def test_garbage_stdin_fails_open(repo: Path) -> None: + """A broken early warning must never block work the hard gate allows.""" + result = subprocess.run( + [sys.executable, str(GATE), "pre-tool-use", "--repo-root", str(repo)], + input="not json at all {", + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 0 + + +def test_the_wrappers_call_the_same_one_body() -> None: + """Three harness wrappers, one script — the xTiles skill's hub pattern. + A wrapper drifting to its own logic is exactly what this pins against.""" + repo_root = GATE.parents[1] + wrappers = [ + repo_root / ".kiro" / "hooks" / "openspec-gate.json", + repo_root / ".claude" / "settings.json", + repo_root / ".codex" / "hooks.json", + ] + for wrapper in wrappers: + assert wrapper.is_file(), f"missing hook wrapper: {wrapper}" + text = wrapper.read_text(encoding="utf-8") + assert "scripts/openspec-gate.py" in text, f"{wrapper} does not call the one body" + # And the one body says what it is: early warning, with per-harness + # verification state — never "enforcement". + body = GATE.read_text(encoding="utf-8") + assert "NOT ENFORCEMENT" in body + assert "release-check" in body diff --git a/packages/studyloop/tests/test_plan_record.py b/packages/studyloop/tests/test_plan_record.py new file mode 100644 index 000000000..a31e53745 --- /dev/null +++ b/packages/studyloop/tests/test_plan_record.py @@ -0,0 +1,221 @@ +"""The learning-record writer (R-93): store function, CLI, and MCP tool. + +Before this writer existed, ``LearningRecord`` was constructed in exactly one +place — the Markdown parser — so a record existed only if the learner typed it +into the plan document by hand, and an xTiles wind-down's learning record lived +only in xTiles, inverting ADR-0010. These tests pin the ruled design: parse → +append → ``save_plan`` (never raw Markdown), ``max(existing) + 1`` numbering, +and byte-level idempotence on a same-title-same-body re-run. +""" + +from __future__ import annotations + +import json + +import pytest +from click.testing import CliRunner + +from studyloop.cli import cli +from studyloop.planning import ( + LearningRecord, + Mission, + StudyPlan, + create_plan, + load_plan, + record_learning, + store, +) +from studyloop.planning.store import PlanNotFoundError, plan_path + + +@pytest.fixture(autouse=True) +def isolated_plans_dir(tmp_path, monkeypatch): + monkeypatch.setenv(store.PLANS_DIR_ENV, str(tmp_path / "study-plans")) + return tmp_path / "study-plans" + + +def _seed(plan_id: str = "decorators", records: list[LearningRecord] | None = None) -> StudyPlan: + plan = StudyPlan( + plan_id=plan_id, + title="Python Decorators", + status="active", + topics=["python"], + mission=Mission(why="They keep appearing in code review."), + learning_records=records or [], + ) + create_plan(plan) + return plan + + +class TestRecordLearning: + def test_first_record_is_lr_0001_and_round_trips(self) -> None: + _seed() + + record, created = record_learning( + "decorators", "Closures carry state", body="The wrapper closes over its cell." + ) + + assert created is True + assert record.number == 1 + # Through the real renderer, in the renderer's format — never our own. + text = plan_path("decorators").read_text(encoding="utf-8") + assert "### LR-0001 — Closures carry state" in text + reloaded = load_plan("decorators") + assert [r.title for r in reloaded.learning_records] == ["Closures carry state"] + assert reloaded.learning_records[0].body == "The wrapper closes over its cell." + + def test_numbering_is_max_plus_one_not_count(self) -> None: + """A superseded LR keeps its number; gaps must not be reused.""" + _seed(records=[LearningRecord(number=3, title="Old insight", body="kept")]) + + record, _ = record_learning("decorators", "New insight") + + assert record.number == 4 + + def test_rerun_with_same_title_and_body_is_a_byte_level_noop(self) -> None: + _seed() + record_learning("decorators", "Once", body="only") + before = plan_path("decorators").read_bytes() + + record, created = record_learning("decorators", "Once", body="only") + + assert created is False + assert record.number == 1 + assert plan_path("decorators").read_bytes() == before + + def test_same_title_different_body_is_a_new_record(self) -> None: + _seed() + record_learning("decorators", "Insight", body="first take") + + record, created = record_learning("decorators", "Insight", body="second take") + + assert created is True + assert record.number == 2 + + def test_non_active_status_renders_and_round_trips(self) -> None: + _seed() + + record_learning("decorators", "Was wrong", body="see LR-0002", status="superseded") + + text = plan_path("decorators").read_text(encoding="utf-8") + assert "Status: superseded" in text + assert load_plan("decorators").learning_records[0].status == "superseded" + + def test_empty_title_is_refused(self) -> None: + _seed() + with pytest.raises(ValueError, match="title"): + record_learning("decorators", " ") + + def test_missing_plan_raises(self) -> None: + with pytest.raises(PlanNotFoundError): + record_learning("nope", "Anything") + + def test_heading_lines_in_the_body_are_refused_not_mangled(self) -> None: + """H1-H3 in a body would be re-parsed as sections/records on reload + (_subsection_items splits on '### ' and ignores code fences), silently + corrupting the document. H4+ is safe prose and stays allowed.""" + _seed() + + with pytest.raises(ValueError, match="###"): + record_learning("decorators", "Trap", body="fine\n### LR-0999 — fake\nmore") + + _record, created = record_learning("decorators", "Fine", body="#### a sub-note\nprose") + assert created is True + assert load_plan("decorators").learning_records[0].body.startswith("#### a sub-note") + + +def _record(*args: str): + """Module-level on purpose: the CLI coverage gate resolves bare-name + helpers from a test's source, not attribute calls on the class.""" + return CliRunner().invoke(cli, ["plan", "record", *args]) + + +class TestPlanRecordCli: + def _run(self, *args: str): + return _record(*args) + + def test_json_shape(self) -> None: + _seed() + + result = self._run("decorators", "--title", "CLI insight", "--body", "prose", "--json") + + assert result.exit_code == 0, result.output + payload = json.loads(result.output) + assert payload == { + "plan_id": "decorators", + "number": 1, + "title": "CLI insight", + "status": "active", + "created": True, + } + + def test_rerun_reports_created_false(self) -> None: + _seed() + self._run("decorators", "--title", "Same", "--body", "same", "--json") + + result = self._run("decorators", "--title", "Same", "--body", "same", "--json") + + assert result.exit_code == 0 + assert json.loads(result.output)["created"] is False + + def test_body_file(self, tmp_path) -> None: + _seed() + body = tmp_path / "body.md" + body.write_text("From a file.", encoding="utf-8") + + result = self._run("decorators", "--title", "Filed", "--body-file", str(body), "--json") + + assert result.exit_code == 0 + assert load_plan("decorators").learning_records[0].body == "From a file." + + def test_body_and_body_file_together_fail(self, tmp_path) -> None: + _seed() + body = tmp_path / "body.md" + body.write_text("x", encoding="utf-8") + + result = self._run( + "decorators", "--title", "T", "--body", "inline", "--body-file", str(body) + ) + + assert result.exit_code == 1 + assert "not both" in result.output + + def test_missing_plan_names_the_fix(self) -> None: + result = self._run("ghost", "--title", "T") + + assert result.exit_code == 1 + assert "studyloop plan list" in result.output + + +class TestMcpTool: + @pytest.fixture(autouse=True) + def _requires_mcp(self): + pytest.importorskip("mcp") + + def _tool(self): + from studyloop.mcp.server import mcp + + return mcp._tool_manager._tools["record_plan_learning"].fn + + def test_records_and_reports(self) -> None: + _seed() + + payload = self._tool()("decorators", "MCP insight", body="prose") + + assert payload["created"] is True + assert payload["number"] == 1 + assert load_plan("decorators").learning_records[0].title == "MCP insight" + + def test_retry_is_safe(self) -> None: + _seed() + self._tool()("decorators", "Again", body="same") + + payload = self._tool()("decorators", "Again", body="same") + + assert payload["created"] is False + + def test_missing_plan_is_a_tool_error(self) -> None: + from mcp.server.fastmcp.exceptions import ToolError + + with pytest.raises(ToolError): + self._tool()("ghost", "Anything") diff --git a/packages/studyloop/tests/test_second_brain_agent_protocol.py b/packages/studyloop/tests/test_second_brain_agent_protocol.py index 2d94ea4ee..631e69c3c 100644 --- a/packages/studyloop/tests/test_second_brain_agent_protocol.py +++ b/packages/studyloop/tests/test_second_brain_agent_protocol.py @@ -45,14 +45,16 @@ def _extract_offer(path: Path) -> str: def test_the_wind_down_protocol_gates_the_offer_on_both_flags() -> None: - """`configured` alone is not enough. + """`configured` alone is not enough — and the decision is the CLI's. xTiles stage 1 IS configured but cannot be published to, so an agent that checked only `configured` would offer a command that cannot work — and would do - it at the end of every session. + it at the end of every session. Since WD-1 the protocol delegates the decision + to `studyloop brain wind-down --json`, but it must still EXPLAIN the two + flags, or a reader cannot tell why the command answers as it does. """ text = WIND_DOWN.read_text(encoding="utf-8") - assert "studyloop brain status --json" in text + assert "studyloop brain wind-down --json" in text assert "supports_publish" in text assert "configured" in text diff --git a/packages/studyloop/tests/test_second_brain_docs.py b/packages/studyloop/tests/test_second_brain_docs.py index 98b0182c4..d5e6b84a1 100644 --- a/packages/studyloop/tests/test_second_brain_docs.py +++ b/packages/studyloop/tests/test_second_brain_docs.py @@ -204,11 +204,16 @@ def test_plan_authority_language_is_present(guide: str) -> None: def test_the_wind_down_offer_matches_the_agent_protocol(guide: str) -> None: - """One sentence, two files, byte-identical. + """One sentence, THREE locations, byte-identical (WD-3). Documentation describing a slightly different offer than the one the agent makes is how a learner ends up unable to tell whether the tool is behaving. + The third location is the CLI's own copy — what + ``studyloop brain wind-down --json`` actually emits — so the command, the + protocol and the guide cannot drift apart. """ + from studyloop.second_brain.wind_down import PUBLISH_OFFER_SENTENCE + protocol = (REPO_ROOT / "agents" / "shared" / "wind-down-protocol.md").read_text( encoding="utf-8" ) @@ -222,6 +227,34 @@ def extract(text: str) -> str: ) assert extract(guide) == extract(protocol) + assert extract(protocol) == PUBLISH_OFFER_SENTENCE + + +def test_the_xtiles_offer_matches_the_skill(guide: str) -> None: + """The xTiles channel's sentence, pinned skill ↔ CLI (WD-3, D1). + + A separate sentence from the publish offer on purpose: the two channels are + two rules, and pinning them to one sentence would recreate the conjunction + the design council rejected. The skill renders it as a blockquote, so the + leading ``>`` is stripped before comparing. + """ + from studyloop.second_brain.wind_down import XTILES_OFFER_SENTENCE + + skill = (REPO_ROOT / "agents" / "skills" / "studyloop-xtiles-wind-down" / "SKILL.md").read_text( + encoding="utf-8" + ) + + assert "<!-- xtiles-wind-down-offer -->" in skill + sentence = ( + skill.split("<!-- xtiles-wind-down-offer -->", 1)[1] + .split("<!-- /xtiles-wind-down-offer -->", 1)[0] + .strip() + .removeprefix(">") + .strip() + ) + + assert sentence == XTILES_OFFER_SENTENCE + assert sentence != "" # --------------------------------------------------------------------------- diff --git a/packages/studyloop/tests/test_session_ws_grace.py b/packages/studyloop/tests/test_session_ws_grace.py index 9ed483f36..8b4206dbc 100644 --- a/packages/studyloop/tests/test_session_ws_grace.py +++ b/packages/studyloop/tests/test_session_ws_grace.py @@ -279,17 +279,28 @@ async def test_agent_exit_while_detached_releases_immediately( # triple when an unrelated selection was large. Still comfortably inside # the 30s grace window above, so a pass here means the dead agent was # reaped rather than the window merely expiring. + # + # Poll for the SETTLED lifecycle — the timer popped from ``_pending`` — + # not the first observable effect. ``release()`` clears the slot, then + # awaits ``transport.end()`` and an executor hop before the grace + # task's ``finally`` pops the timer. Breaking out on "slot is empty" + # and immediately asserting "timer is gone" races that tail, and on a + # loaded CI runner loses (twice on ``main``, 2026-09-04). The pop is + # the last effect in ``_deferred_release``, so once it is observed the + # whole release has landed and every assert below runs against a + # settled state. The test still fails when the liveness poll is broken: + # the 30s grace window above can never expire inside this deadline. deadline = time.monotonic() + 15.0 while time.monotonic() < deadline: await asyncio.sleep(0.1) - if await active.current() is None: + if not _grace.has_pending_release(config.study_session_id): break - assert await active.current() is None, ( + assert not _grace.has_pending_release(config.study_session_id), ( "dead agent pinned the session slot — is_running() poll not working" ) + assert await active.current() is None assert transport.end_calls == 1 - assert not _grace.has_pending_release(config.study_session_id) async def test_release_now_cancels_the_timer_and_releases(self, config: SessionConfig) -> None: """Case 5 (unit half): explicit end during the window, no orphan timer.""" diff --git a/packages/studyloop/tests/test_web_smoke_browser.py b/packages/studyloop/tests/test_web_smoke_browser.py index 0d87ec71f..f83dc2c89 100644 --- a/packages/studyloop/tests/test_web_smoke_browser.py +++ b/packages/studyloop/tests/test_web_smoke_browser.py @@ -324,7 +324,12 @@ def test_main_controls_have_accessible_button_names(web_page: Page) -> None: "Courses", "Settings", ): - assert web_page.get_by_role("button", name=name).is_visible() + # exact=True, because these are substring-matched otherwise and the + # empty-state CTA "Generate content →" (index.html, Today view) also + # matches "Generate" — a strict-mode violation on any machine with no + # decks, which is CI's state and rarely a dev machine's. Red on main + # twice, 2026-09-04. + assert web_page.get_by_role("button", name=name, exact=True).is_visible() def test_failed_struggling_post_shows_error_without_success(web_page: Page) -> None: diff --git a/packages/studyloop/tests/test_wind_down_cli.py b/packages/studyloop/tests/test_wind_down_cli.py new file mode 100644 index 000000000..fe884f8fc --- /dev/null +++ b/packages/studyloop/tests/test_wind_down_cli.py @@ -0,0 +1,180 @@ +"""WD-2 — ``studyloop brain wind-down --json`` in a real subprocess. + +Each truth-table row from ``test_wind_down_decision.py`` is re-run through the +actual CLI entry point (``python -m studyloop``), with the provider selected by +a real config file via ``STUDYLOOP_CONFIG``. Red when the CLI and the pure +function disagree, or when the emitted sentence drifts from the pinned +constants by even one byte. + +Artefact: the observed table is written to ``wind-down-truth-table.json`` +under ``STUDYLOOP_EVIDENCE_DIR`` when that is set (how ``just gate-checks`` +captures evidence), and under pytest's tmp dir otherwise. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from studyloop.second_brain.wind_down import ( + PUBLISH_OFFER_SENTENCE, + XTILES_OFFER_SENTENCE, +) + + +def _run_wind_down(config_path: Path, *cli_args: str) -> subprocess.CompletedProcess[str]: + env = os.environ.copy() + env["STUDYLOOP_CONFIG"] = str(config_path) + return subprocess.run( + [sys.executable, "-m", "studyloop.cli", "brain", "wind-down", *cli_args], + capture_output=True, + text=True, + env=env, + timeout=60, + ) + + +def _config(tmp_path: Path, mapping: dict) -> Path: + path = tmp_path / "config.yaml" + path.write_text(yaml.dump(mapping, default_flow_style=False, sort_keys=False)) + return path + + +def _decide(config_path: Path, *cli_args: str) -> dict: + result = _run_wind_down(config_path, "--json", *cli_args) + assert result.returncode == 0, result.stderr + return json.loads(result.stdout) + + +#: (row id, second_brain section, extra CLI args, expected channel, offer, sentence). +#: Row 5's unwritable vault is a config whose vault_path is a FILE — configured, +#: publish-capable, not available. +_ROWS = [ + ("row1-none-no-connector", None, (), "none", False, ""), + ("row2-none-connector", None, ("--connector", "xtiles"), "none", False, ""), + ( + "row3-obsidian-writable", + {"provider": "obsidian", "vault_path": "@VAULT@"}, + (), + "publish", + True, + PUBLISH_OFFER_SENTENCE, + ), + ( + "row4-obsidian-writable-connector", + {"provider": "obsidian", "vault_path": "@VAULT@"}, + ("--connector", "xtiles"), + "publish", + True, + PUBLISH_OFFER_SENTENCE, + ), + ( + "row5-obsidian-unwritable", + {"provider": "obsidian", "vault_path": "@NOT_A_DIR@"}, + (), + "publish", + True, + PUBLISH_OFFER_SENTENCE, + ), + ("row6-xtiles-no-connector", {"provider": "xtiles"}, (), "none", False, ""), + ( + "row7-xtiles-connector", + {"provider": "xtiles"}, + ("--connector", "xtiles"), + "xtiles", + True, + XTILES_OFFER_SENTENCE, + ), +] + + +def _materialise(tmp_path: Path, section: dict | None) -> Path: + mapping: dict = {"topics": []} + if section is not None: + section = dict(section) + if section.get("vault_path") == "@VAULT@": + vault = tmp_path / "vault" + (vault / ".obsidian").mkdir(parents=True, exist_ok=True) + section["vault_path"] = str(vault) + elif section.get("vault_path") == "@NOT_A_DIR@": + not_a_dir = tmp_path / "not-a-dir" + not_a_dir.write_text("occupied") + section["vault_path"] = str(not_a_dir) + mapping["second_brain"] = section + return _config(tmp_path, mapping) + + +@pytest.mark.parametrize( + ("row_id", "section", "cli_args", "channel", "offer", "sentence"), + _ROWS, + ids=[row[0] for row in _ROWS], +) +def test_cli_matches_the_truth_table_row( + tmp_path: Path, + row_id: str, + section: dict | None, + cli_args: tuple[str, ...], + channel: str, + offer: bool, + sentence: str, +) -> None: + payload = _decide(_materialise(tmp_path, section), *cli_args) + + assert payload["channel"] == channel + assert payload["offer"] is offer + # Byte-identical, never a substring: the sentence IS the contract. + assert payload["sentence"] == sentence + assert payload["reason"] + + +def test_row8_unknown_provider_fails_naming_the_provider(tmp_path: Path) -> None: + config = _config(tmp_path, {"topics": [], "second_brain": {"provider": "notion"}}) + result = _run_wind_down(config, "--json") + + assert result.returncode != 0 + combined = result.stdout + result.stderr + assert "notion" in combined + + +def test_the_whole_table_as_one_artefact(tmp_path: Path) -> None: + """One run over every decidable row, written as the WD-2 artefact.""" + observed = [] + for row_id, section, cli_args, channel, offer, sentence in _ROWS: + row_dir = tmp_path / row_id + row_dir.mkdir() + payload = _decide(_materialise(row_dir, section), *cli_args) + observed.append( + { + "row": row_id, + "expected": {"channel": channel, "offer": offer, "sentence": sentence}, + "observed": payload, + "verdict": ( + "pass" + if (payload["channel"], payload["offer"], payload["sentence"]) + == (channel, offer, sentence) + else "FAIL" + ), + } + ) + + evidence_dir = Path(os.environ.get("STUDYLOOP_EVIDENCE_DIR", tmp_path)) + artefact = evidence_dir / "wind-down-truth-table.json" + artefact.write_text(json.dumps(observed, indent=2)) + + failures = [row["row"] for row in observed if row["verdict"] != "pass"] + assert not failures, f"rows disagreeing with the truth table: {failures} ({artefact})" + + +def test_human_form_prints_the_same_decision(tmp_path: Path) -> None: + config = _materialise(tmp_path, {"provider": "xtiles"}) + result = _run_wind_down(config, "--connector", "xtiles") + + assert result.returncode == 0 + assert "channel: xtiles" in result.stdout + assert "offer: True" in result.stdout diff --git a/packages/studyloop/tests/test_wind_down_decision.py b/packages/studyloop/tests/test_wind_down_decision.py new file mode 100644 index 000000000..aa57a9d82 --- /dev/null +++ b/packages/studyloop/tests/test_wind_down_decision.py @@ -0,0 +1,144 @@ +"""WD-1 — the wind-down decision truth table, as a pure function. + +Every row of the acceptance-suite truth table +(`reviews/2026-09-04-acceptance-harness/PLAN.md`), asserted against +:func:`studyloop.second_brain.wind_down.decide_wind_down` with hand-built +``BrainDescription`` values — no config file, no probe, no I/O. If any row's +``channel`` or ``offer`` flips, exactly one of these tests goes red. + +The two-rules-not-one-conjunction property (D1) gets its own tests: the +publish sentence must never be offered to an xTiles learner, and the xTiles +channel must not be computed from ``supports_publish``. +""" + +from __future__ import annotations + +import pytest + +from studyloop.second_brain.core import ( + BrainDescription, + NullBackend, + XtilesStageOneBackend, +) +from studyloop.second_brain.wind_down import ( + PUBLISH_OFFER_SENTENCE, + XTILES_OFFER_SENTENCE, + decide_wind_down, +) + + +def _obsidian_description(*, available: bool) -> BrainDescription: + """What ObsidianBackend.describe() reports; only ``available`` varies.""" + return BrainDescription( + provider="obsidian", + configured=True, + available=available, + supports_publish=True, + supports_pull_notes=True, + vault_path="/tmp/vault", + folder="Study", + detail="test double", + ) + + +# --------------------------------------------------------------------------- +# The 8 rows. Descriptions for none/xtiles come from the REAL backends so a +# drift in what they report flips these rows rather than a test double. +# --------------------------------------------------------------------------- + + +class TestTruthTable: + def test_row_1_provider_none_no_connector(self) -> None: + decision = decide_wind_down(NullBackend().describe(), ()) + assert (decision.channel, decision.offer) == ("none", False) + assert decision.sentence == "" + + def test_row_2_provider_none_connector_present(self) -> None: + """An attached connector alone is not consent — provider gates it.""" + decision = decide_wind_down(NullBackend().describe(), ("xtiles",)) + assert (decision.channel, decision.offer) == ("none", False) + assert decision.sentence == "" + + def test_row_3_obsidian_writable_no_connector(self) -> None: + decision = decide_wind_down(_obsidian_description(available=True), ()) + assert (decision.channel, decision.offer) == ("publish", True) + assert decision.sentence == PUBLISH_OFFER_SENTENCE + + def test_row_4_obsidian_writable_connector_present(self) -> None: + """Publish outranks the connector: an Obsidian learner gets ONE offer.""" + decision = decide_wind_down(_obsidian_description(available=True), ("xtiles",)) + assert (decision.channel, decision.offer) == ("publish", True) + assert decision.sentence == PUBLISH_OFFER_SENTENCE + + def test_row_5_obsidian_unwritable_vault_offer_stands(self) -> None: + """``available`` is a runtime condition the publish itself reports.""" + decision = decide_wind_down(_obsidian_description(available=False), ()) + assert (decision.channel, decision.offer) == ("publish", True) + assert decision.sentence == PUBLISH_OFFER_SENTENCE + + def test_row_6_xtiles_no_connector(self) -> None: + decision = decide_wind_down(XtilesStageOneBackend().describe(), ()) + assert (decision.channel, decision.offer) == ("none", False) + assert decision.sentence == "" + + def test_row_7_xtiles_connector_present(self) -> None: + decision = decide_wind_down(XtilesStageOneBackend().describe(), ("xtiles",)) + assert (decision.channel, decision.offer) == ("xtiles", True) + assert decision.sentence == XTILES_OFFER_SENTENCE + + def test_row_8_unknown_provider_raises_config_error(self) -> None: + """Unknown providers never reach the decision — get_backend raises.""" + from studyloop.second_brain import get_backend + from studyloop.settings import ConfigError, SecondBrainConfig, Settings + + settings = Settings(second_brain=SecondBrainConfig(provider="notion")) + with pytest.raises(ConfigError, match="notion"): + get_backend(settings) + + +# --------------------------------------------------------------------------- +# D1 — two rules, not one conjunction +# --------------------------------------------------------------------------- + + +class TestTwoRulesNotOneConjunction: + def test_the_publish_sentence_never_reaches_an_xtiles_learner(self) -> None: + for connectors in ((), ("xtiles",), ("xtiles", "playwright")): + decision = decide_wind_down(XtilesStageOneBackend().describe(), connectors) + assert decision.sentence != PUBLISH_OFFER_SENTENCE + assert decision.channel != "publish" + + def test_the_xtiles_channel_is_not_permanently_false(self) -> None: + """The straw man computed everything from supports_publish, which + XtilesStageOneBackend sets False on purpose — the suite would have + 'passed' by asserting silence in the one state whose point is an offer.""" + decision = decide_wind_down(XtilesStageOneBackend().describe(), ("xtiles",)) + assert decision.offer is True + + def test_connector_matching_is_by_exact_name(self) -> None: + decision = decide_wind_down( + XtilesStageOneBackend().describe(), ("xtiles-staging", "my-xtiles") + ) + assert (decision.channel, decision.offer) == ("none", False) + + +# --------------------------------------------------------------------------- +# The payload shape the harness (and the agent) reads +# --------------------------------------------------------------------------- + + +class TestPayload: + def test_json_dict_carries_exactly_the_four_ruled_fields(self) -> None: + """No ``command`` field: nothing guarantees a plan id at wind-down.""" + payload = decide_wind_down(NullBackend().describe(), ()).to_json_dict() + assert sorted(payload) == ["channel", "offer", "reason", "sentence"] + + def test_every_decision_names_a_reason(self) -> None: + cases = [ + (NullBackend().describe(), ()), + (XtilesStageOneBackend().describe(), ()), + (XtilesStageOneBackend().describe(), ("xtiles",)), + (_obsidian_description(available=True), ()), + ] + for description, connectors in cases: + assert decide_wind_down(description, connectors).reason diff --git a/packages/studyloop/tests/test_xtiles_stub_server.py b/packages/studyloop/tests/test_xtiles_stub_server.py new file mode 100644 index 000000000..c26e76746 --- /dev/null +++ b/packages/studyloop/tests/test_xtiles_stub_server.py @@ -0,0 +1,159 @@ +"""WD-4 — the stub ``xtiles`` MCP server speaks MCP, logs every call, no network. + +The stub's one job is to make "a connector named ``xtiles`` is attached" real +for the transcript acceptance tests. So the properties pinned here are exactly +the ones those tests lean on: the server completes a real stdio handshake +under the name ``xtiles``, exposes the three write tools the prompts select +between, appends every call to the JSON artefact, and cannot reach a network. + +Red when: a call is unlogged, a tool disappears, the server name drifts, or a +network-capable import creeps into the stub. +""" + +from __future__ import annotations + +import ast +import json +import sys +from pathlib import Path + +import pytest + +pytest.importorskip("mcp") + +from mcp import ClientSession, StdioServerParameters +from mcp.client.stdio import stdio_client + +STUB = Path(__file__).parent / "_xtiles_stub_server.py" + +EXPECTED_TOOLS = { + "xtiles_create_tasks", + "xtiles_create_tiles_from_markdown_in_my_planner", + "xtiles_create_view_from_markdown", +} + +#: Modules that can open a connection. The stub's transport is stdio and its +#: whole value is being offline; one of these appearing in it is the defect. +_NETWORK_MODULES = { + "socket", + "ssl", + "http", + "urllib", + "urllib3", + "requests", + "httpx", + "aiohttp", + "websockets", +} + + +def _server_params(tmp_path: Path) -> tuple[StdioServerParameters, Path]: + log = tmp_path / "stub-calls.json" + params = StdioServerParameters( + command=sys.executable, + args=[str(STUB)], + env={"XTILES_STUB_CALL_LOG": str(log)}, + ) + return params, log + + +@pytest.mark.asyncio +async def test_handshake_tools_and_call_log(tmp_path: Path) -> None: + """initialize → tools/list → one call per tool → every call in the artefact.""" + params, log = _server_params(tmp_path) + + async with ( + stdio_client(params) as (read, write), + ClientSession(read, write) as session, + ): + init = await session.initialize() + # The NAME is the gate: the wind-down skill's second half is "an MCP + # server named `xtiles` is connected", so a drifted name here would + # make the whole harness test a connector the skill cannot see. + assert init.serverInfo.name == "xtiles" + + tools = {t.name for t in (await session.list_tools()).tools} + assert tools == EXPECTED_TOOLS + + task_call = await session.call_tool( + "xtiles_create_tasks", + {"tasks": [{"title": "Study: decorators"}]}, + ) + assert not task_call.isError + + tile_call = await session.call_tool( + "xtiles_create_tiles_from_markdown_in_my_planner", + {"period": "day", "date": "2026-09-04", "markdown": "### Study: decorators"}, + ) + assert not tile_call.isError + # The one bit of response fidelity the prompts depend on: the planner + # tile is the shape that returns a URL. + tile_payload = tile_call.content[0] + assert tile_payload.type == "text" + assert "resource_url" in tile_payload.text + + page_call = await session.call_tool( + "xtiles_create_view_from_markdown", + {"projectId": "stub-project", "markdown": "## LR — page"}, + ) + assert not page_call.isError + + calls = json.loads(log.read_text(encoding="utf-8")) + assert [c["tool"] for c in calls] == [ + "xtiles_create_tasks", + "xtiles_create_tiles_from_markdown_in_my_planner", + "xtiles_create_view_from_markdown", + ], "a call was dropped from the log, or logged out of order" + assert calls[0]["arguments"]["tasks"] == [{"title": "Study: decorators"}] + assert calls[1]["arguments"]["markdown"] == "### Study: decorators" + + +@pytest.mark.asyncio +async def test_every_call_is_appended_not_overwritten(tmp_path: Path) -> None: + """Two calls to the SAME tool → two log entries. A log that keeps only the + last call would grade a multi-write transcript as a single write.""" + params, log = _server_params(tmp_path) + + async with ( + stdio_client(params) as (read, write), + ClientSession(read, write) as session, + ): + await session.initialize() + for title in ("first", "second"): + await session.call_tool("xtiles_create_tasks", {"tasks": [{"title": title}]}) + + calls = json.loads(log.read_text(encoding="utf-8")) + assert len(calls) == 2 + assert calls[0]["arguments"]["tasks"][0]["title"] == "first" + assert calls[1]["arguments"]["tasks"][0]["title"] == "second" + + +def test_unconfigured_stub_refuses_to_run() -> None: + """No log path → exit before the handshake. A stub that ran unlogged would + pass WD-5's silence checks vacuously — the exact trap (D4) this suite is + built to avoid.""" + import subprocess + + result = subprocess.run( + [sys.executable, str(STUB)], + capture_output=True, + text=True, + env={"PATH": "/usr/bin:/bin"}, + timeout=30, + ) + assert result.returncode == 2 + assert "XTILES_STUB_CALL_LOG" in result.stderr + + +def test_the_stub_imports_nothing_that_can_open_a_socket() -> None: + """Structural half of "reaches no network": stdio transport by + construction, plus no network-capable import in the stub itself.""" + tree = ast.parse(STUB.read_text(encoding="utf-8")) + imported: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + imported.update(alias.name.split(".")[0] for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + imported.add(node.module.split(".")[0]) + offenders = imported & _NETWORK_MODULES + assert not offenders, f"the stub imports network-capable modules: {sorted(offenders)}" diff --git a/scripts/check-release-consistency.py b/scripts/check-release-consistency.py index 1b9af6d42..f75f90cb4 100755 --- a/scripts/check-release-consistency.py +++ b/scripts/check-release-consistency.py @@ -2,6 +2,8 @@ from __future__ import annotations import argparse +import re +import subprocess import sys import tomllib import zipfile @@ -96,6 +98,168 @@ def validate_sdist(repo_root: Path, version: str) -> None: raise ValueError(f"missing source distribution: dist/studyloop-{version}.tar.gz") +def _git(repo_root: Path, *args: str) -> str: + return subprocess.run( + ["git", *args], + cwd=repo_root, + capture_output=True, + text=True, + check=True, + timeout=60, + ).stdout.strip() + + +def _deferred_reason(change_dir: Path) -> str | None: + """The change's explicit ``deferred:`` reason, or None. + + Read with a line match rather than a YAML parser so this script keeps its + stdlib-only property. A bare ``deferred:`` with no reason does NOT count — + an unexplained deferral is indistinguishable from a forgotten one, which is + the state this guard exists to catch. + """ + meta = change_dir / ".openspec.yaml" + if not meta.is_file(): + return None + for line in meta.read_text(encoding="utf-8").splitlines(): + match = re.match(r"\s*deferred:\s*(\S.*)$", line) + if match: + return match.group(1).strip() + return None + + +def validate_openspec_changes_shipped(repo_root: Path) -> None: + """Release mode only: a change that shipped work must be archived. + + The 0.2.0 cut shipped the whole second-brain layer while its change sat at + 0/19 tasks, unarchived — nothing read change state, so nothing objected + (2026-09-04 review, Q5). This fails the release gate when any directory + under ``openspec/changes/`` (``archive/`` excluded) has commits since the + last tag and is not archived, unless its ``.openspec.yaml`` carries an + explicit ``deferred: <reason>``. + + Deliberately NOT part of preflight: open changes are legal during a cycle; + only shipping one is not. + """ + changes_dir = repo_root / "openspec" / "changes" + if not changes_dir.is_dir(): + return + try: + last_tag = _git(repo_root, "describe", "--tags", "--abbrev=0") + except subprocess.CalledProcessError: + return # no tag yet: the first release has nothing to compare against + + offenders: list[str] = [] + for change_dir in sorted(p for p in changes_dir.iterdir() if p.is_dir()): + if change_dir.name == "archive": + continue + touched = _git( + repo_root, + "log", + "--oneline", + f"{last_tag}..HEAD", + "--", + str(change_dir.relative_to(repo_root)), + ) + if not touched: + continue + reason = _deferred_reason(change_dir) + if reason: + print(f"openspec change {change_dir.name!r} deferred: {reason}") + continue + offenders.append(change_dir.name) + + if offenders: + raise ValueError( + "openspec change(s) with commits since " + f"{last_tag} are neither archived nor deferred: {', '.join(offenders)}. " + "Reconcile and run `openspec archive <name>`, or add " + "`deferred: <reason>` to the change's .openspec.yaml." + ) + + +def validate_new_archives(repo_root: Path) -> None: + """Archive entries added since the last tag must pass ``openspec validate``. + + Scoped to NEW archives, not ``--archived --all``: an archive from July + predates this guard and has unticked tasks nobody has evidence to + reconcile; re-failing every future release on it would teach people to + ignore the gate. Soft-skips when the openspec CLI is absent — the same + convention as ``just spec-check``. + """ + import shutil + + if shutil.which("openspec") is None: + print("openspec CLI not found — skipping archived-change validation") + return + try: + last_tag = _git(repo_root, "describe", "--tags", "--abbrev=0") + except subprocess.CalledProcessError: + return + + changed = _git( + repo_root, "diff", "--name-only", f"{last_tag}..HEAD", "--", "openspec/changes/archive/" + ) + new_names = sorted( + {parts[3] for line in changed.splitlines() if len(parts := line.split("/")) > 4} + ) + if not new_names: + return + + report = subprocess.run( + ["openspec", "validate", "--archived", "--all", "--no-interactive"], + cwd=repo_root, + capture_output=True, + text=True, + timeout=300, + check=False, + ).stdout + failing = [name for name in new_names if f"✓ change/{name}" not in report] + if failing: + raise ValueError( + f"newly archived openspec change(s) failed validation: {', '.join(failing)}. " + "Run `openspec validate --archived --all` for the detail." + ) + print(f"openspec archives validated: {', '.join(new_names)}") + + +def validate_adr_statuses(repo_root: Path) -> None: + """An ADR that predates the latest tag must not still say Proposed. + + ADR-0010 shipped in 0.2.0 still marked Proposed — the decision was acted + on, released, and its record claimed it was still being considered. Runs + in every mode (always-on, per the Q5 ruling): an ADR's status is a + statement of fact whenever it is read, not only at release time. + """ + adr_dir = repo_root / "docs" / "adr" + if not adr_dir.is_dir(): + return + try: + last_tag = _git(repo_root, "describe", "--tags", "--abbrev=0") + except subprocess.CalledProcessError: + return + + stale: list[str] = [] + for adr in sorted(adr_dir.glob("[0-9]*.md")): + in_tag = subprocess.run( + ["git", "cat-file", "-e", f"{last_tag}:{adr.relative_to(repo_root)}"], + cwd=repo_root, + capture_output=True, + timeout=60, + check=False, + ) + if in_tag.returncode != 0: + continue # added after the tag; Proposed is honest for it + head = adr.read_text(encoding="utf-8", errors="replace")[:600] + if re.search(r"\*\*Status:\*\*\s*Proposed", head): + stale.append(adr.name) + if stale: + raise ValueError( + f"ADR(s) released in {last_tag} still say Status: Proposed: " + f"{', '.join(stale)}. A shipped decision is Accepted (or Superseded) — " + "update the ADR and its docs/adr/README.md row." + ) + + def parse_args(argv: list[str]) -> argparse.Namespace: parser = argparse.ArgumentParser( description="Check StudyLoop release notes and wheel metadata match pyproject version.", @@ -111,6 +275,15 @@ def parse_args(argv: list[str]) -> argparse.Namespace: action="store_true", help="Skip built wheel METADATA validation.", ) + parser.add_argument( + "--release", + action="store_true", + help=( + "Also fail on unarchived, undeferred openspec changes with commits " + "since the last tag (release-check mode; open changes are legal " + "during a cycle, so preflight does not pass this)." + ), + ) return parser.parse_args(argv) @@ -122,10 +295,21 @@ def main(argv: list[str] | None = None) -> int: version = read_studyloop_version(repo_root) validate_root_version_matches_package(repo_root, version) validate_release_note(repo_root, version) + validate_adr_statuses(repo_root) + if args.release: + validate_openspec_changes_shipped(repo_root) + validate_new_archives(repo_root) if not args.skip_wheel: validate_sdist(repo_root, version) validate_wheel_metadata(repo_root, version) - except (OSError, tomllib.TOMLDecodeError, ValueError, zipfile.BadZipFile) as exc: + except ( + OSError, + subprocess.CalledProcessError, + subprocess.TimeoutExpired, + tomllib.TOMLDecodeError, + ValueError, + zipfile.BadZipFile, + ) as exc: print(f"release consistency failed: {exc}", file=sys.stderr) return 1 diff --git a/scripts/openspec-gate.py b/scripts/openspec-gate.py new file mode 100644 index 000000000..cc50d6377 --- /dev/null +++ b/scripts/openspec-gate.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +"""OpenSpec early-warning hook — one body, three harness wrappers. + +EARLY WARNING, NOT ENFORCEMENT. The hard gate is `just release-check` +(scripts/check-release-consistency.py --release) plus CI; this hook exists so +a release action is questioned at the keyboard instead of failing twenty +minutes later. Per the 2026-09-04 arbitration (Q5 step 3), and per its own +caution: a hook that cannot be shown to block must not be described as +enforcement. Verification state of the block mechanism, per harness: + +* Kiro CLI — VERIFIED against the harness's own hook documentation + (PreToolUse, exit 2 blocks; stderr forwarded). +* Claude Code — DOCUMENTED by the vendor (PreToolUse, exit 2 blocks); + not attested by a recorded run in this repository. +* Codex — events VERIFIED against the vendor hooks page in the + 2026-09-04 review; the exit-2 block is documented there. + +Behaviour (stdin carries the tool-call JSON; all three harnesses use the +``tool_input.command`` shape for their shell tool): + +* ``git tag …`` or ``prepare-release`` → run the REAL release guard + (imported from check-release-consistency.py, never a re-implementation); + exit 2 with the guard's message when an openspec change with commits since + the last tag is neither archived nor deferred. +* ``git commit …`` → same check, but WARN on stdout and exit 0: open changes + are legal during a cycle, so a commit is never blocked. +* anything else → exit 0 immediately, no subprocess spawned. +* ``remind`` mode (UserPromptSubmit) → one terse line when the guard would + fail, silence otherwise; always exit 0. + +Fails OPEN on its own errors (exit 0): a broken early warning must never +block work the hard gate would allow. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import re +import sys +from pathlib import Path + +_RELEASE_RE = re.compile(r"\bgit\s+tag\b|\bprepare-release\b") +_COMMIT_RE = re.compile(r"\bgit\s+commit\b") + + +def _load_release_guard(repo_root: Path): + """Import validate_openspec_changes_shipped from the real gate script. + + Imported, never copied: two implementations of "what counts as shipped" + is how a hook and a release gate come to disagree. + """ + path = repo_root / "scripts" / "check-release-consistency.py" + spec = importlib.util.spec_from_file_location("_release_consistency", path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module.validate_openspec_changes_shipped + + +def _guard_failure(repo_root: Path) -> str | None: + """The release guard's message when it would fail, else None.""" + try: + _load_release_guard(repo_root)(repo_root) + except ValueError as exc: + return str(exc) + return None + + +def _command_from_stdin() -> str: + try: + payload = json.load(sys.stdin) + except Exception: + return "" + tool_input = payload.get("tool_input") or payload.get("toolInput") or {} + if isinstance(tool_input, dict): + return str(tool_input.get("command", "")) + return "" + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "mode", nargs="?", default="pre-tool-use", choices=["pre-tool-use", "remind"] + ) + parser.add_argument("--repo-root", type=Path, default=Path(__file__).resolve().parents[1]) + args = parser.parse_args() + repo_root = args.repo_root.resolve() + + try: + if args.mode == "remind": + failure = _guard_failure(repo_root) + if failure: + print(f"openspec early warning: {failure}") + return 0 + + command = _command_from_stdin() + if _RELEASE_RE.search(command): + failure = _guard_failure(repo_root) + if failure: + print( + f"openspec gate: {failure}\n" + "(early warning; the hard gate is `just release-check`)", + file=sys.stderr, + ) + return 2 + return 0 + if _COMMIT_RE.search(command): + failure = _guard_failure(repo_root) + if failure: + # Warn only: open changes are LEGAL during a cycle. Blocking + # commits would gate normal work on a release-time rule. + print(f"openspec reminder (not blocking): {failure}") + return 0 + return 0 + except Exception as exc: # fail OPEN — see module docstring + print(f"openspec gate skipped (internal error: {exc})", file=sys.stderr) + return 0 + + +if __name__ == "__main__": + sys.exit(main())