From 64c641ce80dc8c491885c73ef99bed6d6db8ce1e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 21 Aug 2026 11:23:47 -0400 Subject: [PATCH] refactor(routes): one supersession seam for REST and MCP; PUT/PATCH notes share a handler; shared mask/not-found/caller helpers (#2829, milestone 296 area 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the 28 route modules against each other and against the MCP tools: - routes/notes.py carried a PUT and a PATCH handler that were the same function minus the supersedes contract on one of them — one handler now serves both verbs, so both carry it. - The two _attach_supersession copies (REST + MCP) become supersession_svc.attach_relations(uid, note_id, data, hint=) — the seam the two surfaces must agree through; only the agent surface adds the one-sentence reading hint. - Three local _uid() wrappers over g.user.id → scribe.auth.get_current_user_id like every other module; design_systems' private _not_found → routes.utils. not_found; the four "********" literals → settings_svc.SECRET_MASK with the read/write contract written once. - routes/plugin.py: the project_id/repo resolution block and the comma-separated id parse were copied into three endpoints — _project_scope() and _int_list() now. Co-Authored-By: Claude Fable 5 --- src/scribe/mcp/tools/notes.py | 30 ++--------- src/scribe/mcp/tools/systems.py | 2 +- src/scribe/routes/admin.py | 12 ++--- src/scribe/routes/design_systems.py | 61 ++++++++++----------- src/scribe/routes/notes.py | 62 +++------------------ src/scribe/routes/plugin.py | 84 ++++++++++++----------------- src/scribe/routes/rulebooks.py | 53 +++++++++--------- src/scribe/routes/settings.py | 11 ++-- src/scribe/routes/trash.py | 13 ++--- src/scribe/services/settings.py | 7 +++ src/scribe/services/supersession.py | 32 +++++++++++ 11 files changed, 155 insertions(+), 212 deletions(-) diff --git a/src/scribe/mcp/tools/notes.py b/src/scribe/mcp/tools/notes.py index f4e2dc9..223ac18 100644 --- a/src/scribe/mcp/tools/notes.py +++ b/src/scribe/mcp/tools/notes.py @@ -62,30 +62,6 @@ async def list_notes( return {"notes": [n.to_dict() for n in rows], "total": total} -async def _attach_supersession(uid: int, note_id: int, data: dict) -> None: - """Add both directions of the supersession relation to a note payload. - - Both, because they answer different questions and only one of them is - obvious. `supersedes` is what the author claimed. `superseded_by` is what a - READER needs and what the note itself cannot know — a stale record handed - over without that marker gets acted on confidently, which is worse than - never surfacing it. - - Omitted entirely when empty, so an ordinary note's payload doesn't grow two - permanently-empty lists. A field that always says nothing trains readers to - skip fields, which is the lesson `consolidated_at` cost us (#2483). - """ - rel = await supersession_svc.get_relations(uid, note_id) - if rel["supersedes"]: - data["supersedes"] = rel["supersedes"] - if rel["superseded_by"]: - data["superseded_by"] = rel["superseded_by"] - data["superseded_note"] = ( - "A later note claims to bring this up to date — see superseded_by. " - "Read this as what was true when written, and check the newer one " - "before acting on it." - ) - async def get_note(note_id: int) -> dict: """Fetch the full content of a single Scribe note by its ID. @@ -113,7 +89,7 @@ async def get_note(note_id: int) -> dict: # snippets would leave those permanently at zero pulls and make them look # like dead weight next to snippets that merely had a counter (#2085). record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_note") - await _attach_supersession(uid, note_id, out) + await supersession_svc.attach_relations(uid, note_id, out, hint=True) await systems_tools.attach_systems( uid, getattr(note, "user_id", uid) or uid, out, note.id, note.project_id ) @@ -186,7 +162,7 @@ async def create_note( raise ValueError(str(exc)) from exc data = note.to_dict() await systems_tools.attach_systems(uid, uid, data, note.id, project_id or None) - await _attach_supersession(uid, note.id, data) + await supersession_svc.attach_relations(uid, note.id, data, hint=True) return data @@ -237,7 +213,7 @@ async def update_note( await systems_tools.attach_systems( uid, getattr(note, "user_id", uid) or uid, data, note_id, note.project_id ) - await _attach_supersession(uid, note_id, data) + await supersession_svc.attach_relations(uid, note_id, data, hint=True) return data diff --git a/src/scribe/mcp/tools/systems.py b/src/scribe/mcp/tools/systems.py index acaa9a0..2ccf049 100644 --- a/src/scribe/mcp/tools/systems.py +++ b/src/scribe/mcp/tools/systems.py @@ -134,7 +134,7 @@ async def attach_systems( tagged record shows its areas (the touching-a-System reflex needs the affiliation visible on read, not just settable on write), an untagged project record carries the question instead. Neither field is ever - attached empty (same reasoning as notes._attach_supersession / #2483 — a + attached empty (same reasoning as supersession_svc.attach_relations / #2483 — a field that always says nothing trains readers to skip fields). The hint goes only to the record's owner: tagging someone else's record in someone else's project is not the caller's call to make. Fail-open — decoration diff --git a/src/scribe/routes/admin.py b/src/scribe/routes/admin.py index ec5fcfe..9edf548 100644 --- a/src/scribe/routes/admin.py +++ b/src/scribe/routes/admin.py @@ -22,6 +22,7 @@ from scribe.services.email import SMTP_SETTING_KEYS, get_base_url, get_smtp_conf from scribe.services.logging import get_logs, get_log_stats, log_audit from scribe.services.notifications import send_invitation_email from scribe.services.settings import ( + SECRET_MASK, get_admin_setting, set_admin_setting, set_setting, @@ -116,7 +117,7 @@ async def get_smtp(): config = await get_smtp_config() # Mask password if config.get("smtp_password"): - config["smtp_password"] = "********" + config["smtp_password"] = SECRET_MASK return jsonify(config) @@ -130,7 +131,7 @@ async def update_smtp(): for key in SMTP_SETTING_KEYS: if key in data: # Skip password if it's the mask placeholder - if key == "smtp_password" and data[key] == "********": + if key == "smtp_password" and data[key] == SECRET_MASK: continue settings_to_save[key] = str(data[key]) @@ -157,9 +158,6 @@ async def test_smtp(): return jsonify({"error": str(e)}), 500 -_TOKEN_MASK = "********" - - # The forge CONFIG moved to per-user keyring rows (#2778, Settings → Git # forges); what stays admin is the webhook secret, because the push endpoint # is one URL per instance and authenticates deliveries, not users. @@ -178,7 +176,7 @@ async def get_forge_webhook_settings(): return jsonify({ # Secrets never leave the server — the smtp_password convention: # masked when set, empty when not. - "webhook_secret": _TOKEN_MASK if webhook_secret else "", + "webhook_secret": SECRET_MASK if webhook_secret else "", }) @@ -192,7 +190,7 @@ async def update_forge_webhook_settings(): webhook_secret = data.get("webhook_secret") # The mask coming back means "unchanged" — the form round-trips what GET # showed it, and storing the mask would silently break the integration. - if webhook_secret is not None and webhook_secret != _TOKEN_MASK: + if webhook_secret is not None and webhook_secret != SECRET_MASK: await set_admin_setting(FORGE_WEBHOOK_SECRET_KEY, str(webhook_secret)) # The secret is deliberately absent from the audit detail. await log_audit( diff --git a/src/scribe/routes/design_systems.py b/src/scribe/routes/design_systems.py index 1bb3628..ceff5ae 100644 --- a/src/scribe/routes/design_systems.py +++ b/src/scribe/routes/design_systems.py @@ -15,9 +15,10 @@ one and returns None for the other: those two IS the intent: distinguishing them would confirm the existence of records the caller may not see. """ -from quart import Blueprint, g, jsonify, request +from quart import Blueprint, jsonify, request -from scribe.auth import login_required +from scribe.auth import get_current_user_id, login_required +from scribe.routes.utils import not_found from scribe.services import design_systems as ds_svc from scribe.services.design_starter_roles import ( DEFAULT_TOKEN_PREFIX, @@ -28,12 +29,6 @@ from scribe.services.design_systems import DesignSystemCycle design_systems_bp = Blueprint("design_systems", __name__, url_prefix="/api") -def _uid() -> int: - return g.user.id - - -def _not_found(what: str = "design system"): - return jsonify({"error": f"{what} not found"}), 404 # ── Design systems ────────────────────────────────────────────────────── @@ -43,7 +38,7 @@ def _not_found(what: str = "design system"): async def list_design_systems(): """The caller's design systems. An empty list is the ordinary state for an install that has never made one, not an error.""" - rows = await ds_svc.list_design_systems(_uid()) + rows = await ds_svc.list_design_systems(get_current_user_id()) return jsonify({"design_systems": [s.to_dict() for s in rows]}) @@ -55,7 +50,7 @@ async def create_design_system(): if not title: return jsonify({"error": "title is required"}), 400 system = await ds_svc.create_design_system( - user_id=_uid(), + user_id=get_current_user_id(), title=title, description=data.get("description") or None, guidance=data.get("guidance") or None, @@ -84,9 +79,9 @@ async def list_starter_role_groups(): @design_systems_bp.get("/design-systems/") @login_required async def get_design_system(design_system_id: int): - system = await ds_svc.get_design_system(_uid(), design_system_id) + system = await ds_svc.get_design_system(get_current_user_id(), design_system_id) if system is None: - return _not_found() + return not_found("Design system") return jsonify(system.to_dict()) @@ -102,19 +97,19 @@ async def update_design_system(design_system_id: int): if "parent_id" in data: fields["parent_id"] = data["parent_id"] try: - system = await ds_svc.update_design_system(_uid(), design_system_id, **fields) + system = await ds_svc.update_design_system(get_current_user_id(), design_system_id, **fields) except DesignSystemCycle as exc: return jsonify({"error": str(exc)}), 400 if system is None: - return _not_found() + return not_found("Design system") return jsonify(system.to_dict()) @design_systems_bp.delete("/design-systems/") @login_required async def delete_design_system(design_system_id: int): - if not await ds_svc.delete_design_system(_uid(), design_system_id): - return _not_found() + if not await ds_svc.delete_design_system(get_current_user_id(), design_system_id): + return not_found("Design system") return "", 204 @@ -127,9 +122,9 @@ async def resolve_design_system(design_system_id: int): this returns what it ends up being. Both are real questions and answering only one would make the other a client-side computation. """ - resolved = await ds_svc.resolve_design_system(_uid(), design_system_id) + resolved = await ds_svc.resolve_design_system(get_current_user_id(), design_system_id) if resolved is None: - return _not_found() + return not_found("Design system") return jsonify({ "design_system_id": design_system_id, "tokens": [t.to_dict() for t in resolved], @@ -149,9 +144,9 @@ async def get_design_system_stylesheet(design_system_id: int): `:root`, so the generator takes it as a parameter. """ root = (request.args.get("root") or ":root").strip() or ":root" - result = await ds_svc.stylesheet_for_system(_uid(), design_system_id, root) + result = await ds_svc.stylesheet_for_system(get_current_user_id(), design_system_id, root) if result is None: - return _not_found() + return not_found("Design system") if request.args.get("format") == "css": return result["css"], 200, {"Content-Type": "text/css; charset=utf-8"} return jsonify(result) @@ -168,10 +163,10 @@ async def check_snippets_against_system(design_system_id: int): """ project_id = request.args.get("project_id", type=int) or 0 result = await ds_svc.check_snippets_against_system( - _uid(), design_system_id, project_id + get_current_user_id(), design_system_id, project_id ) if result is None: - return _not_found() + return not_found("Design system") return jsonify(result) @@ -181,9 +176,9 @@ async def check_snippets_against_system(design_system_id: int): @login_required async def list_design_tokens(design_system_id: int): """This system's OWN tokens — its override set, not its effective set.""" - if await ds_svc.get_design_system(_uid(), design_system_id) is None: - return _not_found() - rows = await ds_svc.list_tokens(_uid(), design_system_id) + if await ds_svc.get_design_system(get_current_user_id(), design_system_id) is None: + return not_found("Design system") + rows = await ds_svc.list_tokens(get_current_user_id(), design_system_id) return jsonify({"tokens": [t.to_dict() for t in rows]}) @@ -195,7 +190,7 @@ async def create_design_token(design_system_id: int): if not name: return jsonify({"error": "name is required"}), 400 token = await ds_svc.create_token( - user_id=_uid(), + user_id=get_current_user_id(), design_system_id=design_system_id, name=name, value_by_mode=data.get("value_by_mode"), @@ -206,7 +201,7 @@ async def create_design_token(design_system_id: int): order_index=data.get("order_index") or 0, ) if token is None: - return _not_found() + return not_found("Design system") return jsonify(token.to_dict()), 201 @@ -221,17 +216,17 @@ async def update_design_token(token_id: int): "supersedes", "order_index", ) } - token = await ds_svc.update_token(_uid(), token_id, **fields) + token = await ds_svc.update_token(get_current_user_id(), token_id, **fields) if token is None: - return _not_found("design token") + return not_found("Design token") return jsonify(token.to_dict()) @design_systems_bp.delete("/design-tokens/") @login_required async def delete_design_token(token_id: int): - if not await ds_svc.delete_token(_uid(), token_id): - return _not_found("design token") + if not await ds_svc.delete_token(get_current_user_id(), token_id): + return not_found("Design token") return "", 204 @@ -247,9 +242,9 @@ async def set_project_design_system(project_id: int): """ data = await request.get_json() or {} ok = await ds_svc.set_project_design_system( - _uid(), project_id, data.get("design_system_id") + get_current_user_id(), project_id, data.get("design_system_id") ) if not ok: - return _not_found("project or design system") + return not_found("Project or design system") return jsonify({"project_id": project_id, "design_system_id": data.get("design_system_id")}) diff --git a/src/scribe/routes/notes.py b/src/scribe/routes/notes.py index cb81507..17a13ad 100644 --- a/src/scribe/routes/notes.py +++ b/src/scribe/routes/notes.py @@ -26,22 +26,6 @@ from scribe.services import dedup as dedup_svc from scribe.services import supersession as supersession_svc from scribe.services.note_usage import record_pulled - -async def _attach_supersession(uid: int, note_id: int, data: dict) -> None: - """Both directions of the supersession relation on a note payload. - - Mirrors the MCP helper of the same name — the two surfaces must agree about - what a note's payload says, or the web UI and the agent would disagree about - whether a record is current. - - Omitted when empty: a field that always says nothing trains readers to skip - fields, which is what `consolidated_at` cost (#2483). - """ - rel = await supersession_svc.get_relations(uid, note_id) - if rel["supersedes"]: - data["supersedes"] = rel["supersedes"] - if rel["superseded_by"]: - data["superseded_by"] = rel["superseded_by"] from scribe.services.note_versions import list_versions, get_version logger = logging.getLogger(__name__) @@ -142,7 +126,7 @@ async def create_note_route(): # may not write the target. The note itself was created. return jsonify({"error": str(exc), "note": note.to_dict()}), 403 out = note.to_dict() - await _attach_supersession(uid, note.id, out) + await supersession_svc.attach_relations(uid, note.id, out) return jsonify(out), 201 @@ -241,13 +225,17 @@ async def get_note_route(note_id: int): # injected line useful?" is answered by agent pulls alone, and a human # clicking a link would inflate exactly the number #1038 and #2085 gate on. record_pulled(user_id=uid, note_id=note_id, source="rest_note") - await _attach_supersession(uid, note_id, data) + await supersession_svc.attach_relations(uid, note_id, data) return jsonify(data) -@notes_bp.route("/", methods=["PUT"]) +@notes_bp.route("/", methods=["PUT", "PATCH"]) @login_required async def update_note_route(note_id: int): + """Partial update — only the keys present in the payload change. PUT and + PATCH are the same handler on purpose: the form sends the field set it + edited, and the two verbs used to be two near-identical copies of this + function that drifted (one carried the supersedes contract, one did not).""" uid = get_current_user_id() # Share-aware: resolve through the ACL and write as the OWNER, so a shared # editor's save isn't rejected by the owner-scoped update service. @@ -290,44 +278,10 @@ async def update_note_route(note_id: int): except PermissionError as exc: return jsonify({"error": str(exc)}), 403 out = note.to_dict() - await _attach_supersession(uid, note_id, out) + await supersession_svc.attach_relations(uid, note_id, out) return jsonify(out) -@notes_bp.route("/", methods=["PATCH"]) -@login_required -async def patch_note_route(note_id: int): - uid = get_current_user_id() - result = await get_note_for_user(uid, note_id) - if result is None: - return not_found("Note") - note_obj, _ = result - if not await can_write_note(uid, note_id): - return jsonify({"error": "Permission denied"}), 403 - owner_uid = note_obj.user_id - data = await request.get_json() - fields = {} - for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"): - if key in data: - fields[key] = data[key] - if "due_date" in data: - if data["due_date"]: - result = parse_iso_date(data["due_date"], "due_date") - if isinstance(result, tuple): - return result - fields["due_date"] = result - else: - fields["due_date"] = None - if "tags" in data: - fields["tags"] = data["tags"] - try: - note = await update_note(owner_uid, note_id, **fields) - except ValueError as e: - return jsonify({"error": str(e)}), 400 - if note is None: - return not_found("Note") - return jsonify(note.to_dict()) - @notes_bp.route("/", methods=["DELETE"]) @login_required diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index c66d2e0..71e48b5 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -24,6 +24,35 @@ plugin_bp = Blueprint("plugin", __name__, url_prefix="/api/plugin") _MARKETPLACE_KEY = "plugin_marketplace_url" +def _int_list(raw: str | None) -> list[int]: + """A comma-separated id list from the query string; non-ints dropped.""" + return [int(p) for p in (raw or "").split(",") if p.strip().isdigit()] + + +async def _project_scope() -> tuple[int, str, str]: + """(project_id, repo, unbound_repo) from the request's `project_id` / + `repo` query args — the one resolution every plugin endpoint shares. + + An explicit project_id wins; otherwise the repo remote is resolved + through the caller's bindings, and a remote nobody bound comes back as + `unbound_repo` (normalised) so /context can say "bind this repo". + """ + try: + project_id = int(request.args.get("project_id", 0) or 0) + except (TypeError, ValueError): + project_id = 0 + repo = (request.args.get("repo") or "").strip() + unbound_repo = "" + if repo and not project_id: + resolved = await repo_bindings_svc.resolve_project(g.user.id, repo) + if resolved: + project_id = resolved + else: + unbound_repo = repo_bindings_svc.normalize_repo_key(repo) + return project_id, repo, unbound_repo + + + @plugin_bp.get("/context") @login_required async def session_context(): @@ -37,20 +66,7 @@ async def session_context(): project_id (optional int) — explicit override, mainly for manual/ad-hoc curl testing; takes precedence over `repo` when set. """ - try: - project_id = int(request.args.get("project_id", 0) or 0) - except (TypeError, ValueError): - project_id = 0 - - unbound_repo = "" - repo = (request.args.get("repo") or "").strip() - if repo and not project_id: - resolved = await repo_bindings_svc.resolve_project(g.user.id, repo) - if resolved: - project_id = resolved - else: - unbound_repo = repo_bindings_svc.normalize_repo_key(repo) - + project_id, _repo, unbound_repo = await _project_scope() result = await plugin_ctx_svc.build_session_context( g.user.id, project_id, unbound_repo=unbound_repo ) @@ -77,22 +93,8 @@ async def autoinject_retrieve(): session; skipped so each note injects at most once. """ q = (request.args.get("q") or "").strip() - try: - project_id = int(request.args.get("project_id", 0) or 0) - except (TypeError, ValueError): - project_id = 0 - - repo = (request.args.get("repo") or "").strip() - if repo and not project_id: - resolved = await repo_bindings_svc.resolve_project(g.user.id, repo) - if resolved: - project_id = resolved - - exclude_ids = [ - int(p) for p in (request.args.get("exclude_ids") or "").split(",") - if p.strip().isdigit() - ] - + project_id, _repo, _unbound = await _project_scope() + exclude_ids = _int_list(request.args.get("exclude_ids")) result = await plugin_ctx_svc.build_autoinject_hint( g.user.id, q, project_id=project_id, exclude_ids=exclude_ids ) @@ -139,25 +141,9 @@ async def write_path_prior_art(): """ path = (request.args.get("path") or "").strip() code = request.args.get("code") or "" - try: - project_id = int(request.args.get("project_id", 0) or 0) - except (TypeError, ValueError): - project_id = 0 - - repo = (request.args.get("repo") or "").strip() - if repo and not project_id: - resolved = await repo_bindings_svc.resolve_project(g.user.id, repo) - if resolved: - project_id = resolved - - exclude_ids = [ - int(p) for p in (request.args.get("exclude_ids") or "").split(",") - if p.strip().isdigit() - ] - exclude_sync_ids = [ - int(p) for p in (request.args.get("exclude_sync_ids") or "").split(",") - if p.strip().isdigit() - ] + project_id, repo, _unbound = await _project_scope() + exclude_ids = _int_list(request.args.get("exclude_ids")) + exclude_sync_ids = _int_list(request.args.get("exclude_sync_ids")) shapes = _parse_shapes(request.args.get("shapes") or "") api_key = getattr(g, "api_key", None) may_stamp = api_key is None or getattr(api_key, "scope", "") == "write" diff --git a/src/scribe/routes/rulebooks.py b/src/scribe/routes/rulebooks.py index 81617bc..4c340b0 100644 --- a/src/scribe/routes/rulebooks.py +++ b/src/scribe/routes/rulebooks.py @@ -1,29 +1,26 @@ """Rulebook / topic REST endpoints. -Wraps services/rulebooks.py. Standard Scribe auth: g.user.id is the +Wraps services/rulebooks.py. Standard Scribe auth: get_current_user_id() is the authenticated owner; the service enforces ownership scoping. """ from __future__ import annotations -from quart import Blueprint, g, jsonify, request +from quart import Blueprint, jsonify, request -from scribe.auth import login_required +from scribe.auth import get_current_user_id, login_required import scribe.services.rulebooks as rulebooks_svc from scribe.services.trash import delete as trash_delete rulebooks_bp = Blueprint("rulebooks", __name__, url_prefix="/api") -def _uid() -> int: - return g.user.id - # ── Rulebooks ─────────────────────────────────────────────────────────── @rulebooks_bp.get("/rulebooks") @login_required async def list_rulebooks(): - rows = await rulebooks_svc.list_rulebooks(_uid()) + rows = await rulebooks_svc.list_rulebooks(get_current_user_id()) return jsonify({"rulebooks": [rb.to_dict() for rb in rows]}) @@ -35,7 +32,7 @@ async def create_rulebook(): if not title: return jsonify({"error": "title is required"}), 400 rb = await rulebooks_svc.create_rulebook( - user_id=_uid(), + user_id=get_current_user_id(), title=title, description=data.get("description", ""), ) @@ -45,7 +42,7 @@ async def create_rulebook(): @rulebooks_bp.get("/rulebooks/") @login_required async def get_rulebook(rulebook_id: int): - rb = await rulebooks_svc.get_rulebook(rulebook_id, _uid()) + rb = await rulebooks_svc.get_rulebook(rulebook_id, get_current_user_id()) if rb is None: return jsonify({"error": "rulebook not found"}), 404 return jsonify(rb.to_dict()) @@ -56,7 +53,7 @@ async def get_rulebook(rulebook_id: int): async def update_rulebook(rulebook_id: int): data = await request.get_json() or {} fields = {k: v for k, v in data.items() if k in ("title", "description", "always_on")} - rb = await rulebooks_svc.update_rulebook(rulebook_id, _uid(), **fields) + rb = await rulebooks_svc.update_rulebook(rulebook_id, get_current_user_id(), **fields) if rb is None: return jsonify({"error": "rulebook not found"}), 404 return jsonify(rb.to_dict()) @@ -65,7 +62,7 @@ async def update_rulebook(rulebook_id: int): @rulebooks_bp.delete("/rulebooks/") @login_required async def delete_rulebook(rulebook_id: int): - await trash_delete(_uid(), "rulebook", rulebook_id) + await trash_delete(get_current_user_id(), "rulebook", rulebook_id) return "", 204 @@ -75,7 +72,7 @@ async def delete_rulebook(rulebook_id: int): @login_required async def list_topics(rulebook_id: int): try: - rows = await rulebooks_svc.list_topics(rulebook_id, _uid()) + rows = await rulebooks_svc.list_topics(rulebook_id, get_current_user_id()) except ValueError as exc: return jsonify({"error": str(exc)}), 404 return jsonify({"topics": [t.to_dict() for t in rows]}) @@ -91,7 +88,7 @@ async def create_topic(rulebook_id: int): try: topic = await rulebooks_svc.create_topic( rulebook_id=rulebook_id, - user_id=_uid(), + user_id=get_current_user_id(), title=title, description=data.get("description", ""), order_index=data.get("order_index", 0), @@ -109,7 +106,7 @@ async def update_topic(topic_id: int): k: v for k, v in data.items() if k in ("title", "description", "order_index") } - topic = await rulebooks_svc.update_topic(topic_id, _uid(), **fields) + topic = await rulebooks_svc.update_topic(topic_id, get_current_user_id(), **fields) if topic is None: return jsonify({"error": "topic not found"}), 404 return jsonify(topic.to_dict()) @@ -118,7 +115,7 @@ async def update_topic(topic_id: int): @rulebooks_bp.delete("/rulebook-topics/") @login_required async def delete_topic(topic_id: int): - if await trash_delete(_uid(), "topic", topic_id) is None: + if await trash_delete(get_current_user_id(), "topic", topic_id) is None: return jsonify({"error": "topic not found"}), 404 return "", 204 @@ -140,7 +137,7 @@ async def list_rules(): return jsonify({"error": "rulebook_id, topic_id, project_id must be integers"}), 400 rows = await rulebooks_svc.list_rules( - user_id=_uid(), + user_id=get_current_user_id(), rulebook_id=rulebook_id, topic_id=topic_id, project_id=project_id, @@ -159,7 +156,7 @@ async def create_rule(topic_id: int): try: rule = await rulebooks_svc.create_rule( topic_id=topic_id, - user_id=_uid(), + user_id=get_current_user_id(), title=title, statement=statement, why=data.get("why", ""), @@ -174,7 +171,7 @@ async def create_rule(topic_id: int): @rulebooks_bp.get("/rules/") @login_required async def get_rule(rule_id: int): - rule = await rulebooks_svc.get_rule(rule_id, _uid()) + rule = await rulebooks_svc.get_rule(rule_id, get_current_user_id()) if rule is None: return jsonify({"error": "rule not found"}), 404 return jsonify(rule.to_dict()) @@ -188,7 +185,7 @@ async def update_rule(rule_id: int): k: v for k, v in data.items() if k in ("title", "statement", "why", "how_to_apply", "order_index") } - rule = await rulebooks_svc.update_rule(rule_id, _uid(), **fields) + rule = await rulebooks_svc.update_rule(rule_id, get_current_user_id(), **fields) if rule is None: return jsonify({"error": "rule not found"}), 404 return jsonify(rule.to_dict()) @@ -197,7 +194,7 @@ async def update_rule(rule_id: int): @rulebooks_bp.delete("/rules/") @login_required async def delete_rule(rule_id: int): - if await trash_delete(_uid(), "rule", rule_id) is None: + if await trash_delete(get_current_user_id(), "rule", rule_id) is None: return jsonify({"error": "rule not found"}), 404 return "", 204 @@ -213,7 +210,7 @@ async def subscribe_project(project_id: int): return jsonify({"error": "rulebook_id is required"}), 400 try: await rulebooks_svc.subscribe_project( - project_id=project_id, rulebook_id=int(rulebook_id), user_id=_uid(), + project_id=project_id, rulebook_id=int(rulebook_id), user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -227,7 +224,7 @@ async def subscribe_project(project_id: int): async def unsubscribe_project(project_id: int, rulebook_id: int): try: await rulebooks_svc.unsubscribe_project( - project_id=project_id, rulebook_id=rulebook_id, user_id=_uid(), + project_id=project_id, rulebook_id=rulebook_id, user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -238,7 +235,7 @@ async def unsubscribe_project(project_id: int, rulebook_id: int): @login_required async def get_project_rules(project_id: int): result = await rulebooks_svc.get_applicable_rules( - project_id=project_id, user_id=_uid(), + project_id=project_id, user_id=get_current_user_id(), ) return jsonify(result) @@ -248,7 +245,7 @@ async def get_project_rules(project_id: int): async def suppress_project_rule(project_id: int, rule_id: int): try: await rulebooks_svc.suppress_rule_for_project( - project_id=project_id, rule_id=rule_id, user_id=_uid(), + project_id=project_id, rule_id=rule_id, user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -260,7 +257,7 @@ async def suppress_project_rule(project_id: int, rule_id: int): async def unsuppress_project_rule(project_id: int, rule_id: int): try: await rulebooks_svc.unsuppress_rule_for_project( - project_id=project_id, rule_id=rule_id, user_id=_uid(), + project_id=project_id, rule_id=rule_id, user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -272,7 +269,7 @@ async def unsuppress_project_rule(project_id: int, rule_id: int): async def suppress_project_topic(project_id: int, topic_id: int): try: await rulebooks_svc.suppress_topic_for_project( - project_id=project_id, topic_id=topic_id, user_id=_uid(), + project_id=project_id, topic_id=topic_id, user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -284,7 +281,7 @@ async def suppress_project_topic(project_id: int, topic_id: int): async def unsuppress_project_topic(project_id: int, topic_id: int): try: await rulebooks_svc.unsuppress_topic_for_project( - project_id=project_id, topic_id=topic_id, user_id=_uid(), + project_id=project_id, topic_id=topic_id, user_id=get_current_user_id(), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -303,7 +300,7 @@ async def create_project_rule(project_id: int): try: rule = await rulebooks_svc.create_project_rule( project_id=project_id, - user_id=_uid(), + user_id=get_current_user_id(), title=title, statement=statement, why=data.get("why", ""), diff --git a/src/scribe/routes/settings.py b/src/scribe/routes/settings.py index 35f31f2..ea3a4f0 100644 --- a/src/scribe/routes/settings.py +++ b/src/scribe/routes/settings.py @@ -9,7 +9,9 @@ from quart import Blueprint, jsonify, request from scribe.auth import login_required, get_current_user_id from scribe.config import Config -from scribe.services.settings import delete_setting, get_all_settings, get_setting, set_settings_batch +from scribe.services.settings import ( + SECRET_MASK, delete_setting, get_all_settings, get_setting, set_settings_batch, +) logger = logging.getLogger(__name__) @@ -22,12 +24,11 @@ settings_bp = Blueprint("settings", __name__, url_prefix="/api/settings") # rows live on the admin's own user_id, so the plain GET returned them raw. # (forge_token left with 0078: forge credentials are keyring rows now, #2778.) _SECRET_KEYS = frozenset({"smtp_password", "forge_webhook_secret"}) -_SECRET_MASK = "********" def _masked(settings: dict) -> dict: return { - k: (_SECRET_MASK if k in _SECRET_KEYS and v else v) + k: (SECRET_MASK if k in _SECRET_KEYS and v else v) for k, v in settings.items() } @@ -53,7 +54,7 @@ async def update_settings_route(): str_v = str(v) # A masked secret round-tripping through a client is "unchanged", not # a request to store the mask over the real credential. - if k in _SECRET_KEYS and str_v == _SECRET_MASK: + if k in _SECRET_KEYS and str_v == SECRET_MASK: continue if not str_v: await delete_setting(uid, k) @@ -127,7 +128,7 @@ async def update_forge_connection_route(connection_id: int): token = str(data.get("token", "")) # The mask coming back means "unchanged" — the form round-trips what the # list showed, and storing the mask would silently break the connection. - if token == _SECRET_MASK: + if token == SECRET_MASK: token = "" try: row = await update_connection( diff --git a/src/scribe/routes/trash.py b/src/scribe/routes/trash.py index 674194a..4e8b19b 100644 --- a/src/scribe/routes/trash.py +++ b/src/scribe/routes/trash.py @@ -1,33 +1,30 @@ """Trash REST API — list / restore / purge soft-deleted content by batch.""" from __future__ import annotations -from quart import Blueprint, g, jsonify +from quart import Blueprint, jsonify -from scribe.auth import login_required +from scribe.auth import get_current_user_id, login_required import scribe.services.trash as trash_svc trash_bp = Blueprint("trash", __name__, url_prefix="/api/trash") -def _uid() -> int: - return g.user.id - @trash_bp.get("") @login_required async def list_trash(): - return jsonify({"batches": await trash_svc.list_trash(_uid())}) + return jsonify({"batches": await trash_svc.list_trash(get_current_user_id())}) @trash_bp.post("//restore") @login_required async def restore_batch(batch_id: str): - n = await trash_svc.restore(_uid(), batch_id) + n = await trash_svc.restore(get_current_user_id(), batch_id) return jsonify({"restored": n}) @trash_bp.delete("/") @login_required async def purge_batch(batch_id: str): - n = await trash_svc.purge(_uid(), batch_id) + n = await trash_svc.purge(get_current_user_id(), batch_id) return jsonify({"purged": n}) diff --git a/src/scribe/services/settings.py b/src/scribe/services/settings.py index a914916..fb044aa 100644 --- a/src/scribe/services/settings.py +++ b/src/scribe/services/settings.py @@ -8,6 +8,13 @@ from scribe.models.user import User logger = logging.getLogger(__name__) +# What a stored credential looks like on the wire. Every surface that READS a +# secret (smtp_password, forge_webhook_secret, a forge token) returns this +# when one is set; every surface that WRITES one treats this value coming back +# as "unchanged", never as a request to store eight asterisks over the real +# credential. One constant so the read and write halves cannot disagree. +SECRET_MASK = "********" + async def get_admin_setting(key: str, default: str = "") -> str: """Read an instance-global setting (one stored on an admin account). diff --git a/src/scribe/services/supersession.py b/src/scribe/services/supersession.py index eb2fac1..9444b55 100644 --- a/src/scribe/services/supersession.py +++ b/src/scribe/services/supersession.py @@ -181,3 +181,35 @@ async def superseded_ids(note_ids: list[int]) -> set[int]: .where(NoteSupersession.superseded_id.in_(note_ids)) )).scalars().all() return {int(r) for r in rows} + + +SUPERSEDED_HINT = ( + "A later note claims to bring this up to date — see superseded_by. " + "Read this as what was true when written, and check the newer one " + "before acting on it." +) + + +async def attach_relations(user_id: int, note_id: int, data: dict, *, hint: bool = False) -> None: + """Add both directions of the supersession relation to a note payload. + + ONE seam for the REST and MCP surfaces, which must agree about what a + note's payload says — or the web UI and the agent would disagree about + whether a record is current. Both directions, because they answer + different questions and only one is obvious: `supersedes` is what the + author claimed; `superseded_by` is what a READER needs and what the note + itself cannot know — a stale record handed over without that marker gets + acted on confidently, which is worse than never surfacing it. + + Omitted entirely when empty, so an ordinary note's payload doesn't grow + two permanently-empty lists (#2483 — a field that always says nothing + trains readers to skip fields). `hint=True` (the agent surface) also + attaches `superseded_note`, the one-sentence reading instruction. + """ + rel = await get_relations(user_id, note_id) + if rel["supersedes"]: + data["supersedes"] = rel["supersedes"] + if rel["superseded_by"]: + data["superseded_by"] = rel["superseded_by"] + if hint: + data["superseded_note"] = SUPERSEDED_HINT