gin style guide and acep

[dexhorthy]

Important

Adds support for delegated tokens in contact channels by introducing channelToken in server.go, updating CRDs, and modifying controllers.

• Behavior:
  • Introduces channelToken in CreateTaskRequest in server.go to support delegated tokens for contact channels.
  • Updates createTask in server.go to create ContactChannel and Task with ContactChannelRef if channelToken is provided.
  • Maintains backward compatibility with responseURL.
• CRD Updates:
  • Adds ChannelToken to ContactChannelSpec in ContactChannel CRD.
  • Adds ContactChannelRef to TaskSpec in Task CRD, deprecating ResponseURL.
• Controller Updates:
  • Modifies ToolCallReconciler to use ContactChannelRef in getContactChannel().
• Documentation:
  • Adds aceps/contact_channels.md detailing the new contact channel approach and migration strategy.
• Misc:
  • Renames SmallChain Development Guide to AgentControlPlane Development Guide in CLAUDE.md.

^{This description was created by for 2ec0bd8. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 2ec0bd8 in 2 minutes and 29 seconds. Click for details.

• Reviewed 453 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 9 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. aceps/contact_channels.md:97

• Draft comment:
  Typo: 'reponse_token' should be 'response_token' in the discussion of stripping the token from the URL.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a documentation file explaining a design proposal. While typos should generally be fixed, this particular typo appears in a section describing the current state/motivation, not in actual technical specifications or code. The meaning is still clear despite the typo. The comment, while technically correct, doesn't add significant value. The typo could potentially cause confusion for readers trying to understand the current state. Consistency in technical terms is important in documentation. While consistency is good, this typo appears in prose explaining motivation, not in technical specifications or code. The meaning is clear from context and it doesn't impact implementation. This comment should be deleted as it points out a minor typo that doesn't impact understanding or implementation.

2. acp/internal/server/server.go:203

• Draft comment:
  Consider using Gin's builtin JSON binding (e.g., c.ShouldBindJSON) instead of manually reading raw JSON and using DisallowUnknownFields, to improve clarity and consistency.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. acp/internal/server/server.go:38

• Draft comment:
  The proposal in 'aceps/contact_channels.md' suggests adding a 'ChannelToken' field to CreateTaskRequest and updating createTask accordingly. Consider adding this field to support delegated token functionality.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. acp/internal/server/server.go:44

• Draft comment:
  The deprecated 'ResponseUrl' field (alternative casing) is marked with a 'todo remove' comment. Consider removing it in future releases to maintain consistency.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

5. CLAUDE.md:246

• Draft comment:
  Typo: "context object that be used" should be "context object that can be used".
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is a real grammatical error in changed code, the PR review rules discourage purely informative comments that don't require code changes. Grammar fixes, while helpful, are not critical to code functionality or quality. The meaning is still clear despite the minor grammatical error. The grammar error could make the documentation less professional and harder to read. Documentation quality is important for developer experience. While documentation quality matters, this is a very minor grammatical error that doesn't significantly impact readability or understanding. The PR rules specifically discourage non-critical comments. Delete this comment as it's a minor grammatical fix that doesn't impact code functionality or significantly affect documentation usability.

6. CLAUDE.md:261

• Draft comment:
  Typo: "all gin endpoint" should be "all gin endpoints".
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is technically a typo in new text being added, the PR comment rules emphasize not making purely informative comments or obvious fixes. The meaning is completely clear despite the missing 's'. This kind of minor grammatical fix doesn't require a PR comment and just adds noise to the review. The typo is in newly added text, so it is related to a change. And technically, fixing typos does improve code quality. However, the rules specifically say not to make obvious or unimportant comments. This typo doesn't impact understanding and is the kind of minor thing that just adds noise to a PR review. Delete this comment. While it points out a real typo, it's too minor to warrant a PR comment and violates the rule about not making obvious/unimportant comments.

7. CLAUDE.md:296

• Draft comment:
  Typo: "Merthods" should be corrected to "Methods".
• Reason this comment was not posted:
  Marked as duplicate.

8. aceps/contact_channels.md:99

• Draft comment:
  Typo: 'reponse_token' should be corrected to 'response_token'.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a typo in a markdown documentation file. While it is technically incorrect, it's in a descriptive paragraph explaining the motivation, not in actual code or API documentation. The meaning is still clear from context. The typo appears only once and doesn't affect functionality. The typo could potentially confuse readers who are searching the document for specific terms. Consistency in terminology is important in technical documentation. While consistency is good, this is a minor typo in a prose section that doesn't affect understanding or implementation. The correct spelling is used everywhere else in the document. This comment should be deleted as it points out a trivial typo that doesn't impact understanding or implementation.

9. acp/internal/server/server.go:44

• Draft comment:
  Typo note: Consider capitalizing 'todo' to 'TODO' for consistency in the comment.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is technically about a change in the diff, it's an extremely minor stylistic suggestion about comment formatting. The meaning is clear either way. The comment doesn't point out any actual code issues or suggest meaningful improvements. It's the kind of nitpicky comment that adds noise without value. The Go style guide might have an official stance on TODO capitalization. This could be a valid standardization request. Even if there is a style guide preference, this is too minor to warrant a PR comment. The code works the same either way, and this kind of nitpicking can slow down PR reviews. Delete this comment as it's an overly nitpicky stylistic suggestion that doesn't meaningfully improve the code.

Workflow ID: wflow_3lGIqa7yI5mfsEFk

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[dexhorthy]

thinking now... maybe its token and baseURL - like https://api.humanlayer.dev/humanlayer/v1 - or https://api.example.com/a2h/v1 - anything that implements

POST $BASE/human_contacts
GET $BASE/human_contacts/{id}
POST $BASE/function_calls
GET $BASE/function_calls/{id}

[allisoneer]

what is aceps directory? Should this go in acp/docs? This is just brain storming to get in the codebase or should we (or agents) want to look at this?

[dexhorthy]

agent control enhancement proposals

its dumb but its different from docs

[allisoneer]

Gonna end up incorporating this in my CLAUDE.md changes. Will send PR number in a comment there when ready.

[dexhorthy]

hi from mcp