Skip to content

Keep scoped approvals out of inferred engagement signers - #164

Merged
suboss87 merged 1 commit into
Mainfrom
fix/scoped-signoff-local
Oct 1, 2026
Merged

suboss87 merged 1 commit into
Mainfrom
fix/scoped-signoff-local

Conversation

@suboss87

@suboss87 suboss87 commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Smart debrief could turn “Jo approves API compatibility only” into a generic engagement signer proposal. Restrict automatic extraction to bare authority statements and explicit acceptance-test/outcome forms, preserving the original notes and their sources for review. Singular and plural “acceptance test(s)” remain supported; additional scope qualifiers do not become global signer authority. Explicit signer: input and the review/apply boundary are unchanged.

Adds ten synthetic regressions for scoped approvals, scope before the name, and plural acceptance tests. Less explicit wording such as “signs off today” stays context for agent review instead of automatically setting a signer.

Validation: npm run check passed with 490 passing tests, no failures, and one macOS-specific skip on Linux. Routing smoke and context-budget checks passed. Independent CLI review verified scoped approvals, preserved signer forms, source retention and unchanged saved records during proposal/review. These checks validate deterministic CLI behavior, not agent judgment.


Devin Review

@suboss87
suboss87 marked this pull request as ready for review October 1, 2026 12:46
@suboss87
suboss87 merged commit 351564a into Main Oct 1, 2026
1 of 2 checks passed

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 1 potential issue.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment thread bin/fde.js
const before = t.slice(0, match.index)
const after = t.slice(match.index + match[0].length).replace(/\[source:[^\]]*\]/gi, '').trim()
if (before.trim() && (!rolePrefix || !roleWords(before))) return ''
if (!/^(?:(?:on\s+)?(?:the\s+)?(?:acceptance tests?|(?:customer |delivered )?outcome))?[.!]?$/i.test(after)) return ''

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.

🔴 Adjacent scope qualifier creates global signer

For 'Jo signs off on the acceptance tests. Only for API compatibility.', candidate accepts the first sentence as unqualified authority. smartProposeText checks sentences independently, so applying the proposal records Jo as the engagement signer.

Learn more

Smart debrief examines each sentence for signers but preserves the full input line as context. The suffix check sees only the first sentence, so a restriction in the next sentence cannot stop a generic signer proposal. Applying the proposal writes that name through routeDebriefInput.

Example: Jo signs off on the acceptance tests. Only for API compatibility. [source: meeting:42] yields signer: Jo [source: meeting:42]; applying it records Jo as the engagement signer despite the qualification.

Recommended fix: Check adjacent sentences for scope restrictions in smartProposeText before proposing a signer, while retaining the original sourced note.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@suboss87
suboss87 deleted the fix/scoped-signoff-local branch October 2, 2026 13:54
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