Repository navigation
connect: admit boosts on the agent's own posts as a boosted trigger - #877
Conversation
|
🤖 @codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
All reported issues were addressed across 14 files
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
🤖 Review summary, head 699cd5d
|
|
🤖 @cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 14 files
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
A boost on the agent's own post arrives as a boosted reply (#877), and the skill treated it as approving nothing. An agent asking the operator for a go had to wait for a mention. The agent now declares an ask when it posts one, with `basecamp connect ask <url|id>`, recorded in the ledger's new asks table. Nothing reads a post's text to decide it is an ask. The operator's boost on a declared ask, and only the operator's in every trust mode, is the go: admission names the boosted post on the operator's verdict, and the ledger takes the ask's go in the verdict's own transaction, so one ask gets one go. A second boost, or one removed and given again, is an ordinary boosted reply; the event that took the go keeps it when decided again; a go closed unwritten (before this run, unreadable) is given back. Asks do not expire. The request line carries "go": true on that request. The skill says the go approves what the ask names and nothing else: not a merge or deploy the ask did not name, and never a connector change.
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
🤖 @cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 14 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
|
🤖 Head dd1b6b7. Codex and cubic reported no issues on this head. CI: everything passes except
|
dd1b6b7 to
f9ae377
Compare
A boost on the agent's own post arrives as a boosted reply (#877), and the skill treated it as approving nothing. An agent asking the operator for a go had to wait for a mention. The agent now declares an ask when it posts one, with `basecamp connect ask <url|id>`, recorded in the ledger's new asks table. Nothing reads a post's text to decide it is an ask. The operator's boost on a declared ask, and only the operator's in every trust mode, is the go: admission names the boosted post on the operator's verdict, and the ledger takes the ask's go in the verdict's own transaction, so one ask gets one go. A second boost, or one removed and given again, is an ordinary boosted reply; the event that took the go keeps it when decided again; a go closed unwritten (before this run, unreadable) is given back. Asks do not expire. The request line carries "go": true on that request. The skill says the go approves what the ask names and nothing else: not a merge or deploy the ask did not name, and never a connector change.
|
🤖 @codex review |
|
🤖 @cubic-dev-ai review |
@jeremy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 14 files
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
f9ae377 to
f1bbe9a
Compare
A boost on the agent's own post arrives as a boosted reply (#877), and the skill treated it as approving nothing. An agent asking the operator for a go had to wait for a mention. The agent now declares an ask when it posts one, with `basecamp connect ask <url|id>`, recorded in the ledger's new asks table. Nothing reads a post's text to decide it is an ask. The operator's boost on a declared ask, and only the operator's in every trust mode, is the go: admission names the boosted post on the operator's verdict, and the ledger takes the ask's go in the verdict's own transaction, so one ask gets one go. A second boost, or one removed and given again, is an ordinary boosted reply; the event that took the go keeps it when decided again; a go closed unwritten (before this run, unreadable) is given back. Asks do not expire. The request line carries "go": true on that request. The skill says the go approves what the ask names and nothing else: not a merge or deploy the ask did not name, and never a connector change.
|
🤖 Rebased onto What conflicted: only How I resolved it: I took
No code conflicted. |
|
🤖 @codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
🤖 Head f1bbe9a, rebased onto
|
Basecamp addresses a boost to the person whose recording was boosted, but the admission matrix knew only mentioned, subscribed, assigned and completed, so boost.created was discarded as not_in_matrix and the agent never heard a boost on its own comment, line or post. boost.created now admits under boosted when the boost is on the recording itself and the recording's creator is the agent. The booster is trusted like a mention's performer in every trust mode (not operator-only); agent, delegated and allowed-agent boosts stay discarded. The boost is read first, for the boosted recording's type (the summary read routes by type, and boost.created names none) and for its content, which is what is handed off. The request is answered where the boosted recording is: a comment's thread, a line's Campfire, otherwise the recording. Acknowledge is false, owner is false, and the rule requires a served project. A boost on one of a recording's events is discarded as not_addressed before any read: Basecamp addresses that to the event's performer, which this rule does not read. queued (the rest of step 25) and edited (step 28) are not built.
…he recording A missing key said nothing about where the boost landed, but decoded as null and passed as a boost on the recording itself. Require the key, and read only an explicit null as the recording. Also qualify the skill's summary: only requests that ask for something are acknowledged.
The poll lane carries no actor type, so an Agent person the operator wrongly put in the allowlist passes the gate. The boost read names the booster's personable type; refuse an Agent there as agent_authored, as authorTrusted refuses an Agent author.
|
@codex review |
f1bbe9a to
62118b1
Compare
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Head 62118b1, rebased onto |
A boost on the agent's own post arrives as a boosted reply (#877), and the skill treated it as approving nothing. An agent asking the operator for a go had to wait for a mention. The agent now declares an ask when it posts one, with `basecamp connect ask <url|id>`, recorded in the ledger's new asks table. Nothing reads a post's text to decide it is an ask. The operator's boost on a declared ask, and only the operator's in every trust mode, is the go: admission names the boosted post on the operator's verdict, and the ledger takes the ask's go in the verdict's own transaction, so one ask gets one go. A second boost, or one removed and given again, is an ordinary boosted reply; the event that took the go keeps it when decided again; a go closed unwritten (before this run, unreadable) is given back. Asks do not expire. The request line carries "go": true on that request. The skill says the go approves what the ask names and nothing else: not a merge or deploy the ask did not name, and never a connector change.
* connect: the operator's boost on a declared ask is the go A boost on the agent's own post arrives as a boosted reply (#877), and the skill treated it as approving nothing. An agent asking the operator for a go had to wait for a mention. The agent now declares an ask when it posts one, with `basecamp connect ask <url|id>`, recorded in the ledger's new asks table. Nothing reads a post's text to decide it is an ask. The operator's boost on a declared ask, and only the operator's in every trust mode, is the go: admission names the boosted post on the operator's verdict, and the ledger takes the ask's go in the verdict's own transaction, so one ask gets one go. A second boost, or one removed and given again, is an ordinary boosted reply; the event that took the go keeps it when decided again; a go closed unwritten (before this run, unreadable) is given back. Asks do not expire. The request line carries "go": true on that request. The skill says the go approves what the ask names and nothing else: not a merge or deploy the ask did not name, and never a connector change. * connect: a go taken stays taken, even when its line is never handed off Giving the go back when its record was closed before_this_run could approve twice: a connector that printed the line and stopped before closing the record looks the same on restart, and the next boost would become a second go after the session already acted on the first. Losing a go costs the operator a mention; a second go costs a second action. The release is removed, and the skill says a boost given while the connector isn't running uses the ask up. Also: declare the ask before anything else after posting it, and list connect ask as taking a url or an id. * connect ask: refuse a URL that does not plainly name one post in the agent's account A declaration that names the wrong recording fails silently later: the operator's boost on the real post arrives as a reply, not a go. A listing URL such as /chats/{id}/lines parsed to its parent's id and was declared with a success message. Follow the mutating-URL pattern files.go uses: a trusted host, the profile's account, and a single recording (the fragment's comment when there is one), else a usage error and nothing declared. * connect: Owner and the go are separate approvals; say so where Owner is set The Owner rationale still said a boost names nothing it approves, which the declared-ask go made untrue. Owner stays false for every boost: it is the owner's word on the connector, and a go approves only what its ask names. * connect: an ask edited after it was declared gives no go The go was keyed by recording id alone, and the worker reads the live post, so an edit after the operator's boost would change what the go approved without the operator seeing it. connect ask now reads the post once, as the agent, through the same summary read admission makes of the boosted recording, refuses one the agent did not write or that is not published, and stores the SHA-256 of its content exactly as the read returns it, with nothing normalized. At admission the operator's boost carries the hash of the content that decision read; the ledger gives the go only when it equals the declared one. An edited ask's boost is an ordinary boosted reply with go false, and the ask is not used up. Re-declaring keeps the first hash, so an edited ask stays void: the agent posts a new one. connect ask takes the post's URL, not a bare id: the read routes by the type the URL names. Migration 15 gains content_hash; it has not shipped. * connect ask: test URL refusals against the agent's own post; say when later boosts lose the go The URL cases ran against a post the operator wrote, so the creator check refused them all and a regressed URL guard would have passed. They now run against the agent's own post; someone else's post is its own case. The skill now says later boosts arrive with go false once the operator's boost has taken the go: another person's boost doesn't use the ask up. * connect: a go carries the ask's words as read, and they include its title The fingerprint covered content alone, and the worker read the live post, so a title edit or an edit after admission could change what a go approved without the operator seeing it. AskFingerprint now hashes a titled post's title and content, each length-prefixed ("<len>:<title><len>:<content>") so the two can't run together; a comment or chat line, whose title is not its own, hashes its content alone. Both ends compute it from the same summary read. Admission keeps the ask's title and content from that same read in the snapshot, and a go is handed off with them under "ask": the exact words the ledger matched against the declaration. The skill acts on those words only, never on a fresh read of the post; "recording" stays for replying. A go whose record carries no such words is not handed off as a go. * connect: a check-in answer's ask is its content, not its question's title An answer's title is its parent question's: hashing it let a rename of the question void an unchanged ask, and handing it off put the question's words in ask.title as if the operator had approved them. A check-in answer now fingerprints and hands off its content alone, like a comment or a chat line. * connect skill: the declared-ask go, on the split skill and inside the cut main moved the subagent's instructions into subagent.md and capped the per-request part of SKILL.md at 14,000 characters (the compaction cut), with main itself at 13,995. This branch's skill changes were written against the single file, so they are re-applied here in one place. SKILL.md carries only what the request line needs: the example's "go" and "ask" keys and a one-line pointer, paid for by tightening the reply_to, recording and log-line wording and a shorter example. The declaration, the go, the ask's words and the boosted trigger live in subagent.md, where the subagent handling the request reads them. * connect: a boost made before its ask was declared is no go, however late it commits takeGo checked only that a declaration existed when the boost's verdict committed, so a boost the connector held (blocked for a retry, or seen late) from before the declaration could take a go nobody gave. The go now also requires the event's Basecamp time to be after the declaration's. Skew between the two clocks can only turn a boost given in the seconds after a declaration into a reply. Also: connect ask compares the URL's account the way the profile's is compared (accountIDsEqual), so "0999" is account 999; and the request line's acknowledge says a boost is context unless it is a go. * connect: order a go's boost against its declaration by boost id, not clock 254f27c compared the declaration's time, from this machine's clock, with the boost event's, from Basecamp's. A local clock behind Basecamp's let a boost given before the declaration pass as one given after. connect ask now reads the boosts already on the post and records the highest id as the ask's boost_floor (migration 15, unshipped). Admission carries the boost's own id, and the go requires it to be above the floor. Basecamp's boost ids only grow, so both sides are ordered on Basecamp's clock alone, and no skew can reorder them.
Problem
Someone boosts a comment, Campfire line or post the agent wrote, and the agent never hears about it. Basecamp does deliver the boost:
Boost#track_boost_eventtracks aboostedevent on the boosted recording, andEvent::Addresseesaddresses it (reasonboosted) to the boostee. Butbasecamp connect's admission matrix only knewmentioned,subscribed,assignedandcompleted, soboost.createdwas discarded asnot_in_matrix. People boost an agent's question and wait, and the agent waits too.Card: https://app.basecamp.com/2914079/buckets/46292715/card_tables/cards/10396224165
Change
boost.creatednow admits under a newboostedtrigger (internal/connector/admission/matrix.go,boost.go,verdict.go):not_addressed.OperatorOnly. That means the operator and allowlisted people as operators, and project members (after the membership read) as participants. Someone outside the trust set is discarded at the gate, before any read. The agent's own boosts are discarded asagent_authored. So are other agents' boosts on the push lane. Delegated boosts are discarded asdelegated. An--allow-agentagent gets in by mention only, so its boosts are discarded asnot_addressed. Two agents that allow each other can't boost each other into a loop.boost.creatednames no recording type, and the SDK'sSummarizerefuses to route it (ErrNoRecordingType). So the boost is read first (GET /boosts/:id, a newReads.Boosts). That read gives the boosted recording's type, whichSummarizethen routes by, and the boost's content. If the boost is gone, is missing its recording or booster, or names a different recording, the event is blocked asread_failed(retained, as for any read that doesn't answer). A booster other than the trusted performer is discarded asuntrusted_author.requester_idis the booster,roleis the booster's role, andrequester_nameis the booster's name.acknowledgeis false: a boost is a reply, not a request.owneris false.contentis the boost itself;recordingis the agent's post it answers.recording:<parent>). A boost on a chat line goes to its Campfire. Anything else replies on the recording itself.boostedis built andqueuedis still pending.skills/basecamp-connect/SKILL.mdlistsboostedamong the triggers. Forboostedit skips the acknowledgement and the card move, defaults to silence, and says a boost is never an operator's go.RequiresServed: true
A boost in a project
connect.jsondoesn't serve is discarded at the gate, before any read, rather than blocked asno_route. Onlymentionedandassignedare blocked in an unserved project, and they get a holding reply ("this project isn't served yet"). That reply exists because someone asked the agent for something and is waiting. A boost asks for nothing, so a holding reply to a thumbs-up would answer nobody.subscribedandcompleted, the other non-request triggers, usetruefor the same reason. It also keeps boosts in unserved projects from spending two reads each.Deliberately not done
queuedandedited. The rest of step 25 and step 28 are out of scope.details.boosted_event_idis non-null for these. Basecamp addresses such a boost to whoever performed the event, not to the recording's author, so the creator check would be wrong for it. Telling whether the agent performed that event would take an events read this rule doesn't make. These are discarded asnot_addressed, from the details alone, at no read cost.published_byvs. creator. bc3's boostee for a recording ispublished_by || edited_by. The summary carriescreator, and the two differ only for a draft someone else published. A message the agent drafted but someone else published would be admitted although Basecamp addressed the boost to the publisher. One the agent published from someone else's draft would be missed. None of the agent's comments or lines are drafts.owner. Whether an operator's boost on an agent's explicit ask should count as the go is a separate policy question; the skill says it doesn't.What can now reach the agent that couldn't before
A boost on a recording the agent created, in a served, in-scope project, given by a person the trust settings already let mention the agent. The recording must not be drafted or trashed, and the boost must not be delegated. That's the whole set, and each case is intended. Cost: each such boost takes one boost read plus one summary read. For a chat line, the summary read includes the SDK's Campfire discovery. In project trust mode this applies to every member's boost on anything in a served project, not just the agent's posts, because the creator is only known after the summary read.
Tests
internal/connector/admission/boost_test.gocovers:TestABoostOnTheAgentsCommentIsHandedOffAsAReplyruns end to end: details survive the ledger, then admission, then the request line.NewSDKReads().Boostsreads/boosts/:idand returns the recording type.RequiresServedor the event-boost discard each fails a test.bin/cipasses on Linux. Locally on macOS, lint flagsinternal/connector/driver/proctime_darwin.go, a darwin-only file this PR doesn't touch.Summary by cubic
Lets
basecamp connecthear when someone boosts a comment, Campfire line, or post the agent wrote.boost.createdwas previously rejected asnot_in_matrix, so boosts sat unanswered even though Basecamp addresses them to the recording's author.Adds a
boostedtrigger that admits a boost only when it lands on the recording itself and the recording's creator is the agent. The booster is trusted like a mention's performer in every trust mode; an agent's own, delegated, and--allow-agent boosts stay discarded. Sinceboost.creatednames no recording type, admission reads the boost first (GET /boosts/:id), which supplies the type the summary read routes on and the boost's content, handed off as the request. The reply goes where the boosted recording lives: a comment's thread, a line's Campfire, or the recording itself.acknowledgeandownerare false, and the rule requires a served project so boosts in unserved projects are discarded rather than held (a holding reply to a thumbs-up answers nobody).Details that lack a boost id, or whose
boosted_event_idkey is absent, are an invalid pointer and discarded before any read; only an explicitnullis read as a boost on the recording. A boost on one of a recording's events is discarded asnot_addressed: Basecamp addresses that to the event's performer, not the recording's author. AnAgentbooster is refused asagent_authoredat the boost read — the poll lane carries no actor type, so an allowlisted Agent passes the gate, and the read is where its type shows.Out of scope
queued(rest of step 25) andedited(step 28) triggers.published_byvs. creator mismatches, and whether an operator's boost on an explicit ask counts as a go.Written for commit 62118b1. Summary will update on new commits.