MVP rewrite: OpenClaw + OpenBao + Kashif + Aga

[stranma]

Summary

Full rewrite of the Sultanate design docs onto a new MVP baseline. Replaces the original V1/V2/V3 PRDs (Hermes-native, Infisical-vault, Kashif-deferred, Sentinel-named) with a coherent MVP doc set built around four locked architectural decisions:

• OpenClaw as the agent runtime (was Hermes)
• OpenBao as the Secret Vault, with Aga as sole client (was Infisical placeholder)
• Kashif is in MVP (was deferred), as a three-layer CPU-only content inspector
• Aga replaces Sentinel (Ottoman: Agha of the Janissaries — commands the guard corps)

This PR is doc-only — no implementation. Every file is Markdown.

Why this PR exists

The repo previously had two parallel histories:

1. The V1/V2/V3 PRDs on main (commits 4dda7f4..176d609) — original design phase
2. The Hermes/Infisical MVP attempt on origin/archive-hermes-infisical (preserved, not on main) — substantially-rewritten MVP docs that committed to Hermes + Infisical + Sentinel

This PR consolidates everything onto a single MVP baseline that:

• Picks the better implementation choices (OpenClaw is more capable; OpenBao is more capable; Kashif is cheap to add given the foundation stack we identified)
• Locks the threat model to MVP scope (only hostile Pasha; trusted-core compromise is Phase 2)
• Targets a concrete host (Hetzner AX41-NVMe — Ryzen 5 3600, 64 GB, no GPU)

Locked architectural decisions

Concern	Choice	Why
Agent runtime	OpenClaw	Multi-provider (Anthropic / OpenAI / OpenRouter / Ollama / LiteLLM); SOUL.md + AGENTS.md auto-load; native Telegram channel; Docker-shipped
Secret Vault	OpenBao (Apache-2.0 fork of Vault)	Dynamic secret minting (GitHub App tokens, etc.) with TTL-bounded leases; lease-based revocation is a real security property regardless of Aga's reliability
Network proxy	Sandcat fork (mitmproxy + WireGuard)	Already implements the hardest primitive (transparent credential injection); WireGuard kill-switch is fail-closed by default
Content inspector	Kashif — in MVP	Three layers on CPU: LLM Guard regex + Prompt Guard 2 22M + Llama Guard 3 1B Q4. ~2-4 GB RAM, ~1-2 s per appeal. Promotes from deferred because the cost is small and Aga's ingestion path needs the screener from day one
Threat model	Only hostile Pasha in scope	Hardening against compromised Aga / Janissary / Kashif / host / OpenBao is Phase 2
Target host	Hetzner AX41-NVMe	Core RAM ~6-10 GB, leaves ~50 GB for 5-10 concurrent provinces

File changes (high level)

New files (16):

• Cross-cutting: MOTIVATION.md, ARCHITECTURE.md, SULTANATE_MVP.md, CLAUDE.md
• Security perimeter: JANISSARY_MVP_PRD.md, JANISSARY_SPEC.md, KASHIF_MVP_PRD.md, AGA_MVP_PRD.md, AGA_SPEC.md, DIVAN_MVP_PRD.md, DIVAN_API_SPEC.md
• Orchestration: VIZIER_MVP_PRD.md, VIZIER_SPEC.md
• Province runtime: OPENCLAW_FIRMAN_MVP_PRD.md, OPENCLAW_FIRMAN_SPEC.md, OPENCLAW_CODING_BERAT_MVP_PRD.md, OPENCLAW_CODING_BERAT_SPEC.md

Deleted files (5):

• SULTANATE.md → superseded by SULTANATE_MVP.md
• JANISSARY_PRD_V2.md → split across JANISSARY_* + KASHIF_* + AGA_* + DIVAN_*
• VIZIER_PRD_V3.md → superseded by VIZIER_*
• HERMES_FIRMAN_PRD_V1.md → superseded by OPENCLAW_FIRMAN_*
• HERMES_CODING_BERAT_PRD_V1.md → superseded by OPENCLAW_CODING_BERAT_*

Modified (2):

• README.md → new doc index by layer, OpenBao + Sandcat + OpenClaw + Kashif implementation notes, Hetzner target
• SENTINELGATE_ANALYSIS.md → only the Sentinel→Aga rename (kept as Phase-2 reference)

Preserved branches:

• origin/archive-hermes-infisical — the prior Hermes/Infisical MVP attempt, untouched
• origin/mvp-rewrite — this branch (the work being PR'd)

Highlights worth reviewing carefully

1. Appeal flow (ARCHITECTURE.md §2) — T+0.0s timeline narrative covering all three Kashif outcomes (allow / block / escalate). Locks in the rule that only Kashif=allow stays silent; both block and escalate notify Sultan AND Aga (so they can spot Pasha drift, e.g. 3 blocks in 10 min → consider destroying the province).

2. Mid-task credential request flow (ARCHITECTURE.md §2.3) — parallel to the appeal flow, but the allow path always involves Sultan because granting a long-lived credential is more consequential than letting one write through.

3. GitHub App as Phase 1 default (SULTANATE_MVP.md GitHub Token Strategy) — Sultan's only manual action is the one-time GitHub App install. Aga then mints per-province installation tokens (1-hour TTL, auto-renewed every ~15 min while province is running). Sultan does not paste tokens day-to-day. KV fallback exists for services without a dynamic mint path.

4. Communication Channels (SULTANATE_MVP.md) — explicit per-province Telegram bot model (2 + N bots: Vizier, Aga, one per active Pasha). Compromise of a Pasha bot token has per-province blast radius — cannot reach other Pashas, cannot reach Aga, cannot bypass Janissary.

5. Lease-aware credential injection (JANISSARY_SPEC.md §6) — Janissary checks lease_expires_at before injecting; expired lease → skip + audit severity=alert (request continues without header so upstream returns 401, signalling staleness; Aga sees the audit and re-issues).

6. Kashif verdict → Divan auto-transition (DIVAN_API_SPEC.md Appeals) — formalised as part of the API contract: Kashif=allow auto-approves the appeal, Kashif=block auto-denies, Kashif=escalate stays pending for Sultan.

7. Dashboard via Tailscale (DIVAN_MVP_PRD.md) — primary access path (mobile-friendly, identity-gated). SSH-tunnel + 127.0.0.1 binding documented as fallback for non-Tailscale environments.

Test plan

• Doc set renders correctly on GitHub's markdown viewer (no broken internal links, tables aligned)
• All cross-references between docs use the new file names (SULTANATE_MVP.md, not SULTANATE.md; OPENCLAW_FIRMAN_*, not HERMES_FIRMAN_*; etc.)
• git grep -i "sentinel" -- '*.md' returns only SentinelGate (product name) + explicit archive-branch references
• git grep -i "infisical" -- '*.md' returns only archive-hermes-infisical branch references
• git grep -i "hermes-firman\|hermes-coding-berat" -- '*.md' returns zero
• git grep -i "kashif deferred" -- '*.md' returns zero
• CLAUDE.md invariants are consistent with what the rewritten docs actually say (architectural invariants table, Kashif verdict transitions, MVP threat model scope)
• CodeRabbit review pass

Out of scope

• Component implementation (still doc-only)
• Phase 2 hardening (compromised-core threat model, dual audit sinks, signed audit chains, Aga AppRole rotation, multi-operator Shamir, GPU upgrade for Llama Guard 8B)
• Additional firmans / berats beyond openclaw-coding-berat
• CI / CONTRIBUTING / LICENSE files

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation

  • Added a full Sultanate MVP: architecture, threat model, fail‑closed workflows, and Phase‑1 scope.
  • New PRDs/specs covering orchestration (Vizier), secret management (Aga/OpenBao), shared state (Divan), egress proxy (Janissary), content screening (Kashif), and OpenClaw firmans/berats.
  • Expanded README, onboarding order, and operational playbooks.

• Chores

  • Reorganized docs and removed legacy/archival PRDs to consolidate the design surface.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds extensive PRDs and technical specifications defining Sultanate’s MVP components (Aga, Divan, Janissary, Kashif, Vizier, OpenBao, OpenClaw firmans/berats), API contracts, Divan SQLite schema, credential lifecycle (OpenBao/GitHub), Janissary proxy rules, Kashif screening, Vizier orchestration, docs index updates, and deletes legacy PRDs.

Changes

Sultanate MVP & Component Specs

Layer / File(s)	Summary
High-level Motivation & Architecture MOTIVATION.md, ARCHITECTURE.md, SULTANATE_MVP.md	Introduces platform goals, component responsibilities, trust boundaries, network flows, province lifecycle, CA lifecycle, and Phase‑1 scope vs deferred items.
API & State Contract (Data Shape / Schema) DIVAN_API_SPEC.md, DIVAN_MVP_PRD.md	Adds Divan FastAPI JSON contract, API-key RBAC, endpoint shapes for provinces/grants/whitelists/blacklist/appeals/access_requests/port_requests/audit, GET /janissary/state, and full SQLite schema + WAL semantics.
Agent/Runtime Artifact Specs OPENCLAW_FIRMAN_SPEC.md, OPENCLAW_CODING_BERAT_SPEC.md, OPENCLAW_FIRMAN_MVP_PRD.md, OPENCLAW_CODING_BERAT_MVP_PRD.md	Defines firman/berat artifact layouts and schemas, template rendering rules (str.replace), OpenClaw file placement, CA installation, and ordering constraints for bootstrap vs gateway start.
Orchestration & CLI Flows VIZIER_SPEC.md, VIZIER_MVP_PRD.md	Documents vizier-cli commands, firman/berat validation/rendering, WireGuard sidecar + Docker steps, Divan interactions, appeal-relay polling, detailed startup/health sequencing, and DivanClient design.
Secret Management (Aga) AGA_SPEC.md, AGA_MVP_PRD.md	Specifies Aga as the OpenBao AppRole client: AppRole policy expectations, KV v2 operations, GitHub App private-key storage, dynamic GitHub installation-token mint/renew/revoke flows, KV-fallback path with Kashif screening, Divan grant writes (lease metadata), port‑request approval workflow, iptables host-rule application, polling loops, and deployment/runbook notes.
Egress Proxy (Janissary) JANISSARY_SPEC.md, JANISSARY_MVP_PRD.md	Defines WireGuard + mitmproxy transparent routing, exact-match domain normalization, request-rule evaluation order (fail‑closed → blacklist → whitelist → GET/HEAD allow → write‑block), lease-aware header injection, appeal API, Divan poll/cache semantics, passthrough_domains for cert‑pinning, and deployment/startup scripts.
Content Screening (Kashif) KASHIF_MVP_PRD.md	Adds Kashif PRD: stateless three-layer screening endpoints (/screen/appeal, /screen/ingress), unanimous-allow requirement, 5s hard timeout → escalate, /health readiness, Divan integration for verdict persistence, resource budgeting, and image bake-time model artifacts.
Documentation, Governance & Indexing README.md, CLAUDE.md, ARCHITECTURE.md, SENTINELGATE_ANALYSIS.md	Reworks README (start guide, document index), adds CLAUDE editing guidance and invariants, updates architecture docs, and replaces Sentinel→Aga & Infisical→OpenBao references.
Removed / Replaced Legacy Files SULTANATE.md, JANISSARY_PRD_V2.md, VIZIER_PRD_V3.md, HERMES_FIRMAN_PRD_V1.md, HERMES_CODING_BERAT_PRD_V1.md	Deletes older PRD/spec artifacts superseded by the new MVP/spec documents.
Operational & Deployment Notes README.md, AGA_SPEC.md, JANISSARY_SPEC.md, VIZIER_SPEC.md	Adds Docker/docker‑compose snippets, startup ordering, healthcheck semantics, restart-recovery state files, iptables rule patterns, and pre-deployment checklist items.

sequenceDiagram
participant Sultan as rgba(0,102,204,0.5)
participant Vizier as rgba(0,153,51,0.5)
participant Divan as rgba(204,102,0,0.5)
participant Aga as rgba(153,51,255,0.5)
participant OpenBao as rgba(255,153,51,0.5)
participant Janissary as rgba(51,153,204,0.5)
participant Province as rgba(102,102,102,0.5)

Sultan->>Vizier: create province (firman + berat)
Vizier->>Divan: POST /provinces (status=creating)
Divan-->>Vizier: stored
Vizier->>Aga: province recorded (polling trigger)
Aga->>OpenBao: AppRole login & read private key
Aga->>OpenBao: mint GitHub installation token
OpenBao-->>Aga: token + lease metadata
Aga->>Divan: POST /grants (inject header, lease_expires_at)
Janissary->>Divan: GET /janissary/state (poll)
Divan-->>Janissary: grants/whitelist/cache
Province->>Janissary: outbound request
Janissary->>Janissary: evaluate rules, inject header (if lease valid)
Janissary->>Upstream: proxied request

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and specifically identifies the four core architectural components being rewritten in this MVP baseline: OpenClaw (agent runtime), OpenBao (secret vault), Kashif (content inspector), and Aga (operator/secret manager).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mvp-rewrite

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 22

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

MOTIVATION.md (1)

58-60: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify Phase 1 runtime boundary in this section.

Line 59 reads as if OpenHands/CrewAI are active MVP options, which conflicts with the locked Phase 1 runtime scope elsewhere. Add a short qualifier like “Phase 2” here to avoid scope drift.

Based on learnings: “Phase 1 runtime is OpenClaw only — openclaw-firman is the only firman and openclaw-coding-berat is the only berat; additional runtimes (OpenHands, CrewAI) are Phase 2 concerns”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@MOTIVATION.md` around lines 58 - 60, Update the sentence "runtime-agnostic:
it works the same whether the province runs OpenClaw, OpenHands, CrewAI, or
anything else." to explicitly limit Phase 1 by adding a Phase 2 qualifier (e.g.,
"...OpenClaw, OpenHands, CrewAI (Phase 2), or anything else") and, if helpful,
add a short clarifying clause that "Phase 1 runtime is OpenClaw only —
openclaw-firman and openclaw-coding-berat are the Phase 1 components" so the
section no longer implies OpenHands/CrewAI are Phase 1 options.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@AGA_MVP_PRD.md`:
- Around line 234-237: The MVP spec forbids direct Aga→Kashif calls, so update
the aga-ops MCP action screen_and_reply to route all coordination through Divan
instead of invoking Kashif directly: remove any direct call or callback to
Kashif inside screen_and_reply and replace it with a Divan-facing request (e.g.,
emit a Divan operation or message containing the screening payload and intended
Kashif action), ensure Divan is the component that subsequently invokes Kashif,
and update any related MCP endpoints/handlers (screen_and_reply in aga-ops, and
any consumers expecting direct Kashif responses) to use the Divan-mediated flow.

In `@AGA_SPEC.md`:
- Around line 386-387: Update the spec to consistently require Kashif screening
for all Sultan-originated token text by changing the source label in the
/screen/ingress step to source=`sultan` (instead of `pasha`) and remove or
reconcile the exception in §6 that exempts Sultan-authored messages; ensure the
flow explicitly states "abort if Kashif=`block`" for Sultan-originated/pasted
text and add a clarifying sentence in §6 that aligns with §3 (or replace the §6
exception with a precise condition when screening is not required).
- Around line 411-412: Update the HTTP method for the GitHub installation token
revocation in the spec: change the line currently stating "POST
https://api.github.com/installation/token" to use "DELETE
https://api.github.com/installation/token" (the correct REST API endpoint that
returns HTTP 204 No Content) so the revocation call is accurate and will
succeed.

In `@ARCHITECTURE.md`:
- Around line 398-401: The flow currently requires Sultan to paste a GitHub
token; change it so Aga performs GitHub App dynamic-minting and writes
lease-aware grants to Divan instead: remove the manual "Ask Sultan for GitHub
token" and "Sultan provides token" steps and replace them with "Aga mints
installation token via GitHub App, receives 1-hour TTL and schedules ~15min
auto-renew, then writes the lease-aware grant to Divan (POST /grants)"; also
state OpenBao is the sole secret vault with Aga as its sole client and Pashas
authenticate via Janissary-injected headers so no manual secret entry is needed.
- Around line 424-437: The architecture doc currently mixes two lifecycle styles
(direct DELETE /provinces/{id} and status transitions like destroying); pick and
enforce one authoritative province lifecycle contract across the document and
update the flow diagram and all references accordingly: either replace the
DELETE /provinces/{id} step and “record deleted (or status=destroyed)” with
explicit status transitions (e.g., PATCH /provinces/{id} -> {status: destroying}
then {status: destroyed}) in the Vizier "destroy" path and elsewhere (also
update the "Stop" path that uses PATCH /provinces/{id} -> {status: stopped}), or
conversely change the PATCH usages to a single DELETE approach everywhere;
ensure all occurrences (including the lines referencing "destroying", DELETE
/provinces/{id}, PATCH /provinces/{id}, and status=destroyed/stopped) are
consistent and document the chosen lifecycle state names and transitions.
- Around line 275-337: The documentation introduces a new API domain
(/access_request and /access_requests) and Divan state transitions that are not
defined in DIVAN_API_SPEC.md; update the spec to formally document the new
resources, request/response schemas, status values (pending/approved/denied),
fields like kashif_verdict, grant/lease semantics, and relevant endpoints used
by Vizier/Aga/Janissary, or alternatively rewrite this ARCHITECTURE.md section
to reference only currently supported endpoints; locate references to
"/access_request(s)" and "Divan" in the diff and either add the corresponding
definitions to DIVAN_API_SPEC.md or change the flow to map to existing API
surfaces.

In `@DIVAN_API_SPEC.md`:
- Around line 12-16: The spec currently lists both binding to the host Tailscale
interface (port 8601) and binding to 127.0.0.1:8601 as runtime defaults—make one
authoritative and describe the other as an explicit fallback: update the wording
that mentions "Tailscale interface bind" and "bind to `127.0.0.1:8601`" to state
a single primary mode (e.g., "By default the dashboard binds to the host's
Tailscale interface on port 8601") and then add a clear fallback sentence (e.g.,
"If Tailscale is not available, the dashboard will bind to 127.0.0.1:8601 and
must be accessed via SSH tunnel or similar"). Apply the same unification to the
other occurrence referenced (lines ~678-680) and add a short config/flag note
(dashboard bind mode or explicit --bind-address) so runtime behavior is
deterministic and documented.

In `@DIVAN_MVP_PRD.md`:
- Around line 135-136: Replace the sentence that says "Kashif continues to
screen (it's stateless) but its verdicts are buffered in-process until Divan
returns" with a fail-closed escalation policy: specify that when Divan is
unreachable Kashif should use last-cached rules if available, otherwise block
all requests; on Kashif timeouts escalate (do not buffer deferred writes);
document that OpenBao sealed means existing leases remain valid but expired
leases are treated as fail-closed; and remove any mention of in-process
buffering to avoid stale/reordered decisions.
- Around line 46-48: The PRD and API spec disagree on who can read the secret
field `inject.value` (PRD says only role Janissary; API spec allows Aga), so
pick the authoritative access-control contract and make both docs consistent: if
Janissary-only is correct, update the API spec role matrix to remove Aga's read
permission for `inject.value`; if Aga should also read it, update
DIVAN_MVP_PRD.md to include Aga alongside Janissary for `inject.value`. Ensure
the chosen contract is explicitly documented in both places (mention
`inject.value` and the roles Janissary and Aga) and adjust any related wording
or tables so the access-control statement is identical across the PRD and API
spec.
- Around line 76-90: The dashboard networking text is self-contradictory: it
claims "Primary access: Tailscale" and "dashboard listener is never bound to
0.0.0.0" but elsewhere locks startup/scope to 127.0.0.1; unify by picking one
clear binding policy (e.g., Tailscale-first with a documented fallback to
127.0.0.1) and update all references ("Primary access: Tailscale", "dashboard
listener", instances of "127.0.0.1" and the "never bound to 0.0.0.0" sentence)
so the document consistently states the default bind (Tailscale interface IP
like 100.x.y.z:8601), the fallback bind (127.0.0.1) and the
precedence/conditions that trigger the fallback.

In `@JANISSARY_MVP_PRD.md`:
- Around line 29-32: Replace non-Ottoman terminology ("agent") used in PRD prose
with repository-standard Ottoman terms (e.g., Pasha, Aga, Province, Firman) to
ensure consistency; specifically update the "Write block" rule and its
description (the sentence beginning "POST, PUT, PATCH, DELETE to a
non-whitelisted domain? Block. Return 403...") to use the chosen Ottoman term
(e.g., "Pasha" or "Aga") and reword accompanying guidance to reference the
appeal tool in Ottoman vocabulary if applicable; apply the same replacement to
the other flagged occurrences (the prose at the other noted locations) so all
mentions conform to the Ottoman glossary.

In `@JANISSARY_SPEC.md`:
- Around line 1133-1137: The docker-compose/service orchestration currently
lists a depends_on block requiring kashif (and divan) to be healthy before
Janissary but the documented startup sequence says Janissary starts before
kashif; fix the contradiction by making the declared startup order and the
depends_on directive consistent: either change the depends_on entries (the
depends_on: divan / kashif / condition: service_healthy) so Janissary does not
require kashif (remove or reorder kashif) if Janissary must start first, or
update the startup sequence/runbook to start kashif (and divan) before Janissary
so depends_on remains correct; apply the same change to the second occurrence
referenced (the block around the later section) so both the depends_on directive
and the operator runbook for Janissary vs kashif/divan are aligned.
- Around line 1347-1356: The health-check snippet for Divan and Kashif currently
loops forever (the until curl ... loops) but the prose states a 60s timeout;
pick one and make them consistent—either update the prose to say the script
blocks indefinitely, or (recommended) add a timeout implementation to the
script: introduce a WAIT_TIMEOUT (or MAX_ATTEMPTS) variable and replace the
infinite until loops for Divan and Kashif with a bounded loop that exits with a
non-zero status after the timeout, ensuring the same behavior is documented in
the startup section; update the corresponding prose lines (also mentioned around
1393-1394) to reflect the chosen timeout behavior.

In `@KASHIF_MVP_PRD.md`:
- Around line 156-162: The table row for "Kashif container unreachable (from
Janissary's perspective)" is missing the Verdict and Reason columns; update that
row so it has three cells: Condition = "Kashif container unreachable (from
Janissary's perspective)", Verdict = "escalate", and Reason = "Janissary writes
`kashif_verdict=escalate` itself with `kashif_timeout=true`" (ensuring the
backtick-wrapped token stays in the Reason cell).

In `@OPENCLAW_CODING_BERAT_MVP_PRD.md`:
- Around line 7-23: The PRD uses non‑canonical terms like "agent" and "template"
in the prose; update the section headings and body to use Ottoman naming
consistently (e.g., replace "agent" -> "Pasha" or "Berat" where referring to the
employee/profile, "container"/"template" -> "Firman" or "Province/Realm" as
appropriate) and ensure references to the five defined items use canonical names
(SOUL.md, AGENTS.md -> verify AGENTS.md maps to "Instructions"/"Divan" language,
IDENTITY.md stays as Identity, openclaw.json -> "OpenClaw configuration",
security policy -> use "whitelist, grants, non‑HTTP port declarations" but adopt
Sultanate terms like Firman/Province); keep meaning identical but eliminate
alternate vocabulary to conform to the guideline.

In `@OPENCLAW_FIRMAN_MVP_PRD.md`:
- Around line 1-10: Update the document to use Ottoman-first terminology:
replace occurrences of "container template", "container", "template", and
"agent" in the title and core definition sections with Firman and Province
(e.g., change the title and the sentence "The default container template
(firman) for Sultanate" to prioritize "Firman" and describe it as the default
Firman for the Province), keep only minimal parenthetical glosses if absolutely
necessary (e.g., "Firman (container template)") and ensure other core terms use
Sultan/Pasha/Berat/Divan/Realm/Aga consistently; also apply the same
replacements and consistency fixes in the later block referenced (lines 33-43)
so all prose uses Ottoman naming exclusively.

In `@OPENCLAW_FIRMAN_SPEC.md`:
- Around line 262-263: The spec currently references a non-existent field
channels.telegram.botTokenEnv; update the documentation to use the correct
OpenClaw config key channels.telegram.botToken (replace any occurrence of
channels.telegram.botTokenEnv with channels.telegram.botToken) so the SPEC
matches the actual configuration contract and the berat artifacts already using
channels.telegram.botToken remain consistent.

In `@SENTINELGATE_ANALYSIS.md`:
- Line 43: Update the flow text/diagram that currently shows "Kashif -> Aga ->
Sultan" to route all hops through Divan (e.g., "Kashif -> Divan -> Aga -> Divan
-> Sultan") so component-to-component communication is always mediated by Divan;
specifically modify the occurrences referencing Kashif, Aga, and Sultan (lines
around the flow diagram entries at the shown diff) and any other occurrences
(notably the similar chain at lines ~242-243) to reflect Divan as the
intermediary and adjust wording to state “All component inter-communication must
be routed through Divan — no direct component calls.”

In `@SULTANATE_MVP.md`:
- Around line 71-78: Update the Kashif timeout semantics: change the fail-closed
behavior for Kashif (the "Kashif" component handling POST /screen/appeal and
POST /screen/ingress) so that a timeout or unavailable LLM results in "escalate
+ alert Sultan" instead of "block + alert Sultan"; edit the line that currently
reads "Fail-closed: timeout or LLM down -> block + alert Sultan" to
"Fail-closed: timeout or LLM down -> escalate + alert Sultan" and ensure related
references in the appeal-flow and fail-closed contract sections consistently use
"escalate" for Kashif timeouts/unavailability.

In `@VIZIER_MVP_PRD.md`:
- Around line 8-12: Rewrite the PRD prose to use Ottoman terms as the primary
nouns: replace occurrences of "agent", "container", "template", and similar
alternates with "Pasha", "Province", "Firman", "Berat", etc., keeping a single
one-time gloss only at the first mention (e.g., "Pasha (agent)"). Ensure all
core definitions and sentences that currently read "CLI tool + OpenClaw agent",
"isolated containers", "container templates", "agent profiles", and any other
alternates are converted to the Ottoman vocabulary (Vizier, Pasha, Province,
Firman, Berat, Divan, Realm, Aga, Sultan, Janissary) and remove repeated
alternate terms; apply the same normalization to the other affected paragraphs
referenced in the comment.

In `@VIZIER_SPEC.md`:
- Around line 116-123: The spec incorrectly instructs vizier-cli destroy to call
Divan's DELETE /grants?province_id={id}; remove that step and update the Vizier
behavior text so vizier-cli destroy does not attempt to call the Divan grants
endpoint (DELETE /grants) and instead documents that grant cleanup is performed
exclusively by Aga; also make vizier-cli destroy resilient by noting it should
ignore or gracefully handle any 403s if such a call is attempted elsewhere
(e.g., in code paths named vizier-cli destroy or steps referencing DELETE
/grants) so the cleanup semantics remain unambiguous.
- Around line 155-181: The VIZIER_SPEC.md firman.yaml example uses an
object-structured schema (nested keys like image.repository, bootstrap.commands,
startup.command, defaults.branch) that conflicts with the flat-field shape
defined in OPENCLAW_FIRMAN_SPEC.md; choose one canonical schema and update both
files to match: either convert VIZIER_SPEC.md to the flat field names used by
OPENCLAW_FIRMAN_SPEC (or vice versa), ensuring keys such as image.repository,
image.tag, bootstrap.commands, startup.command/args and defaults.branch are
present and consistently documented, and run a quick validation of the chosen
spec against any schema validator or examples referenced by OPENCLAW_FIRMAN_SPEC
to prevent artifact drift.

---

Outside diff comments:
In `@MOTIVATION.md`:
- Around line 58-60: Update the sentence "runtime-agnostic: it works the same
whether the province runs OpenClaw, OpenHands, CrewAI, or anything else." to
explicitly limit Phase 1 by adding a Phase 2 qualifier (e.g., "...OpenClaw,
OpenHands, CrewAI (Phase 2), or anything else") and, if helpful, add a short
clarifying clause that "Phase 1 runtime is OpenClaw only — openclaw-firman and
openclaw-coding-berat are the Phase 1 components" so the section no longer
implies OpenHands/CrewAI are Phase 1 options.

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f265a701-e4cd-4917-91ae-e09066e2d18e

📥 Commits

Reviewing files that changed from the base of the PR and between 176d609 and 7de1860.

📒 Files selected for processing (24)

• AGA_MVP_PRD.md
• AGA_SPEC.md
• ARCHITECTURE.md
• CLAUDE.md
• DIVAN_API_SPEC.md
• DIVAN_MVP_PRD.md
• HERMES_CODING_BERAT_PRD_V1.md
• HERMES_FIRMAN_PRD_V1.md
• JANISSARY_MVP_PRD.md
• JANISSARY_PRD_V2.md
• JANISSARY_SPEC.md
• KASHIF_MVP_PRD.md
• MOTIVATION.md
• OPENCLAW_CODING_BERAT_MVP_PRD.md
• OPENCLAW_CODING_BERAT_SPEC.md
• OPENCLAW_FIRMAN_MVP_PRD.md
• OPENCLAW_FIRMAN_SPEC.md
• README.md
• SENTINELGATE_ANALYSIS.md
• SULTANATE.md
• SULTANATE_MVP.md
• VIZIER_MVP_PRD.md
• VIZIER_PRD_V3.md
• VIZIER_SPEC.md

💤 Files with no reviewable changes (5)

• HERMES_FIRMAN_PRD_V1.md
• VIZIER_PRD_V3.md
• SULTANATE.md
• HERMES_CODING_BERAT_PRD_V1.md
• JANISSARY_PRD_V2.md

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Route these flow diagrams through Divan, not direct component hops.

Line 43 and Line 242 depict Kashif -> Aga -> Sultan as a direct chain. In this repo’s MVP contract, component-to-component communication must be mediated by Divan; otherwise this introduces architecture drift in a document that readers will treat as design guidance.

Suggested wording update

-     -> Layer 4: Appeal -> Kashif (LLM triage) -> Aga -> Sultan
+     -> Layer 4: Appeal -> Kashif (LLM triage) -> Divan -> (Aga + Vizier) -> Sultan
...
-              -> Blocked? -> Kashif (LLM triage) -> Aga -> Sultan
+              -> Blocked? -> Kashif (LLM triage) -> Divan -> (Aga + Vizier) -> Sultan

Based on learnings: “All component inter-communication must be routed through Divan — no component should call another directly”.

Also applies to: 242-243

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@SENTINELGATE_ANALYSIS.md` at line 43, Update the flow text/diagram that
currently shows "Kashif -> Aga -> Sultan" to route all hops through Divan (e.g.,
"Kashif -> Divan -> Aga -> Divan -> Sultan") so component-to-component
communication is always mediated by Divan; specifically modify the occurrences
referencing Kashif, Aga, and Sultan (lines around the flow diagram entries at
the shown diff) and any other occurrences (notably the similar chain at lines
~242-243) to reflect Divan as the intermediary and adjust wording to state “All
component inter-communication must be routed through Divan — no direct component
calls.”

[stranma]

Declining. SENTINELGATE_ANALYSIS.md is marked "exploratory, not normative" in CLAUDE.md and preserved as Phase-2 reference material. The diagrams compare Sultanate's appeal flow against SentinelGate's at a simplified level; injecting "via Divan" hops into a SentinelGate-comparison diagram would clutter without clarifying the comparison.

The authoritative appeal-flow diagram is in ARCHITECTURE.md §2 — which DOES show Kashif writing verdicts to Divan via PATCH /appeals/{id}/kashif_verdict. This file is contextual reading, not the source of truth.

[coderabbitai]

@stranma, understood — the simplified flow in SENTINELGATE_ANALYSIS.md is intentionally abstracted for comparison purposes, and ARCHITECTURE.md §2 is the correct normative reference for the Divan-mediated appeal flow. I'll defer to that document for architecture accuracy.

---

✏️ Learnings added

Learnt from: stranma
Repo: stranma/sultanate PR: 1
File: SENTINELGATE_ANALYSIS.md:43-43
Timestamp: 2026-05-05T13:05:06.435Z
Learning: `SENTINELGATE_ANALYSIS.md` in the stranma/sultanate repo is explicitly marked "exploratory, not normative" in `CLAUDE.md` and is preserved as Phase-2 reference material. Its simplified flow diagrams (e.g., `Kashif -> Aga -> Sultan`) are intentional comparisons against SentinelGate's architecture and should not be flagged for missing Divan-mediation hops. The authoritative appeal-flow diagram showing Divan mediation via `PATCH /appeals/{id}/kashif_verdict` lives in `ARCHITECTURE.md` §2.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Normalize PRD prose to Ottoman terms as the primary vocabulary.

This _MVP_PRD uses “agent”/“container” as primary nouns in core definitions (Line 8 onward). Keep Ottoman terms primary (Vizier/Pasha/Province/Firman/Berat), and only use one-time glosses where needed. This doc is likely to be copied into other PRDs, so terminology drift here propagates quickly.

As per coding guidelines: “Use Ottoman naming consistently (Sultan, Pasha, Province, Firman, Berat, Divan, Realm, Aga) — do not introduce alternate names like 'agent', 'container', or 'template' in PRD prose”.

Also applies to: 18-23

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@VIZIER_MVP_PRD.md` around lines 8 - 12, Rewrite the PRD prose to use Ottoman
terms as the primary nouns: replace occurrences of "agent", "container",
"template", and similar alternates with "Pasha", "Province", "Firman", "Berat",
etc., keeping a single one-time gloss only at the first mention (e.g., "Pasha
(agent)"). Ensure all core definitions and sentences that currently read "CLI
tool + OpenClaw agent", "isolated containers", "container templates", "agent
profiles", and any other alternates are converted to the Ottoman vocabulary
(Vizier, Pasha, Province, Firman, Berat, Divan, Realm, Aga, Sultan, Janissary)
and remove repeated alternate terms; apply the same normalization to the other
affected paragraphs referenced in the comment.

[stranma]

Pushing back. CLAUDE.md's Ottoman-naming invariant explicitly permits first-use gloss: "First use per document may gloss the term, e.g. 'Pasha (agent inside a province)'." The flagged passages follow exactly that pattern — "agent profile (berat)", "container template (firman)", "firmans (container templates) and berats (agent profiles)" — gloss on first mention, then Ottoman terms downstream.

A strict "no alternate names ever" reading would require updating CLAUDE.md to forbid the gloss. Current state matches the invariant as written. Closing as "matches documented invariant".

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check CLAUDE.md for the Ottoman naming invariant and gloss rules
cat CLAUDE.md

Length of output: 8857

---

@stranma, you are correct — I misread the invariant. CLAUDE.md explicitly states: "First use per document may gloss the term, e.g. 'Pasha (agent inside a province)'." The flagged passages (firmans (container templates), berats (agent profiles), provinces (isolated containers)) all follow exactly that pattern: Ottoman term primary, alternate in parentheses, on first mention only. My original comment applied the "no alternate names" clause as an absolute prohibition while ignoring the gloss carve-out in the same sentence. The current prose matches the documented invariant. Withdrawing the suggestion.

[coderabbitai]

Actionable comments posted: 9

♻️ Duplicate comments (2)

ARCHITECTURE.md (1)

447-448: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Province destroy lifecycle still conflicts with the documented status model.

This section still mixes “deleted record” / status=destroyed semantics, while the API lifecycle contract is status-driven and does not document deletion as the normal terminal behavior. Please make the lifecycle wording fully consistent.

Also applies to: 663-664

🤖 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 `@ARCHITECTURE.md` around lines 447 - 448, Update the lifecycle wording so it
reflects the status-driven API contract instead of implying record deletion:
replace occurrences under the PATCH /provinces/{id} example that show "{status:
destroyed}" or language implying deletion with the documented terminal status
term used by the spec (e.g., "{status: archived}" or the canonical terminal
status name), and adjust the surrounding sentence to describe a status
transition to the terminal state rather than physical deletion; apply the same
change to the other occurrence that currently mixes deletion semantics (the
second instance referenced).

AGA_SPEC.md (1)

741-742: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

SOUL policy contradicts the mandatory Kashif screening path for Sultan-pasted secret material.

Line 741-742 says Sultan content bypasses Kashif, but Line 386-387 and Section 6 require screening for Sultan-pasted secret payloads. Keep one rule to avoid divergent implementations.

As per coding guidelines: “All Aga ingress … must be pre-screened by Kashif …”.

🤖 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 `@AGA_SPEC.md` around lines 741 - 742, The document currently contradicts
itself by exempting "Sultan's Telegram channel" content from Kashif screening
while Section 6 and the SOUL policy mandate Kashif pre-screening for
Sultan-pasted secret payloads; update the sentence that claims Sultan content is
trusted to instead state that all Sultan-originating ingress (including
Telegram/pasted content) must pass Kashif screening, and remove or reword any
language that creates an exception so the AGA_SPEC and SOUL policy/Section 6 are
consistent.

🤖 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 `@AGA_SPEC.md`:
- Around line 292-295: The spec currently treats openbao_lease_id as an
opaque/fake metadata field; change the model and any code that writes or reads
grant records so openbao_lease_id holds the real backing lease reference (or
null when no lease exists) to match the Divan grant contract—update validation,
serializers/deserializers, and any places that generate values for
openbao_lease_id (and related logic using lease_expires_at) to accept a real
lease identifier format or null and ensure consumers expect a real lease ref
rather than an opaque placeholder.

In `@ARCHITECTURE.md`:
- Around line 269-276: Update the flow diagram to use the canonical
access-request endpoint name everywhere: replace occurrences of "POST /api/req"
with "POST /access_requests" (and ensure any related arrows/labels that
reference /api/req are changed as well) so the sequence diagram consistently
matches the API contract; verify symbols like "POST /access_requests",
"screen/ingress(text)", "regex", "PromptGuard", and "LlamaGuard" remain
unchanged aside from the endpoint rename.

In `@DIVAN_API_SPEC.md`:
- Around line 233-235: Update the doc text to enforce fail-closed semantics:
replace the current sentence that implies forwarding without credentials with
language that an expired lease causes Janissary to deny/block the request for
that credentialed path (no pass-through), log the audit entry for Aga to detect
and re-issue credentials, and return the appropriate 401/403 to the client;
reference the components "Janissary" and "Aga" and the "expired lease" /
"credentialed path" concepts so readers know where the behavior applies and that
the system never falls back to pass-through.

In `@DIVAN_MVP_PRD.md`:
- Around line 3-5: Replace the current top-line subordination text with the
exact mandated sentence: "For shared glossary and architecture see
[SULTANATE_MVP.md](SULTANATE_MVP.md)." — update the header in DIVAN_MVP_PRD.md
so that the very first line matches that sentence exactly (remove or replace the
existing line "For shared glossary, deployment model, and component overview see
[SULTANATE_MVP.md](SULTANATE_MVP.md). For HTTP contract detail see
[DIVAN_API_SPEC.md](DIVAN_API_SPEC.md)." with the required canonical
subordination sentence), preserving the rest of the document.

In `@JANISSARY_SPEC.md`:
- Around line 816-818: The spec and client currently describe Janissary making a
direct POST to Kashif at /screen/appeal with {appeal_id, payload, justification,
url, method, province_id}; change this so Janissary routes the request through
Divan instead of calling Kashif directly: update the spec text and client code
that constructs the POST to call Divan's forwarding endpoint (e.g.,
Divan.forward or Divan proxy API) with the same payload and configurable timeout
handling, remove the hardcoded "http://127.0.0.1:8082/screen/appeal" target and
any direct Kashif URL usage, and ensure downstream handlers (Kashif) expect the
forwarded envelope format and tests/docs are updated to reflect
Janissary→Divan→Kashif flow.
- Around line 868-870: The spec currently offers two incompatible persistence
paths ("POST /access_requests" vs overloading "/appeals" with
"access-request://{service}"), causing divergent implementations; pick a single
explicit path (recommend using POST /access_requests), remove the alternative
overloading of /appeals and the "access-request://{service}" scheme, and update
all occurrences, examples, and API descriptions (e.g., the section referencing
"POST /access_requests" and the paragraph mentioning "/appeals" and
"access-request://{service}") to use only the chosen endpoint and its
payload/response shape.

In `@SULTANATE_MVP.md`:
- Around line 170-172: The doc inconsistently states two GitHub App renewal
intervals (~30 min here vs ~15 min later); pick one canonical interval (e.g., 15
or 30 minutes) and update every occurrence so they match: change the sentence
referencing `lease_expires_at` and the description of the "background renewal
loop in Aga" (and any other mention of renewal cadence) to use the chosen
interval, and ensure notes about stopping on `destroy` and GitHub token lifetime
("GitHub kills the token within 1 hour") remain accurate and consistent with
that interval.
- Around line 62-77: Update the architecture text so Janissary does not call
Kashif directly: make Divan the authoritative contract for routing screening
state transitions and describe Janissary as sending appeal/ingress screening
requests to Divan (which then forwards to Kashif), remove any language implying
direct Janissary→Kashif coupling, and clarify endpoints (e.g., POST
/screen/appeal, POST /screen/ingress) and fail-closed semantics under the
Divan-mediated flow so all inter-component coordination references point to
Divan as the single router.

In `@VIZIER_SPEC.md`:
- Around line 479-482: The code currently mutates Janissary config via
add_peer_to_janissary_config(public_key, wg_peer_ip) and signals the container
with subprocess.run(["docker", "kill", "-s", "HUP", "janissary"], check=True);
instead, remove these direct actions and record the peer lifecycle intent in
Divan's state store (e.g., create or update a peer-intent record containing
public_key and wg_peer_ip and desired state like "add"), using the existing
Divan state API/functionality your repo provides (replace references to
add_peer_to_janissary_config and the subprocess.run call with a single Divan
state mutation call such as publish_peer_intent or update_divan_state). Ensure
the intent includes sufficient metadata for Janissary to reconcile (public_key,
wg_peer_ip, desired_action, caller/ts) and return or log the Divan operation
result; Janissary will be responsible for watching Divan state and applying the
actual wg0.conf changes and HUP.

---

Duplicate comments:
In `@AGA_SPEC.md`:
- Around line 741-742: The document currently contradicts itself by exempting
"Sultan's Telegram channel" content from Kashif screening while Section 6 and
the SOUL policy mandate Kashif pre-screening for Sultan-pasted secret payloads;
update the sentence that claims Sultan content is trusted to instead state that
all Sultan-originating ingress (including Telegram/pasted content) must pass
Kashif screening, and remove or reword any language that creates an exception so
the AGA_SPEC and SOUL policy/Section 6 are consistent.

In `@ARCHITECTURE.md`:
- Around line 447-448: Update the lifecycle wording so it reflects the
status-driven API contract instead of implying record deletion: replace
occurrences under the PATCH /provinces/{id} example that show "{status:
destroyed}" or language implying deletion with the documented terminal status
term used by the spec (e.g., "{status: archived}" or the canonical terminal
status name), and adjust the surrounding sentence to describe a status
transition to the terminal state rather than physical deletion; apply the same
change to the other occurrence that currently mixes deletion semantics (the
second instance referenced).

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2d4461b5-ee8e-406a-8bee-c1499805a398

📥 Commits

Reviewing files that changed from the base of the PR and between 7de1860 and 379b2cc.

📒 Files selected for processing (10)

• AGA_SPEC.md
• ARCHITECTURE.md
• DIVAN_API_SPEC.md
• DIVAN_MVP_PRD.md
• JANISSARY_SPEC.md
• KASHIF_MVP_PRD.md
• MOTIVATION.md
• OPENCLAW_FIRMAN_SPEC.md
• SULTANATE_MVP.md
• VIZIER_SPEC.md

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the canonical access-request endpoint name consistently.

This sequence uses POST /api/req, but the API contract elsewhere specifies /access_requests. Please align this flow to the canonical endpoint to prevent implementation drift.

🤖 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 `@ARCHITECTURE.md` around lines 269 - 276, Update the flow diagram to use the
canonical access-request endpoint name everywhere: replace occurrences of "POST
/api/req" with "POST /access_requests" (and ensure any related arrows/labels
that reference /api/req are changed as well) so the sequence diagram
consistently matches the API contract; verify symbols like "POST
/access_requests", "screen/ingress(text)", "regex", "PromptGuard", and
"LlamaGuard" remain unchanged aside from the endpoint rename.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Route Janissary↔Kashif interactions through Divan, not direct HTTP calls.

Line 816 and Line 862 define direct Janissary→Kashif calls, and the client code (Line 970 onward) hardcodes that path. This breaks the documented one-way dependency rule and couples two components at runtime.

As per coding guidelines: “Enforce one-way component dependencies via Divan: no component should call another directly. Route all inter-component communication through Divan instead”.

Also applies to: 862-864, 970-1013

🤖 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 `@JANISSARY_SPEC.md` around lines 816 - 818, The spec and client currently
describe Janissary making a direct POST to Kashif at /screen/appeal with
{appeal_id, payload, justification, url, method, province_id}; change this so
Janissary routes the request through Divan instead of calling Kashif directly:
update the spec text and client code that constructs the POST to call Divan's
forwarding endpoint (e.g., Divan.forward or Divan proxy API) with the same
payload and configurable timeout handling, remove the hardcoded
"http://127.0.0.1:8082/screen/appeal" target and any direct Kashif URL usage,
and ensure downstream handlers (Kashif) expect the forwarded envelope format and
tests/docs are updated to reflect Janissary→Divan→Kashif flow.

[stranma]

Pushing back. The "one-way dependencies via Divan" invariant in CLAUDE.md is about state coordination — records that live in Divan (provinces, grants, appeals, audit, etc.). Kashif's /screen/appeal and /screen/ingress are stateless function calls (input → verdict). Routing them through Divan would add ~5-15s poll-cycle latency per appeal (caller→Divan→Kashif poll→verdict→Divan→caller poll) for the same trust boundary.

Codified the exception in CLAUDE.md (commit 8e413c5): the invariant now explicitly distinguishes state coordination (must go through Divan) from synchronous capability calls to stateless services (direct HTTP allowed). Kashif still writes verdicts to Divan for audit and appeal-state; only the request/response path is direct. Janissary→Kashif and Aga→Kashif both fall under this exception consistently.

[coderabbitai]

@stranma, that's a well-drawn distinction — the invariant applies to stateful coordination (records that live in Divan), not to synchronous capability calls against inherently stateless services. Routing a blocking content-inspection call through a 5s poll cycle would be architecturally harmful, not safe. The pattern where Kashif answers the request/response path directly while still writing the verdict and audit entry to Divan keeps the right things in the right place.

Acknowledged — my original comment was wrong on this one.

---

✏️ Learnings added

Learnt from: stranma
Repo: stranma/sultanate PR: 1
File: JANISSARY_SPEC.md:816-818
Timestamp: 2026-05-05T13:05:13.353Z
Learning: In the Sultanate architecture (repository: stranma/sultanate), the "one-way dependency via Divan" invariant applies only to *state coordination* — records that live in Divan (provinces, grants, appeals, audit, etc.). Direct HTTP calls between components are explicitly allowed for *synchronous capability calls to stateless services*. Kashif (`/screen/appeal`, `/screen/ingress`) is a stateless input→verdict service; Janissary→Kashif and Aga→Kashif direct HTTP calls are the correct pattern. Kashif still writes verdicts and audit entries to Divan; only the synchronous request/response path is direct. This exception is codified in CLAUDE.md (commit 8e413c5).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Route screening state transitions through Divan instead of direct Janissary→Kashif coupling.

