Skip to content

feat(cli)!: complyctl redesign#381

Open
jpower432 wants to merge 27 commits intocomplytime:mainfrom
jpower432:001-gemara-native-workflow
Open

feat(cli)!: complyctl redesign#381
jpower432 wants to merge 27 commits intocomplytime:mainfrom
jpower432:001-gemara-native-workflow

Conversation

@jpower432
Copy link
Member

@jpower432 jpower432 commented Feb 17, 2026

Summary

The PR implements a complyctl to include plugin API and UX breaking changes.

Related Issues

Inform any issues relevant to this PR. For example:

  • Closes #ISSUE_NUMBER

Review Hints

For specs look at the first commit
For code changes look at the third commit

@jpower432 jpower432 force-pushed the 001-gemara-native-workflow branch 2 times, most recently from 1a8c16d to 0dc86bc Compare February 18, 2026 21:19
@jpower432
Copy link
Member Author

@marcusburghardt I think we may need to exclude vendor directories from the linting checks

Copy link
Contributor

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments Jenn.

- `targets`: Repeated `Target` entries (one per target to scan)
- `target_id`: Target identifier from workspace configuration
- `variables`: Plugin-defined variables map (plugin-specific configuration such as authentication tokens, connection strings, API endpoints, credentials). Each plugin defines what variables it expects.
- `requirement_ids`: Requirement IDs to evaluate (repeated string field). Plugins evaluate only the specified requirements.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to inform the requirement_ids during the SCAN considering the plugin policy is already "generated" including all requirements ids?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right the only thing I think we need at scan time is target information.

- `plugin_info`: Plugin metadata
- `duration_seconds`: Execution time

**Note**: All targets configured in workspace config for the specified policy ID are scanned in a single Scan call.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included a plugin configuration file for targets in ampel-plugin (#380). I wonder if we can create a targets configuration file generic enough for all plugins or should we keep this on the plugin responsibility since we are now considering more than only a local system.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be multiple plugins evaluating a single target. The plugin might have more granular knowledge of the specific target, but the host conveys the user selection.

2. **Health Check**: Must respond to `HealthCheck` within 1 second
3. **API Version**: Must report correct API version in `HealthCheckResponse`
4. **Error Handling**: Must return appropriate gRPC status codes
5. **Timeout**: Must complete `Scan` within configured timeout (default: 5 minutes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we define what to do when the timeout is reached? Maybe checking the plugin health before stopping?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Maybe. I would wonder if the plugin would respond because it's up, but just has a blocking process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea to check the status after the timeout is to finish the process only if it not responding for this timeout. In case a plugin is working in a time-consuming task for a particular target, it should be better to wait it to finish. Essentially it would differentiate time-consuming tasks than unresponsive plugins.

I understand this PR is already bringing many changes so all fine to discuss this further in a next opportunity.

2. Configure `buf.yaml` and `buf.gen.yaml` in `api/proto/`
3. Run `buf generate` to generate Go code

**References**:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be good to include references for Gemara project, where the schemes are available.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!


### AssessmentConfiguration

Configuration extracted from the policy graph and passed to plugins via Generate RPC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Refactor constitution from human-oriented style guide into
agent-enforceable directives. Normalize all rules to RFC 2119
(MUST/SHOULD/MAY), remove repository-specific references, add lint
configuration awareness rule, and define the incremental constitution
model for downstream repositories.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@jpower432 jpower432 force-pushed the 001-gemara-native-workflow branch from 5c8ec26 to 0b59700 Compare February 26, 2026 01:19
@jpower432 jpower432 force-pushed the 001-gemara-native-workflow branch from 0b59700 to 78189bf Compare February 26, 2026 02:02
marcusburghardt and others added 14 commits February 26, 2026 12:16
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
It was also causing shellcheck to fail.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

# Conflicts:
#	vendor/github.com/cloudflare/circl/internal/sha3/xor_unaligned.go
#	vendor/github.com/cloudflare/circl/sign/sign.go
#	vendor/modules.txt
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432 jpower432 force-pushed the 001-gemara-native-workflow branch from 78189bf to b34d5d8 Compare February 26, 2026 12:12
for _, p := range cfg.Policies {
ref := complytime.ParsePolicyRef(p.URL)
if _, ok := cached[ref.Repository]; !ok {
cached[ref.Repository] = []string{"(not cached — " + p.EffectiveID() + ")"}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Thanks @sonupreetam.

if plain {
terminal.ShowPlainTable(w, columns, rows)
return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpower432 Just a thought for UX part, I liked the generate output which actually more boxed than the list command terminal.ShowPlainTable. We can use the same border styles. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sonupreetam. I ended up removing these options and just went to with plain, tabular output. Seems like a much simpler solution and still look good (IMO).


routeRows := make([]table.Row, 0, len(routes))
for _, r := range routes {
pluginPath := r.PluginPath
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpower432 For UX, I think the plugin path could be shortened.

Copy link
Contributor

@sonupreetam sonupreetam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpower432 Added minor suggestions on UX front. Everything else with UX part LGTM.

Copy link
Member

@gvauter gvauter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just left a few nit comments

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432
Copy link
Member Author

jpower432 commented Feb 27, 2026

Testing farm is failing here because all of the plans are removed, we might put them back, but the only plan was for OSCAL content. It may be simpler to squash-merge this.

@jpower432 jpower432 marked this pull request as ready for review February 27, 2026 01:30
@jpower432 jpower432 requested review from a team as code owners February 27, 2026 01:30
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants