Keep scoped approvals out of inferred engagement signers - #164
Conversation
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| 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 '' |
There was a problem hiding this comment.
🔴 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
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 checkpassed 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.