Skip to content

feat: four-layer member system prompt for crew DM threads - #7235

Merged
chenmingwei23 merged 1 commit into
mainfrom
feat/member-system-prompt
Sep 5, 2026
Merged

feat: four-layer member system prompt for crew DM threads#7235
chenmingwei23 merged 1 commit into
mainfrom
feat/member-system-prompt

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

A crew member DM session previously received the same system prompt as any ordinary session — the only difference was the [CURRENT AGENT] label, so every member spoke and behaved identically regardless of its name, role, or remit. This PR gives member DM threads a four-layer member identity block with distinct ownership per layer:

Layer Content Who owns it
1. [MEMBER IDENTITY] name, role (description), remit (triggers), durable-DM framing derived from the crew record — nobody hand-writes it
2. [HOW YOU WORK] worker-not-Q&A-bot stance; front-desk-vs-workshop dispatch; 4-rung stuck ladder (different approach → alternative around the wall → escalate only at permission/reachability/one-way-door walls → park and continue); zero-context escalation format with subagent validation; quiet-run reporting; briefing self-maintenance product (module constant)
3. [PERMANENT RULES] approval boundaries, forbidden actions user — stored at trust/member-rules/<slug>.json under the keystone-gated trust/ subtree, so the member's own file tools cannot rewrite its safety boundary; written only via the new GET/PUT /api/members/{slug}/rules (human dashboard action, app tokens denied)
4. [CURRENT ASSIGNMENT] the member's own working memory member — members/<slug>/briefing.md, agent-writable by design, injection-capped with a visible truncation marker

Precedence is the injection order (rules outrank the briefing). The section builder degrades any failure to an ordinary crew session — EXCEPT an unreadable rules file: for the one safety-relevant layer that degrade would be the fail-open (a member the user bounded would run with no bounds at all), so MemberRulesUnreadable deliberately propagates and aborts the turn until the user repairs or clears the file. Ordinary (non-member) sessions are byte-for-byte unchanged.

Changes

  • members.py: member_rules_path/read_member_rules/write_member_rules (trust-gated, refuse-not-truncate cap, empty write clears), member_briefing_path/read_member_briefing (agent-writable, capped with marker)
  • context.py: _MEMBER_HOW_YOU_WORK constant, ContextBuilder._build_member_section, member= kwarg threaded through build_messagebuild_session_context, injected after the agent-identity block for mode="member" sessions only
  • chat_runner.py: passes member=slot.agent for member slots (slot.agent is the crew the human picked; agent= is the resolved template)
  • handlers/members.py + routes: GET/PUT /api/members/{slug}/rules with member_slug_mismatch / member_not_found / rules_too_long codes
  • Spec updated in the same commit (docs/system-specs/modules/learn-cron-dashboard.md)

