Skip to content

Remove model-specific Fable write restriction#646

Merged
Th0rgal merged 6 commits into
masterfrom
fix/fable-readonly-orchestration-enforcement
Jul 21, 2026
Merged

Remove model-specific Fable write restriction#646
Th0rgal merged 6 commits into
masterfrom
fix/fable-readonly-orchestration-enforcement

Conversation

@Th0rgal

@Th0rgal Th0rgal commented Jul 21, 2026

Copy link
Copy Markdown
Owner

What changed

  • remove the Fable model-name detector
  • remove the prompt-keyword integration guard and its special-case tests
  • allow Fable missions to request writer=true under the same generic writer lease and merge gates as every other model

Why

The Fable restriction was a local response to one historical routing incident, not a provider or model limitation. The owner explicitly authorizes Fable as a writer. Model identity should not override the structured mission capability system.

This also supersedes #645: writer=false alone did not make Claude Code's shell read-only, so a Fable-specific read-only carve-out was neither sufficient nor the desired policy.

Validation

  • cargo fmt --all --check
  • cargo clippy --all-targets -- -D warnings
  • cargo test --all-targets (all passing: 1395 library tests plus all binary targets)

Note

High Risk
Expands which models can obtain repository write authority at mission creation; mis-inference in inferred_pr_writer could grant or deny writer capability incorrectly.

Overview
Fable missions are no longer rejected at admission based on model name or prompt keywords. The dedicated fable_mandate_* helpers and the 400 on Fable + write/orchestration wording are gone, so Fable uses the same writer field, PR writer lease, and merge gates as other models.

inferred_pr_writer is reworked for all missions: text is split into clauses; integration/implementation nouns imply writer; writer verbs include -ing forms; negation handles immediate, coordinated (“do not commit or push”), stop before, and forbidden/prohibited phrasing—so prompts can forbid git actions without falsely clearing writer intent elsewhere in the text.

Tests drop the Fable guard suite and add coverage for the new inference behavior.

Reviewed by Cursor Bugbot for commit b592d19. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sandboxed-dashboard Ready Ready Preview, Comment Jul 21, 2026 9:25am
sandboxed-sh Ready Ready Preview, Comment Jul 21, 2026 9:25am

Request Review

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_190a5c27-732f-41f3-830d-f16a00681a39)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

let request_is_writer = requested_pr_writer(
req.writer,
normalized_request_tags.as_deref(),
req.intent.as_deref(),
req.prompt.as_deref(),
);

P2 Badge Cover inflected mutation verbs before dropping the guard

With the Fable admission check removed, legacy Fable create requests that include a github_pr but omit writer now rely entirely on requested_pr_writer; however the remaining generic inference only recognizes base tokens like commit, push, merge, and rebase, not the gerunds that the deleted guard handled. For prompts such as “committing and pushing the branch” or “rebasing onto main”, request_is_writer stays false, so the mission is admitted without a durable PR writer lease/tag even though the mandate can mutate the PR branch; please extend the generic inference or otherwise preserve parity before removing the Fable guard.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_db96a0c8-3979-42db-801e-b4a01ef37a4b)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a53ba3ca3f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/api/control/mod.rs
"integrate",
"integration",
"rebase",
"rebasing",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep contextual gerunds from taking writer leases

With writer omitted, requested_pr_writer treats any matching token here as permission to reserve the exclusive PR writer lease. A read-only PR mission whose prompt only discusses policy/status with a gerund, e.g. Audit only; rebasing is forbidden, now becomes pr-writer and can either conflict with the actual writer or block one later, even though before this addition that prompt did not match the generic inference. Please either require explicit writer=true for these inflections or carry over negation/context handling before using them for lease acquisition.

Useful? React with 👍 / 👎.

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_ce52a5ad-75f4-43d2-bd56-6a8ffa07b785)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a2cdcc6ba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/api/control/mod.rs Outdated
"updating",
];
const NEGATORS_BEFORE: [&str; 9] = [
"not", "never", "no", "without", "dont", "cannot", "cant", "forbid", "before",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Don't treat sequencing 'before' as negation

When writer is omitted, prompts such as Run tests before committing to the branch now hit this before entry and cause inferred_pr_writer to treat committing as negated. The create and follow-up paths rely on this inference to add pr-writer and acquire the exclusive PR lease, so a real commit request can be admitted as read-only and run alongside another writer for the same PR. Fresh evidence after the earlier gerund comment is that the final code globally lists before as a negator rather than recognizing only prohibition phrases like stop before ....

Useful? React with 👍 / 👎.

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_9f579db9-1041-4fb2-afd4-4b9d2f6f5060)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d745a6f6c2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/api/control/mod.rs Outdated
Comment on lines +5429 to +5431
let negated_before = words[index.saturating_sub(3)..index]
.iter()
.any(|prior| NEGATORS_BEFORE.contains(prior));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve writer leases after unrelated negators

When writer is omitted, this three-token lookback treats any nearby no/cannot as negating the writer verb, even when it belongs to a separate clause. For example, a PR mission prompt like No tests required; fix the failing branch tokenizes with no in the window before fix, so inferred_pr_writer returns false; the create/follow-up paths then skip adding pr-writer and skip the exclusive PR lease even though the mission is explicitly asked to mutate the PR. Please scope negation to the verb phrase rather than any prior token in the window.

Useful? React with 👍 / 👎.

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3f1ff35c-57a8-4259-b40e-716fafb3ad20)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0066aea556

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/api/control/mod.rs Outdated
Comment on lines +5430 to +5434
let coordinated_negation = (index.saturating_sub(4)..index).any(|negator_index| {
NEGATORS_BEFORE.contains(&words[negator_index])
&& words[(negator_index + 1)..index].iter().all(|between| {
WRITER_VERBS.contains(between) || matches!(*between, "and" | "or")
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound coordinated negation to one clause

When a PR mission forbids Git steps but still asks for a code change, e.g. Do not commit or push; fix the failing branch, tokenization has already removed the semicolon and this lookback treats the prior not as negating fix because only writer verbs/or appear between them. inferred_pr_writer then returns false, so create/follow-up paths skip pr-writer tagging and lease acquisition for an actual branch mutation, allowing it to run alongside another writer. Fresh evidence in the final code is the coordinated_negation window crossing punctuation-free clause boundaries.

Useful? React with 👍 / 👎.

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_161c09b6-f939-453f-8e4f-af6286a9690e)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b592d194f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/api/control/mod.rs
Comment on lines +5446 to +5448
let negated_after = words[(index + 1).min(words.len())..after_end]
.iter()
.any(|next| NEGATORS_AFTER.contains(next));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Constrain after-verb negators to the verb phrase

When writer is omitted for a github_pr mission, this window treats any prohibited/forbidden/banned/disallowed token within the next three words as negating the writer verb. A real mutation request like Fix the prohibited dependency usage or Implement the banned-import repair is therefore classified as read-only because the adjective describes the object, so the create/follow-up paths skip the pr-writer tag and exclusive lease even though the branch will be edited. Please only treat these after-negators as negating the verb in predicate phrasing such as merging is prohibited.

Useful? React with 👍 / 👎.

@Th0rgal
Th0rgal merged commit 701a573 into master Jul 21, 2026
10 checks passed
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.

1 participant