This section defines direct Janissary-to-Kashif workflow semantics in the architecture core, which conflicts with the repository rule that inter-component coordination must be routed via Divan. Please make one contract authoritative and align this section accordingly to avoid split orchestration logic.

As per coding guidelines: "Enforce one-way component dependencies via Divan: no component should call another directly. Route all inter-component communication through Divan instead".

🤖 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 `@SULTANATE_MVP.md` around lines 62 - 77, Update the architecture text so
Janissary does not call Kashif directly: make Divan the authoritative contract
for routing screening state transitions and describe Janissary as sending
appeal/ingress screening requests to Divan (which then forwards to Kashif),
remove any language implying direct Janissary→Kashif coupling, and clarify
endpoints (e.g., POST /screen/appeal, POST /screen/ingress) and fail-closed
semantics under the Divan-mediated flow so all inter-component coordination
references point to Divan as the single router.

[stranma]

Pushing back. The "one-way dependencies via Divan" invariant in CLAUDE.md is about state coordination — records that live in Divan (provinces, grants, appeals, audit, etc.). Kashif's /screen/appeal and /screen/ingress are stateless function calls (input → verdict). Routing them through Divan would add ~5-15s poll-cycle latency per appeal (caller→Divan→Kashif poll→verdict→Divan→caller poll) for the same trust boundary.

Codified the exception in CLAUDE.md (commit 8e413c5): the invariant now explicitly distinguishes state coordination (must go through Divan) from synchronous capability calls to stateless services (direct HTTP allowed). Kashif still writes verdicts to Divan for audit and appeal-state; only the request/response path is direct. Janissary→Kashif and Aga→Kashif both fall under this exception consistently.

[coderabbitai]

@stranma, that's a well-reasoned distinction and I agree with it. The Divan invariant is correctly scoped to state coordination — durable records (provinces, grants, appeals, audit entries, etc.) that multiple components need to read over time. Kashif's /screen/appeal and /screen/ingress are synchronous RPC-style capability calls with ephemeral request/response semantics; routing those through a poll-cycle intermediary would be both high-latency and architecturally backward (Divan has no business orchestrating compute). The fact that Kashif still writes verdicts back to Divan for appeal-state transitions and audit entries means the state layer remains authoritative — only the synchronous invocation path is direct, which is the right split.

Retracting the comment. The codification in CLAUDE.md is the right call.

---

✏️ Learnings added

Learnt from: stranma
Repo: stranma/sultanate PR: 1
File: SULTANATE_MVP.md:62-77
Timestamp: 2026-05-05T13:05:10.753Z
Learning: In the Sultanate project (docs-only repo, Markdown architecture specs), the "route all inter-component coordination through Divan" invariant applies exclusively to *state coordination* — durable records such as provinces, grants, appeals, whitelists, audit entries. It does NOT apply to synchronous, stateless capability calls (e.g., Janissary→Kashif POST /screen/appeal or Aga→Kashif POST /screen/ingress). Those are direct HTTP calls to avoid poll-cycle latency (~5-15s). Kashif still writes verdicts to Divan for audit and appeal-state transitions; only the synchronous request/response path bypasses Divan. This exception is codified in CLAUDE.md.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid direct Janissary config mutation from Vizier.

Having Vizier edit Janissary wg0.conf and signal Janissary directly creates a cross-component control path outside Divan. Move peer lifecycle intent into Divan state and let Janissary reconcile from that state.

As per coding guidelines: "Enforce one-way component dependencies via Divan: no component should call another directly. Route all inter-component communication through Divan instead".

🤖 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 `@VIZIER_SPEC.md` around lines 479 - 482, The code currently mutates Janissary
config via add_peer_to_janissary_config(public_key, wg_peer_ip) and signals the
container with subprocess.run(["docker", "kill", "-s", "HUP", "janissary"],
check=True); instead, remove these direct actions and record the peer lifecycle
intent in Divan's state store (e.g., create or update a peer-intent record
containing public_key and wg_peer_ip and desired state like "add"), using the
existing Divan state API/functionality your repo provides (replace references to
add_peer_to_janissary_config and the subprocess.run call with a single Divan
state mutation call such as publish_peer_intent or update_divan_state). Ensure
the intent includes sufficient metadata for Janissary to reconcile (public_key,
wg_peer_ip, desired_action, caller/ts) and return or log the Divan operation
result; Janissary will be responsible for watching Divan state and applying the
actual wg0.conf changes and HUP.

[coderabbitai]

Actionable comments posted: 8

♻️ Duplicate comments (3)

AGA_SPEC.md (1)

746-747: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Resolve contradiction on Sultan-originated screening in SOUL instructions.

Line 746–747 says Sultan channel content does not require Kashif screening, but Section 6 explicitly requires screening Sultan-pasted secret material via /screen/ingress. Please make SOUL.md consistent with Section 6 to avoid bypass behavior.

Based on learnings: "Aga ingress must be pre-screened by Kashif ..."; and Section 6 already scopes the exception correctly for structured commands vs pasted secret payloads.

🤖 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 `@AGA_SPEC.md` around lines 746 - 747, Update the SOUL instructions to remove
the contradictory statement that "Content from Sultan's Telegram channel is
trusted and does not require Kashif screening" and instead align with Section 6
by stating that any Sultan-originated pasted secret material must be
pre-screened by Kashif via the /screen/ingress flow; keep the existing exception
language for structured, authorized Sultan commands (if any) but explicitly
require Kashif screening for pasted payloads and reference the /screen/ingress
process and Kashif as the required gatekeeper to prevent bypass.

ARCHITECTURE.md (2)

664-664: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pick one province-destroy contract (status transition vs delete).

Line 664 still says “record deleted (or status=destroyed)”, which reintroduces ambiguity after the rest of the doc moved to explicit lifecycle states. Keep only the status-based contract for consistency.

🤖 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 `@ARCHITECTURE.md` at line 664, Replace the ambiguous checklist item "Divan
province record deleted (or status=destroyed)" with a single status-based
contract phrase to match the rest of the doc — e.g., change it to "Divan
province status=destroyed" (remove any mention of record deletion) so the file
consistently uses lifecycle state transitions; update the checklist item that
currently contains "Divan province record deleted (or status=destroyed)"
accordingly.

---

269-269: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use canonical API paths consistently in flow + test assertions.

Line 269 and the test assertions at Line 541/588 still use /api/appeal and /api/request_access, while the canonical contract in this PR is /appeals and /access_requests. This will cause integration/test drift.

Also applies to: 541-541, 588-588

🤖 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 `@ARCHITECTURE.md` at line 269, Update the API path strings so the diagram and
tests use the canonical contract: replace any occurrences of "/api/appeal" with
"/appeals" and "/api/request_access" with "/access_requests" (this affects the
flow diagram entry currently showing "|--POST /api/request_access ->|" and the
test assertions referencing "/api/appeal" and "/api/request_access"); ensure
both the architecture flow text and the tests assert the new "/appeals" and
"/access_requests" endpoints consistently.

🤖 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 `@DIVAN_API_SPEC.md`:
- Around line 723-727: The "alert" bullet in DIVAN_API_SPEC.md still reads
"lease-expired skips" but the spec defines fail-closed blocking as
lease_expired_block; update the wording in that bullet to "lease-expired blocks"
(or "lease_expired_block") so the phrase matches the defined behavior and
symbol. Locate the "alert" list item (the line containing "Kashif block, Kashif
escalate, repeated blacklist hits, lease-expired skips, port request approvals")
and replace the "lease-expired skips" token with "lease-expired blocks" to align
with the lease_expired_block definition.

In `@DIVAN_MVP_PRD.md`:
- Around line 13-15: Rewrite the sentence that currently states "no component
calls another directly" to specify that the rule applies only to
durable/stateful coordination via Divan, and explicitly allow direct synchronous
HTTP calls between stateless services (e.g., Janissary→Kashif and Aga→Kashif)
for screening/validation; reference the components Vizier, Janissary, Kashif,
Aga and the coordination store Divan so readers know the exception is limited to
stateless, synchronous interactions while durable state updates must go through
Divan.

In `@JANISSARY_MVP_PRD.md`:
- Around line 64-69: The doc currently instructs Janissary to forward requests
without the header when lease_expires_at is past, but this conflicts with the
fail-closed contract; update the text for the lease_expires_at branch to instead
block the request (do not forward), return a 503 with error code/label
"credential_renewing", and write an audit entry with severity=alert,
action=lease_expired, credential_injected=false so agents know to renew;
reference the lease_expires_at check and the fail-closed/credential_renewing
behavior so the wording matches other sections describing fail-closed blocking.

In `@JANISSARY_SPEC.md`:
- Around line 565-570: The Grant model currently uses openbao_lease_id as the
stable grant identifier but that field can be null for GitHub tokens; add an
explicit non-optional id field on class Grant (e.g., Grant.id: str) and keep
openbao_lease_id: str | None as-is, then change all emissions/serializations
that set grant_id to use grant.id instead of grant.openbao_lease_id (search for
usages of grant.openbao_lease_id in serialization/emit logic and the code paths
referenced around the Grant class, e.g., any lease_expired_block/ audit recovery
emitters) so consumers get a stable grant_id while preserving openbao_lease_id
as an optional field.
- Around line 47-65: In JANISSARY_SPEC.md update all fenced code blocks (e.g.,
the project tree block under the janissary/ example) to include a language
identifier (like ```text or ```bash) to satisfy MD040, ensure JSON fenced blocks
have the required blank lines before and after the fences to satisfy MD031, and
correct the spelling of “re-establishes” in the failure-mode table (search for
the table entries where that word appears). Apply the same fixes to the other
occurrences listed in the comment ranges so all fenced blocks across the
document include language tags and proper blank-line spacing and the typo is
fixed.
- Around line 810-812: Replace all occurrences of the generic term "agent" in
the runtime/request/appeal flows with the Ottoman term "Pasha" (or introduce a
single first-use gloss like "Pasha (agent)" and thereafter use only
"Pasha")—specifically update the sentence about write requests blocked by rule 4
and the 403 response body wording, and apply the same replacement throughout the
blocks flagged (around the current lines 810, 857–874, and 953–965) so all
request paths, appeal paths, examples, and descriptive text use "Pasha"
consistently.
- Around line 763-770: Update the prose describing the lease-expiry flow to
match the fail-closed behavior: when grant.lease_expires_at <= now the proxy
must block and return a 503 with error code/phrase credential_renewing (do not
forward upstream), and record an audit entry with severity=alert; replace any
wording that says “skip injection, write audit … continue” or “upstream 401/403”
with explicit fail-closed language. Apply the same change to the other
referenced passage that currently describes pass-through behavior (the section
mirroring lines 799-803) so both places consistently state proxy-level block,
503 credential_renewing, and audit logging.

In `@SULTANATE_MVP.md`:
- Around line 221-225: The spec currently states "Grant secret values are only
returned to the Janissary role" which contradicts the Divan API docs that allow
plaintext grant visibility to both Aga and Janissary; update this section to
reflect the correct access-control contract by changing the visibility statement
to indicate that grant secret values are returned to both the Aga and Janissary
roles (not Janissary-only), and add a cross-reference to DIVAN_API_SPEC.md and
the relevant `aga` and `janissary` role entries so readers can verify
endpoint-level read/write permissions.

---

Duplicate comments:
In `@AGA_SPEC.md`:
- Around line 746-747: Update the SOUL instructions to remove the contradictory
statement that "Content from Sultan's Telegram channel is trusted and does not
require Kashif screening" and instead align with Section 6 by stating that any
Sultan-originated pasted secret material must be pre-screened by Kashif via the
/screen/ingress flow; keep the existing exception language for structured,
authorized Sultan commands (if any) but explicitly require Kashif screening for
pasted payloads and reference the /screen/ingress process and Kashif as the
required gatekeeper to prevent bypass.

In `@ARCHITECTURE.md`:
- Line 664: Replace the ambiguous checklist item "Divan province record deleted
(or status=destroyed)" with a single status-based contract phrase to match the
rest of the doc — e.g., change it to "Divan province status=destroyed" (remove
any mention of record deletion) so the file consistently uses lifecycle state
transitions; update the checklist item that currently contains "Divan province
record deleted (or status=destroyed)" accordingly.
- Line 269: Update the API path strings so the diagram and tests use the
canonical contract: replace any occurrences of "/api/appeal" with "/appeals" and
"/api/request_access" with "/access_requests" (this affects the flow diagram
entry currently showing "|--POST /api/request_access ->|" and the test
assertions referencing "/api/appeal" and "/api/request_access"); ensure both the
architecture flow text and the tests assert the new "/appeals" and
"/access_requests" endpoints consistently.

🪄 Autofix (Beta)

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 924a1866-8da6-40e3-a789-87bd0d25cbf7

📥 Commits

Reviewing files that changed from the base of the PR and between 379b2cc and e2d1f02.

📒 Files selected for processing (10)

• AGA_MVP_PRD.md
• AGA_SPEC.md
• ARCHITECTURE.md
• CLAUDE.md
• DIVAN_API_SPEC.md
• DIVAN_MVP_PRD.md
• JANISSARY_MVP_PRD.md
• JANISSARY_SPEC.md
• OPENCLAW_CODING_BERAT_SPEC.md
• SULTANATE_MVP.md

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update audit example wording from “lease-expired skips” to “lease-expired blocks.”

Line 726 still references the old “skip” behavior, but this spec now defines fail-closed blocking (lease_expired_block). Please align the wording to avoid operational confusion.

🤖 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 `@DIVAN_API_SPEC.md` around lines 723 - 727, The "alert" bullet in
DIVAN_API_SPEC.md still reads "lease-expired skips" but the spec defines
fail-closed blocking as lease_expired_block; update the wording in that bullet
to "lease-expired blocks" (or "lease_expired_block") so the phrase matches the
defined behavior and symbol. Locate the "alert" list item (the line containing
"Kashif block, Kashif escalate, repeated blacklist hits, lease-expired skips,
port request approvals") and replace the "lease-expired skips" token with
"lease-expired blocks" to align with the lease_expired_block definition.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Narrow the “no direct calls” rule to state coordination only.

Line 14 currently says no component calls another directly, which contradicts the documented exception for direct synchronous stateless screening calls (Janissary→Kashif and Aga→Kashif). Please scope this sentence to durable state coordination through Divan.

Based on learnings: "all inter-component coordination through Divan applies only to state coordination ... direct synchronous HTTP calls to stateless services are exempt."

🤖 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 `@DIVAN_MVP_PRD.md` around lines 13 - 15, Rewrite the sentence that currently
states "no component calls another directly" to specify that the rule applies
only to durable/stateful coordination via Divan, and explicitly allow direct
synchronous HTTP calls between stateless services (e.g., Janissary→Kashif and
Aga→Kashif) for screening/validation; reference the components Vizier,
Janissary, Kashif, Aga and the coordination store Divan so readers know the
exception is limited to stateless, synchronous interactions while durable state
updates must go through Divan.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align expired-lease behavior to fail-closed blocking (not pass-through).

Line 64–69 currently says Janissary should forward without header when lease_expires_at is expired. That conflicts with the documented fail-closed contract elsewhere in this PR (block with 503 credential_renewing and alert/audit), and will create implementation drift.

Proposed doc fix

-1. Check `lease_expires_at`. If set and past current UTC time, **skip
-   injection** and write an audit entry with `severity=alert,
-   action=lease_expired, credential_injected=false`. The request
-   proceeds without the header; upstream will return `401`/`403`
-   naturally, signalling to the agent that the token is stale. Aga
-   sees the audit entry and re-issues or renews.
+1. Check `lease_expires_at`. If set and past current UTC time, **fail
+   closed**: return `503` with `error=credential_renewing`, do not
+   forward upstream, and write an alert-severity audit entry
+   (`action=lease_expired_block`). Aga uses that alert to expedite
+   renewal, then the Pasha retries.

Based on learnings: "Implement fail-closed behavior everywhere: ... expired leases fail closed".

🤖 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 `@JANISSARY_MVP_PRD.md` around lines 64 - 69, The doc currently instructs
Janissary to forward requests without the header when lease_expires_at is past,
but this conflicts with the fail-closed contract; update the text for the
lease_expires_at branch to instead block the request (do not forward), return a
503 with error code/label "credential_renewing", and write an audit entry with
severity=alert, action=lease_expired, credential_injected=false so agents know
to renew; reference the lease_expires_at check and the
fail-closed/credential_renewing behavior so the wording matches other sections
describing fail-closed blocking.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix markdownlint blockers and spelling issue to keep docs CI clean.

Add language identifiers to fenced blocks (MD040), add required blank lines around JSON fences (MD031), and fix the typo “re-establishes” in the failure-mode table.

Also applies to: 100-104, 182-186, 210-228, 280-307, 467-475, 952-972, 1429-1474, 1491-1493, 1547-1547

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 47-47: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@JANISSARY_SPEC.md` around lines 47 - 65, In JANISSARY_SPEC.md update all
fenced code blocks (e.g., the project tree block under the janissary/ example)
to include a language identifier (like ```text or ```bash) to satisfy MD040,
ensure JSON fenced blocks have the required blank lines before and after the
fences to satisfy MD031, and correct the spelling of “re-establishes” in the
failure-mode table (search for the table entries where that word appears). Apply
the same fixes to the other occurrences listed in the comment ranges so all
fenced blocks across the document include language tags and proper blank-line
spacing and the typo is fixed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

grant_id is sourced from openbao_lease_id, which can be null for GitHub tokens.

The spec currently returns grant_id: grant.openbao_lease_id. With the documented GitHub App path, openbao_lease_id may be null, which breaks correlation for lease_expired_block recovery/audit workflows expecting a stable grant identifier. Add explicit Grant.id and emit that as grant_id; keep openbao_lease_id as a separate optional field.

Also applies to: 727-731, 741-742

🤖 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 `@JANISSARY_SPEC.md` around lines 565 - 570, The Grant model currently uses
openbao_lease_id as the stable grant identifier but that field can be null for
GitHub tokens; add an explicit non-optional id field on class Grant (e.g.,
Grant.id: str) and keep openbao_lease_id: str | None as-is, then change all
emissions/serializations that set grant_id to use grant.id instead of
grant.openbao_lease_id (search for usages of grant.openbao_lease_id in
serialization/emit logic and the code paths referenced around the Grant class,
e.g., any lease_expired_block/ audit recovery emitters) so consumers get a
stable grant_id while preserving openbao_lease_id as an optional field.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Lease-expiry flow prose contradicts the documented fail-closed 503 behavior.

These sections still describe pass-through/continue behavior and upstream 401/403 after expiry, but the implementation snippet and PR objectives define proxy-level block with 503 credential_renewing. Please align this text to the fail-closed handshake (block at proxy, no upstream forward).

Based on learnings: “Implement fail-closed behavior everywhere …”.

Also applies to: 799-803

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 763-763: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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 `@JANISSARY_SPEC.md` around lines 763 - 770, Update the prose describing the
lease-expiry flow to match the fail-closed behavior: when grant.lease_expires_at
<= now the proxy must block and return a 503 with error code/phrase
credential_renewing (do not forward upstream), and record an audit entry with
severity=alert; replace any wording that says “skip injection, write audit …
continue” or “upstream 401/403” with explicit fail-closed language. Apply the
same change to the other referenced passage that currently describes
pass-through behavior (the section mirroring lines 799-803) so both places
consistently state proxy-level block, 503 credential_renewing, and audit
logging.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Ottoman naming consistently (Pasha instead of agent) in runtime flows.

This SPEC still uses “agent” repeatedly in request/appeal paths. Replace with “Pasha” (or use a single first-use gloss, then Ottoman terms consistently) to match repo terminology invariants.

As per coding guidelines: “Use Ottoman naming consistently across all PRDs—use terms (Pasha, Province, Firman, Berat, Divan, Realm, Aga) rather than alternate names like 'agent', 'container', or 'template'”.

Also applies to: 857-874, 953-965

🤖 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 `@JANISSARY_SPEC.md` around lines 810 - 812, Replace all occurrences of the
generic term "agent" in the runtime/request/appeal flows with the Ottoman term
"Pasha" (or introduce a single first-use gloss like "Pasha (agent)" and
thereafter use only "Pasha")—specifically update the sentence about write
requests blocked by rule 4 and the 403 response body wording, and apply the same
replacement throughout the blocks flagged (around the current lines 810,
857–874, and 953–965) so all request paths, appeal paths, examples, and
descriptive text use "Pasha" consistently.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix grant secret-field visibility contract (Aga + Janissary, not Janissary-only).

Line 224 says secret grant values are only returned to Janissary, but Divan docs in this PR specify plaintext visibility for both aga and janissary. Please align this section to the same access-control contract.

🤖 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 `@SULTANATE_MVP.md` around lines 221 - 225, The spec currently states "Grant
secret values are only returned to the Janissary role" which contradicts the
Divan API docs that allow plaintext grant visibility to both Aga and Janissary;
update this section to reflect the correct access-control contract by changing
the visibility statement to indicate that grant secret values are returned to
both the Aga and Janissary roles (not Janissary-only), and add a cross-reference
to DIVAN_API_SPEC.md and the relevant `aga` and `janissary` role entries so
readers can verify endpoint-level read/write permissions.