Review follow-ups (post-rebase, addressing @bolichen97's review):

  • read_member_briefing opens through the central pinned_fs.open_in_pinned_parent helper, pinning the ancestor chain — members/<slug>/ is agent-writable too, and a parent swapped for a symlink redirected the whole traversal past the leaf-only O_NOFOLLOW (the ancestor-swap shape that closed fix: include artifact library and uploads in snapshot/restore #2446); O_NONBLOCK + fstat regularity keep the FIFO refusal
  • member_briefing_supported() names layer-4 availability: where briefing reads fail closed (Windows — no O_NOFOLLOW/pinned walk), the [CURRENT ASSIGNMENT] placeholder now says "not available on this platform" and item 6 of [HOW YOU WORK] is softened, instead of instructing upkeep of a file that is never injected
  • Rebased over main's crew-member operating-mode block (feat: crew member dispatch via per-session session_* mount on ACP backends #8153): both render — dispatch mechanics first, then the four layers

Testing

  • New test/test_member_prompt.py (51 tests): storage round-trips, trust-subtree placement, refuse-not-truncate, injection presence/absence per mode, identity floor for unregistered crews, protocol-clause pinning, route contracts incl. app-token denial and slug-mismatch write refusal; symlinked-parent refusal pinned red-before-green, a fullwidth NFKC scrub forgery per minted header, and the unavailable-platform rendering
  • Full local backend suite green on changed scope (test_member_prompt.py 51 passed; full run's only failures reproduce identically on pristine main — host-env artifacts)
  • Gates green on changed scope: black baseline gate, isort, flake8, mypy, subprocess-encoding, docs-lint

Screenshots

No UI change (backend prompt assembly + API only); the Members-page rules editor is a follow-up.

@CrysisDeu
CrysisDeu requested a review from a team as a code owner August 31, 2026 07:32
@CrysisDeu
CrysisDeu requested a review from patrigao August 31, 2026 07:32
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ human override accepted

Human judgment by @CrysisDeu overrides the GPT 5.6 finding for fc2dc2eb0f78285235a9a47b431c2988849a507b; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt fc2dc2eb0f78285235a9a47b431c2988849a507b: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of fc2dc2eb0f78285235a9a47b431c2988849a507b — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound layered-ownership design with named boundaries; the residual risks are a steering layer dressed in enforcement-grade machinery, and a repair path that predates any UI.

Watch

  • MemberRulesUnreadable aborts every member turn ("the member will not run until the file is repaired"), but the PR ships no editor ("the Members-page rules editor is a follow-up") — until it lands, the only repair path for a bricked member DM is a hand-written PUT /api/members/{slug}/rules. The fail-closed choice is right; the merge-order gap is the risk a human should sign off on.
  • [PERMANENT RULES] gets turn-aborting fail-closed semantics, owner gates, SEL audit, and fsync, yet its enforcement is the model reading prose — the spec's "prompt-level STEERING, not runtime enforcement" caveat is the only thing standing between the follow-up editor and users trusting it as policy. Hold that follow-up to the spec's framing.
  • The member-section delivery invariant spans five session-lifecycle branches across context.py and chat_runner.py (_member_session_start_pending, reinjection re-arm, slim-resume refresh); it is held together by comments plus pinned tests, so any future lifecycle branch must remember this table or a member silently runs unbounded.

Suggestions

  • Ship the rules editor — or at minimum a one-click "clear rules" repair action — in the same release as the fail-closed abort, so the unreadable-file state is user-recoverable from the surface that created it.

[DESIGN-REVIEWED] fc2dc2e

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of fc2dc2eb0f78285235a9a47b431c2988849a507b — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. I have what I need for the verdict.

Summary of what I verified: The trust/ placement for rules is derived (config.json is agent-writable per sections.py:513, trust/ is keystone-gated per security.py:7604), the briefing read reuses the central pinned_fs helper rather than re-spelling it, but the new [HOW YOU WORK] item 2 restates the [CREW MEMBER OPERATING MODE] block that already renders in the same member prompt (context.py:2662), and the new rules API has zero in-repo consumers (the editor is a declared follow-up).

First-Principles-Verdict: CONCERNS

Item 2 of [HOW YOU WORK] restates the [CREW MEMBER OPERATING MODE] block already rendering in the same prompt, and the rules API ships with zero consumers.

What this change ships

Intent: give each crew member DM thread its own identity, working protocol, user-set boundaries, and self-maintained memory — an ADDITION.

  1. Member DMs get an identity block derived from the crew record — justified
  2. Member DMs get a fixed working protocol — justified, except the dispatch clause (duplicate of context.py:2662)
  3. Users can set per-member permanent rules via new GET/PUT routes — declared; zero consumers
  4. A member with an unreadable rules file stops until repaired — justified
  5. Members keep a briefing file injected each session, capped with marker — justified
  6. Windows renders the briefing layer as unavailable, with softened prompt copy — justified
  7. Rules saves reach a warm session on its next turn — justified
  8. Rules reads/writes emit SEL audit events — undeclared, derived (audit floor)
  9. Rules routes are owner-gated, non-owners refused — undeclared, derived (multi-human boundary)
  10. A member first turn that dies re-delivers the section next turn — undeclared, rides along, justified

Watch

  • GET/PUT /api/members/{slug}/rules: 0 consumers (grepped website/src for the endpoint and any rules fetch — no hits; the docstring itself says "nothing in the frontend consumes this yet"). The error-code contract (6 codes, max_chars, empty-clears semantics) freezes one release before its first consumer can exercise it. Declared as a follow-up, so watch, not block — but the editor PR is where this contract should have been proven.
  • The two dispatch spellings already diverge: the operating-mode block teaches session_create/session_send and is backend-gated (_member_backend_can_dispatch), while [HOW YOU WORK] item 2 teaches "spawn_run and the session tools" unconditionally — on a backend that cannot mount session tools, item 2 instructs tools the gate exists to hide.

Subtractions

  • Delete item 2 ("Front desk vs workshop") from _MEMBER_HOW_YOU_WORK_COMMON (context.py:1332-1334): the [CREW MEMBER OPERATING MODE] block (context.py:2662-2676, landed via feat: crew member dispatch via per-session session_* mount on ACP backends #8153) already carries keep-this-thread-light, dispatch-to-worker-sessions, and report-back-here; fold the one novel line ("several work items can run in parallel") into that block.
  • Drop max_chars from the GET response body (handlers/members.py) until the rules editor exists — 0 consumers; the constant is importable when the editor lands.

[FIRST-PRINCIPLES-REVIEWED] fc2dc2e

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed fc2dc2eb0f78285235a9a47b431c2988849a507b — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] fc2dc2e

Verdict parsed from the review's SHA-scoped output markers for commit fc2dc2eb0f78285235a9a47b431c2988849a507b.

False positive or not applicable? A repository writer can comment:
/ai-review override fable fc2dc2eb0f78285235a9a47b431c2988849a507b: <one-sentence reason>

@CrysisDeu
CrysisDeu force-pushed the feat/member-system-prompt branch from 437f14d to a84dfb1 Compare August 31, 2026 08:04
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 31, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 1 findings addressed at head a84dfb1 (all lanes were terminal before this push):

GPT 5.6 (blocking):

  • Event-loop blocking config load in the rules PUT: now await asyncio.to_thread(KiroCrewConfig.load).
  • Slug-colliding members overwriting shared rules: PUT now refuses with 409 rules_slug_ambiguous unless exactly one registered crew derives the slug; the rules file additionally records the exact member name (JSON payload, like dm.json) and the read is name-scoped, so a colliding name reads "never set" rather than inheriting another member's boundary.
  • Post-compaction loss of member identity: the member section is now re-injected on the needs_reinjection turn beside the skills index (re-reading the current briefing), pinned by test_member_section_reinjected_after_compaction.

Design (advisory watches — all adopted):

  • Forgeable authority marker: every variable payload the section frames (description, triggers, rules, briefing) is scrubbed of member-authority markers (_MEMBER_MARKER_RES, normalized view folding Cf/dash confusables) before the genuine headers are minted; deliberately NOT added to _STRUCTURAL_MARKER_RES, whose scan covers the tail containing the genuine section.
  • Lossy-slug rules keying: name-recorded payload + name-scoped read as above.
  • Fail-open rules read: an existing-but-unreadable file now raises MemberRulesUnreadable — the whole member section degrades (and the GET answers 500 rules_unreadable), so silently-absent user rules are no longer representable.

CI (owner-gate invariant, Backend shard 1 x3): the rules PUT is now owner-gated via require_owner_dashboard_request before any input validation — writing the user's safety boundary is owner-only, matching the agent-config mutations. Not added to the ungated-debt list.

Local gates green: black baseline, isort, flake8, mypy, docs-lint, targeted shard 539 passed (including test_agent_config_owner_gate_invariant.py).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 2 findings addressed at head 16c2411:

GPT (4 blocking) + Opus (1 blocking, same root as GPT #1):

  • Post-compaction reinjection now passes the member section through _neutralize_structural_markers (the reinjection path has no session-context tail scrub; genuine member headers are not structural markers so they survive). Pinned by test_reinjected_briefing_cannot_forge_a_user_request.
  • Non-object JSON body on the rules PUT: coded 400 invalid_json instead of a 500 from .get() on a list.
  • Briefing symlink following: the read now opens with O_NOFOLLOW (fd-based, no check-then-open race; is_symlink probe where the flag is absent) so an agent-planted symlink cannot pull gateway-readable files — including trust/ payloads — into the prompt. Pinned by test_symlink_briefing_is_refused.
  • Unbounded briefing read: at most (cap+2)*4 bytes are read before decode, so a huge agent-written file costs a bounded allocation. Pinned by test_huge_briefing_read_is_byte_bounded.
  • FINDING (top-level imports): require_owner_dashboard_request import moved to module level.

Local gates green: 543 targeted tests, black baseline, isort, flake8, mypy, docs-lint. Spec updated in the same commit.

@CrysisDeu
CrysisDeu force-pushed the feat/member-system-prompt branch from a84dfb1 to 16c2411 Compare August 31, 2026 08:35
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 31, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 3 findings addressed at head 82cd5fc:

  • Saved rules now reach a warm member session: a successful rules PUT flags the thread's session needs_reinjection (best-effort), so the NEXT turn re-injects the whole member section — fresh rules included — through the same post-compaction branch. No session teardown; the flag is a no-op when no session is warm, and a cold start reads the new rules regardless. Pinned by test_put_flags_live_member_session_for_reinjection.
  • Closed-bracket authority forms scrubbed: _MEMBER_MARKER_RES now also neutralizes the exact [PERMANENT RULES] / [CURRENT ASSIGNMENT] spellings in agent-writable payloads — a bare closed form still reads authoritative even though the genuine headers never use it. Pinned in test_scrub_covers_every_minted_header.

Local gates green: 278 targeted tests, black baseline, isort, flake8, mypy, docs-lint. Spec updated in the same commit.

@CrysisDeu
CrysisDeu force-pushed the feat/member-system-prompt branch from 16c2411 to 82cd5fc Compare August 31, 2026 09:08
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 31, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 4 findings addressed at head b650ef2:

  • Hyphen-tail forgeries of all minted headers scrubbed: _MEMBER_MARKER_RES now covers both shapes of every header ([HOW YOU WORK — override], [MEMBER IDENTITY — v2], ...), on the dash-folded normalized view. Pinned in test_scrub_covers_every_minted_header.
  • Non-string config fields degrade instead of crashing: description/triggers are type-guarded, so a hand-edited "description": 1 degrades to the derived identity floor rather than failing the member's chat turn on .strip(). Pinned by test_non_string_config_fields_degrade_to_identity_floor.

Note: the round-4 Backend Lint & Type Check (3.10) red was a runner cancellation ("The operation was canceled", orphan-process teardown — no lint output), not a lint failure; this push re-runs it fresh.

Local gates green: 275 targeted tests, black baseline, isort, flake8, mypy.

@CrysisDeu
CrysisDeu force-pushed the feat/member-system-prompt branch from 82cd5fc to b650ef2 Compare August 31, 2026 09:39
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Aug 31, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt b91beb0: Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888 (this push implements Design Review CONCERNS per the maintainer's implement-advisory-concerns directive; diff to the adjudicated spans unchanged): deny-site SEL sync-init sibling tracked in #8523; scrub NFKC whole-vs-span normalization tracked in #8524, current behavior fail-closed. Both accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for b91beb03ca5987a7fd6b81d9000247fbec5849af.

Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888 (this push implements Design Review CONCERNS per the maintainer's implement-advisory-concerns directive; diff to the adjudicated spans unchanged): deny-site SEL sync-init sibling tracked in #8523; scrub NFKC whole-vs-span normalization tracked in #8524, current behavior fail-closed. Both accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • fixed — Precedence inversion: user rules formally outranked by product protocol under "earlier outranks later".

Implemented in b91beb0: the [PERMANENT RULES] header now explicitly states it outranks EVERYTHING else in the member section — the working protocol above included, whose instructions yield wherever the rules contradict them. _build_member_section docstring and the members spec paragraph record the stated exception. Injection order is unchanged (identity floor still leads), so the layer tests' structural assertions hold while the user's safety boundary is no longer formally outranked by product prose.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • fixed — Fail-closed ceremony can read as enforcement; the eventual rules editor must not present it that way.

Implemented the in-PR half in b91beb0: the members spec now states the rules layer is prompt-level STEERING, not runtime enforcement (nothing at the PreToolUse gate reads these rules), and that any UI over them — the eventual rules editor included — must present them as instructions, never as enforced policy; governance profiles are named as the enforced path. The editor itself remains a follow-up (Members page rules editor work item), which will inherit this spec sentence as its contract.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • fixed — Unreadable rules file bricks the member thread with an undiagnosable error.

Implemented in b91beb0: all three MemberRulesUnreadable raise sites now name the exact file path and the remedy (rewrite or clear via PUT /api/members/{slug}/rules), so the chat-side abort and the logs identify the file and the repair path; the GET 500 body carries the same remedy alongside its stable rules_unreadable code.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • accepted-and-deferred — Rules-currency invariant stitched across five hand-coordinated branches; extract a single member_turn_context() chokepoint.

Deferred to #8609, which names the concrete task (single decision point all delivery branches call, pinned by one test instead of a comment table). Deferred rather than done here because the refactor rewrites the exact span hardened across GPT rounds 1-2 — behavior-preserving but review-surface-wide — and folding it into a merge-ready PR re-arms every lane on the whole diff. The five branches are each individually pinned by the existing ~50-test member-prompt suite today, so the invariant is test-covered while the chokepoint consolidation lands as its own reviewable unit.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • fixed — Enforcement-grade machinery around steering-grade text; the follow-up editor must not present rules as enforced policy.

The load-bearing mitigation this finding names is exactly what b91beb0 added: the members spec now states the rules layer is prompt-level STEERING, not runtime enforcement (nothing at the PreToolUse gate reads it), and mandates that any UI over it — the rules editor included — present rules as instructions, never enforced policy, with governance profiles named as the enforced path. The editor follow-up inherits that sentence as its contract; the enforcement-grade storage (trust subtree, fail-closed read, SEL audit) protects the INTEGRITY of the user's text, which is orthogonal to whether the text is enforced.

chenmingwei23
chenmingwei23 previously approved these changes Sep 5, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 2c6f9ae: Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888/b91beb03c (conflict-resolution-only rebase, adjudicated spans unchanged): deny-site SEL sync-init sibling is a pre-existing repo-wide pattern tracked in #8523; scrub NFKC whole-vs-span normalization is a design question tracked in #8524 with current behavior fail-closed. Both accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 2c6f9aeae183fb2c2d7ad3b5c89a623c4e0794dc.

Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888/b91beb03c (conflict-resolution-only rebase, adjudicated spans unchanged): deny-site SEL sync-init sibling is a pre-existing repo-wide pattern tracked in #8523; scrub NFKC whole-vs-span normalization is a design question tracked in #8524 with current behavior fail-closed. Both accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt f741de7: Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888/b91beb03c/2c6f9aeae (conflict-resolution-only rebase, adjudicated spans unchanged): deny-site SEL sync-init sibling is a pre-existing repo-wide pattern tracked in #8523; scrub NFKC whole-vs-span normalization is a design question tracked in #8524 with current behavior fail-closed. Both accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for f741de70487d3e4a60f8e4ea975cb574ed0008d2.

Same two findings already adjudicated on f3d8f5e/b5856b441/8d4c0c888/b91beb03c/2c6f9aeae (conflict-resolution-only rebase, adjudicated spans unchanged): deny-site SEL sync-init sibling is a pre-existing repo-wide pattern tracked in #8523; scrub NFKC whole-vs-span normalization is a design question tracked in #8524 with current behavior fail-closed. Both accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 4b5bd2d: Same two findings already adjudicated on five prior heads (deny-site SEL sync-init sibling tracked in #8523 — since fixed upstream by #8604; scrub NFKC whole-vs-span normalization tracked in #8524, fail-closed today). This head additionally FIXES the Opus await finding from f741de7 (rebase-over-#8604 semantic break, await added at both rules-handler sites). Both original findings accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 4b5bd2d8b0d1e68ece8610189e95346b2e814c66.

Same two findings already adjudicated on five prior heads (deny-site SEL sync-init sibling tracked in #8523 — since fixed upstream by #8604; scrub NFKC whole-vs-span normalization tracked in #8524, fail-closed today). This head additionally FIXES the Opus await finding from f741de7 (rebase-over-#8604 semantic break, await added at both rules-handler sites). Both original findings accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author
  • fixed — Two new rules handlers called async _deny_app_caller without await, returning a coroutine to aiohttp (HTTP 500 on every GET/PUT to /rules).

Fixed in 4b5bd2d: await added at both call sites (members.py rules GET + PUT), matching the awaited form at the existing api_members site. Root cause was a semantically-unclean rebase: main's #8604 (which fixed our own tracking issue #8523) changed _deny_app_caller to async def after this PR's handlers were written; the rebase had no textual conflict so the break surfaced only in CI (mypy Incompatible return at 546/621 + rules-route 500s on shards 1/3, both platforms). Verified: mypy clean on the fixed file, all 5 call sites now awaited.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt cd3a384: Same two findings adjudicated on six prior heads (deny-site SEL sync-init sibling tracked in #8523, since fixed upstream by #8604; scrub NFKC whole-vs-span tracked in #8524, fail-closed today). This head also carries the Opus await fix and rewrites the two rules-audit sites to the lambda-in-to_thread shape #8604's AST guard requires. Both original findings accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for cd3a38475703b715ce7a93e752170755f6694d0b.

Same two findings adjudicated on six prior heads (deny-site SEL sync-init sibling tracked in #8523, since fixed upstream by #8604; scrub NFKC whole-vs-span tracked in #8524, fail-closed today). This head also carries the Opus await fix and rewrites the two rules-audit sites to the lambda-in-to_thread shape #8604's AST guard requires. Both original findings accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Status note: the remaining shard-3 failures are inherited from main, not introduced by this PR.

  • Backend Tests (3.12, 3) and Backend Tests (Windows) (3) fail exclusively in test/test_push_branch_gate.py (TestUnrecognisedOptionsReadProtectively) and test/test_security.py (TestGitPublishSubshellGluing) — 9 failures, zero outside those files. Neither file is touched by this PR's diff.
  • main's own ci.yml run 33953620339 fails the identical tests on the same shard (suspect cross-merge around fix(security): model publish option arity so the floor tag holds #7808).
  • Coverage Gate / PR Readiness reds are cascades of those shards.

Everything this PR owns is green on head cd3a384: GPT 5.6 + Opus 4.8 + Design + UX all pass, 52 checks passing, MERGEABLE. The earlier Opus blocking finding (un-awaited _deny_app_caller after main's #8604 made it async) and the #8604 AST-guard shape requirement are both fixed in this head.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 95e7f9c: Same two findings adjudicated on seven prior heads — deny-site SEL sync-init sibling tracked in #8523 (fixed upstream by #8604) and scrub NFKC whole-vs-span normalization tracked in #8524 (fail-closed today). This head is a rebase onto fixed main (#8712) plus a one-line black reformat in chat_runner.py, then an empty-amend re-trigger; the adjudicated spans are unchanged. Both accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 95e7f9c0d41171a23bc3047f04eb993529aa4a21.

Same two findings adjudicated on seven prior heads — deny-site SEL sync-init sibling tracked in #8523 (fixed upstream by #8604) and scrub NFKC whole-vs-span normalization tracked in #8524 (fail-closed today). This head is a rebase onto fixed main (#8712) plus a one-line black reformat in chat_runner.py, then an empty-amend re-trigger; the adjudicated spans are unchanged. Both accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

A member DM session previously differed from an ordinary session only by
the [CURRENT AGENT] label. Member turns now inject a [MEMBER IDENTITY]
block with four layers of distinct ownership:

1. identity — derived from the crew record (name, description,
   triggers); auto-generated so an empty description still has a floor
2. behavior — product-owned working protocol: worker-not-Q&A-bot,
   front-desk-vs-workshop dispatch, a four-rung stuck ladder that
   escalates only at permission / reachability / one-way-door walls,
   zero-context escalation drafts validated by a context-free subagent,
   and quiet-run reporting
3. permanent rules — user-owned, stored at trust/member-rules/<slug>.json
   under the keystone-gated trust/ subtree so the member cannot rewrite
   its own safety boundary; GET/PUT /api/members/{slug}/rules is the
   only write path (human dashboard action, app tokens denied)
4. briefing — member-owned working memory at members/<slug>/briefing.md,
   agent-writable by design, injection-capped with a visible marker

Precedence is the injection order. The section builder degrades to an
ordinary crew session on any failure EXCEPT an unreadable rules file:
for the one safety-relevant layer that degrade is the fail-open, so
MemberRulesUnreadable propagates and aborts the turn until the user
repairs or clears the file.

Review follow-ups (post-rebase, addressing bolichen97's review; feature
authored by CrysisDeu):

- read_member_briefing now opens through the central
  pinned_fs.open_in_pinned_parent helper, pinning the ancestor chain
  (members/<slug>/ is agent-writable too — a swapped parent redirected
  the whole traversal past the leaf-only O_NOFOLLOW; same shape as
  #2446). Fails closed where the pinned walk is unavailable.
- member_briefing_supported() names the layer-4 availability; on
  platforms where briefing reads fail closed (Windows) the section now
  says "not available on this platform" and item 6 of [HOW YOU WORK] is
  softened instead of instructing upkeep of a never-injected file.
- Rebased onto main over the crew-member operating-mode block (#8153):
  both blocks render — dispatch mechanics first, then the four layers.
- Scrub coverage test now carries a fullwidth NFKC forgery per minted
  header, and a regression test pins the symlinked-parent refusal
  red-before-green.
- The layer-3 fail-closed contract now holds PER-TURN across the whole
  session lifecycle (branch table in build_message): a first-turn
  MemberRulesUnreadable abort leaves a warm session whose turns skipped
  both member-section paths, and an idle-expired session restored via
  session/load carried a STALE [PERMANENT RULES] snapshot. build_message
  now validates rules on every warm member turn, re-injects the CURRENT
  member section on slim resume (rules gate included), and the dashboard
  runner re-arms the reinjection flag when a member DM's first turn dies
  before landing, so the section (fresh rules included) is re-delivered
  on the first turn after the user repairs the file.
- Feature-map row for Crew Members now names the GET/PUT
  /api/members/{slug}/rules endpoints.
- The PUT collision scan uses the central _member_names_for_slug helper
  (grammar filter + MemberSlugError tolerance) instead of an ad-hoc
  comprehension a hand-edited config key could crash.
- Rules saves and clears are crash-durable: atomic_write's file fsync
  covers the data, but the publishing rename, the first-save directory
  creation, and the clearing unlink are directory entries — fsync_dir
  (raising on save, best_effort on the already-committed clear) makes
  the PUT's 200 survive a power-off instead of resurrecting cleared
  rules or losing saved ones.
- The rules GET is owner-gated like the PUT: rules are the owner's
  private safety instructions, and any allowed Slack user can mint a
  dashboard session, so the read would otherwise disclose them.
- Escaped lone surrogates in the rules payload are refused with a coded
  400 (rules_not_encodable) at the PUT boundary, and write_member_rules
  rejects UTF-8-unencodable text before any state changes — previously
  the UnicodeEncodeError from the write escaped as a mislabeled
  "too long" error, and escaping surrogates into the file would only
  defer the crash to prompt-encode time inside the member's turn.

- The feature-map Crew Members row lists the rules API as a combined
  `GET,PUT /api/members/{slug}/rules` entry, keeping the row within the
  feature map's four-route limit.

- The member first-turn re-arm lives in the runner's finally (the one
  block on every exit path), not the except arm alone: a user Stop's
  cancelled completion never raises, so the except-arm-only re-arm left
  the warm session running without [PERMANENT RULES] after a stopped
  first turn.

Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt fc2dc2e: Same two findings adjudicated on eight prior heads — deny-site SEL sync-init sibling tracked in #8523 (fixed upstream by #8604) and scrub NFKC whole-vs-span normalization tracked in #8524 (fail-closed today). This head is a rebase onto fixed main (#8712); the only chat_runner.py change is splitting one 101-char append_and_surface call across three lines to satisfy the pinned black (line-length 100), matching main's own form. The adjudicated spans are unchanged. Both accepted-and-deferred.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for fc2dc2eb0f78285235a9a47b431c2988849a507b.

Same two findings adjudicated on eight prior heads — deny-site SEL sync-init sibling tracked in #8523 (fixed upstream by #8604) and scrub NFKC whole-vs-span normalization tracked in #8524 (fail-closed today). This head is a rebase onto fixed main (#8712); the only chat_runner.py change is splitting one 101-char append_and_surface call across three lines to satisfy the pinned black (line-length 100), matching main's own form. The adjudicated spans are unchanged. Both accepted-and-deferred.

This decision applies only to this commit. A new push requires a new judgment.

@chenmingwei23 chenmingwei23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved: checks green at head, readiness passed, no blocking reviewer verdict outstanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants