docs: read Superlog as source, and file what it says Sync is missing - #2
Merged
Conversation
Four documents, all planning. Nothing scheduled and no code. The reference note is the substance: Superlog's investigation pipeline read from a local checkout at b11acc1, in the form references/notes/supabase-control-plane-mechanism.md set -- mechanism and the authors' stated reasoning, path:line citations, no component copied and no class string. Nine sections: the Incident/Issue fan-in, the incident_events table and the check constraint that makes its nullable parent columns safe, the timeline assembled at read time from four sources rather than stored, the run state machine and the two parked states Sync has no equivalent of, the outcome tool contract with the three rules it carries, the runner seam, repository scoring, queue topology, and what deliberately was not recorded. Their runner is not open -- backend.ts loads it through AGENT_RUNNER_ANTHROPIC_MODULE and only a static model-free runner ships. That turned out to be the useful half: an implementation is one way to satisfy a contract, and the contract is what transfers. M8 through M11 turn that read into milestones. M8 puts the model call behind a protocol so the remediation suite runs without a key. M9 gives a run more outcomes than a diff and an abandonment. M10 parks a run instead of ending it. M11 groups findings sharing a vendor change into one remediation. B122 is per-repository context, with its design and seven-task plan. Superlog probing a repository for CLAUDE.md and AGENTS.md before handing it to an agent is independent corroboration of its seed-file half. Numbering corrected before landing: an earlier draft filed this as B116 against a local main that was 207 commits behind origin/main. B116 is taken, B121 is the real high-water mark, and that draft also claimed graph_views was unmerged. It is not, and nothing here is blocked on it. Co-Authored-By: Claude Opus 5 <[email protected]>
📝 WalkthroughWalkthroughThe pull request adds planning and design documentation for the M8–M11 resolution loop and durable per-repository context. It updates the backlog, records Superlog research, and specifies storage, prompt, CLI, API, MCP, validation, and execution requirements. ChangesResolution loop
Repository context
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-08-06-sync-repo-context.md (1)
151-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the machine-specific working directory.
The command uses
/c/Users/strol/orca/workspaces/Sync/repo-context. This path only exists on one workstation and prevents other implementers from following the plan.Run from the repository root or use a relative path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md` around lines 151 - 154, Update the command sequence in the repository-context plan to remove the machine-specific cd path, so the Docker and pytest commands run from the repository root or use a repository-relative location.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/BACKLOG.md`:
- Around line 1001-1005: Align B122's instruction-file scope with the source
note by either adding .cursor/rules/* to its ingestion and acceptance criteria
or explicitly documenting that Sync intentionally excludes it; update the
relevant backlog wording so both documents define the same repository-context
inputs.
In `@docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md`:
- Line 120: Update the opening state-machine code fences to specify the text
language in both affected files: use ```text at
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:120-120 and
docs/superpowers/references/notes/superlog-investigation-mechanism.md:121-121.
No other fence content requires changes.
- Around line 78-83: Define one canonical Sync outcome contract in the plan at
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:78-83,
including resolve_incident for M11 and specifying whether terminal outcomes
occur once per turn or run. Update
docs/superpowers/references/notes/superlog-investigation-mechanism.md:151-159 to
use the same names and cardinality, or explicitly document its mapping to the
canonical contract; ensure resumed-run behavior follows that contract.
- Around line 120-124: Add the same language tag, using text or mermaid, to the
opening fence of both state-machine blocks:
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md lines 120-124
and docs/superpowers/references/notes/superlog-investigation-mechanism.md lines
121-125. No other diagram content requires changes.
- Around line 126-133: Expand the resolution-loop plan to define idempotent
PR-event resumption: specify the provider event ID or deduplication key,
accepted actions (comment, merge, and close), and how events associate with a
parked run. Document ordering requirements, an atomic parked-to-resuming
transition, and idempotent follow-up effects so webhook retries and concurrent
workers cannot duplicate commits or resumes. Require a durable uniqueness
constraint and one ownership path for forge side effects.
- Around line 160-166: Specify the explicit grouping key and fallback behavior
for findings where Finding.vendor_change_id is NULL, ensuring null values are
not implicitly grouped together. Define how each resulting unit produces its
resolution and how attempts are counted in the corpus, then update the “Evidence
that closes it” section to reflect this behavior.
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md`:
- Around line 533-539: Update the expected pytest result in the Step 7 test
instructions to match the 11 tests defined by the referenced test files, or add
the missing test if 12 is intentional. Keep the existing test command unchanged.
- Around line 1022-1047: Update the test fixture’s context_writer to mutate
stored with the posted body and source, then extend
test_post_writes_and_returns_the_view to assert response.json() contains the
updated body and source in addition to verifying written.
- Around line 459-460: Normalize the body with strip() before enforcing
CONTEXT_BODY_MAX in set_repo_context, matching the existing read_seed and
context_set behavior. Ensure the API path validates and stores the normalized
body consistently, including the corresponding writer logic at the other
referenced locations.
- Around line 1462-1470: Update
test_the_instructions_name_no_tool_that_does_not_exist to send an initialize
request and inspect its actual response payload, asserting that the returned
instructions field equals SERVER_INSTRUCTIONS. Keep the existing published-tool
name validation, and apply the same response assertion to the corresponding test
at the additional location.
- Around line 844-851: Update the run flow around seed_repo_context so its
repo_context upsert is included in the same transaction as indexing, or deferred
until indexing completes successfully; do not commit seed data before the
existing graph-write rollback boundary. Add a test that forces indexing to fail
and verifies both the graph changes and seeded repo_context are rolled back.
- Around line 239-274: Enforce RepoContext invariants at the persistence
boundary used by upsert_repo_context and repo_context: require source to belong
to CONTEXT_SOURCES and body to be non-empty and at most 8000 characters. Add the
constraint at the database/model boundary where appropriate, and ensure
repo_context rejects or does not return existing rows that violate these rules
rather than delivering them to prompts.
- Around line 824-840: The seed_repo_context function must remove stale
seeded-file context when read_seed(repo.local_path) returns None, while
preserving operator-authored rows. Update the None path to delete only the
existing RepoContext row whose source is "seeded-file"; retain the current
upsert behavior for valid content, and add tests covering deletion and
invalidation.
- Around line 654-669: Rewrite the prompt-edit instructions to target the
current sections list in agent_patch.py: locate the fenced_block(VENDOR,
[finding.rationale]) entry followed by _SCOPE_RULES, insert the rendered
repository-context section between the stable vendor/rationale content and
_SCOPE_RULES only when non-empty, and preserve diagnostics after the stable
sections.
- Around line 441-462: Update read_seed and its caller seed_repo_context so
rejected seed files retain diagnostic information instead of collapsing every
failure to None. Log unreadable/non-UTF-8 errors and oversize files with the
path and CONTEXT_BODY_MAX limit, while preserving the existing behavior for
absent, empty, or whitespace-only files. Add a test covering the required
rejection diagnostic.
- Around line 1498-1559: Update CONTEXT_URI_TEMPLATE to use RFC 6570 reserved
expansion with `{+repo_id}` so repository IDs retain their slashes in advertised
URIs. Preserve the existing _read_context suffix handling and ensure the
template expansion and read flow pass `github.com/acme/storefront` unchanged to
context_reader, adding or updating coverage for that behavior.
In `@docs/superpowers/references/notes/superlog-investigation-mechanism.md`:
- Around line 51-57: Clarify the vendor-change grouping rule in
docs/superpowers/references/notes/superlog-investigation-mechanism.md:51-57 so
only non-null vendor_change_id values form grouping keys, and explicitly define
the fallback behavior for findings without a vendor change. Apply the same rule
to the M11 implementation contract in
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:160-166; both
sites require documentation updates only.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md`:
- Around line 151-154: Update the command sequence in the repository-context
plan to remove the machine-specific cd path, so the Docker and pytest commands
run from the repository root or use a repository-relative location.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d05a8c1c-da76-4c1c-9e4e-a8a2023d2e4b
📒 Files selected for processing (5)
docs/superpowers/BACKLOG.mddocs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.mddocs/superpowers/plans/2026-08-06-sync-repo-context.mddocs/superpowers/references/notes/superlog-investigation-mechanism.mddocs/superpowers/specs/2026-08-06-sync-repo-context-design.md
Comment on lines
+1001
to
+1005
| **Independent corroboration.** Superlog probes a candidate repository's default branch for | ||
| `CLAUDE.md`, `AGENTS.md`, `.cursorrules` and `.github/copilot-instructions.md` before handing it to | ||
| an agent, so the agent follows the repository's conventions | ||
| (`references/notes/superlog-investigation-mechanism.md`, section 7). Same instinct, arrived at | ||
| independently — evidence for the seed-file half of this design rather than the operator-written |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the instruction-file list with the source note.
docs/superpowers/references/notes/superlog-investigation-mechanism.md, Lines 229-234, also lists .cursor/rules/*. Either include that source in B122's ingestion and acceptance scope, or state that Sync intentionally excludes it. Otherwise the backlog and source study define different repository-context inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/BACKLOG.md` around lines 1001 - 1005, Align B122's
instruction-file scope with the source note by either adding .cursor/rules/* to
its ingestion and acceptance criteria or explicitly documenting that Sync
intentionally excludes it; update the relevant backlog wording so both documents
define the same repository-context inputs.
Comment on lines
+78
to
+83
| **What to build.** A tool contract in two tiers, which is Superlog's shape: | ||
|
|
||
| - **Non-terminal:** `report_findings` — summary, root cause, confidence, estimated impact. | ||
| Callable repeatedly; every call revises. | ||
| - **Terminal, one per run:** `propose_patch`, `report_external_cause`, `ask_human`, `abandon`. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use one canonical outcome contract across both documents.
The documents disagree on terminal names and scope. The plan omits resolve_incident, which M11 requires, and says one terminal outcome per run. The research note includes resolve_incident and says one terminal outcome per turn. This can produce incompatible schemas and incorrect resumed-run behavior.
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L78-L83: define canonical names, include the M11 fan-in operation, and specify terminal cardinality.docs/superpowers/references/notes/superlog-investigation-mechanism.md#L151-L159: align the research-derived names or document an explicit mapping to the canonical Sync contract.
📍 Affects 2 files
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L78-L83(this comment)docs/superpowers/references/notes/superlog-investigation-mechanism.md#L151-L159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md` around
lines 78 - 83, Define one canonical Sync outcome contract in the plan at
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:78-83,
including resolve_incident for M11 and specifying whether terminal outcomes
occur once per turn or run. Update
docs/superpowers/references/notes/superlog-investigation-mechanism.md:151-159 to
use the same names and cardinality, or explicitly document its mapping to the
canonical contract; ensure resumed-run behavior follows that contract.
|
|
||
| **What to build.** A run that parks instead of ending. Superlog's state machine is the reference: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both state-machine fences.
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L120-L120: change the opening fence to```text.docs/superpowers/references/notes/superlog-investigation-mechanism.md#L121-L121: change the opening fence to```text.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L120-L120(this comment)docs/superpowers/references/notes/superlog-investigation-mechanism.md#L121-L121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md` at line
120, Update the opening state-machine code fences to specify the text language
in both affected files: use ```text at
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:120-120 and
docs/superpowers/references/notes/superlog-investigation-mechanism.md:121-121.
No other fence content requires changes.
Source: Linters/SAST tools
Comment on lines
+120
to
+124
| ``` | ||
| queued → repo_discovery → running → { awaiting_human | awaiting_events | complete | failed } | ||
| ↓ a human replies, or a PR event arrives | ||
| resuming → running | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language tags to both state-machine fences.
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L120-L124: usetextormermaidafter the opening fence.docs/superpowers/references/notes/superlog-investigation-mechanism.md#L121-L125: use the same language tag after the opening fence.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L120-L124(this comment)docs/superpowers/references/notes/superlog-investigation-mechanism.md#L121-L125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md` around
lines 120 - 124, Add the same language tag, using text or mermaid, to the
opening fence of both state-machine blocks:
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md lines 120-124
and docs/superpowers/references/notes/superlog-investigation-mechanism.md lines
121-125. No other diagram content requires changes.
Source: Linters/SAST tools
Comment on lines
+126
to
+133
| `awaiting_events` is a run whose pull requests are out for review; the session stays durable and | ||
| PR events resume it. Their comment on `resuming` names the point: *"the heart of talking to an | ||
| investigation."* | ||
|
|
||
| Sync has the harder half already. LangGraph checkpoints in Postgres are a durable session store, | ||
| and `langgraph-checkpoint-postgres` is a declared dependency. What is missing is the parked states, | ||
| the event ingress (a GitHub webhook, which the forge is already authenticated for), and the rule | ||
| that a parked run is not ticked until something wakes it. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file ---'
sed -n '100,155p' docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md
printf '%s\n' '--- related references ---'
rg -n -i "event dedup|duplicate side.effect|duplicate side effect|awaiting_events|resuming|webhook|event ingress|idempotent|event ID|dedupe" docs .github 2>/dev/null | head -240
printf '%s\n' '--- candidate files ---'
git ls-files | rg -i 'sync|m8|m11|resolution|research|event|webhook|superpowers'Repository: stroland02/Sync
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Superlog event identity and state model ---'
sed -n '55,155p' docs/superpowers/references/notes/superlog-investigation-mechanism.md
printf '%s\n' '--- Superlog duplicate-side-effect discussion ---'
sed -n '230,275p' docs/superpowers/references/notes/superlog-investigation-mechanism.md
printf '%s\n' '--- plan-wide M10 and event references ---'
rg -n -C 4 -i "M10|awaiting_events|resuming|event|webhook|duplicate|idempot" docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md docs/superpowers/references/notes/superlog-investigation-mechanism.mdRepository: stroland02/Sync
Length of output: 29702
Define idempotent PR-event resumption.
Specify the provider event ID or dedupe key, accepted PR actions (comment, merge, close), run association, ordering rules, atomic parked-to-resuming transition, and idempotent follow-up side effects. Webhook retries and concurrent workers must not create duplicate commits or resumes. Use a durable uniqueness constraint and a single ownership path for forge side effects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md` around
lines 126 - 133, Expand the resolution-loop plan to define idempotent PR-event
resumption: specify the provider event ID or deduplication key, accepted actions
(comment, merge, and close), and how events associate with a parked run.
Document ordering requirements, an atomic parked-to-resuming transition, and
idempotent follow-up effects so webhook retries and concurrent workers cannot
duplicate commits or resumes. Require a durable uniqueness constraint and one
ownership path for forge side effects.
Comment on lines
+844
to
+851
| - [ ] **Step 4: Call it from `run`** | ||
|
|
||
| In `src/sync/cli.py`'s `run`, immediately after the clone or local-checkout resolution produces `repo` and the store is available, add: | ||
|
|
||
| ```python | ||
| if seed_repo_context(store, repo): | ||
| print(f"context: seeded from {SEED_RELATIVE_PATH} for {repo.repo_id}") | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep seed persistence inside the run transaction.
The plan calls seed_repo_context before indexing. GraphStore commits writes outside transaction(), while the existing transaction is the rollback boundary for graph writes. A later indexing failure can leave repo_context committed while the graph remains unchanged.
Move the upsert into the same transaction or commit it only after indexing succeeds. Add a failure and rollback test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md` around lines 844 -
851, Update the run flow around seed_repo_context so its repo_context upsert is
included in the same transaction as indexing, or deferred until indexing
completes successfully; do not commit seed data before the existing graph-write
rollback boundary. Add a test that forces indexing to fail and verifies both the
graph changes and seeded repo_context are rolled back.
Comment on lines
+1022
to
+1047
| @pytest.fixture() | ||
| def client(written): | ||
| stored = {"repo_id": "r", "body": "", "source": None, "updated_at": None} | ||
|
|
||
| def context_reader(repo_id: str) -> dict: | ||
| return {**stored, "repo_id": repo_id} | ||
|
|
||
| def context_writer(repo_id: str, body: str) -> None: | ||
| written.append((repo_id, body)) | ||
|
|
||
| return TestClient(app_with(context_reader=context_reader, context_writer=context_writer)) | ||
|
|
||
|
|
||
| def test_get_returns_the_view(client): | ||
| response = client.get("/api/repos/github.com%2Facme%2Fstorefront/context") | ||
| assert response.status_code == 200 | ||
| assert response.json()["body"] == "" | ||
|
|
||
|
|
||
| def test_post_writes_and_returns_the_view(client, written): | ||
| response = client.post( | ||
| "/api/repos/r/context", json={"body": "Package manager is pnpm."} | ||
| ) | ||
| assert response.status_code == 200 | ||
| assert written == [("r", "Package manager is pnpm.")] | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the POST test verify the returned view.
context_writer only appends to written; it does not update stored. The test also does not inspect response.json(). A handler can call the writer and return a stale or unrelated view while this test passes.
Update the fake state in the writer and assert the returned body and source.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md` around lines 1022 -
1047, Update the test fixture’s context_writer to mutate stored with the posted
body and source, then extend test_post_writes_and_returns_the_view to assert
response.json() contains the updated body and source in addition to verifying
written.
Comment on lines
+1462
to
+1470
| def test_the_instructions_name_no_tool_that_does_not_exist(): | ||
| from sync.mcp.registry import schemas_as_data | ||
|
|
||
| published = {schema["name"] for schema in schemas_as_data()} | ||
| for word in SERVER_INSTRUCTIONS.split(): | ||
| candidate = word.strip("`.,()") | ||
| if candidate.startswith("sync_"): | ||
| assert candidate in published, f"instructions name an absent tool: {candidate}" | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the initialize response.
The test inspects SERVER_INSTRUCTIONS but never sends an initialize request or checks the response. An implementation can define the constant and omit "instructions": SERVER_INSTRUCTIONS while these tests still pass.
Add an assertion against the actual initialize payload.
Also applies to: 1594-1598
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md` around lines 1462 -
1470, Update test_the_instructions_name_no_tool_that_does_not_exist to send an
initialize request and inspect its actual response payload, asserting that the
returned instructions field equals SERVER_INSTRUCTIONS. Keep the existing
published-tool name validation, and apply the same response assertion to the
corresponding test at the additional location.
Comment on lines
+1498
to
+1559
| CONTEXT_URI_PREFIX = "sync://context/" | ||
| CONTEXT_URI_TEMPLATE = f"{CONTEXT_URI_PREFIX}{{repo_id}}" | ||
| CONTEXT_MIME_TYPE = "text/markdown" | ||
| ``` | ||
|
|
||
| Add a second entry to `RESOURCE_TEMPLATES`: | ||
|
|
||
| ```python | ||
| ResourceTemplateSpec( | ||
| uri_template=CONTEXT_URI_TEMPLATE, | ||
| name="repository-context", | ||
| description=( | ||
| "What stays true of one repository while its code changes underneath: " | ||
| "conventions, generated directories, the package manager its lockfile names. " | ||
| "Written by an operator or copied from a `.sync/context.md` the repository " | ||
| "itself carries. Never a call site, a finding or telemetry." | ||
| ), | ||
| mime_type=CONTEXT_MIME_TYPE, | ||
| ), | ||
| ``` | ||
|
|
||
| Extend `read` with a trailing defaulted keyword and a branch, leaving the feed path exactly as it is: | ||
|
|
||
| ```python | ||
| def read( | ||
| uri: str, | ||
| feed: FeedCache | None, | ||
| known_vendors: tuple[str, ...], | ||
| context_reader: Callable[[str], str | None] | None = None, | ||
| ) -> dict[str, Any]: | ||
| if uri.startswith(CONTEXT_URI_PREFIX): | ||
| return _read_context(uri, context_reader) | ||
| if not uri.startswith(FEED_URI_PREFIX): | ||
| raise ResourceError(f"unknown resource: {uri}", UNKNOWN_RESOURCE, uri=uri) | ||
| # ... the existing feed body, unchanged ... | ||
| ``` | ||
|
|
||
| And the helper beside it: | ||
|
|
||
| ```python | ||
| def _read_context(uri: str, context_reader: Callable[[str], str | None] | None) -> dict[str, Any]: | ||
| """One repository's context, or a `ResourceError` naming why there is none. | ||
|
|
||
| The repository id keeps every slash after the prefix. A `repo_id` is `host/owner/name`, so | ||
| a parser that split on the first one would look up "github.com" and find nothing, for every | ||
| repository that exists. | ||
|
|
||
| A repository with no context is an error rather than an empty string. A client that received | ||
| "" could not tell "nobody has described this repository" from "somebody described it as | ||
| nothing", and would report the second. | ||
| """ | ||
| repo_id = uri[len(CONTEXT_URI_PREFIX):] | ||
| if context_reader is None: | ||
| raise ResourceError( | ||
| "this server serves no repository context", UNKNOWN_RESOURCE, uri=uri | ||
| ) | ||
| body = context_reader(repo_id) | ||
| if body is None: | ||
| raise ResourceError( | ||
| f"no context is held for '{repo_id}'", UNKNOWN_RESOURCE, uri=uri | ||
| ) | ||
| return {"contents": [{"uri": uri, "mimeType": CONTEXT_MIME_TYPE, "text": body}]} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target excerpt ---'
sed -n '1460,1585p' docs/superpowers/plans/2026-08-06-sync-repo-context.md
printf '%s\n' '--- related identifiers and references ---'
rg -n --glob '!node_modules' 'CONTEXT_URI_TEMPLATE|ResourceTemplateSpec|def read\(|_read_context|context_reader|RESOURCE_TEMPLATES' .
printf '%s\n' '--- repository file map ---'
git ls-files | rg '(^|/)(pyproject.toml|package.json|.*test.*|.*resource.*|.*server.*|.*context.*)$' | head -200Repository: stroland02/Sync
Length of output: 19824
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- current resource implementation ---'
cat -n src/sync/mcp/resources.py | sed -n '1,210p'
printf '%s\n' '--- planned MCP context tests ---'
cat -n docs/superpowers/plans/2026-08-06-sync-repo-context.md | sed -n '1360,1470p'
printf '%s\n' '--- URI-template dependencies and expansion code ---'
rg -n -i 'rfc.?6570|uri.?template|expand\(|percent.?encod|unquote|urlencode|urllib' pyproject.toml src tests docs | head -250
printf '%s\n' '--- MCP resource-template handling ---'
rg -n -i 'resource.?template|resources/read|resource/read|uriTemplate|uri_template|resources/list' src tests | head -250Repository: stroland02/Sync
Length of output: 21787
🌐 Web query:
RFC 6570 simple string expansion reserved characters slash percent-encoding and MCP resource template URI expansion
💡 Result:
In RFC 6570, simple string expansion (the default expression type, {var}) performs percent-encoding on any characters that are not in the unreserved character set (defined in RFC 3986 as alphanumeric characters, hyphen, underscore, period, and tilde) [1][2][3]. Because the slash character ("/") is a reserved character, it is percent-encoded (as "%2F") during simple string expansion [3][4]. Conversely, reserved expansion (using the plus operator, {+var}) allows reserved characters, including the slash ("/") and other reserved URI characters, to be passed through without percent-encoding [3][4]. This makes {+var} suitable for paths or other URI components where structural characters like slashes must remain intact [4][5]. Regarding the Model Context Protocol (MCP), resource templates use the RFC 6570 specification to allow servers to advertise parameterized resource URIs [6][7]. MCP implementations, such as the Python SDK, provide utilities for both expanding these templates (generating a URI from variables) and matching incoming URIs back to their template variables [5][8][9]. In these implementations, operators like {+path} are used to ensure that path separators and other reserved characters are preserved during the expansion process [5][8]. Servers typically advertise these templates via the resources/templates/list method, while clients are responsible for expanding them before requesting a resource via resources/read [6][7].
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc6570.html
- 2: https://datatracker.ietf.org/doc/html/rfc6570
- 3: https://www.rfc-editor.org/rfc/rfc6570.txt
- 4: https://datatracker.ietf.org/doc/rfc6570/
- 5: https://py.sdk.modelcontextprotocol.io/servers/uri-templates/
- 6: https://github.com/yaniv-golan/mcp-bash-framework/blob/c37cc48c/docs/RESOURCE-TEMPLATES.md
- 7: https://mcp-staging.mintlify.app/specification/2025-11-25/server/resources
- 8: https://py.sdk.modelcontextprotocol.io/api/mcp/shared/uri_template/
- 9: https://py.sdk.modelcontextprotocol.io/v2/api/mcp/shared/uri_template/
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- repository ID contract ---'
rg -n -C 3 'repo_id|RepoContext|repo_context' src/sync tests docs/superpowers/specs/2026-08-06-sync-repo-context-design.md | head -300
printf '%s\n' '--- planned URI-related assertions and implementation notes ---'
rg -n -C 5 'URI|uri|slash|repo_id' docs/superpowers/plans/2026-08-06-sync-repo-context.md | sed -n '1,260p'
printf '%s\n' '--- standalone RFC 6570-style expansion probe ---'
python3 - <<'PY'
from urllib.parse import quote
repo_id = "github.com/acme/storefront"
simple = "sync://context/" + quote(repo_id, safe="-._~")
reserved = "sync://context/" + quote(repo_id, safe="-._~:/?#[]@!$&'()*+,;=")
print("repo_id:", repo_id)
print("simple {repo_id}:", simple)
print("reserved {+repo_id}:", reserved)
print("simple lookup suffix:", simple.removeprefix("sync://context/"))
print("reserved lookup suffix:", reserved.removeprefix("sync://context/"))
assert simple.endswith("github.com%2Facme%2Fstorefront")
assert reserved == "sync://context/github.com/acme/storefront"
assert reserved.removeprefix("sync://context/") == repo_id
PYRepository: stroland02/Sync
Length of output: 36718
Preserve slashes in the advertised URI template.
Use RFC 6570 reserved expansion: CONTEXT_URI_TEMPLATE = f"{CONTEXT_URI_PREFIX}{{+repo_id}}". Simple expansion encodes / as %2F, but _read_context passes the suffix to context_reader without decoding. Test expansion with github.com/acme/storefront and assert that read receives the original repo_id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-06-sync-repo-context.md` around lines 1498 -
1559, Update CONTEXT_URI_TEMPLATE to use RFC 6570 reserved expansion with
`{+repo_id}` so repository IDs retain their slashes in advertised URIs. Preserve
the existing _read_context suffix handling and ensure the template expansion and
read flow pass `github.com/acme/storefront` unchanged to context_reader, adding
or updating coverage for that behavior.
Comment on lines
+51
to
+57
| **What transfers.** Sync's grain is one finding per claim per call site. One vendor change touching | ||
| eight call sites is eight findings, and today that is eight remediation runs against one | ||
| repository. The Incident layer is the fix, and Sync's grouping key is stronger than theirs: | ||
| findings sharing a `vendor_change_id` against one repository are deterministically one unit of | ||
| work. Superlog needs a model to decide whether two exceptions are the same incident, and that step | ||
| is exactly what fails when no API key is configured — the running instance showed *"Grouping | ||
| Failed"* on every error for that reason. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Define nullable vendor-change grouping consistently.
docs/superpowers/references/notes/superlog-investigation-mechanism.md#L51-L57: state that only non-null IDs are grouping keys and define the fallback for findings without a vendor change.docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L160-L166: carry the same rule into the M11 implementation contract.
🧰 Tools
🪛 LanguageTool
[style] ~56-~56: Consider an alternative for the overused word “exactly”.
Context: ...are the same incident, and that step is exactly what fails when no API key is configure...
(EXACTLY_PRECISELY)
📍 Affects 2 files
docs/superpowers/references/notes/superlog-investigation-mechanism.md#L51-L57(this comment)docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md#L160-L166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/references/notes/superlog-investigation-mechanism.md` around
lines 51 - 57, Clarify the vendor-change grouping rule in
docs/superpowers/references/notes/superlog-investigation-mechanism.md:51-57 so
only non-null vendor_change_id values form grouping keys, and explicitly define
the fallback behavior for findings without a vendor change. Apply the same rule
to the M11 implementation contract in
docs/superpowers/plans/2026-08-06-sync-m8-m11-resolution-loop.md:160-166; both
sites require documentation updates only.
stroland02
added a commit
that referenced
this pull request
Aug 7, 2026
PR #2 landed the Superlog source study and the M8-M11 and repo-context plans on main directly, which put main two commits ahead of the integration branch and broke the invariant CLAUDE.md's branching section rests on: console-identity is only ever ahead of main, so landing it is a fast-forward and never a merge with a review round attached. Merging main back here restores that. Docs only, and none of the five files is one the substrate session has open.
stroland02
added a commit
that referenced
this pull request
Aug 7, 2026
…what the read-back found M12-W196 exempted /api/corpus/abandonment from the console drift guard, with a comment promising "remove it the day its panel lands" and a check that catches an entry whose route was deleted. Nothing caught the case the comment is actually about: the panel lands, client.ts fetches the path, and the guard is satisfied by the exemption rather than by the fetch. A promise no test holds is how an exemption becomes permanent. Proven RED by exempting a path the console already fetches, then restored. B126 files the finding the first read-back exposed, which is worth more than the query. The pipeline discipline says abandoned runs are data and abandon_reason stays queryable because abandoned attempts are where routing learns what is not mechanically safe. The field is diagnostics prose. You can count two identical sentences and you cannot group two spellings of one cause, so the promise is weaker than it reads. The fix is a coded reason beside the prose rather than instead of it - the sentence an operator reads is what makes an abandonment reviewable - and the vocabulary is derived from the abandonment paths that exist rather than invented from outside. B126 also records the number that ranks it: 4 migration_outcome rows, 3 groups, 1 abandonment. No schema change produces a signal that is not there yet. B122 was taken twice, by two sessions. The Ready entry keeps it because 6f0d7a1 reached main at 00:50 UTC through PR #2 and the other was committed at 10:30; the closed entry moves to B127 and says so. Commits carrying the old label are left alone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five documents, no executable change.
git diff --name-only origin/main...HEADshows nothing outsidedocs/: +2438 lines, 0 deletions.What this adds
references/notes/superlog-investigation-mechanism.md— Superlog read as source, for mechanism.plans/2026-08-06-sync-m8-m11-resolution-loop.md— four milestones, proposed and not scheduled.specs/2026-08-06-sync-repo-context-design.mdandplans/2026-08-06-sync-repo-context.md— per-repository context, designed and planned in seven tasks.BACKLOG.md— B122, at the bottom of Ready on purpose, and an M8–M11 milestone entry.The boundary of what could be read
Superlog's investigation runner is not open source.
backend.tsloads it throughawait import(process.env.AGENT_RUNNER_ANTHROPIC_MODULE), an external module shipping with their hosted product. What is open is a static, model-freecommunityrunner and every piece of orchestration around the seam. The note states this at the top; it is a stated boundary rather than an omission.Verification
docs/. +2438 / −0B122free upstreamorigin/main; high-water mark B121B116not disturbedclassName, no class strings, no colour or spacing values. Two fenced blocks only — a SQLCHECKconstraint and an ASCII state diagram. Section 9 names three exclusions, including that the incident screen's layout was viewed and is theirs. 14 source citations, 8 with line numbersuv run pytestuv run lint-importssync.core depends on nothing KEPT; 1 contract kept, 0 brokenscripts/lint_encoding.py src testslint_dead_links.py src --baseline …) exits 0. That tool checks unreachable Python symbols, not markdown, so the baseline is untouched here — the 6 relative markdown links these documents add were resolved separately: 0 brokenTwo things a reviewer should decide, neither of which blocks this
1. The plan proposes adopting Superlog's confidence scalar, and a written rule refuses it by name.
.claude/rules/interface-originality.mdsays: "Superlog's incident view is the best thing in the reference set and it carriesRoot cause confidence: 9. Take its structure, refuse its scalar."CLAUDE.mdrefuses composite scores on the record three times.The note handles this correctly — it reports
rootCauseConfidenceand its calibration rubric as read, which is what a source study should do.The plan goes further (
2026-08-06-sync-m8-m11-resolution-loop.md:99-107): "Superlog's rubric is worth copying nearly verbatim", and "Every terminal outcome carries a confidence and at least onepath:linecitation, enforced by validation rather than by review." It cites the provenance rung as precedent — but the rung exists precisely as the honest alternative to a scalar, so that argument inverts it.There is a real defence available: this is an agent's evidence tier on a run's conclusion rather than a console-rendered health figure, and it is rubric-backed rather than a vibe. The plan neither raises the conflict nor makes that defence. Nothing is scheduled, so it costs nothing today — but it wants settling before M8 is picked up.
2. A fresh worktree cannot pass the suite, and nothing says so.
The first two gate runs here reported 38 failed / 9 errors, identically under
-n autoand-n0. Not contention:tools/is gitignored, so a new worktree has nooasdiff, and the only signal is aFileNotFoundErrorburied 38 failures deep.bash scripts/bootstrap_tools.shfixed it —oasdiff version 1.26.1, matching.oasdiff-version— and the suite then returned the exact baseline. Worth its own backlog entry; not filed here, because this branch is docs-only and filing it would take a number this PR does not own.Scheduling
Nothing here is scheduled. B122 sits at the bottom of Ready so a tick takes the console items first, and M8–M11 are sequenced behind M7.
🤖 Generated with Claude Code
https://claude.ai/code/session_015XW8USbGHvxvsMVrKig2CK
Summary by CodeRabbit