Derive implement issue titles from command text#232
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@sepo-agent /answer This is not what i meant. The agent should generate a proper title for the issue given the description and the context. |
|
You’re right. PR #232 implements the wrong behavior. Current architecture explains why: explicit routes skip dispatch triage in The fix should not copy text after Concrete revision plan:
So the intended behavior is: |
@sepo-agent /implement |
|
Implementing this request — tracking in #234. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@sepo-agent /answer can you compare it with #236 |
|
Yes. Compared against #236, current #232 is the stronger version. Both PRs now aim at the right product behavior: explicit Key differences:
The biggest concern in #236 is this line in its workflow diff: the metadata generation step uses I would merge or continue with #232, not #236. #236 is conceptually aligned, but it looks like an earlier/parallel implementation that missed the later hardening done on #232. |
|
@sepo-agent /answer Right now we have some additional prompts for the implement request endpoint. Just want to double check that they are expected right? B/c previous for other implement route they do not have the agent generated titles and metadata? |
|
Yes, the additional prompt is expected for this specific path. The distinction is:
So the new That is why #232 also gates the prompt behind explicit route authorization, non-issue target, and read-only permissions in So yes: the extra prompt is intentional because explicit |
|
@sepo-agent /answer But we also added a bunch of auth checker in this PR -> is that intended? |
|
Yes, the auth check is intentional, but I agree it looks heavier than the core product change. Why it was added: the new metadata prompt runs before creating the tracking issue, and it can read PR/discussion context. So #232 first reuses the existing dispatch-policy path to confirm the explicit route is actually authorized before letting that metadata prompt run. That is the new That part is expected. It prevents this sequence:
The current PR avoids that by checking One nuance: the current check runs for all explicit routes, even though the new metadata prompt only needs it for explicit requested_route == 'implement' && target_kind != 'issue'So: do not remove the auth checker entirely, but it is fair to tighten its condition so it is clearly just protecting the new metadata-generation path, not changing the broader explicit-route auth model. |
|
@sepo-agent /answer There are two questions here:
|
|
@sepo-agent /answer so the questions is, after adding this new gate, should we remove the older gate? |
|
No. I would not remove the older/final gate. The new gate is a precheck only. Its job is to prevent an unauthorized explicit The older/final
So the right shape is:
What I would change is not removing the older gate, but narrowing the new one so it does not look like a second general auth system: steps.context.outputs.should_respond == 'true' &&
steps.context.outputs.requested_route == 'implement' &&
steps.context.outputs.target_kind != 'issue'Then keep the final |
@sepo-agent /answer now I really don't get it. Since we have a later gating, why don't we reuse the later gating for the same metadata generation? Or are you saying the later gating cannot be reused easily? |
|
You’re right to push on this. The later gate cannot be reused as a single workflow step because of an ordering dependency. Current desired flow is: The later RESPONSE_FILE: ${{ steps.triage.outputs.response_file || steps.implement_metadata.outputs.response_file }}So if we tried to use only the later gate, we get a cycle: That is why the PR adds an earlier precheck. It is not a separate auth model; it is an early call to the same policy machinery so the metadata prompt does not run before authorization. Could we restructure to avoid this shape? Yes, but it would be more awkward:
That still has two phases; it just moves the duplication around. So the honest answer is: we can reuse the same code, but not the same later workflow step, because the later step depends on metadata that has not been generated yet. The current approach is reasonable if the precheck is narrowly scoped and clearly named as a metadata precheck. |
|
@sepo-agent /answer
Why there's a != issue in the condition? |
|
Because the metadata prompt is only needed when Sepo must create a new tracking issue. For explicit There is no new issue to title, so generated For explicit That is the case where we need generated metadata for the tracking issue. So That said, for readability I’d slightly prefer the condition be more explicit if only PRs/discussions are intended: steps.context.outputs.requested_route == 'implement' &&
(
steps.context.outputs.target_kind == 'pull_request' ||
steps.context.outputs.target_kind == 'discussion'
)Functionally, |
|
@sepo-agent /answer What I would change is not removing the older gate, but narrowing the new one so it does not look like a second general auth system: steps.context.outputs.should_respond == 'true' && Good that's the intended change right? |
|
Yes. That is the intended change. Keep the final steps.context.outputs.should_respond == 'true' &&
steps.context.outputs.requested_route == 'implement' &&
steps.context.outputs.target_kind != 'issue'That way the PR reads as:
|
@sepo-agent /orchestrate can you implement the minimum fix and then review it? |
This comment has been minimized.
This comment has been minimized.
|
Sepo pushed fixes for this PR. Branch:
|
Rubrics Review
Notes
Findings
Final Rubric VerdictPASS |
AI Review Synthesis
Summary of PR/Issue
ReviewThe current PR head looks ready to ship. Codex found no issues, Claude only noted optional INFO-level cleanup, and rubrics review passed all applicable dimensions.
Progress
Recommended Next StepNO_AUTOMATED_ACTION: No unresolved actionable branch changes remain; this is ready for human merge decision. Final VerdictSHIP Action Items
|
Rubrics UpdateUpdated Added one rubric:
Skipped the later gate/precheck feedback as already covered by existing authorization, structured-input, and surgical-change rubrics. Validation passed: |
Summary
/implementtracking issue titles from the command detail.Implement requested changeas the fallback when no detail is provided.Tests
npm --prefix .agent run build && node --test .agent/dist/__tests__/triage.test.jsCloses #228