-
Notifications
You must be signed in to change notification settings - Fork 14
feat: CP-1068 Create MCP tool to bump timestamp of a project #3713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Script: bumpedBS, | ||
}) | ||
if err != nil { | ||
return errs.Wrap(err, "Failed to stage bumped commit") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it does not solve, it errors here.
e.g.:
`Failed to stage bumped commit:
failed to poll build plan:
Request failed: Could not plan build. Platform responded with:
These are the last lines of the error message:
….2.36) requires Ingredient|language/python|fast-agent-mcp (0.2.36) which depends on Feature|language/python|opentelemetry-instrumentation-google-genai (>=0.2b0), Feature|language/python|fast-agent-mcp (0.2.36) requires Feature|language/python|opentelemetry-instrumentation-google-genai (>=0.2b0).
So, because no versions of Feature|language/python|opentelemetry-instrumentation-google-genai match >=0.2b0
and root depends on Feature|language/python|fast-agent-mcp (0.2.36), version solving failed.`
} | ||
} | ||
|
||
func (runner *ProjectErrorsRunner) Run(params *Params) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This func is based off of /internal/runners/upgrade/upgrade.go
It isn't bumping the timestamp and I don't know why yet. Perhaps it's missing something after StageCommitAndPoll
return fmt.Errorf("error fetching default branch: %w", err) | ||
} | ||
|
||
// Collect "before" buildplan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Collect "before" buildplan | |
// Collect "before" buildscript |
return errs.Wrap(err, "Failed to fetch build result") | ||
} | ||
|
||
// Collect "after" buildplan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Collect "after" buildplan | |
// Collect "after" buildscript |
now := captain.TimeValue{} | ||
now.Set("now") | ||
ts, err := commits_runbit.ExpandTime(&now, runner.auth) | ||
if err != nil { | ||
return errs.Wrap(err, "Failed to fetch latest timestamp") | ||
} | ||
bumpedBS.SetAtTime(ts, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for facilitating flag parsing, it's a roundabout way of grabbing the time if you already know that you want "now". In this case you can just call the underlying logic:
cli/internal/runbits/commits_runbit/time.go
Lines 32 to 37 in 7301bd9
latest, err := model.FetchLatestRevisionTimeStamp(auth) | |
if err != nil { | |
return time.Time{}, errs.Wrap(err, "Unable to determine latest revision time") | |
} | |
return latest, nil | |
} |
I wouldn't expect to see any use of the captain package in mcp tooling, that's specifically for command line handling, so a red flag.
"github.com/ActiveState/cli/pkg/project" | ||
) | ||
|
||
type ProjectErrorsRunner struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename to reflect directory name.
type Params struct { | ||
project *project.Namespaced | ||
} | ||
|
||
func NewParams(project *project.Namespaced) *Params { | ||
return &Params{ | ||
project: project, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make Project public. And nitpick but NewParams() should not take any of the params if we are to stick to the pattern.
When interacting with runners via the CLI (ie. via captain) we use params to pass properties by reference, which we can't do if they're private and only provided as arguments to a constructor function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Uh oh!
There was an error while loading. Please reload this page.